mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths
@ 2026-08-31 10:11 Wanwu Li
  2026-08-31 10:11 ` [PATCH 1/2] sched/fair: Use cfs_rq->h_curr in throttle_cfs_rq() Wanwu Li
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Wanwu Li @ 2026-08-31 10:11 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot
  Cc: Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, K Prateek Nayak, Aaron Lu, Wanwu Li,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] sched/fair: Use cfs_rq->h_curr in throttle_cfs_rq()
  2026-08-31 10:11 [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths Wanwu Li
@ 2026-08-31 10:11 ` Wanwu Li
  2026-09-02  7:22   ` [tip: sched/urgent] " tip-bot2 for Wanwu Li
  2026-08-31 10:11 ` [PATCH 2/2] sched/fair: Use cfs_rq->h_curr in distribute_cfs_runtime() Wanwu Li
  2026-09-01  2:38 ` [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths Aaron Lu
  2 siblings, 1 reply; 8+ messages in thread
From: Wanwu Li @ 2026-08-31 10:11 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot
  Cc: Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, K Prateek Nayak, Aaron Lu, Wanwu Li,
	linux-kernel

After commit 85570f10a4c6 ("sched/eevdf: Move to a single runqueue"),
cfs_rq->curr is only maintained on the root cfs_rq (set/cleared from
set_next_task_fair()/put_prev_task_fair()), while cfs_rq->h_curr is
the per-level current entity, set by set_next_entity() at every level
of the hierarchy. For an intermediate cfs_rq (a cgroup), cfs_rq->curr
is always NULL, but cfs_rq->h_curr is the group entity at that level.

throttle_cfs_rq() reads cfs_rq->curr to decide whether there is a
running entity at the throttled level, in which case it should request
a full sched_cfs_bandwidth_slice() of runtime and arm the deferred
throttle task_work via task_throttle_setup_work(). For intermediate
cfs_rqs the check is always false, so bandwidth-controlled cgroups
always get just 1ns of runtime and never arm the deferred throttle
work; the running task then escapes throttling until the next pick
arms the work instead, even though there is an on-rq entity at this
level.

Switch the read to cfs_rq->h_curr so intermediate bandwidth-controlled
cgroups behave consistently with the root cfs_rq, matching the
existing usage of cfs_rq->h_curr in update_curr() and
check_enqueue_throttle().

Fixes: 85570f10a4c6 ("sched/eevdf: Move to a single runqueue")
Signed-off-by: Wanwu Li <liwanwu@kylinos.cn>
---
 kernel/sched/fair.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 6d881e530f89..02b38d84fb2d 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6978,14 +6978,14 @@ static int tg_throttle_down(struct task_group *tg, void *data)
 static bool throttle_cfs_rq(struct cfs_rq *cfs_rq)
 {
 	struct cfs_bandwidth *cfs_b = tg_cfs_bandwidth(cfs_rq->tg);
-	struct sched_entity *curr = cfs_rq->curr;
+	struct sched_entity *curr = cfs_rq->h_curr;
 	struct rq *rq = rq_of(cfs_rq);
 
 	scoped_guard(raw_spinlock, &cfs_b->lock) {
 		u64 target_runtime = 1;
 
 		/*
-		 * If cfs_rq->curr is still runnable, we are here from an
+		 * If cfs_rq->h_curr is still runnable, we are here from an
 		 * update_curr(). Request sysctl_sched_cfs_bandwidth_slice
 		 * worth of bandwidth to continue running.
 		 *
-- 
2.34.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] sched/fair: Use cfs_rq->h_curr in distribute_cfs_runtime()
  2026-08-31 10:11 [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths Wanwu Li
  2026-08-31 10:11 ` [PATCH 1/2] sched/fair: Use cfs_rq->h_curr in throttle_cfs_rq() Wanwu Li
