[PATCH v2 sched_ext/for-7.3-fixes] sched_ext: Don't leave balance callbacks queued across a pick retry

[PATCH v2 sched_ext/for-7.3-fixes] sched_ext: Don't leave balance callbacks queued across a pick retry

[Andrea Righi]

The dispatch path can request deferred work to be run from a balance
callback and pick_task_scx() queues rq->scx.deferred_bal_cb right after
dispatching. However, a balance callback must be consumed before the rq
lock is released and pick_task_scx() is not necessarily the end of the
pick:

 - it returns RETRY_TASK when a higher priority class task has been
   enqueued while the dispatch had the rq lock released;

 - the dl_server pick can return NULL, in which case the lower classes
   are picked next.

In both cases the callback stays queued while the core keeps picking,
and the rq lock can be released again, either by prev_balance() or by
another dispatch. For example, with an RT task as prev:

  __schedule()
    __pick_next_task()
      pick_task_scx()
        dispatch_pick()
          maybe_queue_balance_callback()    <- deferred_bal_cb queued
        return RETRY_TASK
      prev_balance()
        balance_rt()
          pull_rt_task()
            double_lock_balance()           <- rq lock released

Any CPU acquiring the rq lock in that window finds a pending balance
callback and triggers the following warning in rq_pin_lock():

  WARNING: kernel/sched/sched.h:1919 at rq_lock+0x7f/0xe0, CPU#10: stress-ng-sleep/533
  CPU: 10 UID: 0 PID: 533 Comm: stress-ng-sleep Not tainted 7.2.0-rc6-virtme #61 PREEMPT(full)
  Sched_ext: qmap (enabled+all)
  Call Trace:
   <IRQ>
   try_to_wake_up+0x2c1/0x5c0
   hrtimer_wakeup+0x22/0x30
   __hrtimer_run_queues+0x1d5/0x350
   hrtimer_interrupt+0x106/0x1e0
   __sysvec_apic_timer_interrupt+0x6f/0x210
   sysvec_apic_timer_interrupt+0x71/0x90
   </IRQ>

This can be reproduced running scx_qmap -I together with a SCHED_FIFO
workload that frequently sleeps, e.g.:

  $ vng -v --cpus 16 --memory 4G -- \
      'tools/sched_ext/build/bin/scx_qmap -I >/dev/null & \
       stress-ng --sleep 4 --sched fifo --sched-prio 1 -t 30 -q'

Fix this by queueing the balance callbacks only when the pick is final.
Otherwise, defer the pending work to the irq_work path, which doesn't
depend on the current rq lock section, and leave the kick sync wait
pending for the next pick.

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>
---
Changes in v2:
 - Fold the balance callback handling into do_pick_task_scx() with a
   single exit, instead of splitting out __do_pick_task_scx() (Tejun Heo)
 - Update the comment above dispatch_pick(), which no longer queues the
   follow-up work (Tejun Heo)
 - Link to v1: https://lore.kernel.org/r/20261010095208.276806-1-arighi@nvidia.com

 kernel/sched/ext/ext.c | 73 ++++++++++++++++++++++++++----------------
 1 file changed, 45 insertions(+), 28 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 4259578186954..0e93a5c45ede4 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -3353,7 +3353,8 @@ static struct task_struct *first_local_task(struct rq *rq)
 }
 
 /*
- * Run dispatch and queue the follow-up work for a pick.
+ * Run dispatch for a pick. The follow-up work is queued by do_pick_task_scx()
+ * once the pick is final.
  */
 static enum scx_dsp_verdict dispatch_pick(struct rq *rq, struct rq_flags *rf,
 					  struct task_struct *prev)
@@ -3363,18 +3364,6 @@ static enum scx_dsp_verdict dispatch_pick(struct rq *rq, struct rq_flags *rf,
 	rq_unpin_lock(rq, rf);
 	verdict = dispatch_one(rq, prev);
 	rq_repin_lock(rq, rf);
-	maybe_queue_balance_callback(rq);
-
-	/*
-	 * Defer to a balance callback which can drop rq lock and enable IRQs.
-	 * Waiting directly in the pick path would deadlock against CPUs sending
-	 * us IPIs (e.g. TLB flushes) while we wait for them.
-	 */
-	if (unlikely(rq->scx.kick_sync_pending)) {
-		rq->scx.kick_sync_pending = false;
-		queue_balance_callback(rq, &rq->scx.kick_sync_bal_cb,
-				       kick_sync_wait_bal_cb);
-	}
 
 	return verdict;
 }
