[PATCH] sched_ext/scx_flatcg: expire cached hweights on weight changes

[PATCH] sched_ext/scx_flatcg: expire cached hweights on weight changes

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

fcg_cgroup_set_weight() updates cgc->weight and the parent's
child_weight_sum but doesn't bump hweight_gen, so the hweights cached by
cgrp_refresh_hweight() stay stale until some task activation bumps the
generation. For cgroups whose tasks never go through a 0->n runnable
transition (e.g. persistently busy ones), a cpu.weight change never
propagates to scheduling at all.

Bump hweight_gen on weight changes so the next refresh recomputes with
the new weight.

Verified on a flatcg VM: a live cpu.weight 100->800 change on a busy
cgroup leaves HWT update at 0 and the distribution unchanged; with it,
hweight_gen increments and the refresh recomputes.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 tools/sched_ext/scx_flatcg.bpf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/sched_ext/scx_flatcg.bpf.c b/tools/sched_ext/scx_flatcg.bpf.c
index 0fd214cc61da..2d178c2ecacc 100644
--- a/tools/sched_ext/scx_flatcg.bpf.c
+++ b/tools/sched_ext/scx_flatcg.bpf.c
@@ -605,6 +605,9 @@ void BPF_STRUCT_OPS(fcg_cgroup_set_weight, struct cgroup *cgrp, u32 weight)
 		pcgc->child_weight_sum += (s64)weight - cgc->weight;
 	cgc->weight = weight;
 	bpf_spin_unlock(&cgv_tree_lock);
+
+	/* expire cached hweights so the new weight propagates */
+	__sync_fetch_and_add(&hweight_gen, 1);
 }

 static bool try_pick_next_cgroup(u64 *cgidp)
-- 
2.43.0

Re: [PATCH] sched_ext/scx_flatcg: expire cached hweights on weight changes

[Tao Cui]

在 2026/8/14 22:41, Tao Cui 写道:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> fcg_cgroup_set_weight() updates cgc->weight and the parent's
> child_weight_sum but doesn't bump hweight_gen, so the hweights cached by
> cgrp_refresh_hweight() stay stale until some task activation bumps the
> generation. For cgroups whose tasks never go through a 0->n runnable
> transition (e.g. persistently busy ones), a cpu.weight change never
> propagates to scheduling at all.
> 
> Bump hweight_gen on weight changes so the next refresh recomputes with
> the new weight.
> 
> Verified on a flatcg VM: a live cpu.weight 100->800 change on a busy
> cgroup leaves HWT update at 0 and the distribution unchanged; with it,
> hweight_gen increments and the refresh recomputes.
> 
Hello,

Some background on how I found this: I was running scx_flatcg in a VM
with a simple cgroup hierarchy to check how it distributes CPU under
different cpu.weight values --

    A (100) -> A1 (100), A2 (100)
    D (weight varies)

with 3 busy tasks in each leaf cgroup, 4 vCPUs, measuring cpu.stat
usage_usec deltas over 15 seconds.

D=200 gave D roughly 47%; D=800 gave 46% -- the distribution barely
responded to weight at all. That led me to the missing hweight_gen
bump, which this patch fixes.

But even with the fix applied, the distribution still doesn't follow
the documented compounding model. With D=800 (compounded share should
be 800/900 = 89%), D only gets around 49%. Removing the /2 in
cgrp_cap_budget()'s max_budget didn't help either (50%).

From what I can tell, the budget clamp is proportional to hweight, so
light cgroups get a very small budget and are clamped right behind the
cvtime leader, which keeps them competitive regardless of their
weight. Under saturation with persistently busy tasks, the
distribution compresses to near-equal no matter what the weights are.

Is this the intended behavior of the budget clamping, or should the
steady-state distribution converge to the compounded shares over time?
The header comment's model doesn't seem to hold in this scenario.

> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> ---
>  tools/sched_ext/scx_flatcg.bpf.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/sched_ext/scx_flatcg.bpf.c b/tools/sched_ext/scx_flatcg.bpf.c
> index 0fd214cc61da..2d178c2ecacc 100644
> --- a/tools/sched_ext/scx_flatcg.bpf.c
> +++ b/tools/sched_ext/scx_flatcg.bpf.c
> @@ -605,6 +605,9 @@ void BPF_STRUCT_OPS(fcg_cgroup_set_weight, struct cgroup *cgrp, u32 weight)
>  		pcgc->child_weight_sum += (s64)weight - cgc->weight;
>  	cgc->weight = weight;
>  	bpf_spin_unlock(&cgv_tree_lock);
> +
> +	/* expire cached hweights so the new weight propagates */
> +	__sync_fetch_and_add(&hweight_gen, 1);
>  }
> 
>  static bool try_pick_next_cgroup(u64 *cgidp)

Re: [PATCH] sched_ext/scx_flatcg: expire cached hweights on weight changes

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2098 bytes --]

> sched_ext/scx_flatcg: expire cached hweights on weight changes
>
> fcg_cgroup_set_weight() updates cgc->weight and the parent's
> child_weight_sum but doesn't bump hweight_gen, so the hweights cached by
> cgrp_refresh_hweight() stay stale until some task activation bumps the
> generation. For cgroups whose tasks never go through a 0->n runnable
> transition (e.g. persistently busy ones), a cpu.weight change never
> propagates to scheduling at all.
>
> Bump hweight_gen on weight changes so the next refresh recomputes with
> the new weight.
>
> Verified on a flatcg VM: a live cpu.weight 100->800 change on a busy
> cgroup leaves HWT update at 0 and the distribution unchanged; with it,
> hweight_gen increments and the refresh recomputes.

