* [PATCH] kvm: x86: Use DEFINE_DEBUGFS_ATTRIBUTE for debugfs files
@ 2019-07-22 7:33 Yi Wang
2019-09-17 17:18 ` Paolo Bonzini
0 siblings, 1 reply; 5+ messages in thread
From: Yi Wang @ 2019-07-22 7:33 UTC (permalink / raw)
To: pbonzini
Cc: rkrcmar, tglx, mingo, bp, hpa, x86, kvm, linux-kernel,
xue.zhihong, wang.yi59, up2wing, wang.liang82
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@zte.com.cn>
---
arch/x86/kvm/debugfs.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/debugfs.c b/arch/x86/kvm/debugfs.c
index 329361b..24016fb 100644
--- a/arch/x86/kvm/debugfs.c
+++ b/arch/x86/kvm/debugfs.c
@@ -20,7 +20,7 @@ static int vcpu_get_timer_advance_ns(void *data, u64 *val)
return 0;
}
-DEFINE_SIMPLE_ATTRIBUTE(vcpu_timer_advance_ns_fops, vcpu_get_timer_advance_ns, NULL, "%llu\n");
+DEFINE_DEBUGFS_ATTRIBUTE(vcpu_timer_advance_ns_fops, vcpu_get_timer_advance_ns, NULL, "%llu\n");
static int vcpu_get_tsc_offset(void *data, u64 *val)
{
@@ -29,7 +29,7 @@ static int vcpu_get_tsc_offset(void *data, u64 *val)
return 0;
}
-DEFINE_SIMPLE_ATTRIBUTE(vcpu_tsc_offset_fops, vcpu_get_tsc_offset, NULL, "%lld\n");
+DEFINE_DEBUGFS_ATTRIBUTE(vcpu_tsc_offset_fops, vcpu_get_tsc_offset, NULL, "%lld\n");
static int vcpu_get_tsc_scaling_ratio(void *data, u64 *val)
{
@@ -38,7 +38,7 @@ static int vcpu_get_tsc_scaling_ratio(void *data, u64 *val)
return 0;
}
-DEFINE_SIMPLE_ATTRIBUTE(vcpu_tsc_scaling_fops, vcpu_get_tsc_scaling_ratio, NULL, "%llu\n");
+DEFINE_DEBUGFS_ATTRIBUTE(vcpu_tsc_scaling_fops, vcpu_get_tsc_scaling_ratio, NULL, "%llu\n");
static int vcpu_get_tsc_scaling_frac_bits(void *data, u64 *val)
{
@@ -46,20 +46,20 @@ static int vcpu_get_tsc_scaling_frac_bits(void *data, u64 *val)
return 0;
}
-DEFINE_SIMPLE_ATTRIBUTE(vcpu_tsc_scaling_frac_fops, vcpu_get_tsc_scaling_frac_bits, NULL, "%llu\n");
+DEFINE_DEBUGFS_ATTRIBUTE(vcpu_tsc_scaling_frac_fops, vcpu_get_tsc_scaling_frac_bits, NULL, "%llu\n");
int kvm_arch_create_vcpu_debugfs(struct kvm_vcpu *vcpu)
{
struct dentry *ret;
- ret = debugfs_create_file("tsc-offset", 0444,
+ ret = debugfs_create_file_unsafe("tsc-offset", 0444,
vcpu->debugfs_dentry,
vcpu, &vcpu_tsc_offset_fops);
if (!ret)
return -ENOMEM;
if (lapic_in_kernel(vcpu)) {
- ret = debugfs_create_file("lapic_timer_advance_ns", 0444,
+ ret = debugfs_create_file_unsafe("lapic_timer_advance_ns", 0444,
vcpu->debugfs_dentry,
vcpu, &vcpu_timer_advance_ns_fops);
if (!ret)
@@ -67,12 +67,12 @@ int kvm_arch_create_vcpu_debugfs(struct kvm_vcpu *vcpu)
}
if (kvm_has_tsc_control) {
- ret = debugfs_create_file("tsc-scaling-ratio", 0444,
+ ret = debugfs_create_file_unsafe("tsc-scaling-ratio", 0444,
vcpu->debugfs_dentry,
vcpu, &vcpu_tsc_scaling_fops);
if (!ret)
return -ENOMEM;
- ret = debugfs_create_file("tsc-scaling-ratio-frac-bits", 0444,
+ ret = debugfs_create_file_unsafe("tsc-scaling-ratio-frac-bits", 0444,
vcpu->debugfs_dentry,
vcpu, &vcpu_tsc_scaling_frac_fops);
if (!ret)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] kvm: x86: Use DEFINE_DEBUGFS_ATTRIBUTE for debugfs files
2019-07-22 7:33 [PATCH] kvm: x86: Use DEFINE_DEBUGFS_ATTRIBUTE for debugfs files Yi Wang
@ 2019-09-17 17:18 ` Paolo Bonzini
2019-09-17 18:12 ` Greg Kroah-Hartman
[not found] ` <201909180819440437759@zte.com.cn>
0 siblings, 2 replies; 5+ messages in thread
From: Paolo Bonzini @ 2019-09-17 17:18 UTC (permalink / raw)
To: Yi Wang
Cc: rkrcmar, tglx, mingo, bp, hpa, x86, kvm, linux-kernel,
xue.zhihong, up2wing, wang.liang82, Greg Kroah-Hartman
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@zte.com.cn>
It sucks though that you have to use a function with "unsafe" in the name.
Greg, is the patch doing the right thing?
Paolo
> ---
> arch/x86/kvm/debugfs.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kvm/debugfs.c b/arch/x86/kvm/debugfs.c
> index 329361b..24016fb 100644
> --- a/arch/x86/kvm/debugfs.c
> +++ b/arch/x86/kvm/debugfs.c
> @@ -20,7 +20,7 @@ static int vcpu_get_timer_advance_ns(void *data, u64 *val)
> return 0;
> }
>
> -DEFINE_SIMPLE_ATTRIBUTE(vcpu_timer_advance_ns_fops, vcpu_get_timer_advance_ns, NULL, "%llu\n");
> +DEFINE_DEBUGFS_ATTRIBUTE(vcpu_timer_advance_ns_fops, vcpu_get_timer_advance_ns, NULL, "%llu\n");
>
> static int vcpu_get_tsc_offset(void *data, u64 *val)
> {
> @@ -29,7 +29,7 @@ static int vcpu_get_tsc_offset(void *data, u64 *val)
> return 0;
> }
>
> -DEFINE_SIMPLE_ATTRIBUTE(vcpu_tsc_offset_fops, vcpu_get_tsc_offset, NULL, "%lld\n");
> +DEFINE_DEBUGFS_ATTRIBUTE(vcpu_tsc_offset_fops, vcpu_get_tsc_offset, NULL, "%lld\n");
>
> static int vcpu_get_tsc_scaling_ratio(void *data, u64 *val)
> {
> @@ -38,7 +38,7 @@ static int vcpu_get_tsc_scaling_ratio(void *data, u64 *val)
> return 0;
> }
>
> -DEFINE_SIMPLE_ATTRIBUTE(vcpu_tsc_scaling_fops, vcpu_get_tsc_scaling_ratio, NULL, "%llu\n");
> +DEFINE_DEBUGFS_ATTRIBUTE(vcpu_tsc_scaling_fops, vcpu_get_tsc_scaling_ratio, NULL, "%llu\n");
>
> static int vcpu_get_tsc_scaling_frac_bits(void *data, u64 *val)
> {
> @@ -46,20 +46,20 @@ static int vcpu_get_tsc_scaling_frac_bits(void *data, u64 *val)
> return 0;
> }
>
> -DEFINE_SIMPLE_ATTRIBUTE(vcpu_tsc_scaling_frac_fops, vcpu_get_tsc_scaling_frac_bits, NULL, "%llu\n");
> +DEFINE_DEBUGFS_ATTRIBUTE(vcpu_tsc_scaling_frac_fops, vcpu_get_tsc_scaling_frac_bits, NULL, "%llu\n");
>
> int kvm_arch_create_vcpu_debugfs(struct kvm_vcpu *vcpu)
> {
> struct dentry *ret;
>
> - ret = debugfs_create_file("tsc-offset", 0444,
> + ret = debugfs_create_file_unsafe("tsc-offset", 0444,
> vcpu->debugfs_dentry,
> vcpu, &vcpu_tsc_offset_fops);
> if (!ret)
> return -ENOMEM;
>
> if (lapic_in_kernel(vcpu)) {
> - ret = debugfs_create_file("lapic_timer_advance_ns", 0444,
> + ret = debugfs_create_file_unsafe("lapic_timer_advance_ns", 0444,
> vcpu->debugfs_dentry,
> vcpu, &vcpu_timer_advance_ns_fops);
> if (!ret)
> @@ -67,12 +67,12 @@ int kvm_arch_create_vcpu_debugfs(struct kvm_vcpu *vcpu)
> }
>
> if (kvm_has_tsc_control) {
> - ret = debugfs_create_file("tsc-scaling-ratio", 0444,
> + ret = debugfs_create_file_unsafe("tsc-scaling-ratio", 0444,
> vcpu->debugfs_dentry,
> vcpu, &vcpu_tsc_scaling_fops);
> if (!ret)
> return -ENOMEM;
> - ret = debugfs_create_file("tsc-scaling-ratio-frac-bits", 0444,
> + ret = debugfs_create_file_unsafe("tsc-scaling-ratio-frac-bits", 0444,
> vcpu->debugfs_dentry,
> vcpu, &vcpu_tsc_scaling_frac_fops);
> if (!ret)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kvm: x86: Use DEFINE_DEBUGFS_ATTRIBUTE for debugfs files
2019-09-17 17:18 ` Paolo Bonzini
@ 2019-09-17 18:12 ` Greg Kroah-Hartman
2019-09-17 19:34 ` Paolo Bonzini
[not found] ` <201909180819440437759@zte.com.cn>
1 sibling, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2019-09-17 18:12 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Yi Wang, rkrcmar, tglx, mingo, bp, hpa, x86, kvm, linux-kernel,
xue.zhihong, up2wing, wang.liang82
On Tue, Sep 17, 2019 at 07:18:33PM +0200, Paolo Bonzini wrote:
> 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@zte.com.cn>
>
> It sucks though that you have to use a function with "unsafe" in the name.
I agree, why make this change?
> Greg, is the patch doing the right thing?
I can't tell. What coccinelle script generated this patch?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kvm: x86: Use DEFINE_DEBUGFS_ATTRIBUTE for debugfs files
2019-09-17 18:12 ` Greg Kroah-Hartman
@ 2019-09-17 19:34 ` Paolo Bonzini
0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2019-09-17 19:34 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Yi Wang, rkrcmar, tglx, mingo, bp, hpa, x86, kvm, linux-kernel,
xue.zhihong, up2wing, wang.liang82
On 17/09/19 20:12, Greg Kroah-Hartman wrote:
> On Tue, Sep 17, 2019 at 07:18:33PM +0200, Paolo Bonzini wrote:
>> 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@zte.com.cn>
>>
>> It sucks though that you have to use a function with "unsafe" in the name.
>
> I agree, why make this change?
>
>> Greg, is the patch doing the right thing?
>
> I can't tell. What coccinelle script generated this patch?
Seems to be scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci.
//# Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
//# imposes some significant overhead as compared to
//# DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
Paolo
> thanks,
>
> greg k-h
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kvm: x86: Use DEFINE_DEBUGFS_ATTRIBUTE for debugfs files
[not found] ` <201909180819440437759@zte.com.cn>
@ 2019-09-18 12:03 ` Greg KH
0 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2019-09-18 12:03 UTC (permalink / raw)
To: wang.yi59
Cc: pbonzini, rkrcmar, tglx, mingo, bp, hpa, x86, kvm, linux-kernel,
xue.zhihong, up2wing, wang.liang82
On Wed, Sep 18, 2019 at 08:19:44AM +0800, wang.yi59@zte.com.cn 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@zte.com.cn>
> >
> > 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-18 12:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-22 7:33 [PATCH] kvm: x86: Use DEFINE_DEBUGFS_ATTRIBUTE for debugfs files Yi Wang
2019-09-17 17:18 ` Paolo Bonzini
2019-09-17 18:12 ` Greg Kroah-Hartman
2019-09-17 19:34 ` Paolo Bonzini
[not found] ` <201909180819440437759@zte.com.cn>
2019-09-18 12:03 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).