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

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

[Peter Zijlstra]

Hi!

As mentioned [1], I was going to move sched_balance_newidle() into
balance_fair(). And while the patch was quickly done, something didn't feel
right.

The two things that bugged me were:

 - core-sched only calls prev_balance() on one sibling, not allowing the others
   to pull tasks;

 - both ::balance() and ::pick_task() take @rf and do balance operations.

That latter is because of ext; the DSQ move needs to be undone if it turns out
a higher class will get ran after all -- this is where ::pick_task() got the rf
from and why I moved its balance into pick_task_scx().

After a bit of puzzling and a few wrong turns [2], I think these patches do
away with sched_class::balance() instead, less is more and all that.

[1] https://patch.msgid.link/20260611113219.GG187714%40noisy.programming.kicks-ass.net
[2] I'm pretty sure I messed things up a few times, but rt-migration-test never
    threw a failure on me -- which suggests it isn't very good at finding fails.

[PATCH 1/2] sched/core: Allow newidle for core-sched

[Peter Zijlstra]

As reported [1], it is possible for newidle, as called from pick_task_fair(),
to migrate a task that was already selected for the SMT sibling.

That is, in case of core scheduling (pick_next_task()'s restart_multi label),
pick_task() is called for each SMT sibling. Suppose SMT0 picks task A, and SMT1
has no tasks and does newidle, it must not be possible (but currently is) to
migrate A to SMT1 and also select A.

This re-enables newidle balancing for core scheduling.

[1] https://patch.msgid.link/20260603095108.GA1684319@bytedance.com

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/fair.c  |    6 ++----
 kernel/sched/sched.h |   15 ++++++++++++++-
 2 files changed, 16 insertions(+), 5 deletions(-)

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9942,9 +9942,6 @@ struct task_struct *pick_task_fair(struc
 	return p;
 
 idle:
-	if (sched_core_enabled(rq))
-		return NULL;
-
 	new_tasks = sched_balance_newidle(rq, rf);
 	if (new_tasks < 0)
 		return RETRY_TASK;
@@ -10783,7 +10780,8 @@ int can_migrate_task(struct task_struct
 	env->flags &= ~LBF_ALL_PINNED;
 
 	if (task_on_cpu(env->src_rq, p) ||
-	    task_current_donor(env->src_rq, p)) {
+	    task_current_donor(env->src_rq, p) ||
+	    task_on_core(env->src_rq, p)) {
 		schedstat_inc(p->stats.nr_failed_migrations_running);
 		return 0;
 	}
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1570,6 +1570,14 @@ static inline bool task_has_sched_core(s
 	return !!p->core_cookie;
 }
 
+static inline bool task_on_core(struct rq *rq, struct task_struct *p)
+{
+	if (sched_core_disabled())
+		return false;
+
+	return rq->core_pick == p;
+}
+
 #else /* !CONFIG_SCHED_CORE: */
 
 static inline bool sched_core_enabled(struct rq *rq)
@@ -1615,6 +1623,11 @@ static inline bool task_has_sched_core(s
 	return false;
 }
 
+static inline bool task_on_core(struct rq *rq, struct task_struct *p)
+{
+	return false;
+}
+
 #endif /* !CONFIG_SCHED_CORE */
 
 #ifdef CONFIG_RT_GROUP_SCHED
@@ -4131,7 +4144,7 @@ void move_queued_task_locked(struct rq *
 static inline
 bool task_is_pushable(struct rq *rq, struct task_struct *p, int cpu)
 {
-	if (!task_on_cpu(rq, p) &&
+	if (!task_on_cpu(rq, p) && !task_on_core(rq, p) &&
 	    cpumask_test_cpu(cpu, &p->cpus_mask))
 		return true;
 

Re: [PATCH 1/2] sched/core: Allow newidle for core-sched

[K Prateek Nayak]

Hello Peter,

On 6/24/2026 5:43 PM, Peter Zijlstra wrote:
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9942,9 +9942,6 @@ struct task_struct *pick_task_fair(struc
>  	return p;
>  
>  idle:
> -	if (sched_core_enabled(rq))
> -		return NULL;
> -
>  	new_tasks = sched_balance_newidle(rq, rf);

I have a sneaky feeling this might still race with a
sched_setaffinity() like:

     CPU0 (CPU1 is its sibling)           CPU2 (CPU1 is idle in the meanwhile)
     ==========================           ====================================

  rq0->core_pick = p;

  /* Core pick on SMT - CPU1 */
  pick_next_task(rq1)                  __sched_setaffinity(p)
    pick_next_task_fair()                __set_cpus_allowed_ptr() /* p cannot run on CPU0 anymore */
      sched_balance_newidle()              task_rq_lock(p, &rf)
        raw_spin_rq_unlock()                 ...
        ... /* continues newidle */
                                             /* Gets lock */
                                             __set_cpus_allowed_ptr_locked()
                                               dest_cpu = 1; /* SMT of CPU0 */
                                               affine_move_task(rq0, p, 1 /* Move to CPU1 */)
                                                 /*
                                                  * p is not on_cpu
                                                  * p is TASK_ON_RQ_QUEUED
                                                  * p is not migration disabled
                                                  */
                                                 move_queued_task(rq0, p, 1)
                                                   /* Moves task to CPU1 */
                                                 task_rq_unlock();

        ... /* Sees new task on CPU1 */
        raw_spin_rq_lock(rq1);
        pulled_task = 1;
        
      /* Retry pick; Finds p on cPU1 */
      return p;

    rq1->core_pick = p; !!! Two CPUs have the same core pick. !!!


Do we have a guard against this that I might be missing?

Should we treat core_pick similar to task_on_cpu() and go down the
stopper route like:

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 2f4530eb543f..e7f64f34aa4f 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3049,7 +3049,9 @@ static int affine_move_task(struct rq *rq, struct task_struct *p, struct rq_flag
 		return -EINVAL;
 	}
 
-	if (task_on_cpu(rq, p) || READ_ONCE(p->__state) == TASK_WAKING) {
+	if (task_on_cpu(rq, p) ||
+	    task_on_core(rq, p) ||
+	    READ_ONCE(p->__state) == TASK_WAKING) {
 		/*
 		 * MIGRATE_ENABLE gets here because 'p == current', but for
 		 * anything else we cannot do is_migration_disabled(), punt
---

... or we can return a RETRY_TASK when newidle balance succeeds too to
force a core-wide pick when core scheduling is enabled like:

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 0d212bf04885..e393aed58bfa 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9946,8 +9946,16 @@ struct task_struct *pick_task_fair(struct rq *rq, struct rq_flags *rf)
 	if (rq_modified_above(rq, &fair_sched_class))
 		return RETRY_TASK;
 
-	if (cfs_rq->nr_queued)
+	if (cfs_rq->nr_queued) {
+		/*
+		 * Force a core-wide pick if newidle
+		 * balance managed to pull a task since
+		 * the lock was dropped.
+		 */
+		if (sched_core_enabled(rq))
+			return RETRY_TASK;
 		goto again;
+	}
 
 	return NULL;
 }
---

Thoughts?

>  	if (new_tasks < 0)
>  		return RETRY_TASK;

-- 
Thanks and Regards,
Prateek

Re: [PATCH 1/2] sched/core: Allow newidle for core-sched

[Peter Zijlstra]

On Thu, Jun 25, 2026 at 05:26:03AM +0530, K Prateek Nayak wrote:
> Should we treat core_pick similar to task_on_cpu() and go down the
> stopper route like:
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 2f4530eb543f..e7f64f34aa4f 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3049,7 +3049,9 @@ static int affine_move_task(struct rq *rq, struct task_struct *p, struct rq_flag
>  		return -EINVAL;
>  	}
>  
> -	if (task_on_cpu(rq, p) || READ_ONCE(p->__state) == TASK_WAKING) {
> +	if (task_on_cpu(rq, p) ||
> +	    task_on_core(rq, p) ||
> +	    READ_ONCE(p->__state) == TASK_WAKING) {
>  		/*
>  		 * MIGRATE_ENABLE gets here because 'p == current', but for
>  		 * anything else we cannot do is_migration_disabled(), punt

Bah, yes. Let me go stare more at this.

Re: [PATCH 1/2] sched/core: Allow newidle for core-sched

[Peter Zijlstra]

On Thu, Jun 25, 2026 at 05:26:03AM +0530, K Prateek Nayak wrote:

> ... or we can return a RETRY_TASK when newidle balance succeeds too to
> force a core-wide pick when core scheduling is enabled like:
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 0d212bf04885..e393aed58bfa 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9946,8 +9946,16 @@ struct task_struct *pick_task_fair(struct rq *rq, struct rq_flags *rf)
>  	if (rq_modified_above(rq, &fair_sched_class))
>  		return RETRY_TASK;
>  
> -	if (cfs_rq->nr_queued)
> +	if (cfs_rq->nr_queued) {
> +		/*
> +		 * Force a core-wide pick if newidle
> +		 * balance managed to pull a task since
> +		 * the lock was dropped.
> +		 */
> +		if (sched_core_enabled(rq))
> +			return RETRY_TASK;
>  		goto again;
> +	}
>  
>  	return NULL;
>  }

This has forward progress issues. You can ping-pong he one task back and
forth and never actually pick anything.

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

[Peter Zijlstra]

Ever since commit 50653216e4ff ("sched: Add support to pick functions to take
rf"), we have the unfortunate situation that both sched_class::balance() and
sched_class::pick_task() have overlapping functionality in that they drop
rq->lock and balance tasks.

Additionally, prev_balance() is only called for a single RQ in the core-sched
case, resulting in 'missed' balance opportunities in this case.

The only classes with a balance callback are dl and rt, prev_balance() will run
the callbacks from prev->class down, pick_next_task() runs the callbacks from
stop_class down.

Therefore, the only case where there is a difference is if prev->class ==
rt_sched_class and pick_next_task() stops at stop/dl. But in those cases the rt
pull would have been pointless, it would move a high priority task to a
runqueue that will not be able to run it.

A subsequent pick that does reach rt, must have a prev of stop/dl priority
(0,-1 resp.) and this will ensure need_pull_rt_task() is true and do the pull
then.

Therefore, move balance_{rt,dl}() into pick_task_{rt,dl}() and remove
sched_class::balance().

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/core.c      |   23 -----------------------
 kernel/sched/deadline.c  |   10 ++++++----
 kernel/sched/fair.c      |   38 ++++++++++----------------------------
 kernel/sched/idle.c      |    7 -------
 kernel/sched/rt.c        |   14 ++++++--------
 kernel/sched/sched.h     |    5 -----
 kernel/sched/stop_task.c |    7 -------
 7 files changed, 22 insertions(+), 82 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6092,25 +6092,6 @@ static inline void schedule_debug(struct
 	schedstat_inc(this_rq()->sched_count);
 }
 
-static void prev_balance(struct rq *rq, struct rq_flags *rf)
-{
-	const struct sched_class *start_class = rq->donor->sched_class;
-	const struct sched_class *class;
-
-	/*
-	 * We must do the balancing pass before put_prev_task(), such
-	 * that when we release the rq->lock the task is in the same
-	 * state as before we took rq->lock.
-	 *
-	 * We can terminate the balance pass as soon as we know there is
-	 * a runnable task of @class priority or higher.
-	 */
-	for_active_class_range(class, start_class, &idle_sched_class) {
-		if (class->balance && class->balance(rq, rf))
-			break;
-	}
-}
-
 /*
  * Pick up the highest-prio task:
  */
@@ -6148,8 +6129,6 @@ __pick_next_task(struct rq *rq, struct r
 	}
 
 restart:
-	prev_balance(rq, rf);
-
 	for_each_active_class(class) {
 		p = class->pick_task(rq, rf);
 		if (unlikely(p == RETRY_TASK))
@@ -6257,8 +6236,6 @@ pick_next_task(struct rq *rq, struct rq_
 		goto out_set_next;
 	}
 
-	prev_balance(rq, rf);
-
 	smt_mask = cpu_smt_mask(cpu);
 	need_sync = !!rq->core->core_cookie;
 
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -2704,7 +2704,7 @@ static void check_preempt_equal_dl(struc
 	resched_curr(rq);
 }
 
-static int balance_dl(struct rq *rq, struct rq_flags *rf)
+static void balance_dl(struct rq *rq, struct rq_flags *rf)
 {
 	/*
 	 * Note, rq->donor may change during rq lock drops,
@@ -2723,8 +2723,6 @@ static int balance_dl(struct rq *rq, str
 		pull_dl_task(rq);
 		rq_repin_lock(rq, rf);
 	}
-
-	return sched_stop_runnable(rq) || sched_dl_runnable(rq);
 }
 
 /*
@@ -2817,6 +2815,11 @@ static struct task_struct *__pick_task_d
 	struct dl_rq *dl_rq = &rq->dl;
 	struct task_struct *p;
 
+	rq_modified_begin(rq, &dl_sched_class);
+	balance_dl(rq, rf);
+	if (rq_modified_above(rq, &dl_sched_class))
+		return RETRY_TASK;
+
 again:
 	if (!sched_dl_runnable(rq))
 		return NULL;
@@ -3652,7 +3655,6 @@ DEFINE_SCHED_CLASS(dl) = {
 	.put_prev_task		= put_prev_task_dl,
 	.set_next_task		= set_next_task_dl,
 
-	.balance		= balance_dl,
 	.select_task_rq		= select_task_rq_dl,
 	.migrate_task_rq	= migrate_task_rq_dl,
 	.set_cpus_allowed       = set_cpus_allowed_dl,
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5732,7 +5732,7 @@ static inline unsigned long cfs_rq_load_
 	return cfs_rq->avg.load_avg;
 }
 
-static int sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf)
+static void sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf)
 	__must_hold(__rq_lockp(this_rq));
 
 static inline unsigned long task_util(struct task_struct *p)
@@ -9916,7 +9916,6 @@ struct task_struct *pick_task_fair(struc
 	struct cfs_rq *cfs_rq;
 	struct task_struct *p;
 	bool throttled;
-	int new_tasks;
 
 again:
 	cfs_rq = &rq->cfs;
@@ -9942,11 +9941,14 @@ struct task_struct *pick_task_fair(struc
 	return p;
 
 idle:
-	new_tasks = sched_balance_newidle(rq, rf);
-	if (new_tasks < 0)
+	rq_modified_begin(rq, &fair_sched_class);
+	sched_balance_newidle(rq, rf);
+	if (rq_modified_above(rq, &fair_sched_class))
 		return RETRY_TASK;
-	if (new_tasks > 0)
+
+	if (cfs_rq->nr_queued)
 		goto again;
+
 	return NULL;
 }
 
@@ -14334,13 +14336,8 @@ static inline void nohz_newidle_balance(
 /*
  * sched_balance_newidle is called by schedule() if this_cpu is about to become
  * idle. Attempts to pull tasks from other CPUs.
- *
- * Returns:
- *   < 0 - we released the lock and there are !fair tasks present
- *     0 - failed, no new tasks
- *   > 0 - success, new (fair) tasks present
  */
-static int sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf)
+static void sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf)
 	__must_hold(__rq_lockp(this_rq))
 {
 	unsigned long next_balance = jiffies + HZ;
@@ -14357,7 +14354,7 @@ static int sched_balance_newidle(struct
 	 * Return 0; the task will be enqueued when switching to idle.
 	 */
 	if (this_rq->ttwu_pending)
-		return 0;
+		return;
 
 	/*
 	 * We must set idle_stamp _before_ calling sched_balance_rq()
@@ -14370,7 +14367,7 @@ static int sched_balance_newidle(struct
 	 * Do not pull tasks towards !active CPUs...
 	 */
 	if (!cpu_active(this_cpu))
-		return 0;
+		return;
 
 	/*
 	 * This is OK, because current is on_cpu, which avoids it being picked
@@ -14399,7 +14396,6 @@ static int sched_balance_newidle(struct
 	t0 = sched_clock_cpu(this_cpu);
 	__sched_balance_update_blocked_averages(this_rq);
 
-	rq_modified_begin(this_rq, &fair_sched_class);
 	raw_spin_rq_unlock(this_rq);
 
 	for_each_domain(this_cpu, sd) {
@@ -14457,18 +14453,6 @@ static int sched_balance_newidle(struct
 	if (curr_cost > this_rq->max_idle_balance_cost)
 		this_rq->max_idle_balance_cost = curr_cost;
 
-	/*
-	 * While browsing the domains, we released the rq lock, a task could
-	 * have been enqueued in the meantime. Since we're not going idle,
-	 * pretend we pulled a task.
-	 */
-	if (this_rq->cfs.h_nr_queued && !pulled_task)
-		pulled_task = 1;
-
-	/* If a higher prio class was modified, restart the pick */
-	if (rq_modified_above(this_rq, &fair_sched_class))
-		pulled_task = -1;
-
 out:
 	/* Move the next balance forward */
 	if (time_after(this_rq->next_balance, next_balance))
@@ -14480,8 +14464,6 @@ static int sched_balance_newidle(struct
 		nohz_newidle_balance(this_rq);
 
 	rq_repin_lock(this_rq, rf);
-
-	return pulled_task;
 }
 
 /*
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -464,12 +464,6 @@ select_task_rq_idle(struct task_struct *
 	return task_cpu(p); /* IDLE tasks as never migrated */
 }
 
-static int
-balance_idle(struct rq *rq, struct rq_flags *rf)
-{
-	return WARN_ON_ONCE(1);
-}
-
 /*
  * Idle tasks are unconditionally rescheduled:
  */
@@ -581,7 +575,6 @@ DEFINE_SCHED_CLASS(idle) = {
 	.put_prev_task		= put_prev_task_idle,
 	.set_next_task          = set_next_task_idle,
 
-	.balance		= balance_idle,
 	.select_task_rq		= select_task_rq_idle,
 	.set_cpus_allowed	= set_cpus_allowed_common,
 
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1596,7 +1596,7 @@ static void check_preempt_equal_prio(str
 	resched_curr(rq);
 }
 
-static int balance_rt(struct rq *rq, struct rq_flags *rf)
+static void balance_rt(struct rq *rq, struct rq_flags *rf)
 {
 	/*
 	 * Note, rq->donor may change during rq lock drops,
@@ -1615,8 +1615,6 @@ static int balance_rt(struct rq *rq, str
 		pull_rt_task(rq);
 		rq_repin_lock(rq, rf);
 	}
-
-	return sched_stop_runnable(rq) || sched_dl_runnable(rq) || sched_rt_runnable(rq);
 }
 
 /*
@@ -1714,14 +1712,15 @@ static struct task_struct *_pick_next_ta
 
 static struct task_struct *pick_task_rt(struct rq *rq, struct rq_flags *rf)
 {
-	struct task_struct *p;
+	rq_modified_begin(rq, &rt_sched_class);
+	balance_rt(rq, rf);
+	if (rq_modified_above(rq, &rt_sched_class))
+		return RETRY_TASK;
 
 	if (!sched_rt_runnable(rq))
 		return NULL;
 
-	p = _pick_next_task_rt(rq);
-
-	return p;
+	return _pick_next_task_rt(rq);
 }
 
 static void put_prev_task_rt(struct rq *rq, struct task_struct *p, struct task_struct *next)
@@ -2609,7 +2608,6 @@ DEFINE_SCHED_CLASS(rt) = {
 	.put_prev_task		= put_prev_task_rt,
 	.set_next_task          = set_next_task_rt,
 
-	.balance		= balance_rt,
 	.select_task_rq		= select_task_rq_rt,
 	.set_cpus_allowed       = set_cpus_allowed_common,
 	.rq_online              = rq_online_rt,
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2638,11 +2638,6 @@ struct sched_class {
 	void (*wakeup_preempt)(struct rq *rq, struct task_struct *p, int flags);
 
 	/*
-	 * schedule/pick_next_task/prev_balance: rq->lock
-	 */
-	int (*balance)(struct rq *rq, struct rq_flags *rf);
-
-	/*
 	 * schedule/pick_next_task: rq->lock
 	 */
 	struct task_struct *(*pick_task)(struct rq *rq, struct rq_flags *rf);
--- a/kernel/sched/stop_task.c
+++ b/kernel/sched/stop_task.c
@@ -15,12 +15,6 @@ select_task_rq_stop(struct task_struct *
 	return task_cpu(p); /* stop tasks as never migrate */
 }
 
-static int
-balance_stop(struct rq *rq, struct rq_flags *rf)
-{
-	return sched_stop_runnable(rq);
-}
-
 static void
 wakeup_preempt_stop(struct rq *rq, struct task_struct *p, int flags)
 {
@@ -107,7 +101,6 @@ DEFINE_SCHED_CLASS(stop) = {
 	.put_prev_task		= put_prev_task_stop,
 	.set_next_task          = set_next_task_stop,
 
-	.balance		= balance_stop,
 	.select_task_rq		= select_task_rq_stop,
 	.set_cpus_allowed	= set_cpus_allowed_common,
 

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

[Aaron Lu]

Hi Peter,

On Wed, Jun 24, 2026 at 02:13:27PM +0200, Peter Zijlstra wrote:
> Hi!
> 
> As mentioned [1], I was going to move sched_balance_newidle() into
> balance_fair(). And while the patch was quickly done, something didn't feel
> right.
>

Sorry for replying late, been busy with other stuffs.

The test caused a panic with below msg:

[  110.613929] BUG: kernel NULL pointer dereference, address: 0000000000000014
[  110.615988] #PF: supervisor read access in kernel mode
[  110.617444] #PF: error_code(0x0000) - not-present page
[  110.618929] PGD 1044c2067 P4D 0
[  110.619938] Oops: Oops: 0000 [#1] SMP NOPTI
[  110.621491] CPU: 34 UID: 1000 PID: 1394 Comm: sh Not tainted 7.1.0-rc2-00109-gb17be96b4d6c #4 PREEMPT(lazy)
[  110.624764] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[  110.627559] RIP: 0010:pick_next_task+0x42b/0xa20
[  110.628970] Code: 28 60 00 49 89 c7 8b 05 b3 8d 73 05 41 39 c7 0f 83 c3 01 00 00 49 63 cf 48 8b 1c cd 80 7b 96 82 48 01 eb 48 8b b3 50 0f 00 00 <8b> 4e 14 48 8b 0c cd 80 7b 96 82 48 8b 4c 29 20 48 39 ce 74 32 48
[  110.634160] RSP: 0000:ffa000000435bc48 EFLAGS: 00010083
[  110.635261] RAX: 0000000000000040 RBX: ff11000ff85fe140 RCX: 0000000000000022
[  110.636753] RDX: 0000000000000022 RSI: 0000000000000000 RDI: ff110001008b54d8
[  110.638230] RBP: ffffffff87452140 R08: 0000000000000022 R09: 00000000aed806e6
[  110.639719] R10: 0000000000000001 R11: 0000000000000000 R12: ff11000ff85fe140
[  110.641185] R13: 0000000000000000 R14: ff11000102373e88 R15: 0000000000000022
[  110.642688] FS:  00007f698e381780(0000) GS:ff110010711ac000(0000) knlGS:0000000000000000
[  110.644359] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  110.645255] CR2: 0000000000000014 CR3: 00000001044c1001 CR4: 0000000000371ef0
[  110.646366] Call Trace:
[  110.646781]  <TASK>
[  110.647123]  ? sched_clock+0x10/0x30
[  110.647704]  __schedule+0x1a4/0x7c0
[  110.648249]  ? local_clock_noinstr+0xd/0xc0
[  110.648914]  preempt_schedule+0x37/0x50
[  110.649528]  preempt_schedule_thunk+0x16/0x30
[  110.650217]  _raw_spin_unlock+0x37/0x40
[  110.650833]  wp_page_copy+0x2b7/0x5f0
[  110.651408]  __handle_mm_fault+0x45f/0x6e0
[  110.652068]  handle_mm_fault+0x96/0x250
[  110.652685]  do_user_addr_fault+0x236/0x6a0
[  110.653333]  exc_page_fault+0x7a/0x210
[  110.653937]  asm_exc_page_fault+0x26/0x30
[  110.654577] RIP: 0033:0x7f698e3a1f84
[  110.655077] Code: 8d 3d e0 d4 1c 00 e9 0b 15 05 00 0f 1f 00 48 89 7d f8 48 8d 3d cd d4 1c 00 e8 38 14 05 00 48 8b 55 f8 eb 8c 66 90 f3 0f 1e fa <c7> 05 b2 d4 1c 00 00 00 00 00 c3 90 f3 0f 1e fa 31 c9 e9 65 f9 ff
[  110.657442] RSP: 002b:00007fff9a5ff9e8 EFLAGS: 00010246
[  110.658113] RAX: 0000000000000000 RBX: 0000000000000001 RCX: 00007f698e443f63
[  110.659037] RDX: 0000000000000000 RSI: 0000000000000018 RDI: 00007f698e381a60
[  110.659973] RBP: 00007fff9a5ffa10 R08: 0000000000000000 R09: 000000000000000f
[  110.660896] R10: 00007f698e381a50 R11: 0000000000000246 R12: 0000000000000000
[  110.661829] R13: 00000000ffffffff R14: 0000000000000000 R15: 00007fff9a5ffc20
[  110.662745]  </TASK>
[  110.663040] Modules linked in:
[  110.663451] CR2: 0000000000000014
[  110.663893] ---[ end trace 0000000000000000 ]---
[  110.664487] RIP: 0010:pick_next_task+0x42b/0xa20
[  110.665090] Code: 28 60 00 49 89 c7 8b 05 b3 8d 73 05 41 39 c7 0f 83 c3 01 00 00 49 63 cf 48 8b 1c cd 80 7b 96 82 48 01 eb 48 8b b3 50 0f 00 00 <8b> 4e 14 48 8b 0c cd 80 7b 96 82 48 8b 4c 29 20 48 39 ce 74 32 48
[  110.667457] RSP: 0000:ffa000000435bc48 EFLAGS: 00010083
[  110.668146] RAX: 0000000000000040 RBX: ff11000ff85fe140 RCX: 0000000000000022
[  110.669074] RDX: 0000000000000022 RSI: 0000000000000000 RDI: ff110001008b54d8
[  110.670009] RBP: ffffffff87452140 R08: 0000000000000022 R09: 00000000aed806e6
[  110.670942] R10: 0000000000000001 R11: 0000000000000000 R12: ff11000ff85fe140
[  110.671861] R13: 0000000000000000 R14: ff11000102373e88 R15: 0000000000000022
[  110.672787] FS:  00007f698e381780(0000) GS:ff110010711ac000(0000) knlGS:0000000000000000
[  110.673826] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  110.674567] CR2: 0000000000000014 CR3: 00000001044c1001 CR4: 0000000000371ef0
[  110.675487] Kernel panic - not syncing: Fatal exception
[  111.737349] Shutting down cpus with NMI
[  111.738019] Kernel Offset: disabled
[  111.738373] Rebooting in 30 seconds..

$ ./scripts/faddr2line ../obj/guest/vmlinux pick_next_task+0x42b
pick_next_task+0x42b/0xa20:
task_cpu at include/linux/sched.h:2270
(inlined by) is_task_rq_idle at kernel/sched/core.c:6144
(inlined by) cookie_equals at kernel/sched/core.c:6149
(inlined by) pick_next_task at kernel/sched/core.c:6328
This is the cookie_equals(p, cookie) in pick_next_task().

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

BTW, I applied this series on top of commit c095741713d1b("sched/fair:
Fix newidle vs core-sched") of the queue/sched/core branch.

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

[K Prateek Nayak]

Hi Aaron,

On 7/2/2026 5:19 PM, 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

I also feel doing a balance before the core_cookie is finalized can
move tasks wrongly to a particular core, only to make them wait later
because the pick converted on a different core cookie. Stealing via
balance callback still seems like a good option.

Then there is the whole issue of core_pick_seq possibly being updated
by another CPU on the core by the time rq_lock is dropped and grabbed
again. Afaict, anything that drops the core-wide lock should just do
a RETRY_TASK if a new task arrives so everything is re-done withing
a single core-wide lock critical-section.

Up until this cleanup, only fair and ext used the RETRY_TASK
mechanism and fair bypassed the newidle with core-sched enabled so
it never used the RETRY_TASK mechanism with core-sched so I'm still
skeptical on balance for core-sched being done as part of pick.

Maybe Peter will have a way to sort it out after he is back from
vacation if he hasn't got it all figured out already ;-)

-- 
Thanks and Regards,
Prateek

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

[Peter Zijlstra]

On Fri, Jul 03, 2026 at 09:01:37AM +0530, K Prateek Nayak wrote:
> Hi Aaron,
> 
> On 7/2/2026 5:19 PM, 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
> 
> I also feel doing a balance before the core_cookie is finalized can
> move tasks wrongly to a particular core, only to make them wait later
> because the pick converted on a different core cookie. Stealing via
> balance callback still seems like a good option.
> 
> Then there is the whole issue of core_pick_seq possibly being updated
> by another CPU on the core by the time rq_lock is dropped and grabbed
> again. Afaict, anything that drops the core-wide lock should just do
> a RETRY_TASK if a new task arrives so everything is re-done withing
> a single core-wide lock critical-section.
> 
> Up until this cleanup, only fair and ext used the RETRY_TASK
> mechanism and fair bypassed the newidle with core-sched enabled so
> it never used the RETRY_TASK mechanism with core-sched so I'm still
> skeptical on balance for core-sched being done as part of pick.
> 
> Maybe Peter will have a way to sort it out after he is back from
> vacation if he hasn't got it all figured out already ;-)

Only bad ideas so far. The whole sched_ext thing [1] makes it more
complicated than I'd like.

[1] https://lore.kernel.org/r/20260819093751.GI1247881@noisy.programming.kicks-ass.net

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

[Peter Zijlstra]

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.

Bah, let me go poke at this for a bit.

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

[Peter Zijlstra]

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;

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

[Tejun Heo]

Hello,

On Wed, Aug 19, 2026 at 04:36:49PM +0200, Peter Zijlstra wrote:
...
> > 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.

For SCX, rq lock is dropped only when a task needs to be migrated to be put
in the local DSQ and, barring something else happening to it like dequeue or
competing dispatch, the next time pick_task comes around, the task is going
to be on the local DSQ, and won't need to drop the lock for that rq and thus
picking would be able to proceed to the next rq.

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

Oh yeah, if core_seq tracks competing multi-picks, SCX no longer needs to
track lock drops which was kinda ugly.

> @@ -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);

I guess this is to signify that idle pick shouldn't drop rq lock as it's
after seq verification?

Thanks.

-- 
tejun

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

[Peter Zijlstra]

On Wed, Aug 19, 2026 at 09:22:04AM -1000, Tejun Heo wrote:
> Hello,
> 
> On Wed, Aug 19, 2026 at 04:36:49PM +0200, Peter Zijlstra wrote:
> ...
> > > 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.
> 
> For SCX, rq lock is dropped only when a task needs to be migrated to be put
> in the local DSQ and, barring something else happening to it like dequeue or
> competing dispatch, the next time pick_task comes around, the task is going
> to be on the local DSQ, and won't need to drop the lock for that rq and thus
> picking would be able to proceed to the next rq.

Right, but I worry about the cases:

 - there is no ext task pulled to local because $reasons (could be
   cpumask), and we retry, then it will see there are ext tasks, but no
   local and it will try again?

 - custom DSQs, those BPF based things, then we always need to drop the
   lock in order to execute those BPF methods, no?

That is not unlike the case where fair has no local tasks and it will
try and pull some tasks. It will try this every time, and if there are
very few fair tasks in the system, this happens again and again.

As mentioned, one 'hack' I considered was keeping a retry count, and
simply setting 'rf = NULL' after a few cycles, to inhibit any further
balancing and forcing progress.

It just needs making sure all the sched_class::pick_task methods can
deal with !rf, but that shoulnd't be too hard.

> > But the simple thing is something like so, which I think also allows
> > simplifying ext some.
> 
> Oh yeah, if core_seq tracks competing multi-picks, SCX no longer needs to
> track lock drops which was kinda ugly.
> 
> > @@ -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);
> 
> I guess this is to signify that idle pick shouldn't drop rq lock as it's
> after seq verification?

Indeed. Obviously idle doesn't do balancing, so its trivially correct,
but it is indeed to make clear this is after seq validation.

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

[Tejun Heo]

Hello, Peter.

On Thu, Aug 20, 2026 at 09:18:52AM +0200, Peter Zijlstra wrote:
...
> Right, but I worry about the cases:
> 
>  - there is no ext task pulled to local because $reasons (could be
>    cpumask), and we retry, then it will see there are ext tasks, but no
>    local and it will try again?

For SCX, rq lock is not dropped for "trying". It is only dropped when there
is a task to be migrated, so if nothing has changed since the last time,
there won't be a task to migrate and thus no lock drop.

>  - custom DSQs, those BPF based things, then we always need to drop the
>    lock in order to execute those BPF methods, no?

No, those all nest inside rq locks. Think about enqueue path. Enqueue needs
to run all those and we don't drop rq lock while enqueueing (unless we're
bouncing to another rq's local DSQ at least).

Thanks.

-- 
tejun

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

[Peter Zijlstra]

On Wed, Aug 19, 2026 at 09:42:14PM -1000, Tejun Heo wrote:
> Hello, Peter.
> 
> On Thu, Aug 20, 2026 at 09:18:52AM +0200, Peter Zijlstra wrote:
> ...
> > Right, but I worry about the cases:
> > 
> >  - there is no ext task pulled to local because $reasons (could be
> >    cpumask), and we retry, then it will see there are ext tasks, but no
> >    local and it will try again?
> 
> For SCX, rq lock is not dropped for "trying". It is only dropped when there
> is a task to be migrated, so if nothing has changed since the last time,
> there won't be a task to migrate and thus no lock drop.

Hmm, right. I don't think I can get fair to behave that way in a hurry.
Also, even if I could, the figuring out if there is anything to move
might still be somewhat expensive to retry multiple times.

Let me continue pondering if there is anything saner than simply
clearing rf.

> >  - custom DSQs, those BPF based things, then we always need to drop the
> >    lock in order to execute those BPF methods, no?
> 
> No, those all nest inside rq locks. Think about enqueue path. Enqueue needs
> to run all those and we don't drop rq lock while enqueueing (unless we're
> bouncing to another rq's local DSQ at least).

Oh right.. bah, I keep forgetting how all that works :/