[PATCH] cpufreq: conservative: Fix incorrect frequency decrease due to stale target

[PATCH] cpufreq: conservative: Fix incorrect frequency decrease due to stale target

[Lifeng Zheng]

In cs_dbs_update(), the requested frequency is decremented by one freq_step
for each idle period. However, this can cause divergence between
'requested_freq' (target for current update) and 'dbs_info->requested_freq'
(target from previous update).

When the load crosses up_threshold or down_threshold, the decision on
whether to increase or decrease frequency should be based on the *previous*
target (dbs_info->requested_freq), not the current one. Otherwise, the
update step may be skipped entirely if the current target has already hit a
boundary due to prior adjustments.

Ensure that frequency scaling decisions are made using the correct
historical target, fixing cases where frequency fails to decrease despite
sustained idle periods.

Fixes: 00bfe05889e9 ("cpufreq: conservative: Decrease frequency faster for deferred updates")
Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
---
 drivers/cpufreq/cpufreq_conservative.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
index df01d33993d8..f3c3b54e4bf8 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -104,7 +104,7 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
 		dbs_info->down_skip = 0;
 
 		/* if we are already at full speed then break out early */
-		if (requested_freq == policy->max)
+		if (dbs_info->requested_freq == policy->max)
 			goto out;
 
 		requested_freq += freq_step;
@@ -127,7 +127,7 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
 		/*
 		 * if we cannot reduce the frequency anymore, break out early
 		 */
-		if (requested_freq == policy->min)
+		if (dbs_info->requested_freq == policy->min)
 			goto out;
 
 		if (requested_freq > freq_step)
-- 
2.33.0

Re: [PATCH] cpufreq: conservative: Fix incorrect frequency decrease due to stale target

[Stratos Karafotis]

Hi all!

I was struggling to see your point, but I think you are right.

The requested_freq could be equal to policy->min (due to idle periods)
while the dbs_info->requested_freq could be greater than policy->min.
So, in this case it breaks out early without the chance to further 
reduce the frequency, correct?

Stratos Karafotis

On 4/21/26 15:35, Lifeng Zheng wrote:
> In cs_dbs_update(), the requested frequency is decremented by one freq_step
> for each idle period. However, this can cause divergence between
> 'requested_freq' (target for current update) and 'dbs_info->requested_freq'
> (target from previous update).
> 
> When the load crosses up_threshold or down_threshold, the decision on
> whether to increase or decrease frequency should be based on the *previous*
> target (dbs_info->requested_freq), not the current one. Otherwise, the
> update step may be skipped entirely if the current target has already hit a
> boundary due to prior adjustments.
> 
> Ensure that frequency scaling decisions are made using the correct
> historical target, fixing cases where frequency fails to decrease despite
> sustained idle periods.
> 
> Fixes: 00bfe05889e9 ("cpufreq: conservative: Decrease frequency faster for deferred updates")
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>   drivers/cpufreq/cpufreq_conservative.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index df01d33993d8..f3c3b54e4bf8 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -104,7 +104,7 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>   		dbs_info->down_skip = 0;
>   
>   		/* if we are already at full speed then break out early */
> -		if (requested_freq == policy->max)
> +		if (dbs_info->requested_freq == policy->max)
>   			goto out;
>   
>   		requested_freq += freq_step;
> @@ -127,7 +127,7 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>   		/*
>   		 * if we cannot reduce the frequency anymore, break out early
>   		 */
> -		if (requested_freq == policy->min)
> +		if (dbs_info->requested_freq == policy->min)
>   			goto out;
>   
>   		if (requested_freq > freq_step)

Re: [PATCH] cpufreq: conservative: Fix incorrect frequency decrease due to stale target

[zhenglifeng (A)]

On 4/22/2026 4:06 PM, Stratos Karafotis wrote:
> Hi all!
> 
> I was struggling to see your point, but I think you are right.
> 
> The requested_freq could be equal to policy->min (due to idle periods)
> while the dbs_info->requested_freq could be greater than policy->min.
> So, in this case it breaks out early without the chance to further reduce the frequency, correct?

Yes. This is the case where problems arise.

