[PATCH sched_ext/for-7.4-fixes] sched_ext: Fix stale pick when a wakeup resets the donor during dispatch

[PATCH sched_ext/for-7.4-fixes] sched_ext: Fix stale pick when a wakeup resets the donor during dispatch

[Tejun Heo]

With proxy execution, do_pick_task_scx() passes rq->donor to dispatch_one()
as @prev. Unlike rq->curr, the donor can be moved to another CPU by a wakeup
while dispatch_one() has the rq lock released:

1. dispatch_one() releases the rq lock, e.g. to move a task from another
   CPU.
2. Another CPU wakes the donor, which proxy execution migrated to this CPU.
   proxy_needs_return() dequeues it and resets the donor to rq->curr, and
   the wakeup enqueues it, possibly on another CPU.
3. dispatch_one() sees @prev queued and returns SCX_DSP_PREV.
4. do_pick_task_scx() returns @prev, and this CPU runs a task that is queued
   on another rq.

The other verdicts are stale too. If the dispatch finds nothing before the
wakeup enqueues @prev, SCX_DSP_NONE picks idle, and put_prev_task_scx()
parks rq->curr on the local DSQ with nothing to reschedule the CPU, or trips
its SCX_OPS_ENQ_LAST warning. SCX_DSP_LOCAL preempts rq->curr even when the
dispatch would have kept it.

rq_modified_above() catches the reset only when rq->curr is in a class above
ext, and the dl_server pick skips the check.

The verdict, whatever it is, describes the old donor. Restart the pick when
@prev is no longer the donor, as dispatch_core_pick() already does whenever
the dispatch released the rq lock. The retry runs with rq->curr as the
donor, and proxy_needs_return() returns early for the current task, so the
donor can't change again.

7a3a6bfbd62a ("sched: Rework prev_balance() to avoid stale prev references")
closed the same window in prev_balance() by reading rq->donor instead of
taking @prev. That doesn't work here, as the dispatch never evaluated
rq->curr, which may not even be a sched_ext task.

Verified with a synthetic reproducer.

Fixes: c6fe97c34a1a ("sched: Allow enabling proxy exec with sched_ext")
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/sched/ext/ext.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 15123f181b5b..3119a1938c6e 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -3936,6 +3936,16 @@ static enum scx_dsp_verdict dispatch_pick(struct rq *rq, struct rq_flags *rf,
 	verdict = dispatch_one(rq, prev);
 	rq_repin_lock(rq, rf);
 
+	/*
+	 * With proxy execution, @prev is the donor and need not be the running
+	 * task. A wakeup while dispatch_one() has the rq lock released can
+	 * reset the donor and move @prev to another CPU. The verdict was
+	 * computed for @prev and doesn't apply once the donor changed. Restart
+	 * the pick.
+	 */
+	if (unlikely(prev != rq->donor))
+		return SCX_DSP_RETRY;
+
 	return verdict;
 }
 
-- 
2.56.0

Re: [PATCH sched_ext/for-7.4-fixes] sched_ext: Fix stale pick when a wakeup resets the donor during dispatch

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] sched_ext: stale prev and donor use in dispatch_one() after rq lock drop
- [Medium] sched: stale rq->dl_server across RETRY_TASK in __pick_next_task()
--

commit 2376377a1091277fdc742a85c71efe512084aeba
Author: Tejun Heo <tj@kernel.org>

sched_ext: Fix stale pick when a wakeup resets the donor during dispatch

