Re: [PATCH] cpufreq: conservative: Drop cached requested_freq

[zhenglifeng (A)]

On 3/10/2026 2:34 PM, Viresh Kumar wrote:
> A recently reported issue highlighted that the cached requested_freq
> is not guaranteed to stay in sync with policy->cur. If the platform
> changes the actual CPU frequency after the governor sets one (e.g.
> due to platform-specific frequency scaling) and a re-sync occurs
> later, policy->cur may diverge from requested_freq.
> 
> This can lead to incorrect behavior in the conservative governor.
> For example, the governor may assume the CPU is already running at
> the maximum frequency and skip further increases even though there
> is still headroom.
> 
> Avoid this by dropping the cached requested_freq and using
> policy->cur directly.
> 
> Reported-by: Lifeng Zheng <zhenglifeng1@xxxxxxxxxx>
> Link: https://lore.kernel.org/all/20260210115458.3493646-1-zhenglifeng1@xxxxxxxxxx/
> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> ---
> Lifeng Zheng, can you please give this a try and provide your Tested-by as well
> ?

Sure. I will test it in the next few days.

> 
>  drivers/cpufreq/cpufreq_conservative.c | 16 +---------------
>  1 file changed, 1 insertion(+), 15 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index e0e847764511..c69577e4f941 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -14,7 +14,6 @@
>  struct cs_policy_dbs_info {
>  	struct policy_dbs_info policy_dbs;
>  	unsigned int down_skip;
> -	unsigned int requested_freq;
>  };
>  
>  static inline struct cs_policy_dbs_info *to_dbs_info(struct policy_dbs_info *policy_dbs)
> @@ -59,10 +58,10 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>  {
>  	struct policy_dbs_info *policy_dbs = policy->governor_data;
>  	struct cs_policy_dbs_info *dbs_info = to_dbs_info(policy_dbs);
> -	unsigned int requested_freq = dbs_info->requested_freq;
>  	struct dbs_data *dbs_data = policy_dbs->dbs_data;
>  	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
>  	unsigned int load = dbs_update(policy);
> +	unsigned int requested_freq = policy->cur;
>  	unsigned int freq_step;
>  
>  	/*
> @@ -72,16 +71,6 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>  	if (cs_tuners->freq_step == 0)
>  		goto out;
>  
> -	/*
> -	 * If requested_freq is out of range, it is likely that the limits
> -	 * changed in the meantime, so fall back to current frequency in that
> -	 * case.
> -	 */
> -	if (requested_freq > policy->max || requested_freq < policy->min) {
> -		requested_freq = policy->cur;
> -		dbs_info->requested_freq = requested_freq;
> -	}
> -
>  	freq_step = get_freq_step(cs_tuners, policy);
>  
>  	/*
> @@ -113,7 +102,6 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>  
>  		__cpufreq_driver_target(policy, requested_freq,
>  					CPUFREQ_RELATION_HE);
> -		dbs_info->requested_freq = requested_freq;
>  		goto out;
>  	}
>  
> @@ -137,7 +125,6 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
>  
>  		__cpufreq_driver_target(policy, requested_freq,
>  					CPUFREQ_RELATION_LE);
> -		dbs_info->requested_freq = requested_freq;
>  	}
>  
>   out:
> @@ -310,7 +297,6 @@ static void cs_start(struct cpufreq_policy *policy)
>  	struct cs_policy_dbs_info *dbs_info = to_dbs_info(policy->governor_data);
>  
>  	dbs_info->down_skip = 0;
> -	dbs_info->requested_freq = policy->cur;
>  }
>  
>  static struct dbs_governor cs_governor = {