Re: [PATCH 0/9] tools/sched_ext: robustness fixes from cold review

[Tao Cui]

Hello, David,

在 2026/11/10 21:03, David CARLIER 写道:
> Hi Tao
> 
> On Tue, 10 Nov 2026 at 12:50, Tao Cui <cui.tao@xxxxxxxxx <mailto:cui.tao@xxxxxxxxx>> wrote:
> 
>     From: Tao Cui <cuitao@xxxxxxxxxx <mailto:cuitao@xxxxxxxxxx>>
> 
>     Hello,
> 
>     This series fixes nine robustness issues in the example schedulers
>     scx_pair, scx_flatcg, scx_qmap and scx_userland.
> 
>     scx_flatcg resets its stats baseline on restart (stale counters print
>     as u64 underflow garbage) and rejects a stats interval that puts
>     nanosleep in an error loop (a negative or non-finite -i busy-spins at
>     100% CPU). scx_pair stops aborting the scheduler when the dispatching
>     cgroup goes away mid-dispatch, gates attach on an interrupted
>     initialization instead of running with a half-built cgrp_q_arr, resets
>     pair_id on restart, and cleans up a dead busy-check whose fall-through
>     clears another cgroup's busy marker. scx_qmap documents the options it
>     actually accepts and resets its hier stats baseline on restart.
>     scx_userland bounds the batch against the dispatch buffer instead of
>     overflowing it 13ms in, and rejects batch size 0, which livelocks the
>     machine until the stall watchdog fires.
> 
>     Verified in qemu on tj/sched_ext for-next (5b740a72b55c), same
>     kernel and config with and without each patch, comparing stock and
>     patched binaries: a scx_flatcg restart prints six
>     u64-underflow garbage counters on stock and zeros with patch 1; a
>     non-finite -i busy-spins ~17k EINVAL lines in 3s on stock and is
>     rejected at parse time with patch 2; SIGINT storms abort 20/30
>     interrupted inits on stock with the scheduler never loading and exit
>     30/30 clean with patch 4; a scx_pair restart aborts 7ms in with a
>     stale pair_id on stock and runs clean for its full window with patch
>     5; an oversized scx_userland batch aborts 13ms in with "dispatch
>     buffer overflow" on stock and survives 25s with patch 9; and batch
>     size 0, which livelocks the machine until the stall watchdog fires
>     (spinner progress collapses to ~1%), is rejected immediately with
>     patch 8. Beyond the A/B pairs: the scx_pair cgid-cache churn window
>     is too narrow to drive (40 rounds clean on both binaries; patch 3 is
>     graded by inspection), the qmap synopsis diff shows exactly the three
>     added options (patch 6), and the hier stats baseline was verified with
>     a clang-23.1.5-built scx_qmap run through restart cycles (patch 7,
>     hier stats clean throughout).
> 
>     Based on sched_ext/for-next (5b740a72b55c).
> 
>     Tao Cui (9):
>       tools/sched_ext: scx_flatcg: Reset the stats baseline on restart
>       tools/sched_ext: scx_flatcg: Validate the stats interval
>       tools/sched_ext: scx_pair: Don't abort when the dispatching cgroup
>         goes away
>       tools/sched_ext: scx_pair: Don't attach after an interrupted
>         initialization
>       tools/sched_ext: scx_pair: Reset pair_id on restart
>       tools/sched_ext: scx_qmap: Add the missing options to the usage
>         synopsis
>       tools/sched_ext: scx_qmap: Reset the hier stats baseline on restart
>       tools/sched_ext: scx_userland: Check dispatch buffer slots before
>         inserting
>       tools/sched_ext: scx_userland: Reject batch size 0
> 
>      tools/sched_ext/scx_flatcg.c       |  7 ++++++-
>      tools/sched_ext/scx_pair.bpf.c     | 12 ++++++++++--
>      tools/sched_ext/scx_pair.c         | 14 ++++++++++++--
>      tools/sched_ext/scx_qmap.c         |  3 ++-
>      tools/sched_ext/scx_userland.bpf.c |  8 ++++++++
>      tools/sched_ext/scx_userland.c     |  5 +++++
>      6 files changed, 43 insertions(+), 6 deletions(-)
> 
>     -- 
>     2.53.0
> 
> 
> I might be missing something, but the cover letter mentions a scx_pair busy-check cleanup that isn't
> in the series, and the verification paragraph swaps patches 8 and 9 it seems.
> 
> hope it helps. 
You're right on both counts. Thanks for the careful read.

The busy-check sentence is a leftover. The series originally included
that cleanup, but I dropped it before posting because it became clear
the path it removed has no observable behavioral difference (the
would-be hijacking update always fails with -EBUSY anyway). I neglected
to update the cover letter accordingly.

The patch 8/9 swap is also just an editing mistake from the same late
rewrite of the verification paragraph.

Thanks,
Tao