Re: [PATCH] cpufreq: schedutil: Fix uncleared need_freq_update on the adjust_perf path

[Zhongqiu Han]

On 6/17/2026 3:27 PM, Christian Loehle wrote:
> On 6/17/26 05:06, Hongyan Xia wrote:
> > On 6/16/2026 11:47 PM, Zhongqiu Han wrote:
> > > The need_freq_update flag makes sugov_should_update_freq() return true
> > > regardless of the rate_limit_us throttling, and is cleared in
> > > sugov_update_next_freq(). sugov_update_single_freq() and
> > > sugov_update_shared() go through that helper, so the flag does not
> > > persist there.
> > >
> > > However, sugov_update_single_perf() (used by drivers implementing the
> > > ->adjust_perf() callback, e.g. intel_pstate or amd-pstate in passive mode)
> > > calls cpufreq_driver_adjust_perf() directly and never goes through
> > > sugov_update_next_freq(), so the need_freq_update flag is not cleared in
> > > that path.
> > >
> > > Before commit 75da043d8f88 ("cpufreq/sched: Set need_freq_update in
> > > ignore_dl_rate_limit()"), this was effectively harmless because
> > > sugov_should_update_freq() still honoured the rate limit even when
> > > need_freq_update was set. After that change, the flag forces
> > > sugov_should_update_freq() to always return true, so once set, it stays
> > > effective indefinitely on the adjust_perf path.
> > >
> > > As a result, cpufreq_driver_adjust_perf() gets called on every scheduler
> > > utilization update (with the runqueue lock held) rather than being
> > > throttled by rate_limit_us, even if the driver itself may skip redundant
> > > hardware updates.
> > >
> > > Clear need_freq_update at the end of the adjust_perf path as well.
> > >
> > > Fixes: 75da043d8f88 ("cpufreq/sched: Set need_freq_update in ignore_dl_rate_limit()")
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Signed-off-by: Zhongqiu Han <zhongqiu.han@xxxxxxxxxxxxxxxx>
> > > ---
> > >    kernel/sched/cpufreq_schedutil.c | 1 +
> > >    1 file changed, 1 insertion(+)
> > >
> > > diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> > > index ae9fd211cec1..a4e689eefdfb 100644
> > > --- a/kernel/sched/cpufreq_schedutil.c
> > > +++ b/kernel/sched/cpufreq_schedutil.c
> > > @@ -486,6 +486,7 @@ static void sugov_update_single_perf(struct update_util_data *hook, u64 time,
> > >    	cpufreq_driver_adjust_perf(sg_policy->policy, sg_cpu->bw_min,
> > >    				   sg_cpu->util, max_cap);
> > >    
> > > +	sg_policy->need_freq_update = false;
> > >    	sg_policy->last_freq_update_time = time;
> >
> > Nice catch. Thanks.
> >
> > It does seem to me that setting last_freq_update_time should then assert
> > !need_freq_update, otherwise it doesn't make sense, but that's a
> > different topic.
> +1, feel free to submit that too.

Thanks Hongyan and Christian for the review and suggestions. I'll look
into it.

> For $SUBJECT:
> Reviewed-by: Christian Loehle <christian.loehle@xxxxxxx>


--
Thx and BRs,
Zhongqiu Han