[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