From 9b4243d30469e7733730e786792d1560442e229e Mon Sep 17 00:00:00 2001 From: Tao Cui Date: Sat, 15 Aug 2026 12:59:05 +0800 Subject: [PATCH] sched_ext/scx_flatcg: Fix cvtime true-up on slice expiry fcg_dispatch() true-ups the current cgroup's cvtime when its slice expires or its DSQ runs empty while the slice is still active: __sync_fetch_and_add(&cgc->cvtime_delta, (cpuc->cur_at + cgrp_slice_ns - now) * FCG_HWEIGHT_ONE / (cgc->hweight ?: 1)); The true-up should be actual minus charged: on CNS_EXPIRE, the overrun (now - cur_at - cgrp_slice_ns) should be added; on CNS_EMPTY, the unused portion of the slice should be subtracted. The expression above has the sign inverted, and in the CNS_EXPIRE case now is already past cur_at + cgrp_slice_ns, so the u64 subtraction wraps. The multiplication preserves the two's complement encoding but the unsigned division by hweight destroys it, adding roughly 2^64/hweight per expiry instead of a small correction. Under saturation the hweight budget clamp in cgrp_cap_budget() masks most of the garbage, so the weight distribution barely moves, but the accounting is broken all the same. Compute the delta as a signed value and use fetch_and_add()/fetch_and_sub() so that the dividends stay positive, as BPF division is unsigned. Instrumented the true-up and ran a saturated three-leaf cgroup tree on a 4-CPU VM: without the fix, each expiry added ~5e15 (2^64/hweight territory) to cvtime_delta; with it, the corrections are back to slice scale, with the overrun added and the unused portion subtracted. Fixes: a4103eacc2ab ("sched_ext: Add a cgroup scheduler which uses flattened hierarchy") Suggested-by: Tejun Heo Signed-off-by: Tao Cui Signed-off-by: Tejun Heo --- tools/sched_ext/scx_flatcg.bpf.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tools/sched_ext/scx_flatcg.bpf.c b/tools/sched_ext/scx_flatcg.bpf.c index 2d178c2ecacc..64cf4dd964d6 100644 --- a/tools/sched_ext/scx_flatcg.bpf.c +++ b/tools/sched_ext/scx_flatcg.bpf.c @@ -772,10 +772,18 @@ void BPF_STRUCT_OPS(fcg_dispatch, s32 cpu, struct task_struct *prev) * cgroup to execute but the latter needs to be done in a loop * and we can't keep the lock held. Oh well... */ + s64 delta = now - cpuc->cur_at - cgrp_slice_ns; + bpf_spin_lock(&cgv_tree_lock); - __sync_fetch_and_add(&cgc->cvtime_delta, - (cpuc->cur_at + cgrp_slice_ns - now) * - FCG_HWEIGHT_ONE / (cgc->hweight ?: 1)); + /* keep the dividends positive, BPF division is unsigned */ + 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)); bpf_spin_unlock(&cgv_tree_lock); } else { stat_inc(FCG_STAT_CNS_GONE);