All of lore.kernel.org
 help / color / mirror / Atom feed
* [scx_nest] bpf_timer_cancel() unusable from select_cpu()
@ 2026-08-21  1:39 yaoyiqi (A)
  2026-08-21  2:22 ` Zhan Xusheng
  0 siblings, 1 reply; 5+ messages in thread
From: yaoyiqi (A) @ 2026-08-21  1:39 UTC (permalink / raw)
  To: sched-ext@lists.linux.dev
  Cc: tj@kernel.org, void@manifault.com, arighi@nvidia.com,
	changwoo@igalia.com

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [scx_nest] bpf_timer_cancel() unusable from select_cpu()
  2026-08-21  1:39 [scx_nest] bpf_timer_cancel() unusable from select_cpu() yaoyiqi (A)
@ 2026-08-21  2:22 ` Zhan Xusheng
  2026-08-21  4:22   ` yaoyiqi (A)
  0 siblings, 1 reply; 5+ messages in thread
From: Zhan Xusheng @ 2026-08-21  2:22 UTC (permalink / raw)
  To: yaoyiqi3; +Cc: sched-ext, tj, void, arighi, changwoo, zhanxusheng

On Fri, 21 Aug 2026 01:39:23 +0000, yaoyiqi (A) wrote:
> 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.

Which value did it return? From .select_cpu() the expected one is
-ECANCELED, and that is not a refusal: kernel/bpf/helpers.c:4737 lists it
as "-ECANCELED when the timer will be cancelled asynchronously". The path
is helpers.c:4762-4770, where bpf_timer_cancel_async() hands the work to
bpf_async_schedule_op(cb, BPF_ASYNC_CANCEL, 0, 0) and reports the
deferral rather than declining it. A plain "< 0" test reads that as a
failure. The other two possibilities there are -ENOMEM from
kmalloc_nolock() and -ENOENT from a racing deletion, both real errors.

If it was -ECANCELED then the sync/async split is the whole story, and
the gate is neither sleepability nor sched_ext:

  static bool defer_timer_wq_op(void)
  {
	return in_hardirq() || irqs_disabled();
  }

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.

That also answers whether it is intentional: bpf_timer_cancel() refuses
at helpers.c:1571 because it would otherwise reach hrtimer_cancel() at
1615 and wait there for a running callback, while the async variant was
given a deferral path for exactly this context.

One difference if you switch to it: the async variant leaves the callback
installed. bpf_async_update_prog_callback(&t->cb, NULL, NULL) appears
only in the sync path (helpers.c:1611) and in cancel_and_free (1694), so
the bpf_timer_set_callback() re-arm your original code needed after
bpf_timer_cancel() is not needed after bpf_timer_cancel_async().

Your flag-based workaround is what the Rust schedulers do and avoids the
question entirely, so none of this asks you to change it back.

On documentation, the async return codes are all listed at
helpers.c:4731-4740. The gap is on the sync side, where
include/uapi/linux/bpf.h:5456-5461 gives 0, 1, -EINVAL and -EDEADLK for
bpf_timer_cancel() and not the -EOPNOTSUPP you hit.

Thanks,
Zhan Xusheng

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [scx_nest] bpf_timer_cancel() unusable from select_cpu()
  2026-08-21  2:22 ` Zhan Xusheng
@ 2026-08-21  4:22   ` yaoyiqi (A)
  2026-08-21  6:16     ` Zhan Xusheng
  0 siblings, 1 reply; 5+ messages in thread