@@ -3401,16 +3390,8 @@ 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()) {
-		maybe_queue_balance_callback(rq);
-
-		/* see dispatch_pick() */
-		if (unlikely(rq->scx.kick_sync_pending)) {
-			rq->scx.kick_sync_pending = false;
-			queue_balance_callback(rq, &rq->scx.kick_sync_bal_cb,
-					       kick_sync_wait_bal_cb);
-		}
-	} else if (unlikely(rq->scx.flags & SCX_RQ_BAL_CB_PENDING)) {
+	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
@@ -3453,8 +3434,10 @@ do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
 	else
 		verdict = dispatch_pick(rq, rf, prev);
 
-	if (verdict == SCX_DSP_RETRY)
-		return RETRY_TASK;
+	if (verdict == SCX_DSP_RETRY) {
+		p = RETRY_TASK;
+		goto out;
+	}
 
 	/*
 	 * If any higher-priority sched class enqueued a runnable task on this
@@ -3464,8 +3447,10 @@ do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
 	 * If @force_scx is true, always try to pick a SCHED_EXT task,
 	 * regardless of any higher-priority sched classes activity.
 	 */
-	if (!force_scx && rq_modified_above(rq, &ext_sched_class))
-		return RETRY_TASK;
+	if (!force_scx && rq_modified_above(rq, &ext_sched_class)) {
+		p = RETRY_TASK;
+		goto out;
+	}
 
 	/*
 	 * If we're keeping @prev, replenish slice if necessary and keep running
@@ -3481,7 +3466,7 @@ do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
 	} else {
 		p = first_local_task(rq);
 		if (!p)
-			return NULL;
+			goto out;
 
 		if (unlikely(!p->scx.slice) && scx_task_can_stay_on_cpu(rq, p)) {
 			struct scx_sched *sch = scx_task_sched(p);
@@ -3496,6 +3481,38 @@ do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
 		}
 	}
 
+out:
+	if (cpu_of(rq) != smp_processor_id())
+		return p;
+
+	/*
+	 * Balance callbacks must be consumed before the rq lock is released, so
+	 * queue them only when the pick is final. On RETRY_TASK, or after a
+	 * NULL pick from the dl_server, the core continues picking and can
+	 * release the rq lock (e.g., prev_balance() or another dispatch). In
+	 * that case, run the deferred work from irq_work, which doesn't depend
+	 * on the current rq lock section, and leave the kick sync wait pending
+	 * for the next pick.
+	 */
+	if (p != RETRY_TASK && (p || !force_scx)) {
+		maybe_queue_balance_callback(rq);
+
+		/*
+		 * Defer to a balance callback which can drop rq lock and enable
+		 * IRQs. Waiting directly in the pick path would deadlock against
+		 * CPUs sending us IPIs (e.g., TLB flushes) while we wait for
+		 * them.
+		 */
+		if (unlikely(rq->scx.kick_sync_pending)) {
+			rq->scx.kick_sync_pending = false;
+			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) {
+		rq->scx.flags &= ~SCX_RQ_BAL_CB_PENDING;
+		schedule_deferred(rq);
+	}
+
 	return p;
 }
 
-- 
2.56.0

Re: [PATCH v2 sched_ext/for-7.3-fixes] sched_ext: Don't leave balance callbacks queued across a pick retry

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] sched_ext: balance callbacks queued before core-sched pick is final
--

commit 859b8e499322d8a16b0b359431e360cdd5ae7494
Author: Andrea Righi <arighi@nvidia.com>

sched_ext: Don't leave balance callbacks queued across a pick retry

This commit moves the queueing of rq->scx.deferred_bal_cb and
rq->scx.kick_sync_bal_cb to the exit path of do_pick_task_scx() so that
balance callbacks are only queued when do_pick_task_scx() returns a final
task rather than RETRY_TASK or a NULL dl_server pick.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 0403c46342813..e19961164cb01 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c

[ ... ]

