[PATCHSET sched_ext/for-7.3] sched_ext: Dispatch path follow-ups

[PATCHSET sched_ext/for-7.3] sched_ext: Dispatch path follow-ups

[Tejun Heo]

Hello,

Follow-ups to the recent dispatch path rework[1]. 0001 fixes kick_sync waits
running on foreign CPUs now that dispatch can drop the rq lock with the wait
callback queued. 0002 and 0003 clean up after the dispatch verdict
conversion, dropping the keep_prev fixup it obsoleted and renaming the
remaining balance-era names to dispatch terms.

[1] https://lore.kernel.org/r/20260807210221.232543-1-tj@kernel.org

Based on sched_ext/for-7.3 (e72979d3264a).

This patchset contains the following 3 patches.

 0001 sched_ext: Keep kick_sync waiting on the rq's own CPU
 0002 sched_ext: Drop the stale keep_prev fixup in dispatch_pick()
 0003 sched_ext: Rename balance-era identifiers to dispatch terms

The patchset is also available in the following git branch:

 git://git.kernel.org/pub/scm/linux/kernel/git/tj/sched_ext.git coresched-followups

diffstat follows. Thanks.

 kernel/sched/ext/ext.c                          | 83 +++++++++++++------------
 kernel/sched/ext/inlines.h                      |  2 +-
 kernel/sched/ext/sub.c                          |  8 +--
 kernel/sched/sched.h                            |  2 +-
 tools/sched_ext/include/scx/enum_defs.autogen.h |  1 +
 tools/sched_ext/include/scx/enums.autogen.bpf.h |  3 +
 tools/sched_ext/include/scx/enums.autogen.h     |  1 +
 7 files changed, 54 insertions(+), 46 deletions(-)

--
tejun

[PATCH 1/3] sched_ext: Keep kick_sync waiting on the rq's own CPU

[Tejun Heo]

kick_sync_wait_bal_cb() assumes it runs on the rq's CPU from the
__schedule() tail: the snapshots it compares against live in that CPU's
percpu area and the busy-wait runs with the rq lock dropped and IRQs
enabled.

However, dispatch can now drop the rq lock while the callback sits queued,
and rq lock takers in that window (the sched class change paths, the scx
task iterator) flush pending balance callbacks on release, running the
callback on a foreign CPU. Such a run compares against unrelated snapshots
and can deadlock when the executing CPU is itself a wait target.

Bail on a foreign CPU and leave the wait state alone. The wait only observes
progress that the resched kicks already guarantee and the rq's next wait
picks up the stale cpus_to_sync bits.