> 
> Stratos Karafotis
> 
> On 4/21/26 15:35, Lifeng Zheng wrote:
>> In cs_dbs_update(), the requested frequency is decremented by one freq_step
>> for each idle period. However, this can cause divergence between
>> 'requested_freq' (target for current update) and 'dbs_info->requested_freq'
>> (target from previous update).
>>
>> When the load crosses up_threshold or down_threshold, the decision on
>> whether to increase or decrease frequency should be based on the *previous*
>> target (dbs_info->requested_freq), not the current one. Otherwise, the
>> update step may be skipped entirely if the current target has already hit a
>> boundary due to prior adjustments.
>>
>> Ensure that frequency scaling decisions are made using the correct
>> historical target, fixing cases where frequency fails to decrease despite
>> sustained idle periods.
>>
>> Fixes: 00bfe05889e9 ("cpufreq: conservative: Decrease frequency faster for deferred updates")
>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
>> ---
>>   drivers/cpufreq/cpufreq_conservative.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
>> index df01d33993d8..f3c3b54e4bf8 100644
>> --- a/drivers/cpufreq/cpufreq_conservative.c
>> +++ b/drivers/cpufreq/cpufreq_conservative.c
>> @@ -104,7 +104,7 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>>           dbs_info->down_skip = 0;
>>             /* if we are already at full speed then break out early */
>> -        if (requested_freq == policy->max)
>> +        if (dbs_info->requested_freq == policy->max)
>>               goto out;
>>             requested_freq += freq_step;
>> @@ -127,7 +127,7 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>>           /*
>>            * if we cannot reduce the frequency anymore, break out early
>>            */
>> -        if (requested_freq == policy->min)
>> +        if (dbs_info->requested_freq == policy->min)
>>               goto out;
>>             if (requested_freq > freq_step)
> 

Re: [PATCH] cpufreq: conservative: Fix incorrect frequency decrease due to stale target

[Zhongqiu Han]

On 4/21/2026 8:35 PM, Lifeng Zheng wrote:
> In cs_dbs_update(), the requested frequency is decremented by one freq_step
> for each idle period. However, this can cause divergence between
> 'requested_freq' (target for current update) and 'dbs_info->requested_freq'
> (target from previous update).
> 
> When the load crosses up_threshold or down_threshold, the decision on
> whether to increase or decrease frequency should be based on the *previous*
> target (dbs_info->requested_freq), not the current one. Otherwise, the
> update step may be skipped entirely if the current target has already hit a
> boundary due to prior adjustments.
> 
> Ensure that frequency scaling decisions are made using the correct
> historical target, fixing cases where frequency fails to decrease despite
> sustained idle periods.
> 
> Fixes: 00bfe05889e9 ("cpufreq: conservative: Decrease frequency faster for deferred updates")
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>

Hi Lifeng,
Thanks for the patch.

May I know would this ignore conservative idle decay when the previous
requested frequency was policy->max?


Scenario: Increase path, previous target at max, with idle
compensation; the original code does not have the same behavior as the
current patch.

Initial state:
   policy->max               = 2000 MHz
   policy->min               = 200 MHz
   dbs_info->requested_freq  = 2000 MHz  (= policy->max)
   hardware frequency        = 2000 MHz
   idle_periods              = 2
   load                      = 90% (> up_threshold=80)

1.Original code
Step 1: requested_freq = dbs_info->requested_freq = 2000

Step 2: [idle_periods block]
         freq_steps = 2 * 100 = 200
         2000 > (200 + 200) = 400 ?  YES
         requested_freq = 2000 - 200 = 1800
Step 3: [increase path]
         if (requested_freq == policy->max)
           -> 1800 == 2000 ?  NO  -> fall through

Step 4: requested_freq += freq_step
         requested_freq = 1800 + 100 = 1900

Step 5: __cpufreq_driver_target(policy, 1900, HE) -> hardware = 1900 MHz
Step 6: dbs_info->requested_freq = 1900

Result: hardware 2000 -> *1900 MHz* (net 1-step decrease)


2.Current Patch
Step 1: requested_freq = 2000
Step 2: [idle_periods block] -> requested_freq = 1800
Step 3: if (dbs_info->requested_freq == policy->max)
           -> 2000 == 2000 ?  YES  -> goto out
Step 4: hardware stays at 2000 MHz, dbs_info->requested_freq stays at 2000

Result: hardware stays at *2000 MHz* (no change)




> ---
>   drivers/cpufreq/cpufreq_conservative.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index df01d33993d8..f3c3b54e4bf8 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -104,7 +104,7 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>   		dbs_info->down_skip = 0;
>   
>   		/* if we are already at full speed then break out early */
> -		if (requested_freq == policy->max)
> +		if (dbs_info->requested_freq == policy->max)
>   			goto out;
>   
>   		requested_freq += freq_step;
> @@ -127,7 +127,7 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>   		/*
>   		 * if we cannot reduce the frequency anymore, break out early
>   		 */
> -		if (requested_freq == policy->min)
> +		if (dbs_info->requested_freq == policy->min)
>   			goto out;
>   
>   		if (requested_freq > freq_step)