> @@ -3342,16 +3331,8 @@ 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()) {
> -		maybe_queue_balance_callback(rq);
> -
> -		/* see dispatch_pick() */
> -		if (unlikely(rq->scx.kick_sync_pending)) {
> -			rq->scx.kick_sync_pending = false;
> -			queue_balance_callback(rq, &rq->scx.kick_sync_bal_cb,
> -					       kick_sync_wait_bal_cb);
> -		}
> -	} else if (unlikely(rq->scx.flags & SCX_RQ_BAL_CB_PENDING)) {
> +	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
> @@ -3394,8 +3375,10 @@ do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
>  	else
>  		verdict = dispatch_pick(rq, rf, prev);
>  
> -	if (verdict == SCX_DSP_RETRY)
> -		return RETRY_TASK;
> +	if (verdict == SCX_DSP_RETRY) {
> +		p = RETRY_TASK;
> +		goto out;
> +	}

[ ... ]

> @@ -3437,6 +3422,38 @@ do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
>  		}
>  	}
>  
> +out:
> +	if (cpu_of(rq) != smp_processor_id())
> +		return p;
> +
> +	/*
> +	 * Balance callbacks must be consumed before the rq lock is released, so
> +	 * queue them only when the pick is final. On RETRY_TASK, or after a
> +	 * NULL pick from the dl_server, the core continues picking and can
> +	 * release the rq lock (e.g., prev_balance() or another dispatch). In
> +	 * that case, run the deferred work from irq_work, which doesn't depend
> +	 * on the current rq lock section, and leave the kick sync wait pending
> +	 * for the next pick.
> +	 */
> +	if (p != RETRY_TASK && (p || !force_scx)) {
> +		maybe_queue_balance_callback(rq);
> +
> +		/*
> +		 * Defer to a balance callback which can drop rq lock and enable
> +		 * IRQs. Waiting directly in the pick path would deadlock against
> +		 * CPUs sending us IPIs (e.g., TLB flushes) while we wait for
> +		 * them.
> +		 */
> +		if (unlikely(rq->scx.kick_sync_pending)) {
> +			rq->scx.kick_sync_pending = false;
> +			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) {
> +		rq->scx.flags &= ~SCX_RQ_BAL_CB_PENDING;
> +		schedule_deferred(rq);
> +	}
> +
>  	return p;
>  }

[Severity: High]
Under CONFIG_SCHED_CORE, can a non-RETRY_TASK return from
do_pick_task_scx() on the local rq still be followed by further picks that
release the shared core lock before the core-wide pick is final?

In kernel/sched/core.c:pick_next_task(), a single shared core lock is held
across all SMT siblings, and pick_task() can be called multiple times after
the local rq pick succeeds:

1. In the !need_sync fastpath, pick_task(rq, rf) runs on the local rq:

kernel/sched/core.c:pick_next_task() {
	if (!need_sync) {
		next = pick_task(rq, rf);
		...
		if (!next->core_cookie) {
			...
			goto out_set_next;
		}
	}
}

If do_pick_task_scx() returns a cookied SCX task (next->core_cookie != 0),
the out: block sees p != RETRY_TASK && (p || !force_scx) and queues
rq->scx.deferred_bal_cb and/or rq->scx.kick_sync_bal_cb on
rq->balance_callback. Because next->core_cookie is non-zero,
pick_next_task() then falls through into the for_each_cpu_wrap(i, smt_mask,
cpu) loop to re-pick across all siblings.

2. In the for_each_cpu_wrap(i, smt_mask, cpu) loop in pick_next_task(),
iteration starts at i = cpu (where cpu_of(rq_i) == smp_processor_id()):

kernel/sched/core.c:pick_next_task() {
	for_each_cpu_wrap(i, smt_mask, cpu) {
		rq_i = cpu_rq(i);
		...
		p = pick_task(rq_i, rf);
		if (unlikely(p == RETRY_TASK)) {
			...
			goto restart;
		}
	}
}

Once do_pick_task_scx() on the local rq succeeds and queues the balance
callbacks on rq->balance_callback, the loop proceeds to call
pick_task(rq_i, rf) on the remaining SMT siblings (i != cpu). When
dispatch_core_pick(rq_i, rf, prev) runs for a sibling rq_i,
dispatch_one(rq_i, prev) or run_deferred(rq_i) can drop the shared core
lock and return SCX_DSP_RETRY, causing pick_next_task() to jump to restart
with rq->balance_callback already queued on the local rq. Any CPU acquiring
the core lock during that window will trigger the warning in
assert_balance_callbacks_empty() via rq_pin_lock() or flush the callback
early.

