mirror of
https://github.com/torvalds/linux.git
synced 2026-09-22 12:44:03 +02:00
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: a4103eacc2 ("sched_ext: Add a cgroup scheduler which uses flattened hierarchy")
Suggested-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Tejun Heo <tj@kernel.org>
This commit is contained in:
parent
8d8dd8ae89
commit
9b4243d304
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user