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

[Rafael J. Wysocki]

On Fri, Mar 20, 2026 at 11:02 AM Zhongqiu Han
<zhongqiu.han@xxxxxxxxxxxxxxxx> wrote:
>
> 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>

Applied as 7.0-rc material, thanks!


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