3. Also in dispatch_core_pick(), if an SMT sibling already has an in-flight
dispatch on the local rq:

kernel/sched/ext/ext.c:dispatch_core_pick() {
	/* another dispatch is in flight on @rq, let that handle it */
	if (rq->scx.flags & SCX_RQ_IN_DISPATCH)
		return SCX_DSP_NONE;
}

Previously, returning SCX_DSP_NONE here bypassed callback queueing in
dispatch_core_pick(). With the callback handling moved to the out: label in
do_pick_task_scx(), won't this early return now fall through to out: and
consume SCX_RQ_BAL_CB_PENDING and rq->scx.kick_sync_pending while the
sibling's dispatch_one() on rq is still in flight?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017152226.3690249-1-arighi@nvidia.com?part=1

Re: [PATCH v2 sched_ext/for-7.3-fixes] sched_ext: Don't leave balance callbacks queued across a pick retry

[Andrea Righi]

On Sat, Oct 17, 2026 at 03:40:58PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] sched_ext: balance callbacks queued before core-sched pick is final
> --
> 
> commit 859b8e499322d8a16b0b359431e360cdd5ae7494
> Author: Andrea Righi <arighi@nvidia.com>
> 
> sched_ext: Don't leave balance callbacks queued across a pick retry
> 
> This commit moves the queueing of rq->scx.deferred_bal_cb and
> rq->scx.kick_sync_bal_cb to the exit path of do_pick_task_scx() so that
> balance callbacks are only queued when do_pick_task_scx() returns a final
> task rather than RETRY_TASK or a NULL dl_server pick.
> 
> > diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> > index 0403c46342813..e19961164cb01 100644
> > --- a/kernel/sched/ext/ext.c
> > +++ b/kernel/sched/ext/ext.c
> 
> [ ... ]
> 
> > @@ -3342,16 +3331,8 @@ 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()) {
> > -		maybe_queue_balance_callback(rq);
> > -
> > -		/* see dispatch_pick() */
> > -		if (unlikely(rq->scx.kick_sync_pending)) {
> > -			rq->scx.kick_sync_pending = false;
> > -			queue_balance_callback(rq, &rq->scx.kick_sync_bal_cb,
> > -					       kick_sync_wait_bal_cb);
> > -		}
> > -	} else if (unlikely(rq->scx.flags & SCX_RQ_BAL_CB_PENDING)) {
> > +	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
> > @@ -3394,8 +3375,10 @@ do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
> >  	else
> >  		verdict = dispatch_pick(rq, rf, prev);
> >  
> > -	if (verdict == SCX_DSP_RETRY)
> > -		return RETRY_TASK;
> > +	if (verdict == SCX_DSP_RETRY) {
> > +		p = RETRY_TASK;
> > +		goto out;
> > +	}
> 
> [ ... ]
> 
> > @@ -3437,6 +3422,38 @@ do_pick_task_scx(struct rq *rq, struct rq_flags *rf, bool force_scx)
> >  		}
> >  	}
> >  
> > +out:
> > +	if (cpu_of(rq) != smp_processor_id())
> > +		return p;
> > +
> > +	/*
> > +	 * Balance callbacks must be consumed before the rq lock is released, so
> > +	 * queue them only when the pick is final. On RETRY_TASK, or after a
> > +	 * NULL pick from the dl_server, the core continues picking and can
> > +	 * release the rq lock (e.g., prev_balance() or another dispatch). In
> > +	 * that case, run the deferred work from irq_work, which doesn't depend
> > +	 * on the current rq lock section, and leave the kick sync wait pending
> > +	 * for the next pick.
> > +	 */
> > +	if (p != RETRY_TASK && (p || !force_scx)) {
> > +		maybe_queue_balance_callback(rq);
> > +
> > +		/*
> > +		 * Defer to a balance callback which can drop rq lock and enable
> > +		 * IRQs. Waiting directly in the pick path would deadlock against
> > +		 * CPUs sending us IPIs (e.g., TLB flushes) while we wait for
> > +		 * them.
> > +		 */
> > +		if (unlikely(rq->scx.kick_sync_pending)) {
> > +			rq->scx.kick_sync_pending = false;
> > +			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) {
> > +		rq->scx.flags &= ~SCX_RQ_BAL_CB_PENDING;
> > +		schedule_deferred(rq);
> > +	}
> > +
> >  	return p;
> >  }
> 
> [Severity: High]
> Under CONFIG_SCHED_CORE, can a non-RETRY_TASK return from
> do_pick_task_scx() on the local rq still be followed by further picks that
> release the shared core lock before the core-wide pick is final?
> 
> In kernel/sched/core.c:pick_next_task(), a single shared core lock is held
> across all SMT siblings, and pick_task() can be called multiple times after
> the local rq pick succeeds:
> 
> 1. In the !need_sync fastpath, pick_task(rq, rf) runs on the local rq:
> 
> kernel/sched/core.c:pick_next_task() {
> 	if (!need_sync) {
> 		next = pick_task(rq, rf);
> 		...
> 		if (!next->core_cookie) {
> 			...
> 			goto out_set_next;
> 		}
> 	}
> }
> 
> If do_pick_task_scx() returns a cookied SCX task (next->core_cookie != 0),
> the out: block sees p != RETRY_TASK && (p || !force_scx) and queues
> rq->scx.deferred_bal_cb and/or rq->scx.kick_sync_bal_cb on
> rq->balance_callback. Because next->core_cookie is non-zero,
> pick_next_task() then falls through into the for_each_cpu_wrap(i, smt_mask,
> cpu) loop to re-pick across all siblings.
> 
> 2. In the for_each_cpu_wrap(i, smt_mask, cpu) loop in pick_next_task(),
> iteration starts at i = cpu (where cpu_of(rq_i) == smp_processor_id()):
> 
> kernel/sched/core.c:pick_next_task() {
> 	for_each_cpu_wrap(i, smt_mask, cpu) {
> 		rq_i = cpu_rq(i);
> 		...
> 		p = pick_task(rq_i, rf);
> 		if (unlikely(p == RETRY_TASK)) {
> 			...
> 			goto restart;
> 		}
> 	}
> }
> 
> Once do_pick_task_scx() on the local rq succeeds and queues the balance
> callbacks on rq->balance_callback, the loop proceeds to call
> pick_task(rq_i, rf) on the remaining SMT siblings (i != cpu). When
> dispatch_core_pick(rq_i, rf, prev) runs for a sibling rq_i,
> dispatch_one(rq_i, prev) or run_deferred(rq_i) can drop the shared core
> lock and return SCX_DSP_RETRY, causing pick_next_task() to jump to restart
> with rq->balance_callback already queued on the local rq. Any CPU acquiring
> the core lock during that window will trigger the warning in
> assert_balance_callbacks_empty() via rq_pin_lock() or flush the callback
> early.