-- 
Thx and BRs,
Zhongqiu Han

Re: [PATCH] cpufreq: conservative: Fix incorrect frequency decrease due to stale target

[zhenglifeng (A)]

On 4/23/2026 1:39 PM, Zhongqiu Han wrote:
> On 4/21/2026 8:35 PM, Lifeng Zheng wrote:
>> In cs_dbs_update(), the requested frequency is decremented by one freq_step
>> for each idle period. However, this can cause divergence between
>> 'requested_freq' (target for current update) and 'dbs_info->requested_freq'
>> (target from previous update).
>>
>> When the load crosses up_threshold or down_threshold, the decision on
>> whether to increase or decrease frequency should be based on the *previous*
>> target (dbs_info->requested_freq), not the current one. Otherwise, the
>> update step may be skipped entirely if the current target has already hit a
>> boundary due to prior adjustments.
>>
>> Ensure that frequency scaling decisions are made using the correct
>> historical target, fixing cases where frequency fails to decrease despite
>> sustained idle periods.
>>
>> Fixes: 00bfe05889e9 ("cpufreq: conservative: Decrease frequency faster for deferred updates")
>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> 
> Hi Lifeng,
> Thanks for the patch.
> 
> May I know would this ignore conservative idle decay when the previous
> requested frequency was policy->max?
> 
> 
> Scenario: Increase path, previous target at max, with idle
> compensation; the original code does not have the same behavior as the
> current patch.
> 
> Initial state:
>   policy->max               = 2000 MHz
>   policy->min               = 200 MHz
>   dbs_info->requested_freq  = 2000 MHz  (= policy->max)
>   hardware frequency        = 2000 MHz
>   idle_periods              = 2
>   load                      = 90% (> up_threshold=80)
> 
> 1.Original code
> Step 1: requested_freq = dbs_info->requested_freq = 2000
> 
> Step 2: [idle_periods block]
>         freq_steps = 2 * 100 = 200
>         2000 > (200 + 200) = 400 ?  YES
>         requested_freq = 2000 - 200 = 1800
> Step 3: [increase path]
>         if (requested_freq == policy->max)
>           -> 1800 == 2000 ?  NO  -> fall through
> 
> Step 4: requested_freq += freq_step
>         requested_freq = 1800 + 100 = 1900
> 
> Step 5: __cpufreq_driver_target(policy, 1900, HE) -> hardware = 1900 MHz
> Step 6: dbs_info->requested_freq = 1900
> 
> Result: hardware 2000 -> *1900 MHz* (net 1-step decrease)
> 
> 
> 2.Current Patch
> Step 1: requested_freq = 2000
> Step 2: [idle_periods block] -> requested_freq = 1800
> Step 3: if (dbs_info->requested_freq == policy->max)
>           -> 2000 == 2000 ?  YES  -> goto out
> Step 4: hardware stays at 2000 MHz, dbs_info->requested_freq stays at 2000
> 
> Result: hardware stays at *2000 MHz* (no change)
> 
> 

Yes, I think you are right. The behaviors are not the same. I modified this
just in order to keep it consistent with the case exceeding down_threshold.
I'm not sure if this change of behavior is reasonable. Perhaps Rafael or
Viresh could give us some advice.

> 
> 
>> ---
>>   drivers/cpufreq/cpufreq_conservative.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
>> index df01d33993d8..f3c3b54e4bf8 100644
>> --- a/drivers/cpufreq/cpufreq_conservative.c
>> +++ b/drivers/cpufreq/cpufreq_conservative.c
>> @@ -104,7 +104,7 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>>           dbs_info->down_skip = 0;
>>             /* if we are already at full speed then break out early */
>> -        if (requested_freq == policy->max)
>> +        if (dbs_info->requested_freq == policy->max)
>>               goto out;
>>             requested_freq += freq_step;
>> @@ -127,7 +127,7 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>>           /*
>>            * if we cannot reduce the frequency anymore, break out early
>>            */
>> -        if (requested_freq == policy->min)
>> +        if (dbs_info->requested_freq == policy->min)
>>               goto out;
>>             if (requested_freq > freq_step)
> 
>