Re: [PATCH v4 sched_ext/for-7.4-fixes] sched_ext: Fix stale pick when the donor changes during dispatch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] sched_ext: dispatch_one() emits ops.cpu_acquire() before checking stale prev
--

commit 15726c74d0541a9ea4622e6efa11631d9ab6aea0
Author: Tejun Heo <tj@kernel.org>

sched_ext: Fix stale pick when the donor changes during dispatch

This commit fixes a stale task pick in sched_ext when rq->donor changes
while the rq lock is dropped during dispatch under proxy execution or core
scheduling. It introduces scx_dsp_prev_stale() to detect when prev is no
longer rq->donor and restarts the pick with SCX_DSP_RETRY.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 0403c46342813..bcd1f7ee8c89b 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -2950,6 +2950,12 @@ static enum scx_dsp_verdict dispatch_one(struct rq *rq, struct task_struct *prev
>  		rq->scx.cpu_released = false;
>  	}
>  
> +	/* the ecaps sync may have dropped the rq lock */
> +	if (scx_dsp_prev_stale(rq, prev)) {
> +		verdict = SCX_DSP_RETRY;
> +		goto has_tasks;
> +	}

[Severity: Medium]
In dispatch_one(), scx_dsp_prev_stale() is checked after handling
rq->scx.cpu_released rather than immediately after
scx_process_sync_ecaps():

kernel/sched/ext/ext.c:dispatch_one() {
	...
	scx_process_sync_ecaps(rq, prev);

	if ((root_sch->ops.flags & SCX_OPS_HAS_CPU_PREEMPT) &&
	    unlikely(rq->scx.cpu_released)) {
		if (root_sch->ops.cpu_acquire)
			SCX_CALL_OP(root_sch, cpu_acquire, rq, cpu, NULL);
		rq->scx.cpu_released = false;
	}

	/* the ecaps sync may have dropped the rq lock */
	if (scx_dsp_prev_stale(rq, prev)) {
		verdict = SCX_DSP_RETRY;
		goto has_tasks;
	}
	...
}

When scx_process_sync_ecaps() drops the rq lock in
scx_flush_dispatch_buf(), a sibling CPU under core scheduling can switch
that rq to a higher-priority RT or DL task, running
put_prev_set_next_task() -> put_prev_task_scx() -> switch_class(), which
emits ops.cpu_release() and sets rq->scx.cpu_released = true. Under proxy
execution, if rq->scx.cpu_released was already true on entry, a remote
wakeup during that lock drop can reset rq->donor to a non-SCX rq->curr
while an RT or DL task becomes runnable.

When scx_process_sync_ecaps() returns, dispatch_one() will call
ops.cpu_acquire() and clear rq->scx.cpu_released = false before seeing
scx_dsp_prev_stale() and returning SCX_DSP_RETRY. If the restarted pick
then selects the higher-priority RT or DL task without reaching
ext_sched_class, and rq->donor is no longer an SCX task, switch_class()
will not run again to set rq->scx.cpu_released = true.

Should the scx_dsp_prev_stale() check be placed immediately after
scx_process_sync_ecaps(), before emitting ops.cpu_acquire() and clearing
rq->scx.cpu_released?

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