[PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths

Wanwu Li posted 2 patches 3 weeks, 5 days ago
kernel/sched/fair.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths
Posted by Wanwu Li 3 weeks, 5 days ago
Hi Peter, Ingo,

Since commit 85570f10a4c6 ("sched/eevdf: Move to a single runqueue")
the per-level "is something running at this level" information moved
from cfs_rq->curr to cfs_rq->h_curr; cfs_rq->curr is now only
maintained on the root cfs_rq. The cgroup hierarchy is kept for load
tracking and bandwidth accounting, which makes the bandwidth paths the
only code that still runs per-level and consults a per-level current.
Two locations were not updated in this conversion:

  1/2: throttle_cfs_rq() reads cfs_rq->curr to decide whether the
       throttled level has a running entity. For intermediate cfs_rqs
       the check is always false, so quota exhaustion never requests a
       full sched_cfs_bandwidth_slice() and never arms the deferred
       throttle task_work via task_throttle_setup_work(); a running
       task can out-run its group's quota until the next pick armed
       the work instead.

  2/2: distribute_cfs_runtime() gates its clock refresh and runtime
       accounting on cfs_rq->curr, which never fires for cgroup
       cfs_rqs. Since commit 28ad5427682b ("sched/fair: Call
       update_curr() before unthrottling the hierarchy")
       unthrottle_cfs_rq() catches up unconditionally, so this is not
       a correctness hole today, but the refresh the check was written
       for is gone.

I audited all cfs_rq->curr references in kernel/sched/fair.c:

  - Only two sites still run at every level of the hierarchy:
    throttle_cfs_rq() (patch 1/2) and distribute_cfs_runtime()
    (patch 2/2). Both reads are fixed here.

  - Every other reader is confined to the root cfs_rq, where curr is
    correctly maintained (avg_vruntime, place_entity, pick_eevdf,
    enqueue/dequeue paths, update_curr_eevdf, put/set_next_task_fair
    and the two assignment sites), or already reads the per-level
    current via cfs_rq->h_curr (update_curr, check_enqueue_throttle,
    set_next_entity, put_prev_entity).

I noticed that 85570f10a4c6 carries a TODO to eventually get rid of
cfs_rq->h_curr. Until that rework lands, the throttle paths should
observe the per-level current as they did before the conversion; if
you prefer, these fixes can be folded into the planned rework.

Wanwu Li (2):
  sched/fair: Use cfs_rq->h_curr in throttle_cfs_rq()
  sched/fair: Use cfs_rq->h_curr in distribute_cfs_runtime()

 kernel/sched/fair.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

base-commit: 1b78070aaef63512688aebfbc82365ef9d6660f1
-- 
2.34.1
Re: [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths
Posted by Aaron Lu 3 weeks, 4 days ago
On Mon, Aug 31, 2026 at 06:11:39PM +0800, Wanwu Li wrote:
> Hi Peter, Ingo,
> 
> Since commit 85570f10a4c6 ("sched/eevdf: Move to a single runqueue")
> the per-level "is something running at this level" information moved
> from cfs_rq->curr to cfs_rq->h_curr; cfs_rq->curr is now only
> maintained on the root cfs_rq. The cgroup hierarchy is kept for load
> tracking and bandwidth accounting, which makes the bandwidth paths the
> only code that still runs per-level and consults a per-level current.
> Two locations were not updated in this conversion:
> 
>   1/2: throttle_cfs_rq() reads cfs_rq->curr to decide whether the
>        throttled level has a running entity. For intermediate cfs_rqs
>        the check is always false, so quota exhaustion never requests a
>        full sched_cfs_bandwidth_slice() and never arms the deferred
>        throttle task_work via task_throttle_setup_work(); a running
>        task can out-run its group's quota until the next pick armed
>        the work instead.
> 
>   2/2: distribute_cfs_runtime() gates its clock refresh and runtime
>        accounting on cfs_rq->curr, which never fires for cgroup
>        cfs_rqs. Since commit 28ad5427682b ("sched/fair: Call
>        update_curr() before unthrottling the hierarchy")
>        unthrottle_cfs_rq() catches up unconditionally, so this is not
>        a correctness hole today, but the refresh the check was written
>        for is gone.

For this series:
Reviewed-by: Aaron Lu <ziqianlu@bytedance.com>

I also run a test where a nop is affined to a single cpu and I can see
that nop task can use more than its quota from time to time; with this
series applied, this no longer happens, so:
Tested-by: Aaron Lu <ziqianlu@bytedance.com>
Re: [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths
Posted by Wanwu Li 3 weeks, 4 days ago
On Tue, Sep 01, 2026 at 10:38:51AM +0800, Aaron Lu wrote:
> For this series:
> Reviewed-by: Aaron Lu <ziqianlu@bytedance.com>
> 
> I also run a test where a nop is affined to a single cpu and I can see
> that nop task can use more than its quota from time to time; with this
> series applied, this no longer happens, so:
> Tested-by: Aaron Lu <ziqianlu@bytedance.com>

Thanks a lot for the quick review and for taking the time to test,
Aaron. The periodic quota overshoot you observed matches the escaped
deferred-throttle path exactly.

Wanwu
Re: [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths
Posted by Wanwu Li 3 weeks, 4 days ago
On Tue, Sep 01, 2026 at 10:38:51AM +0800, Aaron Lu wrote:
> For this series:
> Reviewed-by: Aaron Lu <ziqianlu@bytedance.com>
> 
> I also run a test where a nop is affined to a single cpu and I can see
> that nop task can use more than its quota from time to time; with this
> series applied, this no longer happens, so:
> Tested-by: Aaron Lu <ziqianlu@bytedance.com>

Thanks a lot for the quick review and for taking the time to test,
Aaron. The periodic quota overshoot you observed matches the escaped
deferred-throttle path exactly.

Wanwu