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

[Qais Yousef]

On 10/26/26 10:12, Peter Zijlstra wrote:

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

Actually maybe we can just check if the flag is not set then this means 
we have a corruption without scanning the buckets. I didn't like the 
scan, but seemed safer for a backport. Let me try that and update.

I can fold the uclamp_flags into buckets, I don't think this will grow 
and I do have a path to help us get rid off uclamp_rq (hopefully) in the 
future.

Okay to do this as a separate follow up patch? Since this will go to 
LTS, I think safer keep it minimalist and do the folding bit in a 
separate patch.