Re: [PATCH v2 01/12] sched/core: Fix pick_next_task() self recursion

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Nov 10, 2026 at 02:30:33PM -1000, Tejun Heo wrote:
> Hello, Peter.
> 
> On Sat, Nov 07, 2026 at 12:59:14PM +0100, Peter Zijlstra wrote:
> > Since RETRY_TASK is only possible when a higher priority task gets enqueued
> > during the lock break, this must mean core_task_seq will also be incremented
> > and is thus completely covered by the seq number mismatch.
> 
> This doesn't hold for sched_ext, which returns RETRY_TASK whenever
> dispatch released the rq lock, e.g. ops.dispatch() inserting a task into
> a remote CPU's local DSQ, with no enqueue or dequeue on this core.
> 
> [ ... ]
> 
> >  		if (!next->core_cookie) {
> > +			/*
> > +			 * pick_task() can drop the core rq lock through
> > +			 * newidle balance. If a sibling established a
> > +			 * core-wide cookie while the lock was dropped, the
> > +			 * uncookied no-sync fast path is no longer valid.
> > +			 */
> > +			if (unlikely(rq->core->core_cookie))
> > +				goto restart;
> > +
> >  			rq->core_pick = NULL;
> >  			rq->core_dl_server = NULL;
> 
> This only catches a sibling selection which has already committed. One
> still in flight has reset core_cookie to 0 and may have set this CPU's
> core_pick, which the fast path then clears without changing
> core_task_seq. It needs three or more SMT siblings (e.g. POWER), so it
> isn't a common case:
> 
> 1. CPU2 is in the fast path and drops the lock in newidle.
> 2. CPU1 starts a core-wide selection: bumps core_task_seq, resets
>    core_cookie, sets rq2->core_pick and drops the lock in
>    pick_task(rq3).
> 3. CPU2 retakes the lock, sees core_cookie == 0, clears rq2->core_pick
>    and commits.
> 4. CPU1 retakes the lock, passes the seq check and hands the NULL
>    rq2->core_pick to cookie_equals().
> 
> This becomes reachable with 10/12 ("sched/fair: Reflow pick_task_fair()
> / newidle"), where pick_task_fair() can drop the lock in newidle without
> returning RETRY_TASK, and reliably oopses in __schedule() on an SMT4 VM.

Nice find.

> One way could be bumping core_task_seq when the fast path commits after
> the seq moved, so that the in-flight selection restarts. Another would
> be testing seq != core_task_seq instead of core_cookie, at the cost of
> restarting on any enqueue or dequeue during the lock break.

It always did bother me this test wasn't the same we used below. Let
them both be: 'seq != rq->core->core_task_seq'.

> [ ... ]
> 
> >  		p = pick_task(rq_i, rf);
> > -		if (unlikely(p == RETRY_TASK)) {
> > +		if (unlikely(seq != rq->core->core_task_seq ||
> > +			     WARN_ON_ONCE(p == RETRY_TASK))) {
> 
> As sched_ext can return RETRY_TASK with core_task_seq unchanged, this
> triggers reliably with a BPF scheduler running cookied tasks. The
> restart is still correct. Can the WARN_ON_ONCE() be dropped? The plain
> RETRY_TASK test is still needed as the seq can match.

Sure, dropped the WARN.

---
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6391,13 +6391,7 @@ pick_next_task(struct rq *rq, struct rq_
 		}
 
 		if (!next->core_cookie) {
-			/*
-			 * pick_task() can drop the core rq lock through
-			 * newidle balance. If a sibling established a
-			 * core-wide cookie while the lock was dropped, the
-			 * uncookied no-sync fast path is no longer valid.
-			 */
-			if (unlikely(rq->core->core_cookie))
+			if (unlikely(seq != rq->core->core_task_seq))
 				goto restart;
 
 			rq->core_pick = NULL;
@@ -6431,8 +6425,7 @@ pick_next_task(struct rq *rq, struct rq_
 			update_rq_clock(rq_i);
 
 		p = pick_task(rq_i, rf);
-		if (unlikely(seq != rq->core->core_task_seq ||
-			     WARN_ON_ONCE(p == RETRY_TASK))) {
+		if (unlikely(seq != rq->core->core_task_seq || p == RETRY_TASK)) {
 			/* rq lock may have been dropped, clocks invalidated */
 			core_clock_updated = false;
 			if (!(rq->clock_update_flags & RQCF_UPDATED))