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

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

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

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

[PATCH 1/9] tools/sched_ext: scx_flatcg: Reset the stats baseline on restart

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

last_stats holds the previous accumulator snapshot for the stats
deltas and is initialized before the restart: label. A hotplug
fail-safe reload starts the scheduler with fresh counters while
last_stats still carries the previous generation's snapshots, so the
first stats interval prints u64-underflow garbage.

Reset it together with the rest of the reload state. last_cpu_sum and
last_cpu_idle anchor to /proc/stat which persists across reloads and
are intentionally left alone.

Fixes: a4103eacc2ab ("sched_ext: Add a cgroup scheduler which uses flattened hierarchy")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 tools/sched_ext/scx_flatcg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/sched_ext/scx_flatcg.c b/tools/sched_ext/scx_flatcg.c
index a223bff3746a..4c3c2cca2b85 100644
--- a/tools/sched_ext/scx_flatcg.c
+++ b/tools/sched_ext/scx_flatcg.c
@@ -140,6 +140,7 @@ int main(int argc, char **argv)
 	signal(SIGINT, sigint_handler);
 	signal(SIGTERM, sigint_handler);
 restart:
+	memset(last_stats, 0, sizeof(last_stats));
 	optind = 1;
 	skel = SCX_OPS_OPEN(flatcg_ops, scx_flatcg);
 
-- 
2.53.0

base-commit: 5b740a72b55c4267ba4d8010f17dde26b26f52d3

[PATCH 2/9] tools/sched_ext: scx_flatcg: Validate the stats interval

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

The interval is parsed with strtod() and split into a timespec with a
float cast in the middle: a negative value produces a negative tv_sec
or tv_nsec (for -0.5 the seconds truncate to zero and the nanoseconds
go negative), and any value that doesn't survive the float round-trip
(e.g. 2^24 + 1) produces tv_nsec >= 1e9. Either way nanosleep()
returns EINVAL, which is never checked, and the stats loop starts
busy-spinning at 100% CPU instead of sampling at the requested
interval.

Reject negative intervals up front and compute the nsec part from the
double rather than through a float so the rounding hole is gone.

Fixes: a4103eacc2ab ("sched_ext: Add a cgroup scheduler which uses flattened hierarchy")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 tools/sched_ext/scx_flatcg.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/sched_ext/scx_flatcg.c b/tools/sched_ext/scx_flatcg.c
index 4c3c2cca2b85..bac6535fa2f0 100644
--- a/tools/sched_ext/scx_flatcg.c
+++ b/tools/sched_ext/scx_flatcg.c
@@ -158,8 +158,12 @@ int main(int argc, char **argv)
 			break;
 		case 'i':
 			v = strtod(optarg, NULL);
+			if (!(v >= 0) || v >= 1e18) {
+				fprintf(stderr, "Invalid interval %f\n", v);
+				exit(1);
+			}
 			intv_ts.tv_sec = v;
-			intv_ts.tv_nsec = (v - (float)intv_ts.tv_sec) * 1000000000;
+			intv_ts.tv_nsec = (v - intv_ts.tv_sec) * 1000000000;
 			break;
 		case 'f':
 			skel->rodata->fifo_sched = true;
-- 
2.53.0

base-commit: 5b740a72b55c4267ba4d8010f17dde26b26f52d3

[PATCH 3/9] tools/sched_ext: scx_pair: Don't abort when the dispatching cgroup goes away

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

try_dispatch() caches the cgroup id in pairc->cgid, drops pairc->lock
and then looks the cgroup back up in cgrp_q_idx_hash. pair_cgroup_exit()
deletes that hash entry when the cgroup is removed, so a plain rmdir of
the cgroup that is currently being dispatched, a normal event for
short-lived containers or cgroup churn, hits the lookup failure and
scx_bpf_error() takes the whole scheduler down.

The expiry path already knows how to re-pick a cgroup; make the fast
path tolerate the disappearance too. Mark the pair as drained on lookup
failure and return, letting the next dispatch expire the batch and pick
the next cgroup instead of aborting.

