| Message ID | 20221101072506.7307-1-liubo03@inspur.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: replace DEFINE_SIMPLE_ATTRIBUTE with DEFINE_DEBUGFS_ATTRIBUTE | expand |
On 11/1/22 08:25, Bo Liu wrote: > Fix the following coccicheck warning: > virt/kvm/kvm_main.c:3847:0-23: WARNING > vcpu_get_pid_fops should be defined with DEFINE_DEBUGFS_ATTRIBUTE > > Signed-off-by: Bo Liu <liubo03@inspur.com> > --- > virt/kvm/kvm_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index f1df24c2bc84..3f383f27d3d7 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -3844,7 +3844,7 @@ static int vcpu_get_pid(void *data, u64 *val) > return 0; > } > > -DEFINE_SIMPLE_ATTRIBUTE(vcpu_get_pid_fops, vcpu_get_pid, NULL, "%llu\n"); > +DEFINE_DEBUGFS_ATTRIBUTE(vcpu_get_pid_fops, vcpu_get_pid, NULL, "%llu\n"); > > static void kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu) > { If you really wanted to do this, you would also have to replace debugfs_create_file with debugfs_create_file_unsafe. However, this is not a good idea. The rationale in the .cocci file is that "DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file() imposes some significant overhead", but this should not really be relevant for a debugfs file. Such a patch would only make sense if there was a version of debugfs_create_file_unsafe() with a less-terrible name (e.g. debugfs_create_simple_attr?), which could _only_ be used with fops created by DEFINE_DEBUGFS_ATTRIBUTE. Without such a type-safe trick, the .cocci file is only adding confusion to perfectly fine code. Paolo
On Wed, Nov 02, 2022, Paolo Bonzini wrote: > On 11/1/22 08:25, Bo Liu wrote: > > Fix the following coccicheck warning: > > virt/kvm/kvm_main.c:3847:0-23: WARNING > > vcpu_get_pid_fops should be defined with DEFINE_DEBUGFS_ATTRIBUTE > > > > Signed-off-by: Bo Liu <liubo03@inspur.com> > > --- > > virt/kvm/kvm_main.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > > index f1df24c2bc84..3f383f27d3d7 100644 > > --- a/virt/kvm/kvm_main.c > > +++ b/virt/kvm/kvm_main.c > > @@ -3844,7 +3844,7 @@ static int vcpu_get_pid(void *data, u64 *val) > > return 0; > > } > > -DEFINE_SIMPLE_ATTRIBUTE(vcpu_get_pid_fops, vcpu_get_pid, NULL, "%llu\n"); > > +DEFINE_DEBUGFS_ATTRIBUTE(vcpu_get_pid_fops, vcpu_get_pid, NULL, "%llu\n"); > > static void kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu) > > { > > If you really wanted to do this, you would also have to replace > debugfs_create_file with debugfs_create_file_unsafe. > > However, this is not a good idea. The rationale in the .cocci file is that > "DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file() imposes some significant > overhead", but this should not really be relevant for a debugfs file. > > Such a patch would only make sense if there was a version of > debugfs_create_file_unsafe() with a less-terrible name (e.g. > debugfs_create_simple_attr?), which could _only_ be used with fops created > by DEFINE_DEBUGFS_ATTRIBUTE. Without such a type-safe trick, the .cocci > file is only adding confusion to perfectly fine code. Heh, some serious deja vu here[1]. This is the second case of identical, flawed patches being sent in response to misguided coccinelle warnings in a rather short amount of time, the "return min(r, 0)" horror being the other case[2][3]. The min() thing is supposed to be fixed by commit aeb300c1dbfc ("coccinelle: misc: minmax: suppress patch generation for err returns"). Is that patch broken, or are folks just running old scripts? As for the DEFINE_DEBUGFS_ATTRIBUTE check, can that warning be downgraded (is that even a thing?) or even deleted? As much as I enjoyed the opportunity to learn more about debugfs, the unnecessary confusion and wasted time was/is annoying. [1] https://lore.kernel.org/all/Yxoo1A2fmlAWruyV@google.com [2] https://lore.kernel.org/all/8881d7b4-0c31-cafd-1158-0d42c1c7f43a@redhat.com [3] https://lore.kernel.org/all/d8a518c4a4014307b30020b38022d633@AcuMS.aculab.com
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index f1df24c2bc84..3f383f27d3d7 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -3844,7 +3844,7 @@ static int vcpu_get_pid(void *data, u64 *val) return 0; } -DEFINE_SIMPLE_ATTRIBUTE(vcpu_get_pid_fops, vcpu_get_pid, NULL, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(vcpu_get_pid_fops, vcpu_get_pid, NULL, "%llu\n"); static void kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu) {
Fix the following coccicheck warning: virt/kvm/kvm_main.c:3847:0-23: WARNING vcpu_get_pid_fops should be defined with DEFINE_DEBUGFS_ATTRIBUTE Signed-off-by: Bo Liu <liubo03@inspur.com> --- virt/kvm/kvm_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)