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

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

[Christian Loehle]

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")
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
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",
-- 
2.34.1

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

[Pengjie Zhang]

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")
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
> 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",
Reviewed-by: Pengjie Zhang <zhangpengjie2@huawei.com>

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@arm.com>
> ---
> 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

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

[Christian Loehle]

On 11/5/26 08:41, Zhongqiu Han wrote:
> 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;

Yes, that wouldn't flip it, although still race between sysfs writers.

> 
> 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?

Yes indeed, otherwise we could fold it into cpufreq_driver_lock.
I can split the two including the Fixes tag

> 
>> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
>> ---
>> 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",
> 
>