linux-kernel - Re: [PATCH] cpufreq: acpi: Don't enable boost on policy exit

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20250422094128.wlmr7u4qitwxiniz@vireshk-i7>
Date: Tue, 22 Apr 2025 15:11:28 +0530
From: Viresh Kumar <viresh.kumar@...aro.org>
To: "zhenglifeng (A)" <zhenglifeng1@...wei.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
	Nicholas Chin <nic.c3.14@...il.com>, linux-kernel@...r.kernel.org,
	linux-pm@...r.kernel.org, rafael.j.wysocki@...el.com,
	vincent.guittot@...aro.org
Subject: Re: [PATCH] cpufreq: acpi: Don't enable boost on policy exit

On 21-04-25, 21:36, zhenglifeng (A) wrote:
> On 2025/4/21 19:37, Viresh Kumar wrote:
> > +static int policy_set_boost(struct cpufreq_policy *policy, bool enable, bool forced)
> > +{
> > +       if (!forced && (policy->boost_enabled == enable))
> > +               return 0;
> > +
> > +       policy->boost_enabled = enable;
> > +
> > +       ret = cpufreq_driver->set_boost(policy, enable);
> > +       if (ret)
> > +               policy->boost_enabled = !policy->boost_enabled;
> 
> This may cause boost_enabled becomes false but actually boosted when forced
> is true and trying to set boost_enabled from true to true.

Can't do much in case of failure. And this is the current behavior
anyway. This was just some code cleanup, doesn't change the behavior
of the code.

> >  static struct freq_attr local_boost = __ATTR(boost, 0644, show_local_boost, store_local_boost);
> > @@ -1617,16 +1624,17 @@ static int cpufreq_online(unsigned int cpu)
> >         if (new_policy && cpufreq_thermal_control_enabled(cpufreq_driver))
> >                 policy->cdev = of_cpufreq_cooling_register(policy);
> > 
> > -       /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> > +       /*
> > +        * Let the per-policy boost flag mirror the cpufreq_driver boost during
> > +        * initialization for a new policy. For an existing policy, maintain the
> > +        * previous boost value unless global boost is disabled now.
> > +        */
> >         if (cpufreq_driver->set_boost && policy->boost_supported &&
> > -           policy->boost_enabled != cpufreq_boost_enabled()) {
> > -               policy->boost_enabled = cpufreq_boost_enabled();
> > -               ret = cpufreq_driver->set_boost(policy, policy->boost_enabled);
> > +           (new_policy || !cpufreq_boost_enabled())) {
> > +               ret = policy_set_boost(policy, cpufreq_boost_enabled(), false);
> 
> I think forced here should be true. If new_policy and
> !cpufreq_boost_enabled() but the cpu is actually boosted by some other
> reason (like what we met in acpi-cpufreq), set_boost() should be forcibly
> executed to make the cpu unboost.

The problem is that setting boost may be time consuming for some
platforms and we may not want to do that unnecessarily. ACPI cpufreq
should be fixed separately for that.

I am sending a series now to fix them all, please review that.

-- 
viresh