@ 2026-08-31 10:11 ` Wanwu Li
  2026-09-02  7:21   ` [tip: sched/urgent] " tip-bot2 for Wanwu Li
  2026-09-01  2:38 ` [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths Aaron Lu
  2 siblings, 1 reply; 8+ messages in thread
From: Wanwu Li @ 2026-08-31 10:11 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot
  Cc: Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, K Prateek Nayak, Aaron Lu, Wanwu Li,
	linux-kernel

distribute_cfs_runtime() refreshes the rq clock and accounts elapsed
runtime with update_curr() before redistributing bandwidth, but gates
this on cfs_rq->curr. Since commit 85570f10a4c6 ("sched/eevdf: Move to
a single runqueue") cfs_rq->curr is only maintained on the root
cfs_rq, so for the cgroup cfs_rqs it walks, the check never fires and
the refresh is dead code.

Use cfs_rq->h_curr, the per-level current entity, restoring the
intended behaviour: only refresh when something is actually running at
the throttled level, i.e. within the deferred throttle window.
Without this, runtime consumed by a still-running task of the
throttled hierarchy is not docked before redistribution;
unthrottle_cfs_rq() catches up unconditionally since
commit 28ad5427682b ("sched/fair: Call update_curr() before
unthrottling the hierarchy"), so this is not a correctness hole today,
but the refresh the check was written for is gone.

Fixes: 85570f10a4c6 ("sched/eevdf: Move to a single runqueue")
Signed-off-by: Wanwu Li <liwanwu@kylinos.cn>
---
 kernel/sched/fair.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 6d881e530f89..0c69d1c0ebf0 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7192,7 +7192,7 @@ static bool distribute_cfs_runtime(struct cfs_bandwidth *cfs_b)
 		if (!list_empty(&cfs_rq->throttled_csd_list))
 			continue;
 
-		if (cfs_rq->curr) {
+		if (cfs_rq->h_curr) {
 			update_rq_clock(rq);
 			update_curr(cfs_rq);
 		}
-- 
2.34.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths
  2026-08-31 10:11 [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths Wanwu Li
  2026-08-31 10:11 ` [PATCH 1/2] sched/fair: Use cfs_rq->h_curr in throttle_cfs_rq() Wanwu Li
  2026-08-31 10:11 ` [PATCH 2/2] sched/fair: Use cfs_rq->h_curr in distribute_cfs_runtime() Wanwu Li
@ 2026-09-01  2:38 ` Aaron Lu
  2026-09-01  4:09   ` Wanwu Li
  2026-09-01  4:09   ` Wanwu Li
  2 siblings, 2 replies; 8+ messages in thread
