* [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* [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
* [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* [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
* 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
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®