The enqueue side is left for a follow-up: a lookup miss is reachable
there in principle (css offline runs under the cgroup mutex alone,
and a sleeping-task migrate-out does not take the rq lock), but
handling it gracefully needs somewhere to put the task, which on the
enqueue path means a fallback queue; that is a larger change than
this fix scopes. The dispatch path, where the miss costs one skipped batch,
is the cheap half.

Fixes: f0262b102c7c ("tools/sched_ext: add scx_pair scheduler")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 tools/sched_ext/scx_pair.bpf.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/sched_ext/scx_pair.bpf.c b/tools/sched_ext/scx_pair.bpf.c
index 0d61b7b812db..d38d3a078ade 100644
--- a/tools/sched_ext/scx_pair.bpf.c
+++ b/tools/sched_ext/scx_pair.bpf.c
@@ -478,8 +478,16 @@ static int try_dispatch(s32 cpu)
 	/* again, it'd be better to do all these with the lock held, oh well */
 	vptr = bpf_map_lookup_elem(&cgrp_q_idx_hash, &cgid);
 	if (!vptr) {
-		scx_bpf_error("failed to lookup q_idx for cgroup[%llu]", cgid);
-		return -ENOENT;
+		/*
+		 * @cgid can disappear from under us: pair_cgroup_exit()
+		 * drops the hash entry when the cgroup is going away.
+		 * Mark the pair as drained and let the next dispatch
+		 * expire it instead of taking the whole scheduler down.
+		 */
+		bpf_spin_lock(&pairc->lock);
+		pairc->draining = true;
+		bpf_spin_unlock(&pairc->lock);
+		goto out_maybe_kick;
 	}
 	q_idx = *vptr;
 
-- 
2.53.0

base-commit: 5b740a72b55c4267ba4d8010f17dde26b26f52d3

[PATCH 4/9] tools/sched_ext: scx_pair: Don't attach after an interrupted initialization

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

The cgrp_q_arr initialization loop checks exit_req and breaks early so
that Ctrl-C doesn't make the user wait through all MAX_CGRPS iterations
- but the break falls straight into SCX_OPS_ATTACH(). Attaching with
part of the inner queue maps missing lets ops.cgroup_init() hand out
slots for cgroups whose inner map was never created, and the first task
in such a cgroup aborts the scheduler with an scx_bpf_error(). On a busy
system the 4096-iteration loop takes long enough that hitting it with
Ctrl-C is likely, so the intended responsive interrupt turns into a
scheduler crash dump.

Exit cleanly when interrupted mid-initialization instead of attaching.

Fixes: f0262b102c7c ("tools/sched_ext: add scx_pair scheduler")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 tools/sched_ext/scx_pair.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/sched_ext/scx_pair.c b/tools/sched_ext/scx_pair.c
index 00f595b58f97..35c8a1a07f56 100644
--- a/tools/sched_ext/scx_pair.c
+++ b/tools/sched_ext/scx_pair.c
@@ -161,8 +161,17 @@ int main(int argc, char **argv)
 	printf("\n");
 
 	/*
-	 * Fully initialized, attach and run.
+	 * Fully initialized, attach and run. An interrupt during the
+	 * initialization loop leaves the tail of cgrp_q_arr without inner
+	 * maps; attaching in that state lets ops.cgroup_init() hand out
+	 * uninitialized slots and abort the scheduler on the first cgroup
+	 * task.
 	 */
