Re: [scx_nest] bpf_timer_cancel() unusable from select_cpu()
"yaoyiqi (A)" <[email protected]>
| Newsgroups | dev.linux.lists.sched-ext |
|---|---|
| Message-ID | <[email protected]> |
On Fri, 21 Aug 2026 02:22:00 +0000, Zhan Xusheng wrote:
> helpers.c:1515. .select_cpu() qualifies through irqs_disabled():
> try_to_wake_up() holds p->pi_lock across the wakeup via
> scoped_guard(raw_spinlock_irqsave) at kernel/sched/core.c:4302 and calls
> select_task_rq() inside it at 4403, which dispatches through sched_class->select_task_rq (core.c:3629) into select_task_rq_scx() and SCX_CALL_OP_TASK_RET(sch, select_cpu, ...) at kernel/sched/ext/ext.c:3530 and 3558. So a sched_ext callback that runs with IRQs enabled is unaffected, and this is not specific to CPU-pinned timers.
Unfortunately I encountered -EOPNOTSUPP with bpf_timer_cancel(), which
means it ran with IRQs disabled or within hard IRQs.
On Fri, 21 Aug 2026 02:22:00 +0000, Zhan Xusheng wrote:
> If it was -ECANCELED then the sync/async split is the whole story, and the gate is neither sleepability nor sched_ext:
Thanks for your clarification, it turns out that the
bpf_timer_cancel_async() kfunc returned -ECANCELED as you pointed out. I
erroneously use "< 0" to come to a bad conclusion without further
investigation. The gate is not about sleepability, is whether
bpf_timer_cancel can happen on .select_cpu, which might be triggered by
hard IRQs. I have changed my comment on the diff to not mislead other
people.
But I think our current workaround might be more compatible with kernels
have bpf_timer_cancel() only. I think this might not be a good solution:
#define __COMPAT_scx_bpf_timer_cancel(timer) \
(bpf_ksym_exists(scx_bpf_timer_cancel_async)? \
bpf_timer_cancel_async((timer)) : bpf_timer_cancel((timer))
/*
* bpf_timer_cancel_async returns -ECANCELED for async cancel.
*/
if ((ret = __COMPAT_scx_bpf_timer_cancel(&pcpu_ctx->timer)) < 0 &&
ret != -ECANCELED)
scx_bpf_error("Failed to cancel pcpu timer");
/*
* bpf_timer_cancel_async does not need to set callback again.
*/
if (ret != -ECANCELED && bpf_timer_set_callback(&pcpu_ctx->timer,
compact_primary_core))
scx_bpf_error("Failed to re-arm pcpu timer");
This runs into a problem: On old kernels the macro gives us a synchronous
cancel. On 7.x it gives you an asynchronous one. The scheduler will have
different behavior on different kernels and thus have different
performance.
So I will keep the flag-based one as my workaround.
Thanks,
Yao YiQi
---
diff --git a/scheds/c/scx_nest.bpf.c b/scheds/c/scx_nest.bpf.c
index 2992f90b..b53a0864 100644
--- a/scheds/c/scx_nest.bpf.c
+++ b/scheds/c/scx_nest.bpf.c
@@ -195,16 +195,26 @@ 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: select_cpu() runs in the task
+ * wakeup path, which can be entered with local IRQs disabled or from
+ * hardirq contexts (e.g. wakeups originating in interrupt handlers),
+ * where the timer-cancel synchronization isn't available. 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 +366,16 @@ 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 (select_cpu() is entered from the wakeup
+ * path, which can run with local IRQs disabled or in hardirq
+ * contexts - not merely "non-sleepable" - so the timer cancel
+ * isn't usable here), 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));
}