[scx_nest] bpf_timer_cancel() unusable from select_cpu()

"yaoyiqi (A)" <[email protected]>
Newsgroups dev.linux.lists.sched-ext
Message-ID <[email protected]>
Hi,

I ran into a regression with the classic C example scheduler scx_nest
(sched-ext/scx-c-examples) on a 7.1.8 kernel (post-bpf_async timer
rework). It loads and attaches fine, but exits immediately once a
re-promoted core's compaction timer needs cancelling:

sched_ext: BPF scheduler "nest" enabled
sched_ext: nest: .../scheds/c/scx_nest.bpf.c:361: Failed to cancel pcpu timer
scx_bpf_error_bstr()
nest_select_cpu()
bpf__sched_ext_ops_select_cpu()

The failing call is bpf_timer_cancel(&pcpu_ctx->timer) from the
non-sleepable .select_cpu() callback. Testing showed that on this kernel
bpf_timer_cancel_async(&timer) also returns an error from the same
context, so this appears to be a broader restriction than the
sync/async split: cancelling a CPU-pinned timer is not usable from a
sched_ext hot-path callback at all.

For context: scx_nest is the only C scheduler in scx-c-examples that
uses bpf_timer_cancel(); scx_central/scx_qmap only init/start timers and
are unaffected. None of the current Rust schedulers in sched-ext/scx
cancel timers either - they all use flags/generations to let stale
callbacks no-op, which is what I applied to scx_nest as a workaround.

Questions / notes for maintainers:
- Is this restriction intentional? I'd expect the timer docs to call out
that bpf_timer_cancel()/bpf_timer_cancel_async() may fail from
non-sleepable sched_ext callbacks such as select_cpu().
- Since scx_nest isn't in tools/sched_ext I'm not submitting this as a
kernel patch; I'm reporting it so the API change is on record and in
case the example deserves updating elsewhere.

Workaround diff (in case it helps, applies to scx-c-examples): instead
of cancelling the pending timer when a core is re-promoted, clear
pcpu_ctx->scheduled_compaction in migrate_primary() and let the
compact_primary_core() callback detect the stale request and bail out.

Thanks,
Yao YiQi

---

diff --git a/scheds/c/scx_nest.bpf.c b/scheds/c/scx_nest.bpf.c
index 2992f90b..ef0cbe78 100644
--- a/scheds/c/scx_nest.bpf.c
+++ b/scheds/c/scx_nest.bpf.c
@@ -195,16 +195,23 @@ static int compact_primary_core(void *map, int *key, struct bpf_timer *timer)
 	struct pcpu_ctx *pcpu_ctx;
 
 	stat_inc(NEST_STAT(CALLBACK_COMPACTED));
-	/*
-	 * If we made it to this callback, it means that the timer callback was
-	 * never cancelled, and so the core needs to be demoted from the
-	 * primary nest.
-	 */
 	pcpu_ctx = bpf_map_lookup_elem(&pcpu_ctxs, &cpu);
 	if (!pcpu_ctx) {
 		scx_bpf_error("Couldn't lookup pcpu ctx");
 		return 0;
 	}
+
+	/*
+	 * The core may have been re-promoted to the primary nest while this
+	 * timer was pending (see migrate_primary in nest_select_cpu()). We no
+	 * longer cancel the timer from there as bpf_timer_cancel() isn't usable
+	 * in that non-sleepable context on newer kernels; instead the pending
+	 * callback detects that scheduled_compaction was cleared and bails out.
+	 * Only demote the core if a compaction is still actually scheduled.
+	 */
+	if (!pcpu_ctx->scheduled_compaction)
+		return 0;
+
 	bpf_rcu_read_lock();
 	primary = primary_cpumask;
 	reserve = reserve_cpumask;
@@ -356,11 +363,14 @@ migrate_primary:
 		tctx->prev_misses = 0;
 	pcpu_ctx = bpf_map_lookup_elem(&pcpu_ctxs, &cpu);
 	if (pcpu_ctx) {
+		/*
+		 * A compaction may have been scheduled for this core. Instead of
+		 * cancelling the timer (bpf_timer_cancel() is not usable from
+		 * this non-sleepable context), clear scheduled_compaction so that
+		 * the timer callback, if it fires after we land a task here,
+		 * detects it as stale and bails out.
+		 */
 		if (pcpu_ctx->scheduled_compaction) {
-			if (bpf_timer_cancel(&pcpu_ctx->timer) < 0)
-				scx_bpf_error("Failed to cancel pcpu timer");
-			if (bpf_timer_set_callback(&pcpu_ctx->timer, compact_primary_core))
-				scx_bpf_error("Failed to re-arm pcpu timer");
 			pcpu_ctx->scheduled_compaction = false;
 			stat_inc(NEST_STAT(CANCELLED_COMPACTION));
 		}
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.