* [PATCH v4 0/2] Fix uclamp race against non atomic static key enable
@ 2026-10-29 13:13 Qais Yousef
2026-10-29 13:13 ` [PATCH v4 1/2] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq Qais Yousef
2026-10-29 13:13 ` [PATCH v4 2/2] sched/uclamp: Use wrappers instead of open coding UCLAMP_FLAG_IDLE Qais Yousef
0 siblings, 2 replies; 10+ messages in thread
From: Qais Yousef @ 2026-10-29 13:13 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Vincent Guittot
Cc: Christian Loehle, Dietmar Eggemann, Steven Rostedt, John Stultz,
linux-kernel, Qais Yousef
Changes since v3:
- Fix Sashiko report about force_idle paths where
rq->nr_running is not 0 highlighted by Christian
- Update patch 1 commit message to reflect jump_label call
sites on the same CPU might not be updated atomically too
Changes since v2:
- Use uclamp_rq_is_idle() instead of scanning buckets to detect
imbalance
- Do some cleanup on accessing the flag as an additional patch
Changes since v1:
- Fix kernel test robot report due to missing static inline in dummy
definition of init_uclamp_rq()
Tested with the patch below to force imbalance via sched_feat. I could see the
warning appearing and frequencies spiking during imbalance. Correct behavior is
restored when sched_feat is disabled where the idle entry reset fixes things
up.
I had a mix of uclamp_max mainly used in the system via schedqosd.
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index a2035d66270b9..14f9f1e419e02 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1889,6 +1889,11 @@ static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p, int flags
for_each_clamp_id(clamp_id)
uclamp_rq_inc_id(rq, p, clamp_id);
+ if (sched_feat(UCLAMP_IMBALANCED)) {
+ for_each_clamp_id(clamp_id)
+ uclamp_rq_inc_id(rq, p, clamp_id);
+ }
+
/* Reset clamp idle holding when there is one RUNNABLE task */
if (uclamp_rq_is_idle(rq))
uclamp_rq_clear_idle(rq);
diff --git a/kernel/sched/features.h b/kernel/sched/features.h
index 8f0dee8fc4755..b59e532500fd9 100644
--- a/kernel/sched/features.h
+++ b/kernel/sched/features.h
@@ -142,3 +142,5 @@ SCHED_FEAT(LATENCY_WARN, false)
*/
SCHED_FEAT(NI_RANDOM, true)
SCHED_FEAT(NI_RATE, true)
+
+SCHED_FEAT(UCLAMP_IMBALANCED, false)
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index 84301fc49e699..dbc3f5e1a7e01 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -519,7 +519,7 @@ static void set_next_task_idle(struct rq *rq, struct task_struct *next, enum snt
* are still queued.
*/
if (uclamp_is_used() &&
- !rq->nr_running && unlikely(!uclamp_rq_is_idle(rq)))
+ !rq->nr_running && unlikely(WARN_ON(!uclamp_rq_is_idle(rq))))
init_uclamp_rq(rq);
}
Qais Yousef (2):
sched/uclamp: Fix race condition leading to imbalanced uclamp_rq
sched/uclamp: Use wrappers instead of open coding UCLAMP_FLAG_IDLE
kernel/sched/core.c | 16 ++++++++--------
kernel/sched/idle.c | 19 +++++++++++++++++++
kernel/sched/sched.h | 13 +++++++++++++
3 files changed, 40 insertions(+), 8 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v4 1/2] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq 2026-10-29 13:13 [PATCH v4 0/2] Fix uclamp race against non atomic static key enable Qais Yousef @ 2026-10-29 13:13 ` Qais Yousef 2026-10-29 15:58 ` Christian Loehle 2026-10-29 13:13 ` [PATCH v4 2/2] sched/uclamp: Use wrappers instead of open coding UCLAMP_FLAG_IDLE Qais Yousef 1 sibling, 1 reply; 10+ messages in thread From: Qais Yousef @ 2026-10-29 13:13 UTC (permalink / raw) To: Ingo Molnar, Peter Zijlstra, Vincent Guittot Cc: Christian Loehle, Dietmar Eggemann, Steven Rostedt, John Stultz, linux-kernel, Qais Yousef, Wei Wang Static keys are not guaranteed to be enabled atomically across CPUs. Or across callsites on the same CPUs. Which means when sched_uclamp_used is enabled, some CPUs/callsites will return true for uclamp_is_used() while others won't, leading to asymmetric uclamp_rq_{inc, dec}_id. If we decrement without increment this is safe as documented in the code. But an increment without a decrement is a problem and will leave the uclamp_rq in a corrupt state. The reported case seems to happen due to callsites on the same CPU not seeing the flipped state. Plugging the race is hard as discussed in [1]. Take it for granted and do correction actions on going to idle to guarantee this state is transient for a short period after uclamp_sched_used is enabled. [1] https://lore.kernel.org/lkml/CANDhNCqyXsZ6G77z+9s=rTO6BZifRG9+hVxMTah+tAv6pB6h3w@mail.gmail.com/ Reported-by: Wei Wang <wvw@google.com> Fixes: 46609ce227039 ("sched/uclamp: Protect uclamp fast path code with static key") Signed-off-by: Qais Yousef <qyousef@layalina.io> --- kernel/sched/core.c | 2 +- kernel/sched/idle.c | 19 +++++++++++++++++++ kernel/sched/sched.h | 3 +++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index d7c3b43158dea..c6780358a7975 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2111,7 +2111,7 @@ static void uclamp_post_fork(struct task_struct *p) uclamp_update_util_min_rt_default(p); } -static void __init init_uclamp_rq(struct rq *rq) +void init_uclamp_rq(struct rq *rq) { enum uclamp_id clamp_id; struct uclamp_rq *uc_rq = rq->uclamp; 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))) + init_uclamp_rq(rq); } struct task_struct *pick_task_idle(struct rq *rq, struct rq_flags *rf) diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index 7d2ec527b8a25..d87bdbd2121df 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -3704,6 +3704,7 @@ static inline unsigned long cpu_util_rt(struct rq *rq) #ifdef CONFIG_UCLAMP_TASK unsigned long uclamp_eff_value(struct task_struct *p, enum uclamp_id clamp_id); +void init_uclamp_rq(struct rq *rq); /* * When uclamp is compiled in, the aggregation at rq level is 'turned off' @@ -3802,6 +3803,8 @@ uclamp_eff_value(struct task_struct *p, enum uclamp_id clamp_id) return SCHED_CAPACITY_SCALE; } +static inline void init_uclamp_rq(struct rq *rq) {} + static inline bool uclamp_rq_is_capped(struct rq *rq) { return false; } static inline bool uclamp_is_used(void) -- 2.53.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq 2026-10-29 13:13 ` [PATCH v4 1/2] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq Qais Yousef @ 2026-10-29 15:58 ` Christian Loehle 2026-10-29 23:43 ` Qais Yousef 0 siblings, 1 reply; 10+ messages in thread From: Christian Loehle @ 2026-10-29 15:58 UTC (permalink / raw) To: Qais Yousef, Ingo Molnar, Peter Zijlstra, Vincent Guittot Cc: Dietmar Eggemann, Steven Rostedt, John Stultz, linux-kernel, Wei Wang On 10/29/26 13:13, Qais Yousef wrote: > Static keys are not guaranteed to be enabled atomically across CPUs. Or > across callsites on the same CPUs. Which means when sched_uclamp_used is > enabled, some CPUs/callsites will return true for uclamp_is_used() while > others won't, leading to asymmetric uclamp_rq_{inc, dec}_id. If we > decrement without increment this is safe as documented in the code. But > an increment without a decrement is a problem and will leave the > uclamp_rq in a corrupt state. > > The reported case seems to happen due to callsites on the same CPU not > seeing the flipped state. > > Plugging the race is hard as discussed in [1]. Take it for granted and > do correction actions on going to idle to guarantee this state is > transient for a short period after uclamp_sched_used is enabled. > > [1] https://lore.kernel.org/lkml/CANDhNCqyXsZ6G77z+9s=rTO6BZifRG9+hVxMTah+tAv6pB6h3w@mail.gmail.com/ > > Reported-by: Wei Wang <wvw@google.com> > Fixes: 46609ce227039 ("sched/uclamp: Protect uclamp fast path code with static key") > Signed-off-by: Qais Yousef <qyousef@layalina.io> > --- > kernel/sched/core.c | 2 +- > kernel/sched/idle.c | 19 +++++++++++++++++++ > kernel/sched/sched.h | 3 +++ > 3 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index d7c3b43158dea..c6780358a7975 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -2111,7 +2111,7 @@ static void uclamp_post_fork(struct task_struct *p) > uclamp_update_util_min_rt_default(p); > } > > -static void __init init_uclamp_rq(struct rq *rq) > +void init_uclamp_rq(struct rq *rq) > { > enum uclamp_id clamp_id; > struct uclamp_rq *uc_rq = rq->uclamp; > 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. > + init_uclamp_rq(rq); > } > > struct task_struct *pick_task_idle(struct rq *rq, struct rq_flags *rf) > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h > index 7d2ec527b8a25..d87bdbd2121df 100644 > --- a/kernel/sched/sched.h > +++ b/kernel/sched/sched.h > @@ -3704,6 +3704,7 @@ static inline unsigned long cpu_util_rt(struct rq *rq) > #ifdef CONFIG_UCLAMP_TASK > > unsigned long uclamp_eff_value(struct task_struct *p, enum uclamp_id clamp_id); > +void init_uclamp_rq(struct rq *rq); > > /* > * When uclamp is compiled in, the aggregation at rq level is 'turned off' > @@ -3802,6 +3803,8 @@ uclamp_eff_value(struct task_struct *p, enum uclamp_id clamp_id) > return SCHED_CAPACITY_SCALE; > } > > +static inline void init_uclamp_rq(struct rq *rq) {} > + > static inline bool uclamp_rq_is_capped(struct rq *rq) { return false; } > > static inline bool uclamp_is_used(void) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq 2026-10-29 15:58 ` Christian Loehle @ 2026-10-29 23:43 ` Qais Yousef 2026-10-30 10:50 ` Christian Loehle 0 siblings, 1 reply; 10+ messages in thread From: Qais Yousef @ 2026-10-29 23:43 UTC (permalink / raw) To: Christian Loehle, Ingo Molnar, Peter Zijlstra, Vincent Guittot Cc: Dietmar Eggemann, Steven Rostedt, John Stultz, linux-kernel, Wei Wang 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? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq 2026-10-29 23:43 ` Qais Yousef @ 2026-10-30 10:50 ` Christian Loehle 2026-10-30 11:28 ` Qais Yousef 0 siblings, 1 reply; 10+ messages in thread From: Christian Loehle @ 2026-10-30 10:50 UTC (permalink / raw) To: Qais Yousef, Ingo Molnar, Peter Zijlstra, Vincent Guittot Cc: Dietmar Eggemann, Steven Rostedt, John Stultz, linux-kernel, Wei Wang On 10/29/26 23:43, Qais Yousef wrote: > 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? It forces idle, I can reproduce active=1 and bucket->tasks=0 and the WARN_ON_ONCE triggers: [ 329.665099] WARNING: kernel/sched/core.c:1845 at uclamp_rq_dec_id+0x10c/0x1dc, CPU#5: systemd-journal/281 [ 329.679279] Modules linked in: [ 329.682333] CPU: 5 UID: 0 PID: 281 Comm: systemd-journal Tainted: G S 7.3.0-uclamp-cfs-repro-20261030 #2 PREEMPT_RT [ 329.694411] Tainted: [S]=CPU_OUT_OF_SPEC [ 329.698323] Hardware name: Radxa Computer (Shenzhen) Co., Ltd. Radxa Orion O6/Radxa Orion O6, BIOS 1.1.0-1 2025-12-25T02:55:53+00:00 [ 329.710222] pstate: 834000c9 (Nzcv daIF +PAN -UAO +TCO +DIT -SSBS BTYPE=--) [ 329.717174] pc : uclamp_rq_dec_id+0x10c/0x1dc [ 329.721524] lr : uclamp_rq_dec_id+0x98/0x1dc [ 329.725786] sp : ffff800081bb3a80 [ 329.729089] x29: ffff800081bb3a90 x28: 0000000000000001 x27: ffffb54253580c40 [ 329.736219] x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000 [ 329.743348] x23: 0000000000000000 x22: ffff000085462380 x21: ffff0001fb6e2c40 [ 329.750478] x20: 0000000000000000 x19: 0000000000000000 x18: 0000000000000000 [ 329.757606] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000 [ 329.764736] x14: 0000000000000000 x13: 0000000000000001 x12: 0000000000000000 [ 329.771865] x11: 0000000000000000 x10: 006c616e72756f6a x9 : 0000000000000000 [ 329.778993] x8 : 006c616e72756f6a x7 : ffff000085462380 x6 : 0000000000000001 [ 329.786122] x5 : 00000000000000e1 x4 : ffff4abfa8162000 x3 : ffff0001fb6e2c40 [ 329.793251] x2 : 000000000000000c x1 : 0000000000000000 x0 : 0000000000000000 [ 329.800380] Call trace: [ 329.802816] uclamp_rq_dec_id+0x10c/0x1dc (P) [ 329.807168] dequeue_task+0x54/0x134 [ 329.810737] __schedule+0x6b0/0x1018 [ 329.814307] schedule+0x34/0xd4 [ 329.817440] schedule_hrtimeout_range_clock+0x108/0x11c [ 329.822657] schedule_hrtimeout_range+0x14/0x20 [ 329.827179] do_epoll_wait+0x47c/0x594 [ 329.830922] do_compat_epoll_pwait.part.0+0x14/0xb4 [ 329.835792] __arm64_sys_epoll_pwait+0x74/0x114 [ 329.840314] invoke_syscall+0x54/0x108 [ 329.844055] el0_svc_common.constprop.0+0xc0/0xe0 [ 329.848750] do_el0_svc+0x1c/0x28 [ 329.852055] el0_svc+0x38/0x250 [ 329.855189] el0t_64_sync_handler+0xa0/0xe4 [ 329.859363] el0t_64_sync+0x17c/0x180 [ 329.863018] ---[ end trace 0000000000000000 ]--- > > 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? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq 2026-10-30 10:50 ` Christian Loehle @ 2026-10-30 11:28 ` Qais Yousef 2026-10-30 12:15 ` Peter Zijlstra 0 siblings, 1 reply; 10+ messages in thread From: Qais Yousef @ 2026-10-30 11:28 UTC (permalink / raw) To: Christian Loehle Cc: Ingo Molnar, Peter Zijlstra, Vincent Guittot, Dietmar Eggemann, Steven Rostedt, John Stultz, linux-kernel, Wei Wang On 10/30/26 10:50, Christian Loehle wrote: > On 10/29/26 23:43, Qais Yousef wrote: > > 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? > > It forces idle, I can reproduce active=1 and bucket->tasks=0 and the WARN_ON_ONCE triggers: > > [ 329.665099] WARNING: kernel/sched/core.c:1845 at uclamp_rq_dec_id+0x10c/0x1dc, CPU#5: systemd-journal/281 > [ 329.679279] Modules linked in: > [ 329.682333] CPU: 5 UID: 0 PID: 281 Comm: systemd-journal Tainted: G S 7.3.0-uclamp-cfs-repro-20261030 #2 PREEMPT_RT > [ 329.694411] Tainted: [S]=CPU_OUT_OF_SPEC > [ 329.698323] Hardware name: Radxa Computer (Shenzhen) Co., Ltd. Radxa Orion O6/Radxa Orion O6, BIOS 1.1.0-1 2025-12-25T02:55:53+00:00 > [ 329.710222] pstate: 834000c9 (Nzcv daIF +PAN -UAO +TCO +DIT -SSBS BTYPE=--) > [ 329.717174] pc : uclamp_rq_dec_id+0x10c/0x1dc > [ 329.721524] lr : uclamp_rq_dec_id+0x98/0x1dc > [ 329.725786] sp : ffff800081bb3a80 > [ 329.729089] x29: ffff800081bb3a90 x28: 0000000000000001 x27: ffffb54253580c40 > [ 329.736219] x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000 > [ 329.743348] x23: 0000000000000000 x22: ffff000085462380 x21: ffff0001fb6e2c40 > [ 329.750478] x20: 0000000000000000 x19: 0000000000000000 x18: 0000000000000000 > [ 329.757606] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000 > [ 329.764736] x14: 0000000000000000 x13: 0000000000000001 x12: 0000000000000000 > [ 329.771865] x11: 0000000000000000 x10: 006c616e72756f6a x9 : 0000000000000000 > [ 329.778993] x8 : 006c616e72756f6a x7 : ffff000085462380 x6 : 0000000000000001 > [ 329.786122] x5 : 00000000000000e1 x4 : ffff4abfa8162000 x3 : ffff0001fb6e2c40 > [ 329.793251] x2 : 000000000000000c x1 : 0000000000000000 x0 : 0000000000000000 > [ 329.800380] Call trace: > [ 329.802816] uclamp_rq_dec_id+0x10c/0x1dc (P) > [ 329.807168] dequeue_task+0x54/0x134 > [ 329.810737] __schedule+0x6b0/0x1018 > [ 329.814307] schedule+0x34/0xd4 > [ 329.817440] schedule_hrtimeout_range_clock+0x108/0x11c > [ 329.822657] schedule_hrtimeout_range+0x14/0x20 > [ 329.827179] do_epoll_wait+0x47c/0x594 > [ 329.830922] do_compat_epoll_pwait.part.0+0x14/0xb4 > [ 329.835792] __arm64_sys_epoll_pwait+0x74/0x114 > [ 329.840314] invoke_syscall+0x54/0x108 > [ 329.844055] el0_svc_common.constprop.0+0xc0/0xe0 > [ 329.848750] do_el0_svc+0x1c/0x28 > [ 329.852055] el0_svc+0x38/0x250 > [ 329.855189] el0t_64_sync_handler+0xa0/0xe4 > [ 329.859363] el0t_64_sync+0x17c/0x180 > [ 329.863018] ---[ end trace 0000000000000000 ]--- Okay, thanks for checking. Peter/Vincent, what do you think? It seems we have a schrodenger task that is both enqueued and dequeued simultaneously in these throttling paths. Shouldn't this be fixed? If yes, any thoughts on the right approach? Should we throttle if there are no other tasks on the rq too? I didn't realize the semantics are that strong for enforcing the quota. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq 2026-10-30 11:28 ` Qais Yousef @ 2026-10-30 12:15 ` Peter Zijlstra 2026-10-30 12:41 ` Qais Yousef 0 siblings, 1 reply; 10+ messages in thread From: Peter Zijlstra @ 2026-10-30 12:15 UTC (permalink / raw) To: Qais Yousef Cc: Christian Loehle, Ingo Molnar, Vincent Guittot, Dietmar Eggemann, Steven Rostedt, John Stultz, linux-kernel, Wei Wang On Fri, Oct 30, 2026 at 11:28:25AM +0000, Qais Yousef wrote: > Peter/Vincent, what do you think? It seems we have a schrodenger task that is > both enqueued and dequeued simultaneously in these throttling paths. Shouldn't > this be fixed? If yes, any thoughts on the right approach? Should we throttle > if there are no other tasks on the rq too? I didn't realize the semantics are > that strong for enforcing the quota. Urgh, nothing is ever straight forward is it ... Bah, so this is because the core does uclamp (for classes that have it on) but then the class does the throttle without going through the core {en,de}queue. Does it make sense to have throttle_Cfs_rq_work() and tg_unthrottle_up() do uclamp_rq_{dec,inc}() respectively, along with the {de,en}queue_task_fair() they're already doing? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq 2026-10-30 12:15 ` Peter Zijlstra @ 2026-10-30 12:41 ` Qais Yousef 2026-10-30 13:21 ` Christian Loehle 0 siblings, 1 reply; 10+ messages in thread From: Qais Yousef @ 2026-10-30 12:41 UTC (permalink / raw) To: Peter Zijlstra Cc: Christian Loehle, Ingo Molnar, Vincent Guittot, Dietmar Eggemann, Steven Rostedt, John Stultz, linux-kernel, Wei Wang On 10/30/26 13:15, Peter Zijlstra wrote: > On Fri, Oct 30, 2026 at 11:28:25AM +0000, Qais Yousef wrote: > > > Peter/Vincent, what do you think? It seems we have a schrodenger task that is > > both enqueued and dequeued simultaneously in these throttling paths. Shouldn't > > this be fixed? If yes, any thoughts on the right approach? Should we throttle > > if there are no other tasks on the rq too? I didn't realize the semantics are > > that strong for enforcing the quota. > > Urgh, nothing is ever straight forward is it ... Exactly my thoughts.. > > Bah, so this is because the core does uclamp (for classes that have it > on) but then the class does the throttle without going through the core > {en,de}queue. > > Does it make sense to have throttle_Cfs_rq_work() and tg_unthrottle_up() > do uclamp_rq_{dec,inc}() respectively, along with the > {de,en}queue_task_fair() they're already doing? Yes it does, just wanted to make sure we agree the bug is there rather than in the current patchset approach. I remember seeing a patch to remove the uclamp_enabled bit and defer to enqueue/dequeue_task_$class to handle this instead. Maybe I can revive it if it makes more sense to go that way. Let me find the path of least resistance and add it to the stack. Not sure we want to tag as Fixes for LTS to pick up this independent fix. Note we have rt group throttling path flagged up - which I also thought was false positive but not sure anymore - but it seems to do partial dequeue and must be fixed in that regard anyway even if it doesn't force idle. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq 2026-10-30 12:41 ` Qais Yousef @ 2026-10-30 13:21 ` Christian Loehle 0 siblings, 0 replies; 10+ messages in thread From: Christian Loehle @ 2026-10-30 13:21 UTC (permalink / raw) To: Qais Yousef, Peter Zijlstra Cc: Ingo Molnar, Vincent Guittot, Dietmar Eggemann, Steven Rostedt, John Stultz, linux-kernel, Wei Wang On 10/30/26 12:41, Qais Yousef wrote: > On 10/30/26 13:15, Peter Zijlstra wrote: >> On Fri, Oct 30, 2026 at 11:28:25AM +0000, Qais Yousef wrote: >> >>> Peter/Vincent, what do you think? It seems we have a schrodenger task that is >>> both enqueued and dequeued simultaneously in these throttling paths. Shouldn't >>> this be fixed? If yes, any thoughts on the right approach? Should we throttle >>> if there are no other tasks on the rq too? I didn't realize the semantics are >>> that strong for enforcing the quota. >> >> Urgh, nothing is ever straight forward is it ... > > Exactly my thoughts.. > >> >> Bah, so this is because the core does uclamp (for classes that have it >> on) but then the class does the throttle without going through the core >> {en,de}queue. >> >> Does it make sense to have throttle_Cfs_rq_work() and tg_unthrottle_up() >> do uclamp_rq_{dec,inc}() respectively, along with the >> {de,en}queue_task_fair() they're already doing? > > Yes it does, just wanted to make sure we agree the bug is there rather than in > the current patchset approach. > > I remember seeing a patch to remove the uclamp_enabled bit and defer to > enqueue/dequeue_task_$class to handle this instead. Maybe I can revive it if it > makes more sense to go that way. > > Let me find the path of least resistance and add it to the stack. Not sure we > want to tag as Fixes for LTS to pick up this independent fix. Note we have rt > group throttling path flagged up - which I also thought was false positive but > not sure anymore - but it seems to do partial dequeue and must be fixed in that > regard anyway even if it doesn't force idle. I can reproduce the idle path without uclamp_rq_dec through sched_rt_rq_dequeue(), too, so IMO that needs fixing up as well. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 2/2] sched/uclamp: Use wrappers instead of open coding UCLAMP_FLAG_IDLE 2026-10-29 13:13 [PATCH v4 0/2] Fix uclamp race against non atomic static key enable Qais Yousef 2026-10-29 13:13 ` [PATCH v4 1/2] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq Qais Yousef @ 2026-10-29 13:13 ` Qais Yousef 1 sibling, 0 replies; 10+ messages in thread From: Qais Yousef @ 2026-10-29 13:13 UTC (permalink / raw) To: Ingo Molnar, Peter Zijlstra, Vincent Guittot Cc: Christian Loehle, Dietmar Eggemann, Steven Rostedt, John Stultz, linux-kernel, Qais Yousef We already have uclamp_rq_is_idle(), but it wasn't used everywhere. Replace open coding with the function and add new wrapper to set and clear the flag which enhances readability. Signed-off-by: Qais Yousef <qyousef@layalina.io> --- kernel/sched/core.c | 14 +++++++------- kernel/sched/sched.h | 10 ++++++++++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index c6780358a7975..a2035d66270b9 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -1626,7 +1626,7 @@ uclamp_idle_value(struct rq *rq, enum uclamp_id clamp_id, * max-clamp. */ if (clamp_id == UCLAMP_MAX) { - rq->uclamp_flags |= UCLAMP_FLAG_IDLE; + uclamp_rq_set_idle(rq); return clamp_value; } @@ -1637,7 +1637,7 @@ static inline void uclamp_idle_reset(struct rq *rq, enum uclamp_id clamp_id, unsigned int clamp_value) { /* Reset max-clamp retention only on idle exit */ - if (!(rq->uclamp_flags & UCLAMP_FLAG_IDLE)) + if (!uclamp_rq_is_idle(rq)) return; uclamp_rq_set(rq, clamp_id, clamp_value); @@ -1890,8 +1890,8 @@ static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p, int flags uclamp_rq_inc_id(rq, p, clamp_id); /* Reset clamp idle holding when there is one RUNNABLE task */ - if (rq->uclamp_flags & UCLAMP_FLAG_IDLE) - rq->uclamp_flags &= ~UCLAMP_FLAG_IDLE; + if (uclamp_rq_is_idle(rq)) + uclamp_rq_clear_idle(rq); } static inline void uclamp_rq_dec(struct rq *rq, struct task_struct *p) @@ -1930,8 +1930,8 @@ static inline void uclamp_rq_reinc_id(struct rq *rq, struct task_struct *p, * Make sure to clear the idle flag if we've transiently reached 0 * active tasks on rq. */ - if (clamp_id == UCLAMP_MAX && (rq->uclamp_flags & UCLAMP_FLAG_IDLE)) - rq->uclamp_flags &= ~UCLAMP_FLAG_IDLE; + if (clamp_id == UCLAMP_MAX && uclamp_rq_is_idle(rq)) + uclamp_rq_clear_idle(rq); } static inline void @@ -2122,7 +2122,7 @@ void init_uclamp_rq(struct rq *rq) }; } - rq->uclamp_flags = UCLAMP_FLAG_IDLE; + uclamp_rq_set_idle(rq); } static void __init init_uclamp(void) diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index d87bdbd2121df..4706f73838a86 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -3743,6 +3743,16 @@ static inline void uclamp_rq_set(struct rq *rq, enum uclamp_id clamp_id, WRITE_ONCE(rq->uclamp[clamp_id].value, value); } +static inline void uclamp_rq_set_idle(struct rq *rq) +{ + rq->uclamp_flags |= UCLAMP_FLAG_IDLE; +} + +static inline void uclamp_rq_clear_idle(struct rq *rq) +{ + rq->uclamp_flags &= ~UCLAMP_FLAG_IDLE; +} + static inline bool uclamp_rq_is_idle(struct rq *rq) { return rq->uclamp_flags & UCLAMP_FLAG_IDLE; -- 2.53.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-10-30 13:21 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-29 13:13 [PATCH v4 0/2] Fix uclamp race against non atomic static key enable Qais Yousef 2026-10-29 13:13 ` [PATCH v4 1/2] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq Qais Yousef 2026-10-29 15:58 ` Christian Loehle 2026-10-29 23:43 ` Qais Yousef 2026-10-30 10:50 ` Christian Loehle 2026-10-30 11:28 ` Qais Yousef 2026-10-30 12:15 ` Peter Zijlstra 2026-10-30 12:41 ` Qais Yousef 2026-10-30 13:21 ` Christian Loehle 2026-10-29 13:13 ` [PATCH v4 2/2] sched/uclamp: Use wrappers instead of open coding UCLAMP_FLAG_IDLE Qais Yousef
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®