在 2025/9/3 21:45, Rafael J. Wysocki 写道:
On Wed, Sep 3, 2025 at 3:18 PM Zihuan Zhang <zhangzih...@kylinos.cn> wrote:Replace the manual cpufreq_cpu_put() with __free(put_cpufreq_policy) annotation for policy references. This reduces the risk of reference counting mistakes and aligns the code with the latest kernel style. No functional change intended. Signed-off-by: Zihuan Zhang <zhangzih...@kylinos.cn> --- drivers/powercap/dtpm_cpu.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/drivers/powercap/dtpm_cpu.c b/drivers/powercap/dtpm_cpu.c index 99390ec1481f..f76594185fa2 100644 --- a/drivers/powercap/dtpm_cpu.c +++ b/drivers/powercap/dtpm_cpu.c @@ -144,19 +144,17 @@ static int update_pd_power_uw(struct dtpm *dtpm) static void pd_release(struct dtpm *dtpm) { struct dtpm_cpu *dtpm_cpu = to_dtpm_cpu(dtpm); - struct cpufreq_policy *policy; if (freq_qos_request_active(&dtpm_cpu->qos_req)) freq_qos_remove_request(&dtpm_cpu->qos_req); - policy = cpufreq_cpu_get(dtpm_cpu->cpu); - if (policy) { + struct cpufreq_policy *policy __free(put_cpufreq_policy) = + cpufreq_cpu_get(dtpm_cpu->cpu); + + 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); } @@ -192,7 +190,6 @@ static int cpuhp_dtpm_cpu_online(unsigned int cpu) static int __dtpm_cpu_setup(int cpu, struct dtpm *parent) { struct dtpm_cpu *dtpm_cpu; - struct cpufreq_policy *policy; struct em_perf_state *table; struct em_perf_domain *pd; char name[CPUFREQ_NAME_LEN]; @@ -202,21 +199,19 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent) if (dtpm_cpu) return 0; - policy = cpufreq_cpu_get(cpu); + struct cpufreq_policy *policy __free(put_cpufreq_policy) = + cpufreq_cpu_get(cpu); + if (!policy) return 0; pd = em_cpu_get(cpu); - if (!pd || em_is_artificial(pd)) { - ret = -EINVAL; - goto release_policy; - } + if (!pd || em_is_artificial(pd)) + return -EINVAL; dtpm_cpu = kzalloc(sizeof(*dtpm_cpu), GFP_KERNEL); - if (!dtpm_cpu) { - ret = -ENOMEM; - goto release_policy; - } + if (!dtpm_cpu) + return -ENOMEM; dtpm_init(&dtpm_cpu->dtpm, &dtpm_ops); dtpm_cpu->cpu = cpu; @@ -239,7 +234,6 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent) if (ret < 0) goto out_dtpm_unregister;So this change kind of goes against another recommendation given in cleanup.h: * Lastly, given that the benefit of cleanup helpers is removal of * "goto", and that the "goto" statement can jump between scopes, the * expectation is that usage of "goto" and cleanup helpers is never * mixed in the same function. I.e. for a given routine, convert all * resources that need a "goto" cleanup to scope-based cleanup, or * convert none of them.
Should I replace all the memory allocation cleanups here with `__free`? That would allow us to drop all the `goto`s, but since this function has quite a few of them, I’m concerned it might introduce new issues. What’s your recommendation? Thanks!
- cpufreq_cpu_put(policy); return 0; out_dtpm_unregister: @@ -251,8 +245,6 @@ 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; } --
