All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHSET sched_ext/for-7.3-fixes] sched_ext: Fix scx balance callbacks queued across a core-sched pick
@ 2026-10-17 20:05 Andrea Righi
  2026-10-17 20:05 ` [PATCH 1/2] sched_ext: Don't queue balance callbacks from " Andrea Righi
  2026-10-17 20:05 ` [PATCH 2/2] sched_ext: Don't leave the kick_sync wait queued across " Andrea Righi
  0 siblings, 2 replies; 5+ messages in thread
From: Andrea Righi @ 2026-10-17 20:05 UTC (permalink / raw)
  To: Tejun Heo, David Vernet, Changwoo Min
  Cc: Emil Tsalapatis, sched-ext, linux-kernel

Commit ff0545e0c263 ("sched_ext: Don't leave balance callbacks queued across a
pick retry") queues the scx balance callbacks only when the pick is final.
However, under core scheduling, a pick that returns a task isn't final:
pick_next_task() goes on to pick the SMT siblings, and their dispatch can
release the shared rq lock with the callbacks still queued. Any CPU acquiring
the rq lock in that window triggers the warning in rq_pin_lock().

This was pointed out by Tejun and sashiko in the review of ff0545e0c263 [1][2]
and these are the follow-up fixes.

Patch 1/2 always defers the pending work to irq_work under core scheduling, for
both the local and the sibling picks, and leaves SCX_RQ_BAL_CB_PENDING to the
dispatch in flight that set it.

Patch 2/2 handles the kick_sync wait, which can't be moved to irq_work, by
taking it off the local rq around each dispatch of a core-wide selection and
queueing it again if the rq lock isn't released.

Testing:

 - The core scheduling case (1/2) was reproduced in a 16-CPU VM with SMT (8
   cores, 2 threads each), running scx_qmap -I with two stress-ng sleeper groups
   tagged with different cookies (coresched new -- ...), one of them SCHED_FIFO,
   plus an untagged CPU-bound workload. The warning itself is racy, so a debug
   check in scx_rq_lock_drop() (not part of this series) also reported any scx
   balance callback still queued on the core when the rq lock is released. With
   only ff0545e0c263 applied, the check fired in 5 of 10 runs (always
   deferred_bal_cb, both for sibling and local re-picks) and the warning showed
   up as well; with 1/2 applied, 0 of 16 runs.

 - The kick_sync case (2/2) was reproduced with the same setup and a temporary
   change to scx_qmap that kicks a CPU on another core with SCX_KICK_WAIT on
   every user task enqueue. With 1/2 only, the debug check reported the
   kick_sync callback queued across an rq lock drop, both by a sibling's
   dispatch and by a local re-pick, in 5 of 6 runs, and the rq_pin_lock()
   warning showed up in 2 of them; with 2/2 applied, 0 of 4 runs.

[1] https://lore.kernel.org/r/20261010095208.276806-1-arighi@nvidia.com
[2] https://lore.kernel.org/r/20261017152226.3690249-1-arighi@nvidia.com

Andrea Righi (2):
  sched_ext: Don't queue balance callbacks from a core-sched pick
  sched_ext: Don't leave the kick_sync wait queued across a core-sched pick

 kernel/sched/ext/ext.c | 78 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 66 insertions(+), 12 deletions(-)

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

* [PATCH 1/2] sched_ext: Don't queue balance callbacks from a core-sched pick
  2026-10-17 20:05 [PATCHSET sched_ext/for-7.3-fixes] sched_ext: Fix scx balance callbacks queued across a core-sched pick Andrea Righi
@ 2026-10-17 20:05 ` Andrea Righi
  2026-10-17 22:14   ` Tejun Heo
  2026-10-17 20:05 ` [PATCH 2/2] sched_ext: Don't leave the kick_sync wait queued across " Andrea Righi
  1 sibling, 1 reply; 5+ messages in thread
From: Andrea Righi @ 2026-10-17 20:05 UTC (permalink / raw)
  To: Tejun Heo, David Vernet, Changwoo Min
  Cc: Emil Tsalapatis, sched-ext, linux-kernel

Commit ff0545e0c263 ("sched_ext: Don't leave balance callbacks queued
across a pick retry") queues the deferred work as a balance callback
only when do_pick_task_scx() returns a final pick. However, under core
scheduling, a pick that returns a task isn't final either:
pick_next_task() goes on to pick the SMT siblings, and their dispatch
can release the shared rq lock with the callback still queued,
triggering the rq_pin_lock() warning.

Moreover, dispatch_core_pick() returns early when another dispatch is
in flight on the rq, and do_pick_task_scx() then consumes
%SCX_RQ_BAL_CB_PENDING set by that dispatch while it's still running.

This can be reproduced running scx_qmap -I on SMT siblings with
coresched-tagged workloads that frequently sleep, mixed with untagged
CPU-bound tasks.

Fix this by always deferring the pending work to the irq_work path from
dispatch_core_pick() under core scheduling, for both the local and the
sibling picks, and by leaving %SCX_RQ_BAL_CB_PENDING alone in
do_pick_task_scx(), so that it's handled by the dispatch that set it.
Sample the core scheduling state once at the start of the pick, since
core scheduling can get enabled while a non-core dispatch has the rq lock
released, and the follow-up work must match the dispatch path that ran.

Fixes: 4c95380701f5 ("sched/ext: Fold balance_scx() into pick_task_scx()")
Cc: stable@vger.kernel.org # v6.19+
Signed-off-by: Andrea Righi <arighi@nvidia.com>
---
 kernel/sched/ext/ext.c | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 0e93a5c45ede4..0c5ed97c4d080 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -3390,15 +3390,20 @@ static enum scx_dsp_verdict dispatch_core_pick(struct rq *rq, struct rq_flags *r
 
 	verdict = dispatch_one(rq, prev);
 
-	if (cpu_of(rq) != smp_processor_id() &&
-	    unlikely(rq->scx.flags & SCX_RQ_BAL_CB_PENDING)) {
-		/*
-		 * Balance callbacks must run in the context that queued them,
-		 * so they can't be queued on another CPU's rq. Run the deferred
-		 * work directly instead.
-		 */
+	/*
+	 * A pick that returns a task isn't final under core scheduling: the
+	 * selection goes on to pick the siblings, and their dispatch can
+	 * release the rq lock, which can't happen with balance callbacks
+	 * queued. Don't queue the deferred work as a balance callback, for this
+	 * CPU or a sibling, and run it from irq_work instead.
+	 *
+	 * do_pick_task_scx() leaves %SCX_RQ_BAL_CB_PENDING alone under core
+	 * scheduling, so that the early return above leaves it to the dispatch
+	 * in flight, which handles it here.
+	 */
+	if (unlikely(rq->scx.flags & SCX_RQ_BAL_CB_PENDING)) {
 		rq->scx.flags &= ~SCX_RQ_BAL_CB_PENDING;
-		run_deferred(rq);
+		schedule_deferred(rq);
 	}
 
 	rq_repin_lock(rq, rf);
@@ -3421,6 +3426,7 @@ static struct task_struct *
 do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
 {
 	struct task_struct *prev = rq->curr;
+	bool core_sched = sched_core_enabled(rq);
 	enum scx_dsp_verdict verdict;
 	struct task_struct *p;
 
@@ -3429,7 +3435,12 @@ do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
 
 	rq_modified_begin(rq, &ext_sched_class);
 
-	if (sched_core_enabled(rq))
+	/*
+	 * Core scheduling can get enabled while dispatch_pick() has the rq lock
+	 * released. Stick to @core_sched, so that the follow-up work below
+	 * matches the dispatch that ran.
+	 */
+	if (core_sched)
 		verdict = dispatch_core_pick(rq, rf, prev);
 	else
 		verdict = dispatch_pick(rq, rf, prev);
@@ -3495,7 +3506,9 @@ do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
 	 * for the next pick.
 	 */
 	if (p != RETRY_TASK && (p || !force_scx)) {
-		maybe_queue_balance_callback(rq);
+		/* under core scheduling, see dispatch_core_pick() */
+		if (!core_sched)
+			maybe_queue_balance_callback(rq);
 
 		/*
 		 * Defer to a balance callback which can drop rq lock and enable
@@ -3508,7 +3521,7 @@ do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
 			queue_balance_callback(rq, &rq->scx.kick_sync_bal_cb,
 					       kick_sync_wait_bal_cb);
 		}
-	} else if (rq->scx.flags & SCX_RQ_BAL_CB_PENDING) {
+	} else if (!core_sched && (rq->scx.flags & SCX_RQ_BAL_CB_PENDING)) {
 		rq->scx.flags &= ~SCX_RQ_BAL_CB_PENDING;
 		schedule_deferred(rq);
 	}
-- 
2.56.0


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

* [PATCH 2/2] sched_ext: Don't leave the kick_sync wait queued across a core-sched pick
  2026-10-17 20:05 [PATCHSET sched_ext/for-7.3-fixes] sched_ext: Fix scx balance callbacks queued across a core-sched pick Andrea Righi
  2026-10-17 20:05 ` [PATCH 1/2] sched_ext: Don't queue balance callbacks from " Andrea Righi
@ 2026-10-17 20:05 ` Andrea Righi
  2026-10-17 22:44   ` Tejun Heo
  1 sibling, 1 reply; 5+ messages in thread
From: Andrea Righi @ 2026-10-17 20:05 UTC (permalink / raw)
  To: Tejun Heo, David Vernet, Changwoo Min
  Cc: Emil Tsalapatis, sched-ext, linux-kernel

The SCX_KICK_WAIT wait is deferred to rq->scx.kick_sync_bal_cb, which a
final pick queues on the local rq. Under core scheduling, a pick that
returns a task isn't final: pick_next_task() goes on to pick the SMT
siblings, and their dispatch can release the shared rq lock with the
callback still queued. A CPU acquiring the rq lock in that window then
finds a pending balance callback and triggers the warning in
rq_pin_lock().

Unlike the deferred work, the kick_sync wait can't be moved to irq_work,
since it needs to wait with IRQs enabled. However, during a core-wide
selection the rq lock is only released by a dispatch, which already
restarts the selection through lock_drop_seq.

Fix this by taking the kick_sync callback off the local rq before each
dispatch of the selection. If the rq lock isn't released, queue it
again right away. Otherwise, set kick_sync_pending again, so that the
next final pick of this CPU queues it, as done for a non-core pick that
returns RETRY_TASK.

Fixes: 415cb193bb97 ("sched_ext: Fix SCX_KICK_WAIT deadlock by deferring wait to balance callback")
Cc: stable@vger.kernel.org # v7.0+
Suggested-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Andrea Righi <arighi@nvidia.com>
---
 kernel/sched/ext/ext.c | 43 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 0c5ed97c4d080..2348d5b72eb26 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -3369,6 +3369,28 @@ static enum scx_dsp_verdict dispatch_pick(struct rq *rq, struct rq_flags *rf,
 }
 
 #ifdef CONFIG_SCHED_CORE
+/*
+ * Take @head off @rq's balance callbacks if it's queued. Returns whether it
+ * was queued.
+ */
+static bool dequeue_balance_callback(struct rq *rq, struct balance_callback *head)
+{
+	struct balance_callback **pprev;
+
+	lockdep_assert_rq_held(rq);
+
+	for (pprev = &rq->balance_callback; *pprev; pprev = &(*pprev)->next) {
+		if (*pprev != head)
+			continue;
+
+		*pprev = head->next;
+		head->next = NULL;
+		return true;
+	}
+
+	return false;
+}
+
 /*
  * Dispatch for a pick when core scheduling is enabled. The selection picks for
  * all SMT siblings and the rq_i->core_pick state it builds must stay atomic
@@ -3379,8 +3401,10 @@ static enum scx_dsp_verdict dispatch_pick(struct rq *rq, struct rq_flags *rf,
 static enum scx_dsp_verdict dispatch_core_pick(struct rq *rq, struct rq_flags *rf,
 					       struct task_struct *prev)
 {
+	struct rq *this_rq = this_rq();
 	enum scx_dsp_verdict verdict;
 	u32 seq = rq->scx.lock_drop_seq;
+	bool kick_sync_queued;
 
 	/* another dispatch is in flight on @rq, let that handle it */
 	if (rq->scx.flags & SCX_RQ_IN_DISPATCH)
@@ -3388,6 +3412,16 @@ static enum scx_dsp_verdict dispatch_core_pick(struct rq *rq, struct rq_flags *r
 
 	rq_unpin_lock(rq, rf);
 
+	/*
+	 * dispatch_one() can release the rq lock while the selection is in
+	 * progress (see below), so this CPU's kick_sync wait queued by an
+	 * earlier pick of the selection can't stay queued across it. Take it
+	 * off and queue it again if the rq lock isn't released. Otherwise, the
+	 * selection restarts, so leave it pending for the next final pick of
+	 * this CPU.
+	 */
+	kick_sync_queued = dequeue_balance_callback(this_rq, &this_rq->scx.kick_sync_bal_cb);
+
 	verdict = dispatch_one(rq, prev);
 
 	/*
@@ -3409,8 +3443,15 @@ static enum scx_dsp_verdict dispatch_core_pick(struct rq *rq, struct rq_flags *r
 	rq_repin_lock(rq, rf);
 
 	/* if dispatch_one() released the rq lock, restart the selection */
-	if (rq->scx.lock_drop_seq != seq)
+	if (rq->scx.lock_drop_seq != seq) {
+		if (kick_sync_queued)
+			this_rq->scx.kick_sync_pending = true;
 		return SCX_DSP_RETRY;
+	}
+
+	if (kick_sync_queued)
+		queue_balance_callback(this_rq, &this_rq->scx.kick_sync_bal_cb,
+				       kick_sync_wait_bal_cb);
 
 	return verdict;
 }
-- 
2.56.0


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

* Re: [PATCH 1/2] sched_ext: Don't queue balance callbacks from a core-sched pick
  2026-10-17 20:05 ` [PATCH 1/2] sched_ext: Don't queue balance callbacks from " Andrea Righi
@ 2026-10-17 22:14   ` Tejun Heo
  0 siblings, 0 replies; 5+ messages in thread
From: Tejun Heo @ 2026-10-17 22:14 UTC (permalink / raw)
  To: Andrea Righi
  Cc: David Vernet, Changwoo Min, Emil Tsalapatis, sched-ext,
	linux-kernel

Applied to sched_ext/for-7.3-fixes.

Thanks.

--
tejun

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

* Re: [PATCH 2/2] sched_ext: Don't leave the kick_sync wait queued across a core-sched pick
  2026-10-17 20:05 ` [PATCH 2/2] sched_ext: Don't leave the kick_sync wait queued across " Andrea Righi
@ 2026-10-17 22:44   ` Tejun Heo
  0 siblings, 0 replies; 5+ messages in thread
From: Tejun Heo @ 2026-10-17 22:44 UTC (permalink / raw)
  To: Andrea Righi
  Cc: David Vernet, Changwoo Min, Emil Tsalapatis, sched-ext,
	linux-kernel

Hello, Andrea.

On Sat, Oct 17, 2026 at 10:05:15PM +0200, Andrea Righi wrote:
> +	kick_sync_queued = dequeue_balance_callback(this_rq, &this_rq->scx.kick_sync_bal_cb);

Instead of taking the callback off around each dispatch, can we queue it
from set_next_task_scx() on SNT_PICK and SNT_REPICK?
put_prev_set_next_task() calls it once the pick is final, after the
core-wide selection, so the rq lock can't be dropped with the callback
queued. If the CPU ends up running idle or a higher class task, no SCX task
runs there and there's nothing to wait for. kick_sync_pending stays set and
the next pick of an SCX task waits before running it.

SNT_REPICK came with c72945693b90 ("sched: Restart fair hrtick after
same-task repicks"), which is only in for-7.4, so let's do this there and
tag it as a stable dependency:

  Cc: stable@vger.kernel.org # v7.0+: c72945693b90: sched: Restart fair hrtick after same-task repicks
  Cc: stable@vger.kernel.org # v7.0+

Also, proxy exec on for-7.4 zaps balance callbacks and picks again when
find_proxy_task() fails, so kick_sync_pending should be cleared when the
callback runs rather than when it's queued.

Thanks.

--
tejun

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

end of thread, other threads:[~2026-10-17 22:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-17 20:05 [PATCHSET sched_ext/for-7.3-fixes] sched_ext: Fix scx balance callbacks queued across a core-sched pick Andrea Righi
2026-10-17 20:05 ` [PATCH 1/2] sched_ext: Don't queue balance callbacks from " Andrea Righi
2026-10-17 22:14   ` Tejun Heo
2026-10-17 20:05 ` [PATCH 2/2] sched_ext: Don't leave the kick_sync wait queued across " Andrea Righi
2026-10-17 22:44   ` Tejun Heo

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.