I'm planning to fix this with a follow-up fix, it was also pointed out by Tejun
in his review of v1.

> 
> 3. Also in dispatch_core_pick(), if an SMT sibling already has an in-flight
> dispatch on the local rq:
> 
> kernel/sched/ext/ext.c:dispatch_core_pick() {
> 	/* another dispatch is in flight on @rq, let that handle it */
> 	if (rq->scx.flags & SCX_RQ_IN_DISPATCH)
> 		return SCX_DSP_NONE;
> }
> 
> Previously, returning SCX_DSP_NONE here bypassed callback queueing in
> dispatch_core_pick(). With the callback handling moved to the out: label in
> do_pick_task_scx(), won't this early return now fall through to out: and
> consume SCX_RQ_BAL_CB_PENDING and rq->scx.kick_sync_pending while the
> sibling's dispatch_one() on rq is still in flight?

Hm... yeah this looks legit for SCX_RQ_BAL_CB_PENDING, I'll fix this in v3.

-Andrea

Re: [PATCH v2 sched_ext/for-7.3-fixes] sched_ext: Don't leave balance callbacks queued across a pick retry

[Tejun Heo]

Applied to sched_ext/for-7.3-fixes.

Thanks.

--
tejun

Re: [PATCH v2 sched_ext/for-7.3-fixes] sched_ext: Don't leave balance callbacks queued across a pick retry

[Andrea Righi]

On Sat, Oct 17, 2026 at 08:44:54AM -1000, Tejun Heo wrote:
> Applied to sched_ext/for-7.3-fixes.
> 
> Thanks.

Oops, I was going to send a v3 based on the review from sashiko. I'll send a
follow-up fix then.

Thanks,
-Andrea