This looks like a bug fix. Should this include a Fixes: tag pointing to
the commit that introduced the scx_flatcg scheduler?

  Fixes: a4103eacc2ab4 ("sched_ext: Add a cgroup scheduler which uses flattened hierarchy")

This isn't a bug, but should the verification sentence say "without this
change" to make the "with it" contrast more explicit? The first clause
lacks a matching counterpart, so on first read the "with it" appears to
refer back to something unstated.

>
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>

> diff --git a/tools/sched_ext/scx_flatcg.bpf.c b/tools/sched_ext/scx_flatcg.bpf.c
> index fec3595818269..059b68ca00cbc 100644
> --- a/tools/sched_ext/scx_flatcg.bpf.c
> +++ b/tools/sched_ext/scx_flatcg.bpf.c
> @@ -604,6 +604,9 @@ void BPF_STRUCT_OPS(fcg_cgroup_set_weight, struct cgroup *cgrp, u32 weight)
>  		pcgc->child_weight_sum += (s64)weight - cgc->weight;
>  	cgc->weight = weight;
>  	bpf_spin_unlock(&cgv_tree_lock);
> +
> +	/* expire cached hweights so the new weight propagates */
> +	__sync_fetch_and_add(&hweight_gen, 1);
>  }
>
>  static bool try_pick_next_cgroup(u64 *cgidp)


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31820156939

Re: [PATCH] sched_ext/scx_flatcg: expire cached hweights on weight changes

[Tejun Heo]

On Fri, Aug 14, 2026 at 10:41:16PM +0800, Tao Cui wrote:
> fcg_cgroup_set_weight() updates cgc->weight and the parent's
> child_weight_sum but doesn't bump hweight_gen, so the hweights cached by
> cgrp_refresh_hweight() stay stale until some task activation bumps the
> generation. For cgroups whose tasks never go through a 0->n runnable
> transition (e.g. persistently busy ones), a cpu.weight change never
> propagates to scheduling at all.

Applied to sched_ext/for-7.3.

Thanks.

--
tejun

Re: [PATCH] sched_ext/scx_flatcg: expire cached hweights on weight changes

[Tejun Heo]

Hello,

On Fri, Aug 14, 2026 at 10:48:35PM +0800, Tao Cui wrote:
> Is this the intended behavior of the budget clamping, or should the
> steady-state distribution converge to the compounded shares over time?
> The header comment's model doesn't seem to hold in this scenario.

The clamp isn't the culprit. I reproduced your setup (3 busy tasks per
leaf, 4 CPUs) and dug in.

The dominant factor is that D is runnable-task limited. At weight 800
its compounded share is 3.55 CPUs but it only has 3 tasks, so 75% is
the best it can do. Below that cap, the window granting loses more:
whenever all three of D's tasks are already running and another CPU
picks D, the pop from D's DSQ comes up empty, the cgv_node gets
stashed and that CPU grants a full cgrp_slice_ns window to another
cgroup. The loss scales with the window size. Measured D shares at
weight 800:

	3 tasks/leaf			55-59%
	3 tasks/leaf, 100ms slices	52%
	5 tasks/leaf			84-86%

Disabling the clamp entirely is the 59% above, so it barely matters.
With enough runnable tasks per cgroup, the distribution converges to
the documented compounding. The model holds, but only when no cgroup
is runnable-task limited, and the window granting degrades sooner than
per-task fair queueing would as that limit is approached. That's an
inherent simplification of this example scheduler.

Separately, while digging into this, I found that the true-up in
fcg_dispatch() is broken:

	__sync_fetch_and_add(&cgc->cvtime_delta,
			     (cpuc->cur_at + cgrp_slice_ns - now) *
			     FCG_HWEIGHT_ONE / (cgc->hweight ?: 1));

In the CNS_EXPIRE case, now is past cur_at + cgrp_slice_ns, so the u64
subexpression wraps. The multiplication preserves the two's complement
encoding but the unsigned division by hweight destroys it, adding about
2^64 / hweight per expiry instead of a small correction. The sign is
also inverted. The true-up should be actual minus charged, so the
expiry overrun should be added and the CNS_EMPTY unused portion
subtracted. Under saturation the budget clamp mostly masks the garbage,
which is why the numbers above barely move with it fixed (the 86% in
the 5 tasks/leaf row), but the accounting is broken all the same. The
following fixes it and tests fine (BPF division is unsigned, keep the
dividends positive):

	s64 delta = now - cpuc->cur_at - cgrp_slice_ns;

	if (delta >= 0)
		__sync_fetch_and_add(&cgc->cvtime_delta,
				     (u64)delta * FCG_HWEIGHT_ONE /
				     (cgc->hweight ?: 1));
	else
		__sync_fetch_and_sub(&cgc->cvtime_delta,
				     (u64)-delta * FCG_HWEIGHT_ONE /
				     (cgc->hweight ?: 1));

Care to send a patch?

Thanks.

--
tejun