* [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings
@ 2018-03-29 19:12 Julia Lawall
2018-03-29 19:11 ` Francisco Jerez
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Julia Lawall @ 2018-03-29 19:12 UTC (permalink / raw)
To: Francisco Jerez
Cc: kbuild-all, Srinivas Pandruvada, Len Brown, Rafael J. Wysocki,
Viresh Kumar, linux-pm, linux-kernel, 0day robot
Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
for debugfs files.
Semantic patch information:
Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
imposes some significant overhead as compared to
DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
Generated by: scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
Fixes: 9eec7989e762 ("OPTIONAL: cpufreq/intel_pstate: Expose LP controller parameters via debugfs.")
CC: Francisco Jerez <currojerez@riseup.net>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>
---
I don't actually know anything about this issue. The change was suggested
by kbuild.
intel_pstate.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -885,7 +885,7 @@ static int lp_param_get(void *data, u64
*val = *(u32 *)data;
return 0;
}
-DEFINE_SIMPLE_ATTRIBUTE(fops_lp_param, lp_param_get, lp_param_set, "%llu\n");
+DEFINE_DEBUGFS_ATTRIBUTE(fops_lp_param, lp_param_get, lp_param_set, "%llu\n");
static struct dentry *debugfs_parent;
@@ -922,9 +922,10 @@ static void intel_pstate_debug_expose_pa
for (i = 0; lp_files[i].name; i++) {
struct dentry *dentry;
- dentry = debugfs_create_file(lp_files[i].name, 0660,
- debugfs_parent, lp_files[i].value,
- &fops_lp_param);
+ dentry = debugfs_create_file_unsafe(lp_files[i].name, 0660,
+ debugfs_parent,
+ lp_files[i].value,
+ &fops_lp_param);
if (!IS_ERR(dentry))
lp_files[i].dentry = dentry;
}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings
2018-03-29 19:12 [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings Julia Lawall
@ 2018-03-29 19:11 ` Francisco Jerez
2018-03-29 19:31 ` [kbuild-all] " Fabio Estevam
2018-03-30 9:51 ` Rafael J. Wysocki
2 siblings, 0 replies; 10+ messages in thread
From: Francisco Jerez @ 2018-03-29 19:11 UTC (permalink / raw)
To: Julia Lawall
Cc: kbuild-all, Srinivas Pandruvada, Len Brown, Rafael J. Wysocki,
Viresh Kumar, linux-pm, linux-kernel, 0day robot
[-- Attachment #1.1: Type: text/plain, Size: 1810 bytes --]
Looks okay to me, I'll squash this into the original patch.
Julia Lawall <julia.lawall@lip6.fr> writes:
> Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
> for debugfs files.
>
> Semantic patch information:
> Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
> imposes some significant overhead as compared to
> DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
>
> Generated by: scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
>
> Fixes: 9eec7989e762 ("OPTIONAL: cpufreq/intel_pstate: Expose LP controller parameters via debugfs.")
> CC: Francisco Jerez <currojerez@riseup.net>
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>
> ---
>
> I don't actually know anything about this issue. The change was suggested
> by kbuild.
>
> intel_pstate.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -885,7 +885,7 @@ static int lp_param_get(void *data, u64
> *val = *(u32 *)data;
> return 0;
> }
> -DEFINE_SIMPLE_ATTRIBUTE(fops_lp_param, lp_param_get, lp_param_set, "%llu\n");
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_lp_param, lp_param_get, lp_param_set, "%llu\n");
>
> static struct dentry *debugfs_parent;
>
> @@ -922,9 +922,10 @@ static void intel_pstate_debug_expose_pa
> for (i = 0; lp_files[i].name; i++) {
> struct dentry *dentry;
>
> - dentry = debugfs_create_file(lp_files[i].name, 0660,
> - debugfs_parent, lp_files[i].value,
> - &fops_lp_param);
> + dentry = debugfs_create_file_unsafe(lp_files[i].name, 0660,
> + debugfs_parent,
> + lp_files[i].value,
> + &fops_lp_param);
> if (!IS_ERR(dentry))
> lp_files[i].dentry = dentry;
> }
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kbuild-all] [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings
2018-03-29 19:12 [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings Julia Lawall
2018-03-29 19:11 ` Francisco Jerez
@ 2018-03-29 19:31 ` Fabio Estevam
2018-03-29 19:23 ` Francisco Jerez
2018-03-29 19:44 ` Julia Lawall
2018-03-30 9:51 ` Rafael J. Wysocki
2 siblings, 2 replies; 10+ messages in thread
From: Fabio Estevam @ 2018-03-29 19:31 UTC (permalink / raw)
To: Julia Lawall
Cc: Francisco Jerez, linux-pm, Viresh Kumar, Rafael J. Wysocki,
linux-kernel, kbuild-all, Srinivas Pandruvada, 0day robot,
Len Brown
Hi Julia,
On Thu, Mar 29, 2018 at 4:12 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
> for debugfs files.
>
> Semantic patch information:
> Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
> imposes some significant overhead as compared to
> DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
Just curious: could you please expand on what "imposes some
significant overhead" means?
Thanks
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kbuild-all] [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings
2018-03-29 19:31 ` [kbuild-all] " Fabio Estevam
@ 2018-03-29 19:23 ` Francisco Jerez
2018-03-29 19:44 ` Julia Lawall
1 sibling, 0 replies; 10+ messages in thread
From: Francisco Jerez @ 2018-03-29 19:23 UTC (permalink / raw)
To: Fabio Estevam, Julia Lawall
Cc: linux-pm, Viresh Kumar, Rafael J. Wysocki, linux-kernel,
kbuild-all, Srinivas Pandruvada, 0day robot, Len Brown
[-- Attachment #1.1: Type: text/plain, Size: 731 bytes --]
Fabio Estevam <festevam@gmail.com> writes:
> Hi Julia,
>
> On Thu, Mar 29, 2018 at 4:12 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
>> for debugfs files.
>>
>> Semantic patch information:
>> Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
>> imposes some significant overhead as compared to
>> DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
>
> Just curious: could you please expand on what "imposes some
> significant overhead" means?
>
Probably negligible given that this code will only be run once at system
boot and then never used again in production systems. But I guess the
micro-optimization doesn't hurt either.
> Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kbuild-all] [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings
2018-03-29 19:31 ` [kbuild-all] " Fabio Estevam
2018-03-29 19:23 ` Francisco Jerez
@ 2018-03-29 19:44 ` Julia Lawall
2018-03-30 6:14 ` Nicolai Stange
1 sibling, 1 reply; 10+ messages in thread
From: Julia Lawall @ 2018-03-29 19:44 UTC (permalink / raw)
To: Fabio Estevam
Cc: Francisco Jerez, linux-pm, Viresh Kumar, Rafael J. Wysocki,
linux-kernel, kbuild-all, Srinivas Pandruvada, 0day robot,
Len Brown, Nicolai Stange
On Thu, 29 Mar 2018, Fabio Estevam wrote:
> Hi Julia,
>
> On Thu, Mar 29, 2018 at 4:12 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
> > for debugfs files.
> >
> > Semantic patch information:
> > Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
> > imposes some significant overhead as compared to
> > DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
>
> Just curious: could you please expand on what "imposes some
> significant overhead" means?
I don't know. I didn't write this rule. Nicolai, can you explain?
thanks,
julia
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kbuild-all] [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings
2018-03-29 19:44 ` Julia Lawall
@ 2018-03-30 6:14 ` Nicolai Stange
2018-03-30 6:22 ` Julia Lawall
0 siblings, 1 reply; 10+ messages in thread
From: Nicolai Stange @ 2018-03-30 6:14 UTC (permalink / raw)
To: Fabio Estevam
Cc: Julia Lawall, Francisco Jerez, linux-pm, Viresh Kumar,
Rafael J. Wysocki, linux-kernel, kbuild-all, Srinivas Pandruvada,
0day robot, Len Brown, Nicolai Stange
Julia Lawall <julia.lawall@lip6.fr> writes:
> On Thu, 29 Mar 2018, Fabio Estevam wrote:
>
>> Hi Julia,
>>
>> On Thu, Mar 29, 2018 at 4:12 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> > Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
>> > for debugfs files.
>> >
>> > Semantic patch information:
>> > Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
>> > imposes some significant overhead as compared to
>> > DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
>>
>> Just curious: could you please expand on what "imposes some
>> significant overhead" means?
>
> I don't know. I didn't write this rule. Nicolai, can you explain?
>From commit 49d200deaa68 ("debugfs: prevent access to removed files' private
data"):
Upon return of debugfs_remove()/debugfs_remove_recursive(), it might
still be attempted to access associated private file data through
previously opened struct file objects. If that data has been freed by
the caller of debugfs_remove*() in the meanwhile, the reading/writing
process would either encounter a fault or, if the memory address in
question has been reassigned again, unrelated data structures could get
overwritten.
[...]
Currently, there are ~1000 call sites of debugfs_create_file() spread
throughout the whole tree and touching all of those struct file_operations
in order to make them file removal aware by means of checking the result of
debugfs_use_file_start() from within their methods is unfeasible.
Instead, wrap the struct file_operations by a lifetime managing proxy at
file open [...]
The additional overhead comes in terms of additional memory needed: for
debugs files created through debugfs_create_file(), one such struct
file_operations proxy is allocated for each struct file instantiation,
c.f. full_proxy_open().
This was needed to "repair" the ~1000 call sites without touching them.
New debugfs users should make their file_operations removal aware
themselves by means of DEFINE_DEBUGFS_ATTRIBUTE() and signal that fact to
the debugfs core by instantiating them through
debugfs_create_file_unsafe().
See commit c64688081490 ("debugfs: add support for self-protecting
attribute file fops") for further information.
Thanks,
Nicolai
--
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton,
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kbuild-all] [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings
2018-03-30 6:14 ` Nicolai Stange
@ 2018-03-30 6:22 ` Julia Lawall
2018-03-30 15:33 ` Fabio Estevam
2018-03-31 4:20 ` Nicolai Stange
0 siblings, 2 replies; 10+ messages in thread
From: Julia Lawall @ 2018-03-30 6:22 UTC (permalink / raw)
To: Nicolai Stange
Cc: Fabio Estevam, Francisco Jerez, linux-pm, Viresh Kumar,
Rafael J. Wysocki, linux-kernel, kbuild-all, Srinivas Pandruvada,
0day robot, Len Brown
[-- Attachment #1: Type: text/plain, Size: 2766 bytes --]
On Fri, 30 Mar 2018, Nicolai Stange wrote:
> Julia Lawall <julia.lawall@lip6.fr> writes:
>
> > On Thu, 29 Mar 2018, Fabio Estevam wrote:
> >
> >> Hi Julia,
> >>
> >> On Thu, Mar 29, 2018 at 4:12 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >> > Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
> >> > for debugfs files.
> >> >
> >> > Semantic patch information:
> >> > Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
> >> > imposes some significant overhead as compared to
> >> > DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
> >>
> >> Just curious: could you please expand on what "imposes some
> >> significant overhead" means?
> >
> > I don't know. I didn't write this rule. Nicolai, can you explain?
>
> From commit 49d200deaa68 ("debugfs: prevent access to removed files' private
> data"):
>
> Upon return of debugfs_remove()/debugfs_remove_recursive(), it might
> still be attempted to access associated private file data through
> previously opened struct file objects. If that data has been freed by
> the caller of debugfs_remove*() in the meanwhile, the reading/writing
> process would either encounter a fault or, if the memory address in
> question has been reassigned again, unrelated data structures could get
> overwritten.
> [...]
> Currently, there are ~1000 call sites of debugfs_create_file() spread
> throughout the whole tree and touching all of those struct file_operations
> in order to make them file removal aware by means of checking the result of
> debugfs_use_file_start() from within their methods is unfeasible.
>
> Instead, wrap the struct file_operations by a lifetime managing proxy at
> file open [...]
>
> The additional overhead comes in terms of additional memory needed: for
> debugs files created through debugfs_create_file(), one such struct
> file_operations proxy is allocated for each struct file instantiation,
> c.f. full_proxy_open().
>
> This was needed to "repair" the ~1000 call sites without touching them.
>
> New debugfs users should make their file_operations removal aware
> themselves by means of DEFINE_DEBUGFS_ATTRIBUTE() and signal that fact to
> the debugfs core by instantiating them through
> debugfs_create_file_unsafe().
>
> See commit c64688081490 ("debugfs: add support for self-protecting
> attribute file fops") for further information.
Thanks. Perhaps it would be good to add a reference to this commit in
the message generated by the semantic patch.
Would it be sufficient to just apply the semantic patch everywhere and
submit the patches?
julia
>
>
> Thanks,
>
> Nicolai
>
>
> --
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton,
> HRB 21284 (AG Nürnberg)
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kbuild-all] [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings
2018-03-30 6:22 ` Julia Lawall
@ 2018-03-30 15:33 ` Fabio Estevam
2018-03-31 4:20 ` Nicolai Stange
1 sibling, 0 replies; 10+ messages in thread
From: Fabio Estevam @ 2018-03-30 15:33 UTC (permalink / raw)
To: Julia Lawall
Cc: Nicolai Stange, Francisco Jerez, linux-pm, Viresh Kumar,
Rafael J. Wysocki, linux-kernel, kbuild-all, Srinivas Pandruvada,
0day robot, Len Brown
On Fri, Mar 30, 2018 at 3:22 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> From commit 49d200deaa68 ("debugfs: prevent access to removed files' private
>> data"):
>>
>> Upon return of debugfs_remove()/debugfs_remove_recursive(), it might
>> still be attempted to access associated private file data through
>> previously opened struct file objects. If that data has been freed by
>> the caller of debugfs_remove*() in the meanwhile, the reading/writing
>> process would either encounter a fault or, if the memory address in
>> question has been reassigned again, unrelated data structures could get
>> overwritten.
>> [...]
>> Currently, there are ~1000 call sites of debugfs_create_file() spread
>> throughout the whole tree and touching all of those struct file_operations
>> in order to make them file removal aware by means of checking the result of
>> debugfs_use_file_start() from within their methods is unfeasible.
>>
>> Instead, wrap the struct file_operations by a lifetime managing proxy at
>> file open [...]
>>
>> The additional overhead comes in terms of additional memory needed: for
>> debugs files created through debugfs_create_file(), one such struct
>> file_operations proxy is allocated for each struct file instantiation,
>> c.f. full_proxy_open().
>>
>> This was needed to "repair" the ~1000 call sites without touching them.
>>
>> New debugfs users should make their file_operations removal aware
>> themselves by means of DEFINE_DEBUGFS_ATTRIBUTE() and signal that fact to
>> the debugfs core by instantiating them through
>> debugfs_create_file_unsafe().
>>
>> See commit c64688081490 ("debugfs: add support for self-protecting
>> attribute file fops") for further information.
Thanks for the detailed explanation, Nicolai!
> Thanks. Perhaps it would be good to add a reference to this commit in
> the message generated by the semantic patch.
Yes, that would be very helpful indeed.
Thanks
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kbuild-all] [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings
2018-03-30 6:22 ` Julia Lawall
2018-03-30 15:33 ` Fabio Estevam
@ 2018-03-31 4:20 ` Nicolai Stange
1 sibling, 0 replies; 10+ messages in thread
From: Nicolai Stange @ 2018-03-31 4:20 UTC (permalink / raw)
To: Julia Lawall
Cc: Nicolai Stange, Fabio Estevam, Francisco Jerez, linux-pm,
Viresh Kumar, Rafael J. Wysocki, linux-kernel, kbuild-all,
Srinivas Pandruvada, 0day robot, Len Brown
Julia Lawall <julia.lawall@lip6.fr> writes:
> On Fri, 30 Mar 2018, Nicolai Stange wrote:
>
>> Julia Lawall <julia.lawall@lip6.fr> writes:
>>
>> > On Thu, 29 Mar 2018, Fabio Estevam wrote:
>> >
>> >> Hi Julia,
>> >>
>> >> On Thu, Mar 29, 2018 at 4:12 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> >> > Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
>> >> > for debugfs files.
>> >> >
>> >> > Semantic patch information:
>> >> > Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
>> >> > imposes some significant overhead as compared to
>> >> > DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
>> >>
>> >> Just curious: could you please expand on what "imposes some
>> >> significant overhead" means?
>> >
>> > I don't know. I didn't write this rule. Nicolai, can you explain?
>>
>> From commit 49d200deaa68 ("debugfs: prevent access to removed files' private
>> data"):
>>
>> Upon return of debugfs_remove()/debugfs_remove_recursive(), it might
>> still be attempted to access associated private file data through
>> previously opened struct file objects. If that data has been freed by
>> the caller of debugfs_remove*() in the meanwhile, the reading/writing
>> process would either encounter a fault or, if the memory address in
>> question has been reassigned again, unrelated data structures could get
>> overwritten.
>> [...]
>> Currently, there are ~1000 call sites of debugfs_create_file() spread
>> throughout the whole tree and touching all of those struct file_operations
>> in order to make them file removal aware by means of checking the result of
>> debugfs_use_file_start() from within their methods is unfeasible.
>>
>> Instead, wrap the struct file_operations by a lifetime managing proxy at
>> file open [...]
>>
>> The additional overhead comes in terms of additional memory needed: for
>> debugs files created through debugfs_create_file(), one such struct
>> file_operations proxy is allocated for each struct file instantiation,
>> c.f. full_proxy_open().
>>
>> This was needed to "repair" the ~1000 call sites without touching them.
>>
>> New debugfs users should make their file_operations removal aware
>> themselves by means of DEFINE_DEBUGFS_ATTRIBUTE() and signal that fact to
>> the debugfs core by instantiating them through
>> debugfs_create_file_unsafe().
>>
>> See commit c64688081490 ("debugfs: add support for self-protecting
>> attribute file fops") for further information.
>
> Thanks. Perhaps it would be good to add a reference to this commit in
> the message generated by the semantic patch.
Thanks for doing this!
>
> Would it be sufficient to just apply the semantic patch everywhere and
> submit the patches?
In principle yes. But I'm note sure whether such a mass application is
worth it: the proxy allocation happens only at file open and the
expectation is that there aren't that many debugfs files kept open at a
time. OTOH, a struct file_operation consumes 256 bytes with
sizeof(long) == 8.
In my opinion, new users should avoid this overhead as it's easily
doable. For existing ones, I don't know.
Thanks,
Nicolai
--
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton,
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings
2018-03-29 19:12 [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings Julia Lawall
2018-03-29 19:11 ` Francisco Jerez
2018-03-29 19:31 ` [kbuild-all] " Fabio Estevam
@ 2018-03-30 9:51 ` Rafael J. Wysocki
2 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2018-03-30 9:51 UTC (permalink / raw)
To: Julia Lawall
Cc: Francisco Jerez, kbuild-all, Srinivas Pandruvada, Len Brown,
Viresh Kumar, linux-pm, linux-kernel, 0day robot
Hi Julia,
On Thursday, March 29, 2018 9:12:06 PM CEST Julia Lawall wrote:
> Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
> for debugfs files.
>
> Semantic patch information:
> Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
> imposes some significant overhead as compared to
> DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
>
> Generated by: scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
We've dropped the debugfs bits from intel_pstate entirely, so this change
is not applicable any more.
Thanks!
> Fixes: 9eec7989e762 ("OPTIONAL: cpufreq/intel_pstate: Expose LP controller parameters via debugfs.")
> CC: Francisco Jerez <currojerez@riseup.net>
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>
> ---
>
> I don't actually know anything about this issue. The change was suggested
> by kbuild.
>
> intel_pstate.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -885,7 +885,7 @@ static int lp_param_get(void *data, u64
> *val = *(u32 *)data;
> return 0;
> }
> -DEFINE_SIMPLE_ATTRIBUTE(fops_lp_param, lp_param_get, lp_param_set, "%llu\n");
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_lp_param, lp_param_get, lp_param_set, "%llu\n");
>
> static struct dentry *debugfs_parent;
>
> @@ -922,9 +922,10 @@ static void intel_pstate_debug_expose_pa
> for (i = 0; lp_files[i].name; i++) {
> struct dentry *dentry;
>
> - dentry = debugfs_create_file(lp_files[i].name, 0660,
> - debugfs_parent, lp_files[i].value,
> - &fops_lp_param);
> + dentry = debugfs_create_file_unsafe(lp_files[i].name, 0660,
> + debugfs_parent,
> + lp_files[i].value,
> + &fops_lp_param);
> if (!IS_ERR(dentry))
> lp_files[i].dentry = dentry;
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-03-31 4:20 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-29 19:12 [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings Julia Lawall
2018-03-29 19:11 ` Francisco Jerez
2018-03-29 19:31 ` [kbuild-all] " Fabio Estevam
2018-03-29 19:23 ` Francisco Jerez
2018-03-29 19:44 ` Julia Lawall
2018-03-30 6:14 ` Nicolai Stange
2018-03-30 6:22 ` Julia Lawall
2018-03-30 15:33 ` Fabio Estevam
2018-03-31 4:20 ` Nicolai Stange
2018-03-30 9:51 ` Rafael J. Wysocki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.