From: yaoyiqi (A) @ 2026-08-21  4:22 UTC (permalink / raw)
  To: Zhan Xusheng
  Cc: sched-ext@lists.linux.dev, tj@kernel.org, void@manifault.com,
	arighi@nvidia.com, changwoo@igalia.com

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [scx_nest] bpf_timer_cancel() unusable from select_cpu()
  2026-08-21  4:22   ` yaoyiqi (A)
@ 2026-08-21  6:16     ` Zhan Xusheng
  2026-08-21  8:06       ` yaoyiqi (A)
  0 siblings, 1 reply; 5+ messages in thread
From: Zhan Xusheng @ 2026-08-21  6:16 UTC (permalink / raw)
  To: yaoyiqi3; +Cc: sched-ext, tj, void, arighi, changwoo, zhanxusheng

On Fri, 21 Aug 2026 04:22:26 +0000, yaoyiqi (A) wrote:
> + if (!pcpu_ctx->scheduled_compaction)
> +   return 0;

stat_inc(NEST_STAT(CALLBACK_COMPACTED)) sits above this, so the stale
expiries get counted as compactions that did not happen. The counters
also stop adding up: the cancel in migrate_primary() takes
CANCELLED_COMPACTION, then the same timer still fires and takes
CALLBACK_COMPACTED, so SCHEDULED_COMPACTION no longer matches the sum
unless the core was re-armed in between. Upstream can reach the same
state, since the callback clears scheduled_compaction only at its end and
a cancel can arrive while it runs, but there it needs that race. Moving
the stat_inc() below your check would cover both.

> +    * path, which can run with local IRQs disabled or in hardirq
> +    * contexts - not merely "non-sleepable" - so the timer cancel

I would drop the "can": it looks unconditional to me. select_task_rq()
carries lockdep_assert_held(&p->pi_lock) at kernel/sched/core.c:3626, and
the paths reaching select_task_rq_scx() all hold that lock with IRQs off:

  try_to_wake_up()    scoped_guard(raw_spinlock_irqsave) 4302, call 4403
  wake_up_new_task()  raw_spin_lock_irqsave() 4957, call 4968
  sched_exec()        scoped_guard(raw_spinlock_irqsave) 5639, call 5640

So it is the caller's lock rather than the wakeup being IRQ-driven, and
sched_exec() is not a wakeup at all.

Thanks,
Zhan Xusheng

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [scx_nest] bpf_timer_cancel() unusable from select_cpu()
  2026-08-21  6:16     ` Zhan Xusheng
@ 2026-08-21  8:06       ` yaoyiqi (A)
  0 siblings, 0 replies; 5+ messages in thread
From: yaoyiqi (A) @ 2026-08-21  8:06 UTC (permalink / raw)
  To: Zhan Xusheng
  Cc: sched-ext@lists.linux.dev, tj@kernel.org, void@manifault.com,
	arighi@nvidia.com, changwoo@igalia.com

On Fri, 21 Aug 2026 06:16:00 +0000, Zhan Xusheng wrote:
> stat_inc(NEST_STAT(CALLBACK_COMPACTED)) sits above this, so the stale expiries get counted as compactions that did not happen. The counters also stop adding up: the cancel in migrate_primary() takes CANCELLED_COMPACTION, then the same timer still fires and takes CALLBACK_COMPACTED, so SCHEDULED_COMPACTION no longer matches the sum unless the core was re-armed in between.

I tried your suggestion, but still CALLBACK_COMPATED + CANCELLED_COMPACTION
< SCHEDULED_COMPACTION. This might be the .dispatch() re-armed the timer
before the callback compact_primary_core was triggered.

On Fri, 21 Aug 2026 06:16:00 +0000, Zhan Xusheng wrote:
> I would drop the "can": it looks unconditional to me.

Thanks for your confirmation. Searching for the callers of
select_task_rq_scx(), all of them have raw_spinlock_irqsave for p->pi_lock.

Newer patch can be reviewed below.

Thanks,
Yao YiQi
---
diff --git a/scheds/c/scx_nest.bpf.c b/scheds/c/scx_nest.bpf.c
index 2992f90b..d93549dc 100644
--- a/scheds/c/scx_nest.bpf.c
+++ b/scheds/c/scx_nest.bpf.c
@@ -194,17 +194,27 @@ static int compact_primary_core(void *map, int *key, struct bpf_timer *timer)
 	s32 cpu = bpf_get_smp_processor_id();
 	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() is invoked with
+	 * p->pi_lock held (its callers take it via raw_spin_lock_irqsave() and
+	 * thus always run with local IRQs disabled, regardless of the entry
+	 * context), 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;
+
+	stat_inc(NEST_STAT(CALLBACK_COMPACTED));
 	bpf_rcu_read_lock();
 	primary = primary_cpumask;
 	reserve = reserve_cpumask;
@@ -356,11 +366,15 @@ 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() holds p->pi_lock, which locks
+		 * with local IRQs disabled within all callers of select_task_rq,
+		 * 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));
 		}

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-21  8:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-21  1:39 [scx_nest] bpf_timer_cancel() unusable from select_cpu() yaoyiqi (A)
2026-08-21  2:22 ` Zhan Xusheng
2026-08-21  4:22   ` yaoyiqi (A)
2026-08-21  6:16     ` Zhan Xusheng
2026-08-21  8:06       ` yaoyiqi (A)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.