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 <zhangzihuan@kylinos.cn>
---
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)
@@ -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);
+ }
+
+ 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;
}
--
2.25.1
On Fri, Sep 5, 2025 at 3:24 PM Zihuan Zhang <zhangzihuan@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 <zhangzihuan@kylinos.cn> > --- > 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) > @@ -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); > + } > + > + 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; > } > -- Applied as 6.18 material under a new subject, with some minor edits in the changelog, and with some minor whitespace breakage fixed. Thanks!
On Fri, Sep 5, 2025 at 3:24 PM Zihuan Zhang <zhangzihuan@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 <zhangzihuan@kylinos.cn> > --- > 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. > @@ -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. > + > + 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; > } > --
在 2025/9/6 04:17, Rafael J. Wysocki 写道: > On Fri, Sep 5, 2025 at 3:24 PM Zihuan Zhang <zhangzihuan@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 <zhangzihuan@kylinos.cn> >> --- >> 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. > >> @@ -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. Thanks! I’ll follow your advice and handle the Fixes tag and whitespace issues. >> + >> + 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; >> } >> --
On Mon, Sep 8, 2025 at 11:16 AM Zihuan Zhang <zhangzihuan@kylinos.cn> wrote: > > > 在 2025/9/6 04:17, Rafael J. Wysocki 写道: > > On Fri, Sep 5, 2025 at 3:24 PM Zihuan Zhang <zhangzihuan@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 <zhangzihuan@kylinos.cn> > >> --- > >> 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. > > > >> @@ -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. > > Thanks! > > I’ll follow your advice and handle the Fixes tag and whitespace issues. Actually, no need to resend. The current code is correct as it registers and unregisters the freq QoS request from cpufreq policy notifiers, so the policy is guaranteed to be there when cpufreq_set_cur_state() runs.
© 2016 - 2025 Red Hat, Inc.