Re: [PATCH 0/2] sched: Remove sched_class::balance()

[Peter Zijlstra <peterz@infradead.org>]

On Wed, Aug 19, 2026 at 09:58:30AM +0200, Peter Zijlstra wrote:
> On Thu, Jul 02, 2026 at 07:49:19PM +0800, Aaron Lu wrote:
> 
> > Assume cpuX and cpuY are siblings, it appears the following happened:
> > 
> >      cpuX                                      cpuY
> > 
> >    pick_next_task()
> >    goto restart_multi
> > 
> >    rqX->core_pick = pick_task(rqX)
> > 
> >    pick_task(rqY)
> >      pick_task_fair(rqY)
> >        sched_balance_newidle(rqY)
> >          raw_spin_rq_unlock(rqY)  // drops core lock
> > 
> >                                            pick_next_task()
> >                                            goto restart_multi
> >                                            rqY->core_pick = pick_task(rqY)
> >                                            rqX->core_pick = pick_task(rqX)
> > 
> >                                            if (rqX->curr == rqX->core_pick)
> >                                              rqX->core_pick = NULL
> > 
> > 					   UNLOCK rq_lockp(rqY)
> > 
> >          raw_spin_rq_lock(rqY)
> > 
> >    rqY->core_pick = pick_task(rqY)
> > 
> >    p = rqX->core_pick        // NULL
> >    cookie_equals(p, cookie)  // NULL deref
> 
> Well, damn :/ That's a nice race. So while we did a lock-break, it does
> not trigger RETRY_TASK and continues.
> 
> The 'easy' fix is taking a local copy of core_task_seq when we increase
> it for the pick, and double checking that is still valid at the end and
> then restarting if not.
> 
> Except that is susceptible to live-locks. It doesn't have forward
> progress guarantees. For that we need to limit the amount of
> lock-breaks/newidle invocations.

So TJ did something like that for ext. I'm not entirely sure I get his
argument on forward progress though.

But the simple thing is something like so, which I think also allows
simplifying ext some.

---
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 8070a347cf3b..ce8bb5036fe1 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6232,14 +6232,14 @@ static struct task_struct *
 pick_next_task(struct rq *rq, struct rq_flags *rf)
 	__must_hold(__rq_lockp(rq))
 {
+	bool core_clock_updated = (rq == rq->core);
 	struct task_struct *next, *p, *max;
 	const struct cpumask *smt_mask;
+	int i, cpu, seq, occ = 0;
 	bool fi_before = false;
-	bool core_clock_updated = (rq == rq->core);
-	unsigned long cookie;
-	int i, cpu, occ = 0;
-	struct rq *rq_i;
 	bool need_sync = false;
+	unsigned long cookie;
+	struct rq *rq_i;
 
 	if (!sched_core_enabled(rq))
 		return __pick_next_task(rq, rf);
@@ -6314,7 +6314,7 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 	 * However, preemptions can cause multiple picks on the same task set.
 	 * 'Fix' this by also increasing @task_seq for every pick.
 	 */
-	rq->core->core_task_seq++;
+	seq = ++rq->core->core_task_seq;
 
 	/*
 	 * Optimize for common case where this CPU has no cookies
@@ -6362,7 +6362,8 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 			update_rq_clock(rq_i);
 
 		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))) {
 			/* rq lock may have been dropped, clocks invalidated */
 			core_clock_updated = false;
 			if (!(rq->clock_update_flags & RQCF_UPDATED))
@@ -6392,7 +6393,7 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 			if (cookie)
 				p = sched_core_find(rq_i, cookie);
 			if (!p)
-				p = idle_sched_class.pick_task(rq_i, rf);
+				p = idle_sched_class.pick_task(rq_i, NULL);
 		}
 
 		rq_i->core_pick = p;