Re: [PATCH] sched/cpufreq: Reevaluate frequency before tickless idle

[Christian Loehle]

On 8/25/26 03:51, Hongyan Xia wrote:
> On 8/24/2026 9:18 PM, Christian Loehle wrote:
>> sugov_hold_freq() can preserve a UCLAMP_MIN-driven high frequency when
>> the runqueue goes idle. If cpuidle then stops the tick, no later
>> utilization update is guaranteed and a CPU using WFI can remain at an
>> unnecessarily high voltage for the entire idle period.
>>
>> Issue a final cpufreq update when the idle tick actually transitions to
>> stopped and force single-policy schedutil past its rate limit. Keep the
>> existing hold behavior when the tick is retained.
>>
>> Signed-off-by: Christian Loehle <christian.loehle@xxxxxxx>
>> ---
>>   include/linux/sched/cpufreq.h    |  1 +
>>   kernel/sched/cpufreq_schedutil.c |  5 ++++-
>>   kernel/sched/idle.c              | 29 +++++++++++++++++++++++++++--
>>   3 files changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/sched/cpufreq.h b/include/linux/sched/cpufreq.h
>> index bdd31ab93bc5..0814f6c79315 100644
>> --- a/include/linux/sched/cpufreq.h
>> +++ b/include/linux/sched/cpufreq.h
>> @@ -9,6 +9,7 @@
>>    */
>>   
>>   #define SCHED_CPUFREQ_IOWAIT	(1U << 0)
>> +#define SCHED_CPUFREQ_IDLE	(1U << 1)
>>   
>>   #ifdef CONFIG_CPU_FREQ
>>   struct cpufreq_policy;
>> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
>> index a1782755efcc..90bf8d8bffb7 100644
>> --- a/kernel/sched/cpufreq_schedutil.c
>> +++ b/kernel/sched/cpufreq_schedutil.c
>> @@ -100,7 +100,7 @@ static bool sugov_should_update_freq(struct sugov_policy *sg_policy, u64 time)
>>   
>>   		return true;
>>   	} else if (sg_policy->need_freq_update) {
>> -		/* ignore_dl_rate_limit() wants a new frequency to be found. */
>> +		/* A forced update needs a new frequency to be found. */
>>   		return true;
>>   	}
>>   
>> @@ -407,6 +407,9 @@ static inline bool sugov_update_single_common(struct sugov_cpu *sg_cpu,
>>   	sugov_iowait_boost(sg_cpu, time, flags);
>>   	sg_cpu->last_update = time;
>>   
>> +	if (flags & SCHED_CPUFREQ_IDLE)
>> +		sg_cpu->sg_policy->need_freq_update = true;
>> +
>>   	ignore_dl_rate_limit(sg_cpu);
>>   
>>   	if (!sugov_should_update_freq(sg_cpu->sg_policy, time))
>> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
>> index eb73b65ce6c4..36f8840d0562 100644
>> --- a/kernel/sched/idle.c
>> +++ b/kernel/sched/idle.c
>> @@ -161,10 +161,35 @@ static int call_cpuidle(struct cpuidle_driver *drv, struct cpuidle_device *dev,
>>   	return cpuidle_enter(drv, dev, next_state);
>>   }
>>   
>> +static void idle_stop_tick(void)
>> +{
>> +#ifdef CONFIG_CPU_FREQ
>> +	bool was_stopped = tick_nohz_tick_stopped();
>> +#endif
>> +
>> +	tick_nohz_idle_stop_tick();
>> +
>> +#ifdef CONFIG_CPU_FREQ
>> +	/*
>> +	 * Run one last cpufreq update before entering idle with the tick
>> +	 * stopped, because no later update is guaranteed.
>> +	 */
>> +	if (!was_stopped && tick_nohz_tick_stopped()) {
>> +		struct rq *rq = this_rq();
>> +		struct rq_flags rf;
>> +
>> +		rq_lock(rq, &rf);
>> +		update_rq_clock(rq);
>> +		cpufreq_update_util(rq, SCHED_CPUFREQ_IDLE);
>> +		rq_unlock(rq, &rf);
>> +	}
>> +#endif
>> +}
>> +
>>   static void idle_call_stop_or_retain_tick(bool stop_tick)
>>   {
>>   	if (stop_tick || tick_nohz_tick_stopped())
>> -		tick_nohz_idle_stop_tick();
>> +		idle_stop_tick();
>>   	else
>>   		tick_nohz_idle_retain_tick();
>>   }
>> @@ -225,7 +250,7 @@ static void cpuidle_idle_call(bool stop_tick)
>>   			max_latency_ns = dev->forced_idle_latency_limit_ns;
>>   		}
>>   
>> -		tick_nohz_idle_stop_tick();
>> +		idle_stop_tick();
>>   
>>   		next_state = cpuidle_find_deepest_state(drv, dev, max_latency_ns);
>>   		call_cpuidle(drv, dev, next_state);
> 
> LGTM, minus the Sashiko issue.
> 
> One side note is that I briefly looked at similar things before, not 
> just for sugov_hold_freq() but for going idle in general. It might be 
> desirable to predict the util *after* wake-up and use it to drive 
> frequency one last time before going idle for any CPU. This avoids 
> holding high frequencies in update_single() and also avoids idling CPUs 
> holding high frequencies for the entire cluster in update_shared().
> 
> But predictions are hard and the energy savings aren't much so I didn't 
> spend much time on it. Not sure if there are better ideas.

Funnily enough the slow-switch case makes this a lot more complicated,
in particular with PREEMPT_RT :/
My current approach is to have an irq work for the next tick for
the sugov_hold_freq() case, that checks if it hasn't seen another update
for the tick duration and then drops the hold_freq.
It's not ideal because of course it disabled the tick-stop by one tick,
but it's the best approach I can come up with now.
We could do something similar for decayed blocked utilization (i.e. have
an irq work for when utilization would reach lowest OPP (or an intermediate
step?) and decay that then).

That being said I do think the hold_freq and the blocked utilization are
separate issues, they just might have a similar-looking solution...
I can post an RFC for the latter, that being said I'm not entirely convinced
myself that it's worth it, while I think the hold_freq is just clearly wrong
behavior.