[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));
}