+	if (exit_req) {
+		scx_pair__destroy(skel);
+		return 1;
+	}
+
 	link = SCX_OPS_ATTACH(skel, pair_ops, scx_pair);
 
 	while (!exit_req && !UEI_EXITED(skel, uei)) {
-- 
2.53.0

base-commit: 5b740a72b55c4267ba4d8010f17dde26b26f52d3

[PATCH 5/9] tools/sched_ext: scx_pair: Reset pair_id on restart

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

pair_id is initialized to 0 before the restart: label, so a reload
triggered by an ops restart (e.g. a hotplug event with the scheduler
providing no cpu_online/cpu_offline callbacks) jumps back to the label
with pair_id still holding the previous run's counter. Since f546c77038ab
("tools/sched_ext: scx_pair: fix pair_ctx indexing for CPU pairs")
pair_id is a dense index into the pair_ctx array, so the second load
starts pairing CPUs past the end of the array and the scheduler can
never come up again.

Initialize pair_id after the label so every reload starts from a clean
counter.

Fixes: f546c77038ab ("tools/sched_ext: scx_pair: fix pair_ctx indexing for CPU pairs")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 tools/sched_ext/scx_pair.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/sched_ext/scx_pair.c b/tools/sched_ext/scx_pair.c
index 35c8a1a07f56..f15093e002f0 100644
--- a/tools/sched_ext/scx_pair.c
+++ b/tools/sched_ext/scx_pair.c
@@ -48,12 +48,13 @@ int main(int argc, char **argv)
 	struct bpf_link *link;
 	__u64 seq = 0, ecode;
 	__s32 stride, i, opt, outer_fd;
-	__u32 pair_id = 0;
+	__u32 pair_id;
 
 	libbpf_set_print(libbpf_print_fn);
 	signal(SIGINT, sigint_handler);
 	signal(SIGTERM, sigint_handler);
 restart:
+	pair_id = 0;
 	optind = 1;
 	skel = SCX_OPS_OPEN(pair_ops, scx_pair);
 
-- 
2.53.0

base-commit: 5b740a72b55c4267ba4d8010f17dde26b26f52d3

[PATCH 6/9] tools/sched_ext: scx_qmap: Add the missing options to the usage synopsis

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

-C, -B and -q are described in the option list but missing from the
usage synopsis. Add them.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 tools/sched_ext/scx_qmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/sched_ext/scx_qmap.c b/tools/sched_ext/scx_qmap.c
index fe2f206f5c8a..f44802247069 100644
--- a/tools/sched_ext/scx_qmap.c
+++ b/tools/sched_ext/scx_qmap.c
@@ -47,7 +47,7 @@ const char help_fmt[] =
 "\n"
 "Usage: %s [-s SLICE_US] [-e COUNT] [-t COUNT] [-T COUNT] [-l COUNT] [-b COUNT]\n"
 "       [-N COUNT] [-P] [-M] [-H] [-c CG_PATH] [-d PID] [-D LEN] [-S] [-p] [-I] [-X]\n"
-"       [-F COUNT] [-i SEC] [-R MS] [-J MODE] [-v]\n"
+"       [-F COUNT] [-i SEC] [-R MS] [-C MODE] [-B PPT] [-q US] [-J MODE] [-v]\n"
 "\n"
 "  -s SLICE_US   Override slice duration\n"
 "  -e COUNT      Trigger scx_bpf_error() after COUNT enqueues\n"
-- 
2.53.0

base-commit: 5b740a72b55c4267ba4d8010f17dde26b26f52d3

[PATCH 7/9] tools/sched_ext: scx_qmap: Reset the hier stats baseline on restart

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

hprev holds the previous per-participant counter snapshots for the
hier stats deltas and is initialized before the restart: label, so a
hotplug-triggered fail-safe reload leaves it holding snapshots from
the previous generation while the scheduler's own counters start from
zero; the first stats interval prints u64-underflow garbage.

Reset it together with optind on reload.

Fixes: e9151ed5c944 ("tools/sched_ext: scx_qmap - Expand hierarchical sub-scheduling")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 tools/sched_ext/scx_qmap.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/sched_ext/scx_qmap.c b/tools/sched_ext/scx_qmap.c
index f44802247069..414afb817e0a 100644
--- a/tools/sched_ext/scx_qmap.c
+++ b/tools/sched_ext/scx_qmap.c
@@ -292,6 +292,7 @@ int main(int argc, char **argv)
 	}
 restart:
 	optind = 1;
