| Message ID | 20231201123205.1996790-1-lukasz.luba@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | powercap: DTPM: Fix the missing cpufreq_cpu_put() calls | expand |
On Fri, Dec 01, 2023 at 12:32:05PM +0000, Lukasz Luba wrote: > The policy returned by cpufreq_cpu_get() has to be released with > the help of cpufreq_cpu_put() to balance its kobject reference counter > properly. > > Add the missing calls to cpufreq_cpu_put() in the code. > > Fixes: 0aea2e4ec2a2 ("powercap/dtpm_cpu: Reset per_cpu variable in the release function") > Fixes: 0e8f68d7f048 ("powercap/drivers/dtpm: Add CPU energy model based support") > Cc: <stable@vger.kernel.org> # v5.10+ But the Fixes: tags are for commits that are only in 5.12 and newer, how can this be relevant for 5.10? thanks, greg k-h
Hi Greg, On 12/1/23 12:44, Greg KH wrote: > On Fri, Dec 01, 2023 at 12:32:05PM +0000, Lukasz Luba wrote: >> The policy returned by cpufreq_cpu_get() has to be released with >> the help of cpufreq_cpu_put() to balance its kobject reference counter >> properly. >> >> Add the missing calls to cpufreq_cpu_put() in the code. >> >> Fixes: 0aea2e4ec2a2 ("powercap/dtpm_cpu: Reset per_cpu variable in the release function") >> Fixes: 0e8f68d7f048 ("powercap/drivers/dtpm: Add CPU energy model based support") >> Cc: <stable@vger.kernel.org> # v5.10+ > > But the Fixes: tags are for commits that are only in 5.12 and newer, how > can this be relevant for 5.10? My apologies, you're right. Somehow I checked that this dtpm_cpu.c was introduced in v5.10. It was in v5.12 indeed. I messed that up. Also, the code in that v5.12 had different implementation and there was a function cpuhp_dtpm_cpu_offline() which had the cpufreq_cpu_get(). I can craft for that v5.12 special extra patch fix addressing it and send directly to stable list. Would that make sense? So this patch would only be applicable for v5.16+ AFAICS. Regards, Lukasz
On Fri, Dec 1, 2023 at 1:31 PM Lukasz Luba <lukasz.luba@arm.com> wrote: > > The policy returned by cpufreq_cpu_get() has to be released with > the help of cpufreq_cpu_put() to balance its kobject reference counter > properly. > > Add the missing calls to cpufreq_cpu_put() in the code. > > Fixes: 0aea2e4ec2a2 ("powercap/dtpm_cpu: Reset per_cpu variable in the release function") > Fixes: 0e8f68d7f048 ("powercap/drivers/dtpm: Add CPU energy model based support") > Cc: <stable@vger.kernel.org> # v5.10+ > Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> > --- > drivers/powercap/dtpm_cpu.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/powercap/dtpm_cpu.c b/drivers/powercap/dtpm_cpu.c > index 45bb7e2849d7..aac278e162d9 100644 > --- a/drivers/powercap/dtpm_cpu.c > +++ b/drivers/powercap/dtpm_cpu.c > @@ -152,6 +152,8 @@ static void pd_release(struct dtpm *dtpm) > if (policy) { > for_each_cpu(dtpm_cpu->cpu, policy->related_cpus) > per_cpu(dtpm_per_cpu, dtpm_cpu->cpu) = NULL; > + > + cpufreq_cpu_put(policy); > } > > kfree(dtpm_cpu); > @@ -204,12 +206,16 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent) > return 0; > > pd = em_cpu_get(cpu); > - if (!pd || em_is_artificial(pd)) > - return -EINVAL; > + if (!pd || em_is_artificial(pd)) { > + ret = -EINVAL; > + goto release_policy; > + } > > dtpm_cpu = kzalloc(sizeof(*dtpm_cpu), GFP_KERNEL); > - if (!dtpm_cpu) > - return -ENOMEM; > + if (!dtpm_cpu) { > + ret = -ENOMEM; > + goto release_policy; > + } > > dtpm_init(&dtpm_cpu->dtpm, &dtpm_ops); > dtpm_cpu->cpu = cpu; > @@ -231,6 +237,7 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent) > if (ret) > goto out_dtpm_unregister; > > + cpufreq_cpu_put(policy); > return 0; > > out_dtpm_unregister: > @@ -242,6 +249,8 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent) > per_cpu(dtpm_per_cpu, cpu) = NULL; > kfree(dtpm_cpu); > > +release_policy: > + cpufreq_cpu_put(policy); > return ret; > } > > -- Applied as 6.7-rc material with the Cc: stable tag fixed. Thanks!
On 12/5/23 19:54, Rafael J. Wysocki wrote: > On Fri, Dec 1, 2023 at 1:31 PM Lukasz Luba <lukasz.luba@arm.com> wrote: >> >> The policy returned by cpufreq_cpu_get() has to be released with >> the help of cpufreq_cpu_put() to balance its kobject reference counter >> properly. >> >> Add the missing calls to cpufreq_cpu_put() in the code. >> >> Fixes: 0aea2e4ec2a2 ("powercap/dtpm_cpu: Reset per_cpu variable in the release function") >> Fixes: 0e8f68d7f048 ("powercap/drivers/dtpm: Add CPU energy model based support") >> Cc: <stable@vger.kernel.org> # v5.10+ >> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> >> --- >> drivers/powercap/dtpm_cpu.c | 17 +++++++++++++---- >> 1 file changed, 13 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/powercap/dtpm_cpu.c b/drivers/powercap/dtpm_cpu.c >> index 45bb7e2849d7..aac278e162d9 100644 >> --- a/drivers/powercap/dtpm_cpu.c >> +++ b/drivers/powercap/dtpm_cpu.c >> @@ -152,6 +152,8 @@ static void pd_release(struct dtpm *dtpm) >> if (policy) { >> for_each_cpu(dtpm_cpu->cpu, policy->related_cpus) >> per_cpu(dtpm_per_cpu, dtpm_cpu->cpu) = NULL; >> + >> + cpufreq_cpu_put(policy); >> } >> >> kfree(dtpm_cpu); >> @@ -204,12 +206,16 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent) >> return 0; >> >> pd = em_cpu_get(cpu); >> - if (!pd || em_is_artificial(pd)) >> - return -EINVAL; >> + if (!pd || em_is_artificial(pd)) { >> + ret = -EINVAL; >> + goto release_policy; >> + } >> >> dtpm_cpu = kzalloc(sizeof(*dtpm_cpu), GFP_KERNEL); >> - if (!dtpm_cpu) >> - return -ENOMEM; >> + if (!dtpm_cpu) { >> + ret = -ENOMEM; >> + goto release_policy; >> + } >> >> dtpm_init(&dtpm_cpu->dtpm, &dtpm_ops); >> dtpm_cpu->cpu = cpu; >> @@ -231,6 +237,7 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent) >> if (ret) >> goto out_dtpm_unregister; >> >> + cpufreq_cpu_put(policy); >> return 0; >> >> out_dtpm_unregister: >> @@ -242,6 +249,8 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent) >> per_cpu(dtpm_per_cpu, cpu) = NULL; >> kfree(dtpm_cpu); >> >> +release_policy: >> + cpufreq_cpu_put(policy); >> return ret; >> } >> >> -- > > Applied as 6.7-rc material with the Cc: stable tag fixed. > > Thanks! > Thank you Rafael!
diff --git a/drivers/powercap/dtpm_cpu.c b/drivers/powercap/dtpm_cpu.c index 45bb7e2849d7..aac278e162d9 100644 --- a/drivers/powercap/dtpm_cpu.c +++ b/drivers/powercap/dtpm_cpu.c @@ -152,6 +152,8 @@ static void pd_release(struct dtpm *dtpm) if (policy) { for_each_cpu(dtpm_cpu->cpu, policy->related_cpus) per_cpu(dtpm_per_cpu, dtpm_cpu->cpu) = NULL; + + cpufreq_cpu_put(policy); } kfree(dtpm_cpu); @@ -204,12 +206,16 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent) return 0; pd = em_cpu_get(cpu); - if (!pd || em_is_artificial(pd)) - return -EINVAL; + if (!pd || em_is_artificial(pd)) { + ret = -EINVAL; + goto release_policy; + } dtpm_cpu = kzalloc(sizeof(*dtpm_cpu), GFP_KERNEL); - if (!dtpm_cpu) - return -ENOMEM; + if (!dtpm_cpu) { + ret = -ENOMEM; + goto release_policy; + } dtpm_init(&dtpm_cpu->dtpm, &dtpm_ops); dtpm_cpu->cpu = cpu; @@ -231,6 +237,7 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent) if (ret) goto out_dtpm_unregister; + cpufreq_cpu_put(policy); return 0; out_dtpm_unregister: @@ -242,6 +249,8 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent) per_cpu(dtpm_per_cpu, cpu) = NULL; kfree(dtpm_cpu); +release_policy: + cpufreq_cpu_put(policy); return ret; }
The policy returned by cpufreq_cpu_get() has to be released with the help of cpufreq_cpu_put() to balance its kobject reference counter properly. Add the missing calls to cpufreq_cpu_put() in the code. Fixes: 0aea2e4ec2a2 ("powercap/dtpm_cpu: Reset per_cpu variable in the release function") Fixes: 0e8f68d7f048 ("powercap/drivers/dtpm: Add CPU energy model based support") Cc: <stable@vger.kernel.org> # v5.10+ Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> --- drivers/powercap/dtpm_cpu.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)