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

[Peter Zijlstra <peterz@infradead.org>]

On Sun, Oct 25, 2026 at 04:26:12PM +0000, Qais Yousef wrote:

> 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.
> +	*/

Whitespace fail here.

> +	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..47dcfd74e150f 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;
> +}

Anyway; I was thinking, since we have a hole in struct uclamp_rq; and
also uclamp_flags. Can we fold uclamp_flags into the hold, and also use
UCLAMP_FLAG_IDLE to detect if we need this bucket scan at all?