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

[Wanwu Li]

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