When proxy execution is enabled, a concurrent wakeup while dispatch_one()
releases the runqueue lock can reset rq->donor to rq->curr and migrate the
previous donor to another CPU. This commit checks whether prev != rq->donor
in dispatch_pick() after dispatch_one() returns and restarts the pick with
SCX_DSP_RETRY.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 904627ef909cb..c3f2f631dfafb 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -3919,6 +3919,16 @@ static enum scx_dsp_verdict dispatch_pick(struct rq *rq, struct rq_flags *rf,
>  	verdict = dispatch_one(rq, prev);
>  	rq_repin_lock(rq, rf);
>  
> +	/*
> +	 * With proxy execution, @prev is the donor and need not be the running
> +	 * task. A wakeup while dispatch_one() has the rq lock released can
> +	 * reset the donor and move @prev to another CPU. The verdict was
> +	 * computed for @prev and doesn't apply once the donor changed. Restart
> +	 * the pick.
> +	 */
> +	if (unlikely(prev != rq->donor))
> +		return SCX_DSP_RETRY;

[Severity: High]
Because dispatch_pick() checks prev != rq->donor only after dispatch_one()
has completed, can a donor change during an internal rq->__lock drop inside
dispatch_one() cause dispatch_one() and scx_dispatch_sched() to operate on a
stale prev task and a mismatched rq->donor before returning?

In dispatch_one(), scx_process_sync_ecaps() -> __scx_process_sync_ecaps() ->
scx_flush_dispatch_buf() can drop and reacquire rq->__lock:

kernel/sched/ext/ext.c:dispatch_one() {
	scx_process_sync_ecaps(rq, prev);
	...
	if (prev->sched_class == &ext_sched_class) {
		update_curr_scx(rq);
		...
		if (scx_bypassing(scx_task_sched(prev), cpu))
			expire_task_slice(prev);
}

If another CPU wakes the blocked donor prev during that window,
proxy_needs_return() resets rq->donor to rq->curr and try_to_wake_up() can
migrate and enqueue prev on a remote CPU. When scx_process_sync_ecaps()
returns:

1. dispatch_one() tests prev->sched_class == &ext_sched_class on the old
   donor and calls update_curr_scx(rq), which reads donor = rq->donor (now
   rq->curr). If rq->curr is not in ext_sched_class, update_curr_scx() runs
   apply_task_slice_oob() and update_curr_common() on the non-scx rq->curr
   and charges its execution delta to rq->ext_server.

2. scx_task_sched(prev) is called without holding prev->pi_lock or
   task_rq(prev)->__lock, triggering the rcu_dereference_protected() check,
   and expire_task_slice(prev) -> set_task_slice_keep_oob(prev, 0) trips
   lockdep_assert_rq_held(task_rq(p)) while zeroing prev->scx.slice on a
   remote runqueue.

Similarly, if rq->__lock is dropped there or inside scx_dispatch_sched()
(via scx_consume_global_dsq(), scx_consume_dispatch_q(), or
scx_flush_dispatch_buf()):

kernel/sched/ext/inlines.h:scx_dispatch_sched() {
		SCX_CALL_OP(sch, dispatch, rq, scx_cpu_arg(cpu),
			    prev_on_sch ? prev : NULL);
}

kernel/sched/ext/ext.c:dispatch_one() {
	if (prev->scx.flags & SCX_TASK_QUEUED) {
		struct scx_sched *prev_sch = scx_task_sched(prev);
		...
	}
	...
has_tasks:
	if (unlikely((rq->scx.local_dsq.nr > 1 || verdict == SCX_DSP_PREV) &&
		     rq->scx.nr_immed))
		scx_schedule_reenq_local(rq, 0);
}

Wouldn't scx_dispatch_sched() pass the migrated prev pointer to ops.dispatch,
dispatch_one() call scx_task_sched(prev) unlocked when prev is queued on its
new CPU, and the has_tasks path schedule a spurious local DSQ re-enqueue for
a lone SCX_ENQ_IMMED task if a stale SCX_DSP_PREV verdict was reached before
dispatch_pick() turns it into SCX_DSP_RETRY?

[Severity: Medium]
Also, when dispatch_pick() returns SCX_DSP_RETRY during a deadline server
pick (do_pick_task_scx() called from ext_server_pick_task() with
sched_core_enabled(rq) false), could returning RETRY_TASK leave a stale
rq->dl_server pointer across the restart in __pick_next_task()?

In __pick_task_dl(), a RETRY_TASK return from server_pick_task() is non-NULL,
so rq->dl_server is set before returning RETRY_TASK:

kernel/sched/deadline.c:__pick_task_dl() {
	if (dl_server(dl_se)) {
		p = dl_se->server_pick_task(dl_se, rf);
		if (!p) {
			dl_server_stop(dl_se);
			goto again;
		}
		rq->dl_server = dl_se;
	}
}

In __pick_next_task(), jumping to restart bypasses rq->dl_server = NULL:

kernel/sched/core.c:__pick_next_task() {
	rq->dl_server = NULL;

	if (scx_enabled())
		goto restart;
	...
restart:
	prev_balance(rq, rf);

	for_each_active_class(class) {
		p = class->pick_task(rq, rf);
		if (unlikely(p == RETRY_TASK))
			goto restart;
		if (p) {
			put_prev_set_next_task(rq, rq->donor, p);
			return p;
		}
	}
}

If the retry pass ends up picking a non-server task (for instance, if the
migrated donor was the last scx task so ext_server_pick_task() returns NULL
and stops ext_server, or a higher-priority task became runnable while the rq
lock was dropped), wouldn't put_prev_set_next_task() ->
__put_prev_set_next_dl_server() assign the stale &rq->ext_server pointer to
next->dl_server on that task?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030142338.3453484-1-tj@kernel.org?part=1

Re: [PATCH sched_ext/for-7.4-fixes] sched_ext: Fix stale pick when a wakeup resets the donor during dispatch

[Andrea Righi]

Hi Tejun,

On Fri, Oct 30, 2026 at 04:23:38AM -1000, Tejun Heo wrote:
> With proxy execution, do_pick_task_scx() passes rq->donor to dispatch_one()
> as @prev. Unlike rq->curr, the donor can be moved to another CPU by a wakeup
> while dispatch_one() has the rq lock released:
> 
> 1. dispatch_one() releases the rq lock, e.g. to move a task from another
>    CPU.
> 2. Another CPU wakes the donor, which proxy execution migrated to this CPU.
>    proxy_needs_return() dequeues it and resets the donor to rq->curr, and
>    the wakeup enqueues it, possibly on another CPU.
> 3. dispatch_one() sees @prev queued and returns SCX_DSP_PREV.
> 4. do_pick_task_scx() returns @prev, and this CPU runs a task that is queued
>    on another rq.
> 
> The other verdicts are stale too. If the dispatch finds nothing before the
> wakeup enqueues @prev, SCX_DSP_NONE picks idle, and put_prev_task_scx()
> parks rq->curr on the local DSQ with nothing to reschedule the CPU, or trips
> its SCX_OPS_ENQ_LAST warning. SCX_DSP_LOCAL preempts rq->curr even when the
> dispatch would have kept it.
> 
> rq_modified_above() catches the reset only when rq->curr is in a class above
> ext, and the dl_server pick skips the check.
> 
> The verdict, whatever it is, describes the old donor. Restart the pick when
> @prev is no longer the donor, as dispatch_core_pick() already does whenever
> the dispatch released the rq lock. The retry runs with rq->curr as the
> donor, and proxy_needs_return() returns early for the current task, so the
> donor can't change again.
> 
> 7a3a6bfbd62a ("sched: Rework prev_balance() to avoid stale prev references")
> closed the same window in prev_balance() by reading rq->donor instead of
> taking @prev. That doesn't work here, as the dispatch never evaluated
> rq->curr, which may not even be a sched_ext task.
> 
> Verified with a synthetic reproducer.
> 
> Fixes: c6fe97c34a1a ("sched: Allow enabling proxy exec with sched_ext")
> Signed-off-by: Tejun Heo <tj@kernel.org>
> ---
>  kernel/sched/ext/ext.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 15123f181b5b..3119a1938c6e 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -3936,6 +3936,16 @@ static enum scx_dsp_verdict dispatch_pick(struct rq *rq, struct rq_flags *rf,
>  	verdict = dispatch_one(rq, prev);
>  	rq_repin_lock(rq, rf);
>  
> +	/*
> +	 * With proxy execution, @prev is the donor and need not be the running
> +	 * task. A wakeup while dispatch_one() has the rq lock released can
> +	 * reset the donor and move @prev to another CPU. The verdict was
> +	 * computed for @prev and doesn't apply once the donor changed. Restart
> +	 * the pick.
> +	 */
> +	if (unlikely(prev != rq->donor))
> +		return SCX_DSP_RETRY;
> +

I think Sashiko's first point is that this check comes too late to
protect accesses inside dispatch_one(). If the donor changes while
scx_process_sync_ecaps() drops the rq lock, the subsequent
prev->sched_class test checks the old donor, but update_curr_scx(rq)
operates on the new rq->donor, which might not be an EXT task.

Maybe we can detect the donor change before those accesses too?

And the second finding also looks valid, __pick_task_dl() sets
rq->dl_server even when the server pick returns RETRY_TASK, but
__pick_next_task() skips clearing it on restart. If the next pass picks
a non-server task, it inherits the stale server association.

Should we clear rq->dl_server on restart? (I haven't reproduced or
tested this).

Thanks,
-Andrea

>  	return verdict;
>  }
>  
> -- 
> 2.56.0
>