Re: [PATCH] sched/fair: Prevent cfs_rq from being unthrottled with zero runtime_remaining

[Aaron Lu <ziqianlu@bytedance.com>]

On Wed, Oct 15, 2025 at 06:21:01PM +0800, Hao Jia wrote:
> On 2025/10/15 16:40, Aaron Lu wrote:
... ...
> > Hao Jia,
> > 
> > Do I understand you correctly that you can only hit the newly added
> > debug warn in tg_unthrottle_up():
> > WARN_ON_ONCE(cfs_rq->runtime_enabled && cfs_rq->runtime_remaining <= 0);
> > but not throttle triggered on unthrottle path?
> > 
> 
> yes. but I'm not sure if there are other corner cases where
> cfs_rq->runtime_remaining <= 0 and cfs_rq->curr is NULL.
> 

Right, I'm not aware of any but might be possible.

> > BTW, I think your change has the advantage of being straightforward and
> > easy to reason about. My concern is, it's not efficient to enqueue tasks
> > to a cfs_rq that has no runtime left, not sure how big a deal that is
> > though.
> 
> Yes, but that's what we're doing now. The case described above involves
> enqueue a task where cfs_rq->runtime_remaining <= 0.
> 
> I previously tried adding a runtime_remaining check for each level of task
> p's cfs_rq in unthrottle_cfs_rq()/tg_unthrottle_up(), but this made the code
> strange and complicated.

Agree that adding a runtime_remaining check for each level in
unthrottle_cfs_rq() looks too complex.

So I think you approach is fine, feel free to submit a formal patch.
With your change, theoretically we do not need to do those
runtime_remaining check in unthrottle_cfs_rq() but keeping that check
could save us some unnecessary enqueues, so I'll leave it to you to
decide if you want to keep it or not. If you want to keep it, please
also change its comments because the current comments will be stale
then.

Thanks.