Re: [scx_nest] bpf_timer_cancel() unusable from select_cpu()

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