+	memset(&hprev, 0, sizeof(hprev));
 	skel = SCX_OPS_CID_OPEN(qmap_ops, scx_qmap);
 
 	skel->rodata->slice_ns = skel->rodata->__SCX_SLICE_DFL;
-- 
2.53.0

base-commit: 5b740a72b55c4267ba4d8010f17dde26b26f52d3

[PATCH 8/9] tools/sched_ext: scx_userland: Check dispatch buffer slots before inserting

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

userland_dispatch() pops up to MAX_ENQUEUED_TASKS pids from the
dispatched map and inserts each task into the global DSQ without
consulting scx_bpf_dispatch_nr_slots(). The kernel's dispatch buffer
holds 32 slots against MAX_ENQUEUED_TASKS (4096), so a single
   full dispatch round overflows it long before that many tasks
pile up between dispatch rounds the insert past the buffer limit aborts
the whole scheduler. Every other tools scheduler that loops on
scx_bpf_dsq_insert() (scx_central, scx_qmap) checks the remaining slot
count; scx_userland is the odd one out.

Break out when the buffer is exhausted and leave the remaining tasks
queued for the next dispatch round.

Fixes: cc4448d0856d ("tools/sched_ext: add scx_userland scheduler")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 tools/sched_ext/scx_userland.bpf.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/sched_ext/scx_userland.bpf.c b/tools/sched_ext/scx_userland.bpf.c
index f29862b89386..0a6daad84528 100644
--- a/tools/sched_ext/scx_userland.bpf.c
+++ b/tools/sched_ext/scx_userland.bpf.c
@@ -233,6 +233,14 @@ void BPF_STRUCT_OPS(userland_dispatch, s32 cpu, struct task_struct *prev)
 		s32 pid;
 		struct task_struct *p;
 
+		/*
+		 * The dispatch buffer is limited. When it runs out, leave the
+		 * remaining tasks queued for the next dispatch round instead
+		 * of inserting past the limit and aborting the scheduler.
+		 */
+		if (!scx_bpf_dispatch_nr_slots())
+			break;
+
 		if (bpf_map_pop_elem(&dispatched, &pid))
 			break;
 
-- 
2.53.0

base-commit: 5b740a72b55c4267ba4d8010f17dde26b26f52d3

[PATCH 9/9] tools/sched_ext: scx_userland: Reject batch size 0

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

-b 0 is accepted silently and leaves batch_size at zero, so
dispatch_batch() never moves a task out of the vruntime list into the
dispatched queue: tasks pile up on the vruntime list while
nr_scheduled stays positive and ops.update_idle() keeps kicking cpus;
the system-wide livelock ends when the stall watchdog kills the
scheduler.

Reject a zero batch size at option parsing.

Fixes: cc4448d0856d ("tools/sched_ext: add scx_userland scheduler")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 tools/sched_ext/scx_userland.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/sched_ext/scx_userland.c b/tools/sched_ext/scx_userland.c
index b16b6db5f365..cbd511685449 100644
--- a/tools/sched_ext/scx_userland.c
+++ b/tools/sched_ext/scx_userland.c
@@ -364,6 +364,11 @@ static void pre_bootstrap(int argc, char **argv)
 		}
 	}
 
+	if (!batch_size) {
+		fprintf(stderr, "Invalid batch size %s\n", optarg);
+		exit(1);
+	}
+
 	/*
 	 * It's not always safe to allocate in a user space scheduler, as an
 	 * enqueued task could hold a lock that we require in order to be able
-- 
2.53.0

base-commit: 5b740a72b55c4267ba4d8010f17dde26b26f52d3

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

[Tejun Heo]

> 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

Applied 1, 5, 6, 7 and 8 to sched_ext/for-7.5, with a garbled sentence
in 8's description fixed. I'm skipping 2, 3, 4 and 9 as they don't
really make a meaningful difference.

Thanks.

--
tejun