[PATCH] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq

[PATCH] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq

[Qais Yousef]

Static keys are not guaranteed to be enabled atomically across CPUs.
Which means when sched_uclamp_used is enabled, some CPUs will return
true for uclamp_is_used() while others won't, leading to asymmetric
ulcamp_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 race for increment without a decrement is interesting as it means
a CPU has seen the static key flipped and incremented while enqueueing
a task, but the task was dequeued on another CPU that hasn't seen the
static key flipped - we need to be wary this might be a symptom of
something else going wrong.

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

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 c6780358a7975..d3e73ba88f22b 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 (rq->uclamp_flags & UCLAMP_FLAG_IDLE)
			rq->uclamp_flags &= ~UCLAMP_FLAG_IDLE;
	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 950e2885bba63..cb31b3c9df23c 100644
	--- a/kernel/sched/idle.c
	+++ b/kernel/sched/idle.c
	@@ -516,7 +516,7 @@ static void set_next_task_idle(struct rq *rq, struct task_struct *next, enum snt
		 * everything should work as expected.
		*/
		if (uclamp_is_used()) {
	-               if (unlikely(uclamp_is_imbalanced(rq)))
	+               if (unlikely(WARN_ON(uclamp_is_imbalanced(rq))))
				init_uclamp_rq(rq);
		}
	 }


 kernel/sched/core.c  |  2 +-
 kernel/sched/idle.c  | 17 +++++++++++++++++
 kernel/sched/sched.h | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 53 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..950e2885bba63 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -502,6 +502,23 @@ 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.