Fixes: 4c95380701f5 ("sched/ext: Fold balance_scx() into pick_task_scx()")
Cc: stable@vger.kernel.org # v6.19+
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/sched/ext/ext.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 2adf2bde0cb1..4867517b71a4 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -3269,11 +3269,26 @@ static void put_prev_task_scx(struct rq *rq, struct task_struct *p,
 
 static void kick_sync_wait_bal_cb(struct rq *rq)
 {
-	struct scx_kick_syncs __rcu *ks = __this_cpu_read(scx_kick_syncs);
-	unsigned long *ksyncs = rcu_dereference_sched(ks)->syncs;
+	struct scx_kick_syncs __rcu *ks;
+	unsigned long *ksyncs;
 	bool waited;
 	s32 cpu;
 
+	/*
+	 * This callback is queued and normally flushed within @rq's own
+	 * scheduling pass. However, dispatch can drop the rq lock while it sits
+	 * queued, and lock takers in that window (the sched class change paths,
+	 * the scx task iterator) flush pending balance callbacks on release,
+	 * running this one on a foreign CPU whose snapshots are unrelated. The
+	 * kicked CPUs are already on their way to advance the kick_syncs being
+	 * waited on. Don't get in the way.
+	 */
+	if (unlikely(cpu_of(rq) != smp_processor_id()))
+		return;
+
+	ks = __this_cpu_read(scx_kick_syncs);
+	ksyncs = rcu_dereference_sched(ks)->syncs;
+
 	/*
 	 * Drop rq lock and enable IRQs while waiting. IRQs must be enabled
 	 * — a target CPU may be waiting for us to process an IPI (e.g. TLB
-- 
2.55.0

[PATCH 2/3] sched_ext: Drop the stale keep_prev fixup in dispatch_pick()

[Tejun Heo]

The fixup demoting a keep verdict when @prev is not on ext_sched_class
guarded against the rq-level SCX_RQ_BAL_KEEP flag going stale back when
balancing and picking were separate operations.

The verdict now travels in the return value, created and consumed in one
invocation against the @prev it evaluated, and every keep decision tests
SCX_TASK_QUEUED under the rq lock, which implies ext_sched_class as a class
switch dequeues first. Drop the fixup along with dispatch_core_pick()'s
copy.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/sched/ext/ext.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 4867517b71a4..19db98f0e727 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -3355,11 +3355,6 @@ static enum scx_dsp_verdict dispatch_pick(struct rq *rq, struct rq_flags *rf,
 				       kick_sync_wait_bal_cb);
 	}
 
-	if (unlikely(verdict == SCX_DSP_PREV && prev->sched_class != &ext_sched_class)) {
-		WARN_ON_ONCE(scx_enable_state() == SCX_ENABLED);
-		verdict = SCX_DSP_LOCAL;
-	}
-
 	return verdict;
 }
 
@@ -3410,13 +3405,6 @@ static enum scx_dsp_verdict dispatch_core_pick(struct rq *rq, struct rq_flags *r
 	if (rq->scx.lock_drop_seq != seq)
 		return SCX_DSP_RETRY;
 
-	/* see dispatch_pick() */
-	if (unlikely(verdict == SCX_DSP_PREV &&
-		     prev->sched_class != &ext_sched_class)) {
-		WARN_ON_ONCE(scx_enable_state() == SCX_ENABLED);
-		verdict = SCX_DSP_LOCAL;
-	}
-
 	return verdict;
 }
 #else	/* CONFIG_SCHED_CORE */
-- 
2.55.0

[PATCH 3/3] sched_ext: Rename balance-era identifiers to dispatch terms

[Tejun Heo]

sched_class->balance() is gone from sched_ext and what balance_one() does is
run dispatch to produce something pickable. Update the balance-era names to
dispatch terms:

- balance_one() -> dispatch_one()
- SCX_RQ_IN_BALANCE -> SCX_RQ_IN_DISPATCH

No BPF scheduler reads the flag. The enum autogen headers gain the new name
with the old entry retained like other removed enumerators, zero-filling at
load time. No functional changes.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/sched/ext/ext.c                        | 52 +++++++++----------
 kernel/sched/ext/inlines.h                    |  2 +-
 kernel/sched/ext/sub.c                        |  8 +--
 kernel/sched/sched.h                          |  2 +-
 .../sched_ext/include/scx/enum_defs.autogen.h |  1 +
 .../sched_ext/include/scx/enums.autogen.bpf.h |  3 ++
 tools/sched_ext/include/scx/enums.autogen.h   |  1 +
 7 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 19db98f0e727..966cde3e169a 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -377,9 +377,9 @@ static bool rq_is_open(struct rq *rq, u64 enq_flags)
 	 * If we're in the dispatch path holding rq lock, $curr may or may not
 	 * be ready depending on whether the on-going dispatch decides to extend
 	 * $curr's slice. We say yes here and resolve it at the end of dispatch.
-	 * See balance_one().
+	 * See dispatch_one().
 	 */
-	if (rq->scx.flags & SCX_RQ_IN_BALANCE)
+	if (rq->scx.flags & SCX_RQ_IN_DISPATCH)
 		return true;
 
 	/*
@@ -1019,7 +1019,7 @@ static void schedule_deferred(struct rq *rq)
 	/*
 	 * This is the fallback when schedule_deferred_locked() can't use
 	 * the cheaper balance callback or wakeup hook paths (the target
-	 * CPU is not in balance or wakeup). Currently, this is primarily
+	 * CPU is not in dispatch or wakeup). Currently, this is primarily
 	 * hit by reenqueue operations targeting a remote CPU.
 	 *
 	 * Queue on the target CPU. The deferred work can run from any CPU
@@ -1055,25 +1055,25 @@ static void schedule_deferred_locked(struct rq *rq)
 		return;
 
 	/*
-	 * If in balance, the balance callbacks will be called before rq lock is
-	 * released. Schedule one.
+	 * If in dispatch, the balance callbacks will be called before rq lock
+	 * is released. Schedule one.
 	 *
 	 *
 	 * We can't directly insert the callback into the
 	 * rq's list: The call can drop its lock and make the pending balance
 	 * callback visible to unrelated code paths that call rq_pin_lock().
 	 *
-	 * Just let balance_one() know that it must do it itself.
+	 * Just let dispatch_one() know that it must do it itself.
 	 */
-	if (rq->scx.flags & SCX_RQ_IN_BALANCE) {
+	if (rq->scx.flags & SCX_RQ_IN_DISPATCH) {
 		rq->scx.flags |= SCX_RQ_BAL_CB_PENDING;
 		return;
 	}
 
 	/*
 	 * No scheduler hooks available. Use the generic irq_work path. The
-	 * above WAKEUP and BALANCE paths should cover most of the cases and the
-	 * time to IRQ re-enable shouldn't be long.
+	 * above WAKEUP and DISPATCH paths should cover most of the cases and
+	 * the time to IRQ re-enable shouldn't be long.
 	 */
 	schedule_deferred(rq);
 }
@@ -1616,14 +1616,14 @@ static void rq_owned_post_enq(struct scx_sched *sch, struct rq *rq,
 		wakeup_preempt(rq, p, 0);
 
 	/*
-	 * If @rq is in balance, the CPU is already vacant and looking for the
+	 * If @rq is in dispatch, the CPU is already vacant and looking for the
 	 * next task to run. No need to preempt or trigger resched after moving
 	 * @p into its local DSQ.
 	 * Note that the wakeup_preempt() above may have already triggered
 	 * a resched if @rq->next_class was idle. It's harmless, since
 	 * need_resched is cleared immediately after task pick.
 	 */
-	if (rq->scx.flags & SCX_RQ_IN_BALANCE)
+	if (rq->scx.flags & SCX_RQ_IN_DISPATCH)
 		return;
 
 	if ((enq_flags & SCX_ENQ_PREEMPT) && p != rq->curr &&
@@ -2348,7 +2348,7 @@ static bool dequeue_task_scx(struct rq *rq, struct task_struct *p, int core_deq_
 	 *
 	 * @p may go through multiple stopping <-> running transitions between
 	 * here and put_prev_task_scx() if task attribute changes occur while
-	 * balance_one() leaves @rq unlocked. However, they don't contain any
+	 * dispatch_one() leaves @rq unlocked. However, they don't contain any
 	 * information meaningful to the BPF scheduler and can be suppressed by
 	 * skipping the callbacks if the task is !QUEUED.
 	 */
@@ -2976,14 +2976,14 @@ static inline void maybe_queue_balance_callback(struct rq *rq)
 	rq->scx.flags &= ~SCX_RQ_BAL_CB_PENDING;
 }
 
-static enum scx_dsp_verdict balance_one(struct rq *rq, struct task_struct *prev)
+static enum scx_dsp_verdict dispatch_one(struct rq *rq, struct task_struct *prev)
 {
 	struct scx_sched *sch = scx_root_protected_live();
 	enum scx_dsp_verdict verdict;
 	s32 cpu = cpu_of(rq);
 
 	lockdep_assert_rq_held(rq);
-	rq->scx.flags |= SCX_RQ_IN_BALANCE;
+	rq->scx.flags |= SCX_RQ_IN_DISPATCH;
 
 	scx_process_sync_ecaps(rq, prev);
 
@@ -3041,7 +3041,7 @@ static enum scx_dsp_verdict balance_one(struct rq *rq, struct task_struct *prev)
 		verdict = SCX_DSP_PREV;
 		goto has_tasks;
 	}
-	rq->scx.flags &= ~SCX_RQ_IN_BALANCE;
+	rq->scx.flags &= ~SCX_RQ_IN_DISPATCH;
 	return SCX_DSP_NONE;
 
 has_tasks:
@@ -3058,7 +3058,7 @@ static enum scx_dsp_verdict balance_one(struct rq *rq, struct task_struct *prev)
 	if (unlikely(rq->scx.local_dsq.nr > 1 && rq->scx.nr_immed))
 		scx_schedule_reenq_local(rq, 0);
 
-	rq->scx.flags &= ~SCX_RQ_IN_BALANCE;
+	rq->scx.flags &= ~SCX_RQ_IN_DISPATCH;
 	return verdict;
 }
 
@@ -3159,7 +3159,7 @@ static void switch_class(struct rq *rq, struct task_struct *next)
 	 * preempted, and it regaining control of the CPU.
 	 *
 	 * ->cpu_release() complements ->cpu_acquire(), which is emitted the
-	 *  next time that balance_one() is invoked.
+	 *  next time that dispatch_one() is invoked.
 	 */
 	if (!rq->scx.cpu_released) {
 		if (sch->ops.cpu_release) {
@@ -3340,7 +3340,7 @@ static enum scx_dsp_verdict dispatch_pick(struct rq *rq, struct rq_flags *rf,
 	enum scx_dsp_verdict verdict;
 
 	rq_unpin_lock(rq, rf);
-	verdict = balance_one(rq, prev);
+	verdict = dispatch_one(rq, prev);
 	rq_repin_lock(rq, rf);
 	maybe_queue_balance_callback(rq);
 
@@ -3373,12 +3373,12 @@ static enum scx_dsp_verdict dispatch_core_pick(struct rq *rq, struct rq_flags *r
 	u32 seq = rq->scx.lock_drop_seq;
 
 	/* another dispatch is in flight on @rq, let that handle it */
-	if (rq->scx.flags & SCX_RQ_IN_BALANCE)
+	if (rq->scx.flags & SCX_RQ_IN_DISPATCH)
 		return SCX_DSP_NONE;
 
 	rq_unpin_lock(rq, rf);
 
-	verdict = balance_one(rq, prev);
+	verdict = dispatch_one(rq, prev);
 
 	if (cpu_of(rq) == smp_processor_id()) {
 		maybe_queue_balance_callback(rq);
@@ -3401,7 +3401,7 @@ static enum scx_dsp_verdict dispatch_core_pick(struct rq *rq, struct rq_flags *r
 
 	rq_repin_lock(rq, rf);
 
-	/* if balance_one() released the rq lock, restart the selection */
+	/* if dispatch_one() released the rq lock, restart the selection */
 	if (rq->scx.lock_drop_seq != seq)
 		return SCX_DSP_RETRY;
 
@@ -3437,7 +3437,7 @@ do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
 
 	/*
 	 * If any higher-priority sched class enqueued a runnable task on this
-	 * rq during balance_one(), abort and return RETRY_TASK, so that the
+	 * rq during dispatch_one(), abort and return RETRY_TASK, so that the
 	 * scheduler loop can restart.
 	 *
 	 * If @force_scx is true, always try to pick a SCHED_EXT task,
@@ -6072,7 +6072,7 @@ static void unbypass_renotify_idle(struct rq *rq, struct scx_sched *pos,
  *
  * - ops.dispatch() is ignored.
  *
- * - balance_one() does not report %SCX_DSP_PREV on non-zero slice as slice
+ * - dispatch_one() does not report %SCX_DSP_PREV on non-zero slice as slice
  *   can't be trusted. Whenever a tick triggers, the running task is rotated to
  *   the tail of the queue with core_sched_at touched.
  *
@@ -8460,13 +8460,13 @@ static bool can_skip_idle_kick(struct rq *rq)
 	 * We can skip idle kicking if @rq is going to go through at least one
 	 * full SCX scheduling cycle before going idle. Just checking whether
 	 * curr is not idle is insufficient because we could be racing
-	 * balance_one() trying to pull the next task from a remote rq, which
+	 * dispatch_one() trying to pull the next task from a remote rq, which
 	 * may fail, and @rq may become idle afterwards.
 	 *
 	 * The race window is small and we don't and can't guarantee that @rq is
 	 * only kicked while idle anyway. Skip only when sure.
 	 */
-	return !is_idle_task(rq->curr) && !(rq->scx.flags & SCX_RQ_IN_BALANCE);
+	return !is_idle_task(rq->curr) && !(rq->scx.flags & SCX_RQ_IN_DISPATCH);
 }
 
 static bool kick_one_cpu(s32 cpu, struct scx_sched_pcpu *pcpu, struct rq *this_rq,
@@ -9199,7 +9199,7 @@ __bpf_kfunc bool scx_bpf_dsq_move_to_local___v2(u64 dsq_id, u64 enq_flags,
 		/*
 		 * A successfully consumed task can be dequeued before it starts
 		 * running while the CPU is trying to migrate other dispatched
-		 * tasks. Bump nr_tasks to tell balance_one() to retry on empty
+		 * tasks. Bump nr_tasks to tell dispatch_one() to retry on empty
 		 * local DSQ.
 		 */
 		dspc->nr_tasks++;
diff --git a/kernel/sched/ext/inlines.h b/kernel/sched/ext/inlines.h
index 8f3be59863e5..ed423bcc26b8 100644
--- a/kernel/sched/ext/inlines.h
+++ b/kernel/sched/ext/inlines.h
@@ -112,7 +112,7 @@ scx_dispatch_sched(struct scx_sched *sch, struct rq *rq,
 		 * ops.dispatch() can trap us in this loop by repeatedly
 		 * dispatching ineligible tasks. Break out once in a while to
 		 * allow the watchdog to run. As IRQ can't be enabled in
-		 * balance(), we want to complete this scheduling cycle and then
+		 * dispatch, we want to complete this scheduling cycle and then
 		 * start a new one. IOW, we want to call resched_curr() on the
 		 * next, most likely idle, task, not the current one. Use
 		 * __scx_bpf_kick_cpu() for deferred kicking.
diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
index ec4729c99763..a7b38c90d095 100644
--- a/kernel/sched/ext/sub.c
+++ b/kernel/sched/ext/sub.c
@@ -918,7 +918,7 @@ static u64 calc_effective_caps(struct scx_pshard *ps, s32 cid)
  * @cid: cid to update
  *
  * Queue an ecaps update for @sch's @cid and kick the cpu so that it syncs in
- * balance_one().
+ * dispatch_one().
  */
 static void queue_sync_ecaps(struct scx_sched *sch, s32 cid)
 {
@@ -953,7 +953,7 @@ static void discard_queued_syncs(struct rq *rq)
 /**
  * scx_process_sync_ecaps - Sync this cpu's ecaps to pshard->caps[]
  * @rq: the cid's cpu rq
- * @prev: @rq's previous task from the in-progress balance
+ * @prev: @rq's previous task from the in-progress dispatch
  *
  * pshard->caps[] is the target configuration. pcpu->ecaps is the effective
  * transposed copy owned by the cid's cpu and written only here under @rq's
@@ -1069,7 +1069,7 @@ void scx_process_sync_ecaps(struct rq *rq, struct task_struct *prev)
  * sync when bypass lifts, so without a replay a cid that never changes again
  * would never be notified. The attach-time initial grants are the acute case
  * as they are consumed during the enable bypass window. Re-queue a sync for
- * any undelivered delta so the next balance delivers it.
+ * any undelivered delta so the next dispatch delivers it.
  */
 void scx_unbypass_replay_ecaps(struct rq *rq, struct scx_sched *sch)
 {
@@ -2248,7 +2248,7 @@ __bpf_kfunc bool scx_bpf_sub_dispatch(u64 cgroup_id, const struct bpf_prog_aux *
 	/*
 	 * Skip a child that does not effectively hold the base cap on this cpu:
 	 * its inserts would only be rejected. ecaps are synced at the top of
-	 * balance_one() before dispatch, so this reflects the in-effect state.
+	 * dispatch_one() before dispatch, so this reflects the in-effect state.
 	 */
 	if (scx_missing_caps(child, cpu_of(rq), SCX_CAP_BASE))
 		return false;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 5ae2d6d2d35b..7701a5a60972 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -790,7 +790,7 @@ enum scx_rq_flags {
 	SCX_RQ_ROOT_IDLE_RENOTIFY	= 1 << 8, /* the root is owed update_idle() */
 
 	SCX_RQ_IN_WAKEUP	= 1 << 16,
-	SCX_RQ_IN_BALANCE	= 1 << 17,
+	SCX_RQ_IN_DISPATCH	= 1 << 17,
 };
 
 /* per-rq rescue execution state, see scx_rescue_timerfn() */
diff --git a/tools/sched_ext/include/scx/enum_defs.autogen.h b/tools/sched_ext/include/scx/enum_defs.autogen.h
index d609f369a337..19aa1de3e700 100644
--- a/tools/sched_ext/include/scx/enum_defs.autogen.h
+++ b/tools/sched_ext/include/scx/enum_defs.autogen.h
@@ -189,6 +189,7 @@
 #define HAVE_SCX_RQ_ROOT_IDLE_RENOTIFY
 #define HAVE_SCX_RQ_IN_WAKEUP
 #define HAVE_SCX_RQ_IN_BALANCE
+#define HAVE_SCX_RQ_IN_DISPATCH
 #define HAVE_SCX_SCHED_PCPU_BYPASSING
 #define HAVE_SCX_SLICE_OOB_DUR_BITS
 #define HAVE_SCX_SLICE_OOB_ID_BITS
diff --git a/tools/sched_ext/include/scx/enums.autogen.bpf.h b/tools/sched_ext/include/scx/enums.autogen.bpf.h
index d74b901688f1..7268131010de 100644
--- a/tools/sched_ext/include/scx/enums.autogen.bpf.h
+++ b/tools/sched_ext/include/scx/enums.autogen.bpf.h
@@ -34,6 +34,9 @@ const volatile u64 __SCX_RQ_IN_WAKEUP __weak;
 const volatile u64 __SCX_RQ_IN_BALANCE __weak;
 #define SCX_RQ_IN_BALANCE __SCX_RQ_IN_BALANCE
 
+const volatile u64 __SCX_RQ_IN_DISPATCH __weak;
+#define SCX_RQ_IN_DISPATCH __SCX_RQ_IN_DISPATCH
+
 const volatile u64 __SCX_DSQ_FLAG_BUILTIN __weak;
 #define SCX_DSQ_FLAG_BUILTIN __SCX_DSQ_FLAG_BUILTIN
 
diff --git a/tools/sched_ext/include/scx/enums.autogen.h b/tools/sched_ext/include/scx/enums.autogen.h
index d58f3e59680e..e61632654517 100644
--- a/tools/sched_ext/include/scx/enums.autogen.h
+++ b/tools/sched_ext/include/scx/enums.autogen.h
@@ -15,6 +15,7 @@
 	SCX_ENUM_SET(skel, scx_rq_flags, SCX_RQ_CLK_VALID); \
 	SCX_ENUM_SET(skel, scx_rq_flags, SCX_RQ_IN_WAKEUP); \
 	SCX_ENUM_SET(skel, scx_rq_flags, SCX_RQ_IN_BALANCE); \
+	SCX_ENUM_SET(skel, scx_rq_flags, SCX_RQ_IN_DISPATCH); \
 	SCX_ENUM_SET(skel, scx_dsq_id_flags, SCX_DSQ_FLAG_BUILTIN); \
 	SCX_ENUM_SET(skel, scx_dsq_id_flags, SCX_DSQ_FLAG_LOCAL_ON); \
 	SCX_ENUM_SET(skel, scx_dsq_id_flags, SCX_DSQ_INVALID); \
-- 
2.55.0