From: Aaron Lu @ 2026-09-01  2:38 UTC (permalink / raw)
  To: Wanwu Li
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, K Prateek Nayak, linux-kernel

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>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths
  2026-09-01  2:38 ` [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths Aaron Lu
@ 2026-09-01  4:09   ` Wanwu Li
  2026-09-01  4:09   ` Wanwu Li
  1 sibling, 0 replies; 8+ messages in thread
From: Wanwu Li @ 2026-09-01  4:09 UTC (permalink / raw)
  To: Aaron Lu
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, K Prateek Nayak, linux-kernel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths
  2026-09-01  2:38 ` [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths Aaron Lu
  2026-09-01  4:09   ` Wanwu Li
@ 2026-09-01  4:09   ` Wanwu Li
  1 sibling, 0 replies; 8+ messages in thread
From: Wanwu Li @ 2026-09-01  4:09 UTC (permalink / raw)
  To: Aaron Lu
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, K Prateek Nayak, linux-kernel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [tip: sched/urgent] sched/fair: Use cfs_rq->h_curr in distribute_cfs_runtime()
  2026-08-31 10:11 ` [PATCH 2/2] sched/fair: Use cfs_rq->h_curr in distribute_cfs_runtime() Wanwu Li
@ 2026-09-02  7:21   ` tip-bot2 for Wanwu Li
  0 siblings, 0 replies; 8+ messages in thread
From: tip-bot2 for Wanwu Li @ 2026-09-02  7:21 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Wanwu Li, Peter Zijlstra (Intel), Aaron Lu, x86, linux-kernel

The following commit has been merged into the sched/urgent branch of tip:

Commit-ID:     b038383526d8c7883ea0486dd1911102b6dda414
Gitweb:        https://git.kernel.org/tip/b038383526d8c7883ea0486dd1911102b6dda414
Author:        Wanwu Li <liwanwu@kylinos.cn>
AuthorDate:    Mon, 31 Aug 2026 18:11:41 +08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 02 Sep 2026 09:17:50 +02:00

sched/fair: Use cfs_rq->h_curr in distribute_cfs_runtime()

distribute_cfs_runtime() refreshes the rq clock and accounts elapsed
runtime with update_curr() before redistributing bandwidth, but gates
this on cfs_rq->curr. Since commit 85570f10a4c6 ("sched/eevdf: Move to
a single runqueue") cfs_rq->curr is only maintained on the root
cfs_rq, so for the cgroup cfs_rqs it walks, the check never fires and
the refresh is dead code.

Use cfs_rq->h_curr, the per-level current entity, restoring the
intended behaviour: only refresh when something is actually running at
the throttled level, i.e. within the deferred throttle window.
Without this, runtime consumed by a still-running task of the
throttled hierarchy is not docked before redistribution;
unthrottle_cfs_rq() catches up unconditionally since
commit 28ad5427682b ("sched/fair: Call update_curr() before
unthrottling the hierarchy"), so this is not a correctness hole today,
but the refresh the check was written for is gone.

Fixes: 85570f10a4c6 ("sched/eevdf: Move to a single runqueue")
Signed-off-by: Wanwu Li <liwanwu@kylinos.cn>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Aaron Lu <ziqianlu@bytedance.com>
Tested-by: Aaron Lu <ziqianlu@bytedance.com>
Link: https://patch.msgid.link/20260831101141.391382-3-liwanwu@kylinos.cn
---
 kernel/sched/fair.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 73797d6..97021a5 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7192,7 +7192,7 @@ static bool distribute_cfs_runtime(struct cfs_bandwidth *cfs_b)
 		if (!list_empty(&cfs_rq->throttled_csd_list))
 			continue;
 
-		if (cfs_rq->curr) {
+		if (cfs_rq->h_curr) {
 			update_rq_clock(rq);
 			update_curr(cfs_rq);
 		}

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [tip: sched/urgent] sched/fair: Use cfs_rq->h_curr in throttle_cfs_rq()
  2026-08-31 10:11 ` [PATCH 1/2] sched/fair: Use cfs_rq->h_curr in throttle_cfs_rq() Wanwu Li
@ 2026-09-02  7:22   ` tip-bot2 for Wanwu Li
  0 siblings, 0 replies; 8+ messages in thread
From: tip-bot2 for Wanwu Li @ 2026-09-02  7:22 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Wanwu Li, Peter Zijlstra (Intel), Aaron Lu, x86, linux-kernel

The following commit has been merged into the sched/urgent branch of tip:

Commit-ID:     f8610c57f4078c63d1d4e2f3d7134f3dc1768403
Gitweb:        https://git.kernel.org/tip/f8610c57f4078c63d1d4e2f3d7134f3dc1768403
Author:        Wanwu Li <liwanwu@kylinos.cn>
AuthorDate:    Mon, 31 Aug 2026 18:11:40 +08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 02 Sep 2026 09:17:49 +02:00

sched/fair: Use cfs_rq->h_curr in throttle_cfs_rq()

After commit 85570f10a4c6 ("sched/eevdf: Move to a single runqueue"),
cfs_rq->curr is only maintained on the root cfs_rq (set/cleared from
set_next_task_fair()/put_prev_task_fair()), while cfs_rq->h_curr is
the per-level current entity, set by set_next_entity() at every level
of the hierarchy. For an intermediate cfs_rq (a cgroup), cfs_rq->curr
is always NULL, but cfs_rq->h_curr is the group entity at that level.

throttle_cfs_rq() reads cfs_rq->curr to decide whether there is a
running entity at the throttled level, in which case it should request
a full sched_cfs_bandwidth_slice() of runtime and arm the deferred
throttle task_work via task_throttle_setup_work(). For intermediate
cfs_rqs the check is always false, so bandwidth-controlled cgroups
always get just 1ns of runtime and never arm the deferred throttle
work; the running task then escapes throttling until the next pick
arms the work instead, even though there is an on-rq entity at this
level.

Switch the read to cfs_rq->h_curr so intermediate bandwidth-controlled
cgroups behave consistently with the root cfs_rq, matching the
existing usage of cfs_rq->h_curr in update_curr() and
check_enqueue_throttle().

Fixes: 85570f10a4c6 ("sched/eevdf: Move to a single runqueue")
Signed-off-by: Wanwu Li <liwanwu@kylinos.cn>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Aaron Lu <ziqianlu@bytedance.com>
Tested-by: Aaron Lu <ziqianlu@bytedance.com>
Link: https://patch.msgid.link/20260831101141.391382-2-liwanwu@kylinos.cn
---
 kernel/sched/fair.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 5d47de5..73797d6 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6978,14 +6978,14 @@ static int tg_throttle_down(struct task_group *tg, void *data)
 static bool throttle_cfs_rq(struct cfs_rq *cfs_rq)
 {
 	struct cfs_bandwidth *cfs_b = tg_cfs_bandwidth(cfs_rq->tg);
-	struct sched_entity *curr = cfs_rq->curr;
+	struct sched_entity *curr = cfs_rq->h_curr;
 	struct rq *rq = rq_of(cfs_rq);
 
 	scoped_guard(raw_spinlock, &cfs_b->lock) {
 		u64 target_runtime = 1;
 
 		/*
-		 * If cfs_rq->curr is still runnable, we are here from an
+		 * If cfs_rq->h_curr is still runnable, we are here from an
 		 * update_curr(). Request sysctl_sched_cfs_bandwidth_slice
 		 * worth of bandwidth to continue running.
 		 *

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-09-02  7:22 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-31 10:11 [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths Wanwu Li
2026-08-31 10:11 ` [PATCH 1/2] sched/fair: Use cfs_rq->h_curr in throttle_cfs_rq() Wanwu Li
2026-09-02  7:22   ` [tip: sched/urgent] " tip-bot2 for Wanwu Li
2026-08-31 10:11 ` [PATCH 2/2] sched/fair: Use cfs_rq->h_curr in distribute_cfs_runtime() Wanwu Li
2026-09-02  7:21   ` [tip: sched/urgent] " tip-bot2 for Wanwu Li
2026-09-01  2:38 ` [PATCH 0/2] sched/fair: Use cfs_rq->h_curr in the bandwidth paths Aaron Lu
2026-09-01  4:09   ` Wanwu Li
2026-09-01  4:09   ` Wanwu Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®