Re: [PATCH v4 1/2] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq

[Qais Yousef <qyousef@layalina.io>]

On 10/29/26 15:58, Christian Loehle wrote:

>> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
>> index 76f3c84ca6846..84301fc49e699 100644
>> --- a/kernel/sched/idle.c
>> +++ b/kernel/sched/idle.c
>> @@ -502,6 +502,25 @@ static void set_next_task_idle(struct rq *rq, struct task_struct *next, enum snt
>>   	 * lost_idle_time of clock_pelt
>>   	 */
>>   	update_idle_rq_clock_pelt(rq);
>> +
>> +	/*
>> +	 * Due to a race condition when enabling sched_uclamp_used static key,
>> +	 * we can end up with imbalance where uclamp_rq_inc_id() has happened
>> +	 * on a CPU without uclamp_rq_dec_id() on another CPU that hasn't seen
>> +	 * the static key flipped. The race condition is unlikely, but ensure
>> +	 * the uclamp_rq() state is set properly on each entry to idle.
>> +	 *
>> +	 * The reset will cause the uclamp_max from uclamp_idle_value() to be
>> +	 * lost. Since we are imbalanced, hard to know what is right. So just
>> +	 * cleanup. This should be a transient error state once we recover
>> +	 * everything should work as expected.
>> +	 *
>> +	 * We need to be careful of some paths that can force idle with tasks
>> +	 * are still queued.
>> +	 */
>> +	if (uclamp_is_used() &&
>> +	    !rq->nr_running && unlikely(!uclamp_rq_is_idle(rq)))
> 
> just checking nr_running isn't enough as CFS bandwidth throttling (a fun one
> recently!) will decrement it through dequeue_task_fair() without calling
> uclamp_rq_dec(), i.e. it's clamps must remain valid AFAICS.

I think this and the rt group throttling are false positives. Do they 
actually force idle or just under contention dequeue the task 
temporarily to allow other queued tasks to run?

If this is the last running task and throttling can force it off the 
queue partially then enter idle, I don't think the bug is here. It 
should do a full dequeue then. But I am not awfully familiar with this 
code, so could be missing something.

For context, below is Sashiko's full report. Proxy and core scheduling 
seems to cause temporarily context switch to idle. No idea about 
sched_ext. 3 and 4 seem false positive to me.

--->8---

Can calling init_uclamp_rq(rq) in set_next_task_idle() corrupt rq->uclamp
when the runqueue schedules the idle task while uclamp-active tasks are 
still
queued on rq?
In kernel/sched/core.c, init_uclamp_rq() zeroes all bucket->tasks counters
and resets rq->uclamp[clamp_id].value without clearing
p->uclamp[clamp_id].active on tasks that remain on rq:
kernel/sched/core.c:init_uclamp_rq() {
	for_each_clamp_id(clamp_id) {
		uc_rq[clamp_id] = (struct uclamp_rq) {
			.value = uclamp_none(clamp_id)
		};
	}
	rq->uclamp_flags = UCLAMP_FLAG_IDLE;
}
When those still-queued tasks are later dequeued, uclamp_rq_dec_id() in
kernel/sched/core.c sees uc_se->active is still true while bucket->tasks was
zeroed (or decrements another task's bucket reference count):
kernel/sched/core.c:uclamp_rq_dec_id() {
	if (unlikely(!uc_se->active))
		return;
	bucket = &uc_rq->bucket[uc_se->bucket_id];
	WARN_ON_ONCE(!bucket->tasks);
	if (likely(bucket->tasks))
		bucket->tasks--;
}
There appear to be several normal scheduling paths that reach
set_next_task_idle() while tasks remain queued on rq with
p->uclamp[clamp_id].active set and UCLAMP_FLAG_IDLE cleared:
1. With proxy execution enabled, proxy_deactivate() and proxy_migrate_task()
in kernel/sched/core.c call proxy_resched_idle(rq) ->
put_prev_set_next_task(rq, rq->donor, rq->idle) -> set_next_task_idle()
immediately before calling block_task() or deactivate_task():
kernel/sched/core.c:proxy_deactivate() {
	proxy_resched_idle(rq);
	block_task(rq, donor, state);
}
Here set_next_task_idle() zeroes bucket->tasks via init_uclamp_rq(rq), 
and on
the very next line block_task() -> dequeue_task() -> uclamp_rq_dec() ->
uclamp_rq_dec_id() triggers WARN_ON_ONCE(!bucket->tasks). Similarly,
find_proxy_task() calls proxy_resched_idle(rq) when a mutex-blocked 
donor and
other runnable tasks remain queued on rq.
2. With core scheduling enabled, pick_next_task() in kernel/sched/core.c can
force-idle an SMT sibling rq_i that still has runnable tasks queued when no
task matches the core cookie:
kernel/sched/core.c:pick_next_task() {
		if (!cookie_equals(p, cookie)) {
			p = NULL;
			if (cookie)
				p = sched_core_find(rq_i, cookie);
			if (!p)
				p = idle_sched_class.pick_task(rq_i, rf);
		}
}
3. With CFS bandwidth throttling enabled, throttle_cfs_rq_work() in
kernel/sched/fair.c dequeues tasks via dequeue_task_fair() directly without
going through dequeue_task() and uclamp_rq_dec():
kernel/sched/fair.c:throttle_cfs_rq_work() {
		dequeue_task_fair(rq, p, DEQUEUE_SLEEP | DEQUEUE_THROTTLE);
		list_add(&p->throttle_node, &cfs_rq->throttled_limbo_list);
}
4. With RT group bandwidth throttling enabled, 
sched_rt_runtime_exceeded() in
kernel/sched/rt.c dequeues the throttled rt_rq via sched_rt_rq_dequeue() ->
dequeue_top_rt_rq() without calling dequeue_task() and uclamp_rq_dec() 
on the
queued RT tasks:
kernel/sched/rt.c:sched_rt_runtime_exceeded() {
		if (rt_rq_throttled(rt_rq)) {
			sched_rt_rq_dequeue(rt_rq);
			return 1;
		}
}
5. With sched_ext enabled (where ext_sched_class sets uclamp_enabled),
do_pick_task_scx() in kernel/sched/ext/ext.c returns NULL when the local DSQ
is empty even if tasks enqueued on rq reside in a non-local DSQ or BPF
custody:
kernel/sched/ext/ext.c:do_pick_task_scx() {
		p = first_local_task(rq);
		if (!p)
			return NULL;
}
When any of these paths causes rq to schedule rq->idle while tasks with
p->uclamp[clamp_id].active remain on rq, wouldn't init_uclamp_rq(rq) wipe
active frequency clamps and corrupt the bucket task counts?