Re: Uclamp races due to non-atomic static_branch_likely updates

[Qais Yousef]

On 10/21/26 15:29, Peter Zijlstra wrote:
> On Wed, Oct 21, 2026 at 02:20:58PM +0100, Qais Yousef wrote:
> 
> > > Also note that a simple global boolean has the exact same problem.
> > > Adding runtime conditionals isn't going to fix this.
> > 
> > I have something in my memory that could be complete garbage that tells me all
> > cpus should see the new update after a grace period, did my brain invent this?
> 
> This is true; rcu_sync() implies a memory barrier on all CPUs, and
> therefore all CPUs must observe the prior state upon completion.
> 
> The thing is that a grace period is of 'arbitrary length' and during
> that you can still have some CPUs seeing your variable set while others
> will not and you get into the very same problem.

Maybe this arbitrary length what contributes to the size of the race window
otherwise I struggle to see this being a problem practically.

> 
> > If not invented, can we do synchronize_rcu() once on any cpu seeing the code
> > enabled for the first time to ensure the rest of the cpus has seen the newly
> > enabled path?
> > 
> > eg (not even compile tested):
> > 
> > 	diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> > 	index 7d2ec527b8a2..a6093ec88103 100644
> > 	--- a/kernel/sched/sched.h
> > 	+++ b/kernel/sched/sched.h
> > 	@@ -3715,7 +3715,16 @@ unsigned long uclamp_eff_value(struct task_struct *p, enum uclamp_id clamp_id);
> > 	  */
> > 	 static inline bool uclamp_is_used(void)
> > 	 {
> > 	-       return static_branch_likely(&sched_uclamp_used);
> > 	+       if (static_branch_likely(&sched_uclamp_used)) {
> > 	+               static bool do_once = true;
> > 	+               if (unlikely(do_once)) {
> > 	+                       do_once = false;
> > 	+                       synchronize_rcu();
> > 	+               }
> > 	+               return true;
> > 	+       }
> > 	+
> > 	+       return false;
> > 	 }
> > 
> > 	 /*
> 
> This cannot work; two immediate problems:
> 
>  - the do_once needs re-evaluating for every switch

I think uclamp can only be turned on, but yeah for a generic solution this is
a problem and the do_once will need to be reset on switch off.

>  - you cannot do rcu_sync while holding rq->lock ;-)

Uh-oh, right :-)

> 
> > The other alternative is to just accept this race can happen and do some sanity
> > check and reset/cleanup on going to idle - which should be pretty cheap and
> > straightforward.
> 
> This seems the sanest option.

I think the below should do

-->8--

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index d7c3b43158de..c6780358a797 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 76f3c84ca684..64ea91b743bb 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -502,6 +502,14 @@ 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.
+        */
+       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 7d2ec527b8a2..6d7be3e3b36b 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;
 }

+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)