linux-kernel - Re: [PATCH v5 2/6] ACPI: processor: thermal: Use scope-based cleanup helper

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0h9pYXi4Op4zAA+Q7QZz5gkJg+83eKUCqM-YKh76CVLKg@mail.gmail.com>
Date: Mon, 8 Sep 2025 19:44:01 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Zihuan Zhang <zhangzihuan@...inos.cn>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Viresh Kumar <viresh.kumar@...aro.org>, 
	Jonathan Cameron <jonathan.cameron@...wei.com>, Catalin Marinas <catalin.marinas@....com>, 
	Will Deacon <will@...nel.org>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, 
	Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>, Michael Ellerman <mpe@...erman.id.au>, 
	Krzysztof Kozlowski <krzk@...nel.org>, Alim Akhtar <alim.akhtar@...sung.com>, 
	Thierry Reding <thierry.reding@...il.com>, MyungJoo Ham <myungjoo.ham@...sung.com>, 
	Kyungmin Park <kyungmin.park@...sung.com>, Chanwoo Choi <cw00.choi@...sung.com>, 
	Jani Nikula <jani.nikula@...ux.intel.com>, Rodrigo Vivi <rodrigo.vivi@...el.com>, 
	Tvrtko Ursulin <tursulin@...ulin.net>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, Daniel Lezcano <daniel.lezcano@...nel.org>, 
	Sascha Hauer <s.hauer@...gutronix.de>, Shawn Guo <shawnguo@...nel.org>, 
	Eduardo Valentin <edubezval@...il.com>, Keerthy <j-keerthy@...com>, Ben Horgan <ben.horgan@....com>, 
	zhenglifeng <zhenglifeng1@...wei.com>, Zhang Rui <rui.zhang@...el.com>, 
	Len Brown <lenb@...nel.org>, Lukasz Luba <lukasz.luba@....com>, 
	Pengutronix Kernel Team <kernel@...gutronix.de>, Beata Michalska <beata.michalska@....com>, 
	Fabio Estevam <festevam@...il.com>, Pavel Machek <pavel@...nel.org>, Sumit Gupta <sumitg@...dia.com>, 
	Prasanna Kumar T S M <ptsm@...ux.microsoft.com>, Sudeep Holla <sudeep.holla@....com>, 
	Yicong Yang <yangyicong@...ilicon.com>, linux-pm@...r.kernel.org, 
	linux-acpi@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org, 
	linux-arm-kernel@...ts.infradead.org, intel-gfx@...ts.freedesktop.org, 
	dri-devel@...ts.freedesktop.org, imx@...ts.linux.dev, 
	linux-omap@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 2/6] ACPI: processor: thermal: Use scope-based cleanup helper

On Mon, Sep 8, 2025 at 11:16 AM Zihuan Zhang <zhangzihuan@...inos.cn> wrote:
>
>
> 在 2025/9/6 04:17, Rafael J. Wysocki 写道:
> > On Fri, Sep 5, 2025 at 3:24 PM Zihuan Zhang <zhangzihuan@...inos.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@...inos.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.