mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] cpufreq: schedutil: Fix uncleared need_freq_update on the adjust_perf path
@ 2026-06-16 15:47 Zhongqiu Han
  2026-06-17  4:06 ` Hongyan Xia
  0 siblings, 1 reply; 5+ messages in thread
From: Zhongqiu Han @ 2026-06-16 15:47 UTC (permalink / raw)
  To: rafael, viresh.kumar, mingo, peterz, juri.lelli, vincent.guittot,
	dietmar.eggemann, rostedt, bsegall, mgorman, vschneid,
	kprateek.nayak, christian.loehle
  Cc: linux-pm, linux-kernel, zhongqiu.han, stable

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@vger.kernel.org
Signed-off-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
---
 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;
 }
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpufreq: schedutil: Fix uncleared need_freq_update on the adjust_perf path
  2026-06-16 15:47 [PATCH] cpufreq: schedutil: Fix uncleared need_freq_update on the adjust_perf path Zhongqiu Han
@ 2026-06-17  4:06 ` Hongyan Xia
  2026-06-17  7:27   ` Christian Loehle
  0 siblings, 1 reply; 5+ messages in thread
From: Hongyan Xia @ 2026-06-17  4:06 UTC (permalink / raw)
  To: Zhongqiu Han, rafael, viresh.kumar, mingo, peterz, juri.lelli,
	vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
	vschneid, kprateek.nayak, christian.loehle
  Cc: linux-pm, linux-kernel, stable

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@vger.kernel.org
> Signed-off-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
> ---
>   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.

>   }
>   

Reviewed-by: Hongyan Xia <hongyan.xia@transsion.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpufreq: schedutil: Fix uncleared need_freq_update on the adjust_perf path
  2026-06-17  4:06 ` Hongyan Xia
@ 2026-06-17  7:27   ` Christian Loehle
  2026-06-17 18:09     ` Rafael J. Wysocki
  2026-06-22  2:34     ` Zhongqiu Han
  0 siblings, 2 replies; 5+ messages in thread
From: Christian Loehle @ 2026-06-17  7:27 UTC (permalink / raw)
  To: Hongyan Xia, Zhongqiu Han, rafael, viresh.kumar, mingo, peterz,
	juri.lelli, vincent.guittot, dietmar.eggemann, rostedt, bsegall,
	mgorman, vschneid, kprateek.nayak
  Cc: linux-pm, linux-kernel, stable

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@vger.kernel.org
>> Signed-off-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
>> ---
>>   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.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpufreq: schedutil: Fix uncleared need_freq_update on the adjust_perf path
  2026-06-17  7:27   ` Christian Loehle
@ 2026-06-17 18:09     ` Rafael J. Wysocki
  2026-06-22  2:34     ` Zhongqiu Han
  1 sibling, 0 replies; 5+ messages in thread
From: Rafael J. Wysocki @ 2026-06-17 18:09 UTC (permalink / raw)
  To: Christian Loehle, Hongyan Xia
  Cc: Zhongqiu Han, viresh.kumar, mingo, peterz, juri.lelli,
	vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
	vschneid, kprateek.nayak, linux-pm, linux-kernel, stable

On Wed, Jun 17, 2026 at 9:28 AM Christian Loehle
<christian.loehle@arm.com> 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@vger.kernel.org
> >> Signed-off-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
> >> ---
> >>   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.
>
> For $SUBJECT:
> Reviewed-by: Christian Loehle <christian.loehle@arm.com>

Applied as 7.2-rc material, thanks!

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpufreq: schedutil: Fix uncleared need_freq_update on the adjust_perf path
  2026-06-17  7:27   ` Christian Loehle
  2026-06-17 18:09     ` Rafael J. Wysocki
@ 2026-06-22  2:34     ` Zhongqiu Han
  1 sibling, 0 replies; 5+ messages in thread
From: Zhongqiu Han @ 2026-06-22  2:34 UTC (permalink / raw)
  To: Christian Loehle, Hongyan Xia, rafael, viresh.kumar, mingo,
	peterz, juri.lelli, vincent.guittot, dietmar.eggemann, rostedt,
	bsegall, mgorman, vschneid, kprateek.nayak
  Cc: linux-pm, linux-kernel, stable, 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@vger.kernel.org
>>> Signed-off-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
>>> ---
>>>    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@arm.com>


-- 
Thx and BRs,
Zhongqiu Han

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-06-22  2:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-16 15:47 [PATCH] cpufreq: schedutil: Fix uncleared need_freq_update on the adjust_perf path Zhongqiu Han
2026-06-17  4:06 ` Hongyan Xia
2026-06-17  7:27   ` Christian Loehle
2026-06-17 18:09     ` Rafael J. Wysocki
2026-06-22  2:34     ` Zhongqiu Han

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox