linux-kernel - Re: [PATCH V3] cpufreq: Fix per-policy boost behavior on SoCs using cpufreq_boost_set

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] [day] [month] [year] [list]

Message-ID: <CAJZ5v0iLvq_4mNTTNw9eXZL4rG25jpRuWAKvY_7HZvLDRmGZNw@mail.gmail.com>
Date: Wed, 13 Mar 2024 20:58:04 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Sibi Sankar <quic_sibis@...cinc.com>
Cc: dietmar.eggemann@....com, rafael@...nel.org, viresh.kumar@...aro.org, 
	xuwei5@...ilicon.com, zhanjie9@...ilicon.com, sudeep.holla@....com, 
	cristian.marussi@....com, linux-kernel@...r.kernel.org, 
	linux-arm-msm@...r.kernel.org, quic_rgottimu@...cinc.com, 
	linux-arm-kernel@...ts.infradead.org, linux-pm@...r.kernel.org, d-gole@...com
Subject: Re: [PATCH V3] cpufreq: Fix per-policy boost behavior on SoCs using cpufreq_boost_set_sw

On Tue, Mar 12, 2024 at 11:37 AM Sibi Sankar <quic_sibis@...cinc.com> wrote:
>
> In the existing code, per-policy flags doesn't have any impact i.e.
> if cpufreq_driver boost is enabled and one or more of the per-policy
> boost is disabled, the cpufreq driver will behave as if boost is
> enabled. Fix this by incorporating per-policy boost flag in the policy->max
> calculus used in cpufreq_frequency_table_cpuinfo and setting the default
> per-policy boost to mirror the cpufreq_driver boost flag.
>
> Fixes: 218a06a79d9a ("cpufreq: Support per-policy performance boost")
> Reported-by: Dietmar Eggemann <dietmar.eggemann@....com>
> Reviewed-by: Viresh Kumar <viresh.kumar@...aro.org>
> Reviewed-by: Dhruva Gole <d-gole@...com>
> Signed-off-by: Sibi Sankar <quic_sibis@...cinc.com>
> ---
>
> v3:
> * Pickup Rbs.
> * Simplify per-policy boost setting. [Viresh]
>
> v2:
> * Enable per-policy boost flag in the core instead. [Viresh]
> * Add more details regarding the bug. [Viresh]
> * Drop cover-letter and patch 2.
>
> Logs reported-by Dietmar Eggemann:
> https://lore.kernel.org/lkml/265e5f2c-9b45-420f-89b1-44369aeb8418@arm.com/
>
>  drivers/cpufreq/cpufreq.c    | 18 ++++++++++++------
>  drivers/cpufreq/freq_table.c |  2 +-
>  2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index f6f8d7f450e7..66e10a19d76a 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -653,14 +653,16 @@ static ssize_t store_local_boost(struct cpufreq_policy *policy,
>         if (policy->boost_enabled == enable)
>                 return count;
>
> +       policy->boost_enabled = enable;
> +
>         cpus_read_lock();
>         ret = cpufreq_driver->set_boost(policy, enable);
>         cpus_read_unlock();
>
> -       if (ret)
> +       if (ret) {
> +               policy->boost_enabled = !policy->boost_enabled;
>                 return ret;
> -
> -       policy->boost_enabled = enable;
> +       }
>
>         return count;
>  }
> @@ -1428,6 +1430,9 @@ static int cpufreq_online(unsigned int cpu)
>                         goto out_free_policy;
>                 }
>
> +               /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> +               policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy);
> +
>                 /*
>                  * The initialization has succeeded and the policy is online.
>                  * If there is a problem with its frequency table, take it
> @@ -2769,11 +2774,12 @@ int cpufreq_boost_trigger_state(int state)
>
>         cpus_read_lock();
>         for_each_active_policy(policy) {
> +               policy->boost_enabled = state;
>                 ret = cpufreq_driver->set_boost(policy, state);
> -               if (ret)
> +               if (ret) {
> +                       policy->boost_enabled = !policy->boost_enabled;
>                         goto err_reset_state;
> -
> -               policy->boost_enabled = state;
> +               }
>         }
>         cpus_read_unlock();
>
> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
> index c4d4643b6ca6..c17dc51a5a02 100644
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -40,7 +40,7 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
>         cpufreq_for_each_valid_entry(pos, table) {
>                 freq = pos->frequency;
>
> -               if (!cpufreq_boost_enabled()
> +               if ((!cpufreq_boost_enabled() || !policy->boost_enabled)
>                     && (pos->flags & CPUFREQ_BOOST_FREQ))
>                         continue;
>
> --

Applied as 6.9-rc material, thanks!