Re: [PATCH] cpufreq: Preserve global boost state on failed updates

[Zhongqiu Han]

On 11/5/2026 12:16 AM, Christian Loehle wrote:
> cpufreq_boost_trigger_state() restores the opposite of the requested
> state when updating policy boost state fails. For example, a failed
> write of 0 when global boost is already disabled enables the global
> flag without enabling any policies.
>
> Save the original global boost state using a mutex and restore it on
> failure.
>
> Fixes: 121baab7b88e ("cpufreq: Force sync policy boost with global boost on sysfs update")
> Fixes: 6f19efc0a1ca ("cpufreq: Add boost frequency support in core")

It seems the only appropriate Fixes tag here is 121baab7b88e ("cpufreq:
Force sync policy boost with global boost on sysfs update"). Commit
6f19efc0a1ca ("cpufreq: Add boost frequency support in core") did not
have this issue, since it returned early when the state was unchanged:

    if (cpufreq_driver->boost_enabled == state)
        return 0;

so restoring "!state" on failure was equivalent to restoring the old
value. 121baab7b88e removed that check, which is what introduced the
bug.

Also, is the new mutex meant to address a separate race between
concurrent sysfs writers? If so, it would be good to mention that in the
commit message or using separate patch?

> Signed-off-by: Christian Loehle <christian.loehle@xxxxxxx>
> ---
> On a platform without boost_supported the unfixed behaviour is:
> # cat /sys/devices/system/cpu/cpufreq/boost
> 0
> # echo 1 > /sys/devices/system/cpu/cpufreq/boost
> echo: write error: invalid argument
> # cat /sys/devices/system/cpu/cpufreq/boost
> 0
> # echo 0 > /sys/devices/system/cpu/cpufreq/boost
> echo: write error: invalid argument
> # cat /sys/devices/system/cpu/cpufreq/boost
> 1
>
>   drivers/cpufreq/cpufreq.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 54dde8419bdc..d95c849f79c5 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -2810,18 +2810,24 @@ int cpufreq_boost_set_sw(struct cpufreq_policy *policy, int state)
>   }
>   EXPORT_SYMBOL_GPL(cpufreq_boost_set_sw);
>   
> +static DEFINE_MUTEX(cpufreq_boost_mutex);
> +
>   static int cpufreq_boost_trigger_state(int state)
>   {
>   	struct cpufreq_policy *policy;
>   	unsigned long flags;
> +	bool boost_enabled;
>   	int ret = -EOPNOTSUPP;
>   
> +	guard(mutex)(&cpufreq_boost_mutex);
> +
>   	/*
>   	 * Don't compare 'cpufreq_driver->boost_enabled' with 'state' here to
>   	 * make sure all policies are in sync with global boost flag.
>   	 */
>   
>   	write_lock_irqsave(&cpufreq_driver_lock, flags);
> +	boost_enabled = cpufreq_driver->boost_enabled;
>   	cpufreq_driver->boost_enabled = state;
>   	write_unlock_irqrestore(&cpufreq_driver_lock, flags);
>   
> @@ -2841,7 +2847,7 @@ static int cpufreq_boost_trigger_state(int state)
>   		return 0;
>   
>   	write_lock_irqsave(&cpufreq_driver_lock, flags);
> -	cpufreq_driver->boost_enabled = !state;
> +	cpufreq_driver->boost_enabled = boost_enabled;
>   	write_unlock_irqrestore(&cpufreq_driver_lock, flags);
>   
>   	pr_err("%s: Cannot %s BOOST\n",


--
Thx and BRs,
Zhongqiu Han