mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "zhenglifeng (A)" <zhenglifeng1@huawei.com>
To: Viresh Kumar <viresh.kumar@linaro.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>
Cc: <linux-pm@vger.kernel.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] cpufreq: conservative: Drop cached requested_freq
Date: Thu, 12 Mar 2026 19:13:14 +0800	[thread overview]
Message-ID: <2edc5f50-896d-410b-95a1-04a55cd3e12f@huawei.com> (raw)
In-Reply-To: <c3e22128e6619d24f3fc6ac07d1c854c664a7f1e.1773124395.git.viresh.kumar@linaro.org>

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@huawei.com>
> Link: https://lore.kernel.org/all/20260210115458.3493646-1-zhenglifeng1@huawei.com/
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Lifeng Zheng, can you please give this a try and provide your Tested-by as well
> ?

I tested it on our platform, and the behavior is as expected.

Tested-by: Lifeng Zheng <zhenglifeng1@huawei.com>

> 
>  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 = {


  parent reply	other threads:[~2026-03-12 11:13 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-10  6:34 Viresh Kumar
2026-03-10 13:13 ` zhenglifeng (A)
2026-03-10 13:17 ` Zhongqiu Han
2026-03-12 10:27   ` Viresh Kumar
2026-03-12 14:03     ` Rafael J. Wysocki
2026-03-12 14:48       ` Viresh Kumar
2026-03-12 15:43         ` Rafael J. Wysocki
2026-03-12 15:59           ` Viresh Kumar
2026-03-12 16:27             ` Rafael J. Wysocki
2026-03-12 16:33               ` Viresh Kumar
2026-03-13  7:46     ` Zhongqiu Han
2026-03-17  8:13     ` zhenglifeng (A)
2026-03-12 11:13 ` zhenglifeng (A) [this message]
2026-03-12 14:53   ` Viresh Kumar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2edc5f50-896d-410b-95a1-04a55cd3e12f@huawei.com \
    --to=zhenglifeng1@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®