Re: [PATCH V2] cpufreq: conservative: Reset requested_freq on limits change

[Zhongqiu Han]

On 3/20/2026 5:38 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 resetting the cached requested_freq to policy->cur on
> detecting a change in policy limits.

Reviewed-by: Zhongqiu Han <zhongqiu.han@xxxxxxxxxxxxxxxx>



> Reported-by: Lifeng Zheng <zhenglifeng1@xxxxxxxxxx>
> Tested-by: Lifeng Zheng <zhenglifeng1@xxxxxxxxxx>
> Link: https://lore.kernel.org/all/20260210115458.3493646-1-zhenglifeng1@xxxxxxxxxx/
> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> ---
> [v2]
> - Completely different design to solve the problem.
>
>   drivers/cpufreq/cpufreq_conservative.c | 12 ++++++++++++
>   drivers/cpufreq/cpufreq_governor.c     |  3 +++
>   drivers/cpufreq/cpufreq_governor.h     |  1 +
>   3 files changed, 16 insertions(+)
>
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index e0e847764511..df01d33993d8 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -313,6 +313,17 @@ static void cs_start(struct cpufreq_policy *policy)
>   	dbs_info->requested_freq = policy->cur;
>   }
>   
> +static void cs_limits(struct cpufreq_policy *policy)
> +{
> +	struct cs_policy_dbs_info *dbs_info = to_dbs_info(policy->governor_data);
> +
> +	/*
> +	 * The limits have changed, so may have the current frequency. Reset
> +	 * requested_freq to avoid any unintended outcomes due to the mismatch.
> +	 */
> +	dbs_info->requested_freq = policy->cur;
> +}
> +
>   static struct dbs_governor cs_governor = {
>   	.gov = CPUFREQ_DBS_GOVERNOR_INITIALIZER("conservative"),
>   	.kobj_type = { .default_groups = cs_groups },
> @@ -322,6 +333,7 @@ static struct dbs_governor cs_governor = {
>   	.init = cs_init,
>   	.exit = cs_exit,
>   	.start = cs_start,
> +	.limits = cs_limits,
>   };
>   
>   #define CPU_FREQ_GOV_CONSERVATIVE	(cs_governor.gov)
> diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
> index 36eb7aee4bcd..acf101878733 100644
> --- a/drivers/cpufreq/cpufreq_governor.c
> +++ b/drivers/cpufreq/cpufreq_governor.c
> @@ -563,6 +563,7 @@ EXPORT_SYMBOL_GPL(cpufreq_dbs_governor_stop);
>   
>   void cpufreq_dbs_governor_limits(struct cpufreq_policy *policy)
>   {
> +	struct dbs_governor *gov = dbs_governor_of(policy);
>   	struct policy_dbs_info *policy_dbs;
>   
>   	/* Protect gov->gdbs_data against cpufreq_dbs_governor_exit() */
> @@ -574,6 +575,8 @@ void cpufreq_dbs_governor_limits(struct cpufreq_policy *policy)
>   	mutex_lock(&policy_dbs->update_mutex);
>   	cpufreq_policy_apply_limits(policy);
>   	gov_update_sample_delay(policy_dbs, 0);
> +	if (gov->limits)
> +		gov->limits(policy);
>   	mutex_unlock(&policy_dbs->update_mutex);
>   
>   out:
> diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
> index 168c23fd7fca..1462d59277bd 100644
> --- a/drivers/cpufreq/cpufreq_governor.h
> +++ b/drivers/cpufreq/cpufreq_governor.h
> @@ -138,6 +138,7 @@ struct dbs_governor {
>   	int (*init)(struct dbs_data *dbs_data);
>   	void (*exit)(struct dbs_data *dbs_data);
>   	void (*start)(struct cpufreq_policy *policy);
> +	void (*limits)(struct cpufreq_policy *policy);
>   };
>   
>   static inline struct dbs_governor *dbs_governor_of(struct cpufreq_policy *policy)


--
Thx and BRs,
Zhongqiu Han