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

[Hao Jia <jiahao.kernel@gmail.com>]

Hi Aaron,

On 2025/10/15 16:40, Aaron Lu wrote:
> On Wed, Oct 15, 2025 at 02:31:27PM +0800, Hao Jia wrote:
>> On 2025/10/15 10:51, Aaron Lu wrote:
>>> On Wed, Oct 15, 2025 at 09:43:20AM +0800, Hao Jia wrote:
>>> ... ...
>>>> Yes, I've already hit the cfs_rq->runtime_remaining < 0 condition in
>>>> tg_unthrottle_up().
>>>>
>>>> This morning, after applying your patch, I still get the same issue.
>>>> However, As before, because cfs_rq->curr isn't NULL,
>>>> check_enqueue_throttle() returns prematurely, preventing the triggering of
>>>> throttle_cfs_rq().
>>>>
>>>>
>>>> Some information to share with you.
>>>
>>> Can you also share your cgroup setup and related quota setting etc. and
>>> how to trigger it? Thanks.
>>
>> I ran some internal workloads on my test machine with different quota
>> settings, and added 10 sched messaging branchmark cgroups, setting their
>> cpu.max to 1000 100000.
>>
>> perf bench sched messaging -g 10 -t -l 50000 &
>>
>> I'm not sure if the issue can be reproduced without these internal
>> workloads.
> 
> Thanks for the report, I think I understand your concern now.
> 
> I managed to trigger a condition in tg_unthrottle_up() for a cfs_rq that
> has runtime_enabled but with a negative runtime_remaining, the setup is
> as before:
> 
>            root
>          /      \
>          A*     ...
>       /  |  \   ...
>          B
>         /  \
>        C*
> 
> where both A and C have quota settings.
> 
> 1 Initially, both cfs_rq_a and cfs_rq_c are in unthrottled state with a
>    positive runtime_remaining.
> 2 At some time, cfs_rq_a is throttled. cfs_rq_c is now in a throttled
>    hierarchy, but it's not throttled and has a positive runtime_remaining.
> 3 Some time later, task @p gets enqueued to cfs_rq_c and starts execution
>    in kernel mode, consumed all cfs_rq_c's runtime_remaining.
>    account_cfs_rq_runtime() properly accounted, but resched_curr() doesn't
>    cause schedule() -> check_cfs_rq_runtime() -> throttle_cfs_rq() to
>    happen immediately, because task @p is still executing in kernel mode
>    (CONFIG_PREEMPT_VOLUNTARY).
> 4 Some time later, cfs_rq_a is unthrottled.
>    tg_unthrottle_up() noticed cfs_rq_c has a negative runtime_remaining.
> 
> In this situation, check_enqueue_throttle() will not do anything though
> because cfs_rq_c->curr is set, throttle will not happen immediately so
> it won't cause throttle to happen on unthrottle path.
> 
> 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.

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

Thanks,
Hao





> 
> Thanks.