Re: [PATCH v5 2/6] ACPI: processor: thermal: Use scope-based cleanup helper
From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: 2025-09-05 20:17:39
Also in:
dri-devel, imx, intel-gfx, linux-acpi, linux-arm-kernel, linux-omap, linux-pm, lkml
On Fri, Sep 5, 2025 at 3:24 PM Zihuan Zhang [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted> --- drivers/acpi/processor_thermal.c | 52 +++++++++++++++++--------------- 1 file changed, 27 insertions(+), 25 deletions(-)diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c index 1219adb11ab9..460713d1414a 100644 --- a/drivers/acpi/processor_thermal.c +++ b/drivers/acpi/processor_thermal.c@@ -62,19 +62,14 @@ static int phys_package_first_cpu(int cpu) return 0; } -static int cpu_has_cpufreq(unsigned int cpu) +static bool cpu_has_cpufreq(unsigned int cpu) { - struct cpufreq_policy *policy; - if (!acpi_processor_cpufreq_init) return 0; - policy = cpufreq_cpu_get(cpu); - if (policy) { - cpufreq_cpu_put(policy); - return 1; - } - return 0; + struct cpufreq_policy *policy __free(put_cpufreq_policy) = cpufreq_cpu_get(cpu); + + return policy != NULL; } static int cpufreq_get_max_state(unsigned int cpu)
The changes above are fine and can be sent as a separate patch.
quoted hunk ↗ jump to hunk
@@ -93,12 +88,31 @@ static int cpufreq_get_cur_state(unsigned int cpu) return reduction_step(cpu); } +static bool cpufreq_update_thermal_limit(unsigned int cpu, struct acpi_processor *pr) +{ + unsigned long max_freq; + int ret; + struct cpufreq_policy *policy __free(put_cpufreq_policy) = cpufreq_cpu_get(cpu); + + if (!policy) + return false; + + max_freq = (policy->cpuinfo.max_freq * + (100 - reduction_step(cpu) * cpufreq_thermal_reduction_pctg)) / 100; + + ret = freq_qos_update_request(&pr->thermal_req, max_freq); + if (ret < 0) { + pr_warn("Failed to update thermal freq constraint: CPU%d (%d)\n", + pr->id, ret); + }
But this silently fixes a bug in the original code which needs to be documented with a Fixes: tag (and it would be better to fix the bug separately before the using the __free()-based cleanup TBH) and introduces some whitespace breakage.
quoted hunk ↗ jump to hunk
+ + return true; +} + static int cpufreq_set_cur_state(unsigned int cpu, int state) { - struct cpufreq_policy *policy; struct acpi_processor *pr; - unsigned long max_freq; - int i, ret; + int i; if (!cpu_has_cpufreq(cpu)) return 0;@@ -120,20 +134,8 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state) if (unlikely(!freq_qos_request_active(&pr->thermal_req))) continue; - policy = cpufreq_cpu_get(i); - if (!policy) + if (!cpufreq_update_thermal_limit(i, pr)) return -EINVAL; - - max_freq = (policy->cpuinfo.max_freq * - (100 - reduction_step(i) * cpufreq_thermal_reduction_pctg)) / 100; - - cpufreq_cpu_put(policy); - - ret = freq_qos_update_request(&pr->thermal_req, max_freq); - if (ret < 0) { - pr_warn("Failed to update thermal freq constraint: CPU%d (%d)\n", - pr->id, ret); - } } return 0; } --