+	*/
+	if (uclamp_is_used()) {
+		if (unlikely(uclamp_is_imbalanced(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..232ed37227ee4 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'
@@ -3783,6 +3784,33 @@ static inline unsigned int uclamp_bucket_id(unsigned int clamp_value)
 	return min_t(unsigned int, clamp_value / UCLAMP_BUCKET_DELTA, UCLAMP_BUCKETS - 1);
 }
 
+static inline bool __uclamp_is_imbalanced(struct rq *rq, enum uclamp_id clamp_id)
+{
+	struct uclamp_bucket *bucket = rq->uclamp[clamp_id].bucket;
+	int bucket_id = UCLAMP_BUCKETS - 1;
+
+	/*
+	 * On idle all bucket.tasks must be 0, or we have hit imbalance
+	 * problem.
+	 */
+	for ( ; bucket_id >= 0; bucket_id--)
+		if (bucket[bucket_id].tasks)
+			return true;
+
+	return false;
+}
+
+static inline bool uclamp_is_imbalanced(struct rq *rq)
+{
+	enum uclamp_id clamp_id;
+
+	for_each_clamp_id(clamp_id)
+		if(__uclamp_is_imbalanced(rq, clamp_id))
+			return true;
+
+	return false;
+}
+
 static inline void
 uclamp_se_set(struct uclamp_se *uc_se, unsigned int value, bool user_defined)
 {
@@ -3802,6 +3830,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)
@@ -3830,6 +3860,11 @@ static inline bool uclamp_rq_is_idle(struct rq *rq)
 	return false;
 }
 
+static inline bool uclamp_is_imbalanced(struct rq *rq)
+{
+	return false;
+}
+
 #endif /* !CONFIG_UCLAMP_TASK */
 
 #ifdef CONFIG_HAVE_SCHED_AVG_IRQ
-- 
2.53.0

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

[kernel test robot]

Hi Qais,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/sched/core]
[also build test WARNING on tip/master peterz-queue/sched/core linus/master v7.3 next-20261023]
[cannot apply to tip/auto-latest]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Qais-Yousef/sched-uclamp-Fix-race-condition-leading-to-imbalanced-uclamp_rq/20261025-003945
base:   tip/sched/core
patch link:    https://lore.kernel.org/r/20261024233945.74794-1-qyousef%40layalina.io
patch subject: [PATCH] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq
config: arm-allnoconfig (https://download.01.org/0day-ci/archive/20261025/202610251644.6weWofAv-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261025/202610251644.6weWofAv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202610251644.6weWofAv-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from kernel/sched/rq-offsets.c:5:
>> kernel/sched/sched.h:3833:6: warning: no previous prototype for function 'init_uclamp_rq' [-Wmissing-prototypes]
    3833 | void init_uclamp_rq(struct rq *rq) {}
         |      ^
   kernel/sched/sched.h:3833:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
    3833 | void init_uclamp_rq(struct rq *rq) {}
         | ^
         | static 
   1 warning generated.
--
   In file included from kernel/sched/rq-offsets.c:5:
>> kernel/sched/sched.h:3833:6: warning: no previous prototype for function 'init_uclamp_rq' [-Wmissing-prototypes]
    3833 | void init_uclamp_rq(struct rq *rq) {}
         |      ^
   kernel/sched/sched.h:3833:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
    3833 | void init_uclamp_rq(struct rq *rq) {}
         | ^
         | static 
   1 warning generated.


vim +/init_uclamp_rq +3833 kernel/sched/sched.h

  3832	
> 3833	void init_uclamp_rq(struct rq *rq) {}
  3834	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

Hi Qais,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/sched/core]
[also build test WARNING on tip/master peterz-queue/sched/core linus/master v7.3 next-20261023]
[cannot apply to tip/auto-latest]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Qais-Yousef/sched-uclamp-Fix-race-condition-leading-to-imbalanced-uclamp_rq/20261025-003945
base:   tip/sched/core
patch link:    https://lore.kernel.org/r/20261024233945.74794-1-qyousef%40layalina.io
patch subject: [PATCH] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq
config: arc-allnoconfig (https://download.01.org/0day-ci/archive/20261025/202610251831.6qMLJiPR-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 16.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261025/202610251831.6qMLJiPR-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202610251831.6qMLJiPR-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from kernel/sched/rq-offsets.c:5:
>> kernel/sched/sched.h:3833:6: warning: no previous prototype for 'init_uclamp_rq' [-Wmissing-prototypes]
    3833 | void init_uclamp_rq(struct rq *rq) {}
         |      ^~~~~~~~~~~~~~
--
   In file included from kernel/sched/rq-offsets.c:5:
>> kernel/sched/sched.h:3833:6: warning: no previous prototype for 'init_uclamp_rq' [-Wmissing-prototypes]
    3833 | void init_uclamp_rq(struct rq *rq) {}
         |      ^~~~~~~~~~~~~~


vim +/init_uclamp_rq +3833 kernel/sched/sched.h

  3832	
> 3833	void init_uclamp_rq(struct rq *rq) {}
  3834	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

Hi Qais,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/sched/core]
[also build test ERROR on tip/master peterz-queue/sched/core linus/master v7.3 next-20261023]
[cannot apply to tip/auto-latest]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Qais-Yousef/sched-uclamp-Fix-race-condition-leading-to-imbalanced-uclamp_rq/20261025-003945
base:   tip/sched/core
patch link:    https://lore.kernel.org/r/20261024233945.74794-1-qyousef%40layalina.io
patch subject: [PATCH] sched/uclamp: Fix race condition leading to imbalanced uclamp_rq
config: arm-allnoconfig (https://download.01.org/0day-ci/archive/20261025/202610252327.6QJlXAiY-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261025/202610252327.6QJlXAiY-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202610252327.6QJlXAiY-lkp@intel.com/

All errors (new ones prefixed by >>):

>> ld.lld: error: duplicate symbol: init_uclamp_rq
   >>> defined at core.c
   >>>            kernel/sched/core.o:(init_uclamp_rq) in archive vmlinux.a
   >>> defined at fair.c
   >>>            kernel/sched/fair.o:(.text+0x1) in archive vmlinux.a
--
>> ld.lld: error: duplicate symbol: init_uclamp_rq
   >>> defined at core.c
   >>>            kernel/sched/core.o:(init_uclamp_rq) in archive vmlinux.a
   >>> defined at build_utility.c
   >>>            kernel/sched/build_utility.o:(.text+0x1) in archive vmlinux.a
--
>> ld.lld: error: duplicate symbol: init_uclamp_rq
   >>> defined at core.c
   >>>            kernel/sched/core.o:(init_uclamp_rq) in archive vmlinux.a
   >>> defined at build_policy.c
   >>>            kernel/sched/build_policy.o:(.text+0x1) in archive vmlinux.a

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki