Re: [PATCH] kvm: x86: Use DEFINE_DEBUGFS_ATTRIBUTE for debugfs files

From: Greg KH
Date: Wed Sep 18 2019 - 08:03:56 EST


On Wed, Sep 18, 2019 at 08:19:44AM +0800, wang.yi59@xxxxxxxxxx wrote:
> Hi Paolo,
>
> > On 22/07/19 09:33, Yi Wang wrote:
> > > We got these coccinelle warning:
> > > ./arch/x86/kvm/debugfs.c:23:0-23: WARNING: vcpu_timer_advance_ns_fops
> > > should be defined with DEFINE_DEBUGFS_ATTRIBUTE
> > > ./arch/x86/kvm/debugfs.c:32:0-23: WARNING: vcpu_tsc_offset_fops should
> > > be defined with DEFINE_DEBUGFS_ATTRIBUTE
> > > ./arch/x86/kvm/debugfs.c:41:0-23: WARNING: vcpu_tsc_scaling_fops should
> > > be defined with DEFINE_DEBUGFS_ATTRIBUTE
> > > ./arch/x86/kvm/debugfs.c:49:0-23: WARNING: vcpu_tsc_scaling_frac_fops
> > > should be defined with DEFINE_DEBUGFS_ATTRIBUTE
> > >
> > > Use DEFINE_DEBUGFS_ATTRIBUTE() rather than DEFINE_SIMPLE_ATTRIBUTE()
> > > to fix this.
> > >
> > > Signed-off-by: Yi Wang <wang.yi59@xxxxxxxxxx>
> >
> > It sucks though that you have to use a function with "unsafe" in the name.
>
> Yes, it does, but I found some patches in the git log:
> https://git.kernel.org/pub/scm/virt/kvm/kvm.git/log/?qt=grep&q=DEFINE_DEBUGFS_ATTRIBUTE+
>
> And, do you think the function name "debugfs_create_file_unsafe" is not proper?

Only if you _KNOW_ you are creating/removing these files in a way that
is safe is it ok to use these calls. Hint, what is your locking
strategy for when these files are removed?

Is that the case here? If not, please stick with what is there today,
as we know it works, and it is "safe" to do so.

thanks,

greg k-h