[PATCH v2 00/12] sched: core-sched fixes and balancing

[PATCH v2 00/12] sched: core-sched fixes and balancing

[Peter Zijlstra]

Hi!

Previous version at:

  https://lore.kernel.org/r/20260828101659.812011872@infradead.org

The main difference is reworking of rq_clock due to TJ noticing the previous
one got it all wrong. The new patches might seem a little aimless at times;
this is because my initial idea for fixing the problem didn't pan out, but I
kept some of the groundwork for it anyway, since it resulted in simpler (I
hope) code.

Code at:

  git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git sched/hackery

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

[Peter Zijlstra]

It is possible for another sibling to end up in pick_next_task() when:

  pick_next_task()
    pick_task()
      sched_class::pick_task()

drops the core wide rq->lock. In this case they end up trampling the core wide
task selection state, possibly leading to NULL derefs. Detect this case by
keeping a local copy of core_task_seq, a value that is incremented on
{en,de}queue and schedule.

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.

If there are forward progress issues; those might possibly be solved by using
core_pick_in_flight.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/core.c |   22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6296,14 +6296,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);
+	bool need_sync = false;
 	unsigned long cookie;
-	int i, cpu, occ = 0;
 	struct rq *rq_i;
-	bool need_sync = false;
 
 	if (!sched_core_enabled(rq))
 		return __pick_next_task(rq, rf);
@@ -6377,7 +6377,7 @@ pick_next_task(struct rq *rq, struct rq_
 	 * 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
@@ -6394,6 +6394,15 @@ 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))
+				goto restart;
+
 			rq->core_pick = NULL;
 			rq->core_dl_server = NULL;
 			/*
@@ -6425,7 +6434,8 @@ pick_next_task(struct rq *rq, struct rq_
 			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))
@@ -6455,7 +6465,7 @@ pick_next_task(struct rq *rq, struct rq_
 			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;

[PATCH v2 02/12] sched/core: Introduce rq_drop_lock()

[Peter Zijlstra]

Currently rq_unpin_lock() serves a dual purpose, it pairs with both
rq_pin_lock() and rq_repin_lock(). That is:

	rq_pin_lock()
	  rq_unpin_lock()
          rq_repin_lock()
	rq_unpin_lock()

In prepration of making the temporary drop and the final drop
distinct, this needs another function. Introduce and use
rq_drop_lock() for the temporary drop, like:

	rq_pin_lock()
	  rq_drop_lock()
          rq_repin_lock()
	rq_unpin_lock()

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

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3910,7 +3910,7 @@ ttwu_do_activate(struct rq *rq, struct t
 		 * Our task @p is fully woken up and running; so it's safe to
 		 * drop the rq->lock, hereafter rq is only used for statistics.
 		 */
-		rq_unpin_lock(rq, rf);
+		rq_drop_lock(rq, rf);
 		p->sched_class->task_woken(rq, p);
 		rq_repin_lock(rq, rf);
 	}
@@ -5042,7 +5042,7 @@ void wake_up_new_task(struct task_struct
 		 * Nothing relies on rq->lock after this, so it's fine to
 		 * drop it.
 		 */
-		rq_unpin_lock(rq, &rf);
+		rq_drop_lock(rq, &rf);
 		p->sched_class->task_woken(rq, p);
 		rq_repin_lock(rq, &rf);
 	}
@@ -5260,7 +5260,7 @@ struct balance_callback *splice_balance_
 void __balance_callbacks(struct rq *rq, struct rq_flags *rf)
 {
 	if (rf)
-		rq_unpin_lock(rq, rf);
+		rq_drop_lock(rq, rf);
 	do_balance_callbacks(rq, __splice_balance_callbacks(rq, false));
 	if (rf)
 		rq_repin_lock(rq, rf);
@@ -6929,7 +6929,7 @@ static inline void proxy_release_rq_lock
 	 * to pick_again, so the callbacks will get re-established.
 	 */
 	zap_balance_callbacks(rq);
-	rq_unpin_lock(rq, rf);
+	rq_drop_lock(rq, rf);
 	raw_spin_rq_unlock(rq);
 }
 
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1129,7 +1129,7 @@ static void __push_dl_task(struct rq *rq
 		 * Nothing relies on rq->lock after this, so its safe to drop
 		 * rq->lock.
 		 */
-		rq_unpin_lock(rq, rf);
+		rq_drop_lock(rq, rf);
 		push_dl_task(rq);
 		rq_repin_lock(rq, rf);
 	}
@@ -2720,7 +2720,7 @@ static int balance_dl(struct rq *rq, str
 		 * disabled avoiding further scheduler activity on it and we've
 		 * not yet started the picking loop.
 		 */
-		rq_unpin_lock(rq, rf);
+		rq_drop_lock(rq, rf);
 		pull_dl_task(rq);
 		rq_repin_lock(rq, rf);
 	}
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -3924,7 +3924,7 @@ static enum scx_dsp_verdict dispatch_pic
 {
 	enum scx_dsp_verdict verdict;
 
-	rq_unpin_lock(rq, rf);
+	rq_drop_lock(rq, rf);
 	verdict = dispatch_one(rq, prev);
 	rq_repin_lock(rq, rf);
 
@@ -3949,7 +3949,7 @@ static enum scx_dsp_verdict dispatch_cor
 	if (rq->scx.flags & SCX_RQ_IN_DISPATCH)
 		return SCX_DSP_NONE;
 
-	rq_unpin_lock(rq, rf);
+	rq_drop_lock(rq, rf);
 
 	verdict = dispatch_one(rq, prev);
 
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -14907,7 +14907,7 @@ static int sched_balance_newidle(struct
 	 * further scheduler activity on it and we're being very careful to
 	 * re-start the picking loop.
 	 */
-	rq_unpin_lock(this_rq, rf);
+	rq_drop_lock(this_rq, rf);
 
 	sd = rcu_dereference_sched_domain(this_rq->sd);
 	if (!sd)
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1611,7 +1611,7 @@ static int balance_rt(struct rq *rq, str
 		 * disabled avoiding further scheduler activity on it and we've
 		 * not yet started the picking loop.
 		 */
-		rq_unpin_lock(rq, rf);
+		rq_drop_lock(rq, rf);
 		pull_rt_task(rq);
 		rq_repin_lock(rq, rf);
 	}
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1991,6 +1991,11 @@ static inline void rq_unpin_lock(struct
 	lockdep_unpin_lock(__rq_lockp(rq), rf->cookie);
 }
 
+static inline void rq_drop_lock(struct rq *rq, struct rq_flags *rf)
+{
+	rq_unpin_lock(rq, rf);
+}
+
 static inline void rq_repin_lock(struct rq *rq, struct rq_flags *rf)
 {
 	lockdep_repin_lock(__rq_lockp(rq), rf->cookie);

[PATCH v2 03/12] sched/core: Decompose update_rq_clock()

[Peter Zijlstra]

Adds __update_rq_clock() which lacks the double update warning. Additionally it
will now always set UPDATED, even though it might skip the actual update.
Because when we skip the update, we must be up-to-date by definition.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/core.c |   35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -867,29 +867,32 @@ static void update_rq_clock_task(struct
 	update_rq_clock_pelt(rq, delta);
 }
 
-void update_rq_clock(struct rq *rq)
+static void __update_rq_clock(struct rq *rq)
 {
-	s64 delta;
-	u64 clock;
-
 	lockdep_assert_rq_held(rq);
 
-	if (rq->clock_update_flags & RQCF_ACT_SKIP)
-		return;
-
-	if (sched_feat(WARN_DOUBLE_CLOCK))
-		WARN_ON_ONCE(rq->clock_update_flags & RQCF_UPDATED);
 	rq->clock_update_flags |= RQCF_UPDATED;
 
-	clock = sched_clock_cpu(cpu_of(rq));
-	scx_rq_clock_update(rq, clock);
+	if (!(rq->clock_update_flags & RQCF_ACT_SKIP)) {
+		u64 clock = sched_clock_cpu(cpu_of(rq));
+		s64 delta;
+
+		scx_rq_clock_update(rq, clock);
+
+		delta = clock - rq->clock;
+		if (delta > 0) {
+			rq->clock += delta;
+			update_rq_clock_task(rq, delta);
+		}
+	}
+}
 
-	delta = clock - rq->clock;
-	if (delta < 0)
-		return;
-	rq->clock += delta;
+void update_rq_clock(struct rq *rq)
+{
+	if (sched_feat(WARN_DOUBLE_CLOCK))
+		WARN_ON_ONCE(rq->clock_update_flags & RQCF_UPDATED);
 
-	update_rq_clock_task(rq, delta);
+	__update_rq_clock(rq);
 }
 
 #ifdef CONFIG_SCHED_HRTICK

[PATCH v2 04/12] sched/core: Make __update_rq_clock() skip updates on UPDATED

[Peter Zijlstra]

Calling update_rq_clock() on a rq which is already up-to-date would still move
the clock (after warning when WARN_DOUBLE_CLOCK is set). This is a bit silly,
so also skip the update when already up-to-date.

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

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -869,11 +869,14 @@ static void update_rq_clock_task(struct
 
 static void __update_rq_clock(struct rq *rq)
 {
+	bool update;
+
 	lockdep_assert_rq_held(rq);
 
+	update = !(rq->clock_update_flags & RQCF_NOP_MASK);
 	rq->clock_update_flags |= RQCF_UPDATED;
 
-	if (!(rq->clock_update_flags & RQCF_ACT_SKIP)) {
+	if (update) {
 		u64 clock = sched_clock_cpu(cpu_of(rq));
 		s64 delta;
 
@@ -890,7 +893,7 @@ static void __update_rq_clock(struct rq
 void update_rq_clock(struct rq *rq)
 {
 	if (sched_feat(WARN_DOUBLE_CLOCK))
-		WARN_ON_ONCE(rq->clock_update_flags & RQCF_UPDATED);
+		WARN_ON_ONCE((rq->clock_update_flags & RQCF_NOP_MASK) == RQCF_UPDATED);
 
 	__update_rq_clock(rq);
 }
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1807,6 +1807,8 @@ extern void update_rq_clock(struct rq *r
 #define RQCF_ACT_SKIP		0x02
 #define RQCF_UPDATED		0x04
 
+#define RQCF_NOP_MASK		(RQCF_ACT_SKIP | RQCF_UPDATED)
+
 static inline void assert_clock_updated(struct rq *rq)
 {
 	/*

[PATCH v2 05/12] sched/core: Abstract/fix rq_clock_start_loop_update()

[Peter Zijlstra]

The existing rq_clock_start_loop_update() / rq_lock_stop_loop_update() almost
do what we need rq_drop_lock() / rq_repin_lock() to do. They freeze time by
setting ACT_SKIP.

Rename them rq_clock_freeze() / rq_clock_thaw() and have them operate on a new
bit RQCF_FREEZE. This is such that schedule() can safely nest inside
rq_clock_freeze() as is required for rq_drop_lock().

Sanitize the rq->clock_update_flags handling while adding the new bit --
notably do away with the full shift left which makes a mess of things.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/core.c  |    4 +--
 kernel/sched/fair.c  |   15 +++++++-----
 kernel/sched/sched.h |   63 +++++++++++++++++++++++++++++----------------------
 3 files changed, 48 insertions(+), 34 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -7185,9 +7185,9 @@ static void __sched notrace __schedule(i
 	hrtick_schedule_enter(rq);
 
 	/* Promote REQ to ACT */
-	rq->clock_update_flags <<= 1;
+	rq->clock_update_flags |= (rq->clock_update_flags & RQCF_REQ_SKIP) << 1;
 	update_rq_clock(rq);
-	rq->clock_update_flags = RQCF_UPDATED;
+	rq->clock_update_flags &= (RQCF_FREEZE|RQCF_UPDATED);
 
 	switch_count = &prev->nivcsw;
 
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5445,6 +5445,7 @@ static inline void clear_tg_load_avg(str
 static void __maybe_unused clear_tg_offline_cfs_rqs(struct rq *rq)
 {
 	struct task_group *tg;
+	unsigned int cuf;
 
 	lockdep_assert_rq_held(rq);
 
@@ -5453,7 +5454,7 @@ static void __maybe_unused clear_tg_offl
 	 * set_rq_offline(), so we should skip updating
 	 * the rq clock again in unthrottle_cfs_rq().
 	 */
-	rq_clock_start_loop_update(rq);
+	cuf = rq_clock_freeze(rq);
 
 	guard(rcu)();
 
@@ -5463,7 +5464,7 @@ static void __maybe_unused clear_tg_offl
 		clear_tg_load_avg(cfs_rq);
 	}
 
-	rq_clock_stop_loop_update(rq);
+	rq_clock_thaw(rq, cuf);
 }
 
 /*
@@ -7331,6 +7332,7 @@ static void __cfsb_csd_unthrottle(void *
 {
 	struct cfs_rq *cursor, *tmp;
 	struct rq *rq = arg;
+	unsigned int cuf;
 
 	guard(rq_lock)(rq);
 
@@ -7340,7 +7342,7 @@ static void __cfsb_csd_unthrottle(void *
 	 * Do it once and skip the potential next ones.
 	 */
 	update_rq_clock(rq);
-	rq_clock_start_loop_update(rq);
+	cuf = rq_clock_freeze(rq);
 
 	/*
 	 * Since we hold rq lock we're safe from concurrent manipulation of
@@ -7359,7 +7361,7 @@ static void __cfsb_csd_unthrottle(void *
 			unthrottle_cfs_rq(cursor);
 	}
 
-	rq_clock_stop_loop_update(rq);
+	rq_clock_thaw(rq, cuf);
 }
 
 static inline void __unthrottle_cfs_rq_async(struct cfs_rq *cfs_rq)
@@ -7865,6 +7867,7 @@ static void __maybe_unused update_runtim
 static void __maybe_unused unthrottle_offline_cfs_rqs(struct rq *rq)
 {
 	struct task_group *tg;
+	unsigned int cuf;
 
 	lockdep_assert_rq_held(rq);
 
@@ -7877,7 +7880,7 @@ static void __maybe_unused unthrottle_of
 	 * set_rq_offline(), so we should skip updating
 	 * the rq clock again in unthrottle_cfs_rq().
 	 */
-	rq_clock_start_loop_update(rq);
+	cuf = rq_clock_freeze(rq);
 
 	guard(rcu)();
 
@@ -7904,7 +7907,7 @@ static void __maybe_unused unthrottle_of
 		unthrottle_cfs_rq(cfs_rq);
 	}
 
-	rq_clock_stop_loop_update(rq);
+	rq_clock_thaw(rq, cuf);
 }
 
 bool cfs_task_bw_constrained(struct task_struct *p)
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1790,24 +1790,33 @@ extern void update_rq_clock(struct rq *r
  * %RQCF_ACT_SKIP - is set from inside of __schedule() when skipping is
  *  in effect and calls to update_rq_clock() are being ignored.
  *
+ * %RQCF_FREEZE - set from rq_clock_freeze(), much like ACT_SKIP it
+ *  makes update_rq_clock() a noop.
+ *
  * %RQCF_UPDATED - is a debug flag that indicates whether a call has been
  *  made to update_rq_clock() since the last time rq::lock was pinned.
  *
- * If inside of __schedule(), clock_update_flags will have been
- * shifted left (a left shift is a cheap operation for the fast path
- * to promote %RQCF_REQ_SKIP to %RQCF_ACT_SKIP), so you must use,
- *
- *	if (rq-clock_update_flags >= RQCF_UPDATED)
- *
- * to check if %RQCF_UPDATED is set. It'll never be shifted more than
- * one position though, because the next rq_unpin_lock() will shift it
- * back.
+ *
+ * yield():
+ *   will generally result in REQ_SKIP.
+ *
+ * update_rq_clock():
+ *   sets UPDATED, will not modify rq->clock when NOP_MASK
+ *
+ * schedule():
+ *   will promote REQ_SKIP to ACT_SKIP before its update_rq_clock() to ensure
+ *   the yield() and schedule() appear to happen instantaneous.
+ *
+ * rq_clock_freeze()/rq_clock_thaw():
+ *   set FREEZE to make 'nested' update_rq_clock() calls NOP and avoid
+ *   time being moved forward. Also used in rq_drop_lock()/rq_repin_lock().
  */
 #define RQCF_REQ_SKIP		0x01
 #define RQCF_ACT_SKIP		0x02
-#define RQCF_UPDATED		0x04
+#define RQCF_FREEZE		0x04
+#define RQCF_UPDATED		0x08
 
-#define RQCF_NOP_MASK		(RQCF_ACT_SKIP | RQCF_UPDATED)
+#define RQCF_NOP_MASK		(RQCF_ACT_SKIP | RQCF_FREEZE | RQCF_UPDATED)
 
 static inline void assert_clock_updated(struct rq *rq)
 {
@@ -1815,7 +1824,7 @@ static inline void assert_clock_updated(
 	 * The only reason for not seeing a clock update since the
 	 * last rq_pin_lock() is if we're currently skipping updates.
 	 */
-	WARN_ON_ONCE(rq->clock_update_flags < RQCF_ACT_SKIP);
+	WARN_ON_ONCE(!(rq->clock_update_flags & RQCF_UPDATED));
 }
 
 static inline u64 rq_clock(struct rq *rq)
@@ -1851,25 +1860,27 @@ static inline void rq_clock_cancel_skipu
 }
 
 /*
- * During cpu offlining and rq wide unthrottling, we can trigger
- * an update_rq_clock() for several cfs and rt runqueues (Typically
- * when using list_for_each_entry_*)
- * rq_clock_start_loop_update() can be called after updating the clock
- * once and before iterating over the list to prevent multiple update.
- * After the iterative traversal, we need to call rq_clock_stop_loop_update()
- * to clear RQCF_ACT_SKIP of rq->clock_update_flags.
+ * FREEZE rq_clock. This is used in a number of places to
+ * ensure things appears to happen at the same time.
+ *
+ * Eg. during cpu offlining and rq wide unthrottling, we can trigger an
+ * update_rq_clock() for several cfs and rt runqueues (Typically when using
+ * list_for_each_entry_*)
  */
-static inline void rq_clock_start_loop_update(struct rq *rq)
+static inline unsigned int rq_clock_freeze(struct rq *rq)
 {
+	unsigned int flags = rq->clock_update_flags;
 	lockdep_assert_rq_held(rq);
-	WARN_ON_ONCE(rq->clock_update_flags & RQCF_ACT_SKIP);
-	rq->clock_update_flags |= RQCF_ACT_SKIP;
+	assert_clock_updated(rq);
+	rq->clock_update_flags |= RQCF_FREEZE;
+	return flags;
 }
 
-static inline void rq_clock_stop_loop_update(struct rq *rq)
+static inline void rq_clock_thaw(struct rq *rq, bool flags)
 {
 	lockdep_assert_rq_held(rq);
-	rq->clock_update_flags &= ~RQCF_ACT_SKIP;
+	rq->clock_update_flags &= ~RQCF_FREEZE;
+	rq->clock_update_flags |= RQCF_UPDATED | (flags & RQCF_FREEZE);
 }
 
 struct rq_flags {
@@ -1938,14 +1949,14 @@ static inline void rq_pin_lock(struct rq
 {
 	rf->cookie = lockdep_pin_lock(__rq_lockp(rq));
 
-	rq->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
+	rq->clock_update_flags &= ~RQCF_UPDATED;
 	rf->clock_update_flags = 0;
 	assert_balance_callbacks_empty(rq);
 }
 
 static inline void rq_unpin_lock(struct rq *rq, struct rq_flags *rf)
 {
-	if (rq->clock_update_flags > RQCF_ACT_SKIP)
+	if (rq->clock_update_flags & RQCF_UPDATED)
 		rf->clock_update_flags = RQCF_UPDATED;
 
 	scx_rq_clock_invalidate(rq);

[PATCH v2 06/12] sched/core: New rq_{pin,drop,repin,unpin}_lock() semantics

[Peter Zijlstra]

The current semantics are such that UPDATED is cleared on rq_pin_lock() but
remains set on rq_unpin_lock(). This works because rq_clock() itself asserts
rq->lock is held.

However, there are two problems with this, both related to the newidle/balance
lock drop.

The code as-is, tries to preserve UPDATED over drop()/repin(), but during the
drop an interleaving lock()+update_rq_clock() will still move the clock --
since it will clear UPDATED on lock(). Make this stronger using the freshly
minted freeze and thaw. This way the clock remains where it was, restoring the
'atomicity' of schedule().

Additionally, there is a variant with core-scheduling. In that case rq->lock is
shared among the sibling threads, but rq->clock and rq->clock_update_flags are
very much per thread. This disparity can cause another form of the above
problem, where one sibling is trying to forward the clock of the other sibling.

Sort this by making lock() clear all sibling UPDATED, but only for non-nested
(using core_pick_in_flight as nesting counter).

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/sched.h |   61 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 10 deletions(-)

--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1936,6 +1936,51 @@ static inline void assert_balance_callba
 }
 
 /*
+ * We want clocks to be updated _once_ during core scheduling.
+ *
+ * rq_lock(smt0)
+ *   __rq_clock_clear(); // outer
+ *
+ * core->core_pick_in_flight++;
+ * update_rq_clock(smt0);
+ * drop()
+ *   freeze();
+ *
+ *			rq_lock(smt1)
+ *			  __rq_clock_clear(); // inner
+ *
+ *			update_rq_clock(smt1)
+ *
+ *			rq_unlock(smt1)
+ *
+ * relock()
+ *   thaw();
+ * update_rq_clock(smt1) // no second update
+ * core_pick_in_flight--;
+ *
+ * rq_unlock()
+ */
+static inline void __rq_clock_clear(struct rq *rq)
+{
+	if (!sched_core_enabled(rq)) {
+		rq->clock_update_flags &= ~RQCF_UPDATED;
+		return;
+	}
+
+#ifdef CONFIG_SCHED_CORE
+	/*
+	 * Inner rq_{,un}lock() should preserve state.
+	 */
+	if (!rq->core->core_pick_in_flight) {
+		int cpu;
+
+		for_each_cpu(cpu, cpu_smt_mask(cpu_of(rq)))
+			cpu_rq(cpu)->clock_update_flags &= ~RQCF_UPDATED;
+	}
+#endif
+}
+
+/*
  * Lockdep annotation that avoids accidental unlocks; it's like a
  * sticky/continuous lockdep_assert_held().
  *
@@ -1949,33 +1994,29 @@ static inline void rq_pin_lock(struct rq
 {
 	rf->cookie = lockdep_pin_lock(__rq_lockp(rq));
 
-	rq->clock_update_flags &= ~RQCF_UPDATED;
+	__rq_clock_clear(rq);
 	rf->clock_update_flags = 0;
 	assert_balance_callbacks_empty(rq);
 }
 
 static inline void rq_unpin_lock(struct rq *rq, struct rq_flags *rf)
 {
-	if (rq->clock_update_flags & RQCF_UPDATED)
-		rf->clock_update_flags = RQCF_UPDATED;
-
+	/* __rq_clock_clear(rq); */
 	scx_rq_clock_invalidate(rq);
 	lockdep_unpin_lock(__rq_lockp(rq), rf->cookie);
 }
 
 static inline void rq_drop_lock(struct rq *rq, struct rq_flags *rf)
 {
-	rq_unpin_lock(rq, rf);
+	rf->clock_update_flags = rq_clock_freeze(rq);
+	scx_rq_clock_invalidate(rq);
+	lockdep_unpin_lock(__rq_lockp(rq), rf->cookie);
 }
 
 static inline void rq_repin_lock(struct rq *rq, struct rq_flags *rf)
 {
 	lockdep_repin_lock(__rq_lockp(rq), rf->cookie);
-
-	/*
-	 * Restore the value we stashed in @rf for this pin context.
-	 */
-	rq->clock_update_flags |= rf->clock_update_flags;
+	rq_clock_thaw(rq, rf->clock_update_flags);
 }
 
 #define __task_rq_lock(...) __acquire_ret(___task_rq_lock(__VA_ARGS__), __rq_lockp(__ret))

[PATCH v2 07/12] sched/core: Simplify/fix time updates

[Peter Zijlstra]

Use __update_rq_clock() and the new update semantics instead of using
convoluted logic to try and divinate if a clock needs updating.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/core.c |   25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6302,7 +6302,6 @@ 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;
@@ -6358,10 +6357,7 @@ pick_next_task(struct rq *rq, struct rq_
 	/* reset state */
 	rq->core->core_cookie = 0UL;
 	if (rq->core->core_forceidle_count) {
-		if (!core_clock_updated) {
-			update_rq_clock(rq->core);
-			core_clock_updated = true;
-		}
+		__update_rq_clock(rq->core);
 		sched_core_account_forceidle(rq);
 		/* reset after accounting force idle */
 		rq->core->core_forceidle_start = 0;
@@ -6390,14 +6386,11 @@ pick_next_task(struct rq *rq, struct rq_
 	 * and there are no cookied tasks running on siblings.
 	 */
 	if (!need_sync) {
+		__update_rq_clock(rq);
+
 		next = pick_task(rq, rf);
-		if (unlikely(next == RETRY_TASK)) {
-			/* rq lock may have been dropped, clocks invalidated */
-			core_clock_updated = false;
-			if (!(rq->clock_update_flags & RQCF_UPDATED))
-				update_rq_clock(rq);
+		if (unlikely(next == RETRY_TASK))
 			goto restart;
-		}
 
 		if (!next->core_cookie) {
 			/*
@@ -6436,18 +6429,12 @@ pick_next_task(struct rq *rq, struct rq_
 		 * pick_next_task(). If the current cpu is not the core,
 		 * the core may also have been updated above.
 		 */
-		if (i != cpu && (rq_i != rq->core || !core_clock_updated))
-			update_rq_clock(rq_i);
+		__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))) {
-			/* rq lock may have been dropped, clocks invalidated */
-			core_clock_updated = false;
-			if (!(rq->clock_update_flags & RQCF_UPDATED))
-				update_rq_clock(rq);
+			     WARN_ON_ONCE(p == RETRY_TASK)))
 			goto restart;
-		}
 
 		rq_i->core_pick = p;
 		rq_i->core_dl_server = rq_i->dl_server;

[PATCH v2 08/12] 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.

Avoids the 'stealing', but doesn't yet enable newidle just yet.

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

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

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3065,7 +3065,8 @@ static int affine_move_task(struct rq *r
 		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
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -11184,7 +11184,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
@@ -1594,6 +1594,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)
@@ -1639,6 +1647,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
@@ -4269,7 +4282,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;
 

[PATCH v2 09/12] sched/rt: Add early exit on balance path

[Peter Zijlstra]

Add/make consistent an early exit on the balance_{rt,dl}() path. When there are
no pushable tasks, there is no point in taking the rq lock and all that jazz.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/deadline.c |   10 +++-------
 kernel/sched/rt.c       |    3 +++
 2 files changed, 6 insertions(+), 7 deletions(-)

--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -3250,17 +3250,13 @@ static void pull_dl_task(struct rq *this
 				   src_rq->dl.earliest_dl.next))
 			continue;
 
+		if (!has_pushable_dl_tasks(src_rq))
+			continue;
+
 		/* Might drop this_rq->lock */
 		push_task = NULL;
 		double_lock_balance(this_rq, src_rq);
 
-		/*
-		 * If there are no more pullable tasks on the
-		 * rq, we're done with it.
-		 */
-		if (src_rq->dl.dl_nr_running <= 1)
-			goto skip;
-
 		p = pick_earliest_pushable_dl_task(src_rq, this_cpu);
 
 		/*
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -2307,6 +2307,9 @@ static void pull_rt_task(struct rq *this
 		    this_rq->rt.highest_prio.curr)
 			continue;
 
+		if (!has_pushable_tasks(src_rq))
+			continue;
+
 		/*
 		 * We can potentially drop this_rq's lock in
 		 * double_lock_balance, and another CPU could

[PATCH v2 10/12] sched/fair: Reflow pick_task_fair() / newidle

[Peter Zijlstra]

This is all histerical artifacts. Clean up the code to be less convoluted.

Note: this also re-enables balance for core-sched.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/fair.c |   51 +++++++++++++--------------------------------------
 1 file changed, 13 insertions(+), 38 deletions(-)

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6100,7 +6100,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)
@@ -10287,11 +10287,17 @@ struct task_struct *pick_task_fair(struc
 	struct cfs_rq *cfs_rq = &rq->cfs;
 	struct sched_entity *se;
 	struct task_struct *p;
-	int new_tasks;
 
 again:
-	if (!cfs_rq->h_nr_queued)
-		goto idle;
+	if (unlikely(!cfs_rq->h_nr_queued)) {
+		rq_modified_begin(rq, &fair_sched_class);
+		sched_balance_newidle(rq, rf);
+		if (rq_modified_above(rq, &fair_sched_class))
+			return RETRY_TASK;
+
+		if (!cfs_rq->h_nr_queued)
+			return NULL;
+	}
 
 	/* Might not have done put_prev_entity() */
 	if (cfs_rq->curr && cfs_rq->curr->on_rq)
@@ -10303,17 +10309,6 @@ struct task_struct *pick_task_fair(struc
 
 	p = task_of(se);
 	return p;
-
-idle:
-	if (sched_core_enabled(rq))
-		return NULL;
-
-	new_tasks = sched_balance_newidle(rq, rf);
-	if (new_tasks < 0)
-		return RETRY_TASK;
-	if (new_tasks > 0)
-		goto again;
-	return NULL;
 }
 
 static struct task_struct *
@@ -14869,13 +14864,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;
@@ -14892,7 +14882,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()
@@ -14903,7 +14893,7 @@ static int sched_balance_newidle(struct
 
 	/* Do not pull tasks towards !preferred CPUs */
 	if (!cpu_preferred(this_cpu))
-		return 0;
+		return;
 
 	/*
 	 * This is OK, because current is on_cpu, which avoids it being picked
@@ -14932,7 +14922,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) {
@@ -14990,18 +14979,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))
@@ -15013,8 +14990,6 @@ static int sched_balance_newidle(struct
 		nohz_newidle_balance(this_rq);
 
 	rq_repin_lock(this_rq, rf);
-
-	return pulled_task;
 }
 
 /*

[PATCH v2 11/12] sched/fair: Push sched_balance_newidle() unlock down

[Peter Zijlstra]

By pushing down the this_rq unlock, there are more cases where the lock isn't
dropped.

XXX needs numbers

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/fair.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -13706,6 +13706,7 @@ static int sched_balance_rq(int this_cpu
 			struct sched_domain *sd, enum cpu_idle_type idle,
 			int *continue_balancing)
 {
+	bool lock_rq = false, unlock_rq = idle == CPU_NEWLY_IDLE;
 	int ld_moved, cur_ld_moved, active_balance = 0;
 	struct sched_domain *sd_parent = sd->parent;
 	struct sched_group *group;
@@ -13774,6 +13775,13 @@ static int sched_balance_rq(int this_cpu
 		 */
 		env.loop_max  = min(sysctl_sched_nr_migrate, busiest->nr_running);
 
+		if (unlock_rq) {
+			lockdep_assert_rq_held(this_rq);
+			raw_spin_rq_unlock(this_rq);
+			unlock_rq = false;
+			lock_rq = true;
+		}
+
 more_balance:
 		rq_lock_irqsave(busiest, &rf);
 		update_rq_clock(busiest);
@@ -13902,6 +13910,13 @@ static int sched_balance_rq(int this_cpu
 	if (!need_active_balance(&env))
 		goto out_unbalanced;
 
+	if (unlock_rq) {
+		lockdep_assert_rq_held(this_rq);
+		raw_spin_rq_unlock(this_rq);
+		unlock_rq = false;
+		lock_rq = true;
+	}
+
 	scoped_guard (raw_spin_rq_lock_irqsave, busiest) {
 		/*
 		 * Don't kick the active_load_balance_cpu_stop,
@@ -13995,6 +14010,10 @@ static int sched_balance_rq(int this_cpu
 out:
 	if (need_unlock)
 		atomic_set_release(&sched_balance_running, 0);
+	if (lock_rq) {
+		raw_spin_rq_lock(this_rq);
+		__release_ctx_lock(__rq_lockp(this_rq));
+	}
 
 	return ld_moved;
 }
@@ -14878,8 +14897,6 @@ static void sched_balance_newidle(struct
 	t0 = sched_clock_cpu(this_cpu);
 	__sched_balance_update_blocked_averages(this_rq);
 
-	raw_spin_rq_unlock(this_rq);
-
 	for_each_domain(this_cpu, sd) {
 		u64 domain_cost;
 
@@ -14930,8 +14947,6 @@ static void sched_balance_newidle(struct
 			break;
 	}
 
-	raw_spin_rq_lock(this_rq);
-
 	if (curr_cost > this_rq->max_idle_balance_cost)
 		this_rq->max_idle_balance_cost = curr_cost;
 

[PATCH v2 12/12] 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.

Just like pick_task(), prev_balance() would iterate the classes and stop when
there is a runnable task found. The only difference is that pick_task() starts
from the top class, while prev_balance() would start from prev->class.

This means that if prev was an RT task, we'd get an extra visit to balance_dl()
-- and similary, if prev is fair, we'd visit balance_{dl,rt}() both. However,
this is not a problem, because both need_pull_{dl,rt}_task() will DTRT.

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/idle.c      |    7 -------
 kernel/sched/rt.c        |   14 ++++++--------
 kernel/sched/sched.h     |    5 -----
 kernel/sched/stop_task.c |    7 -------
 6 files changed, 12 insertions(+), 54 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6185,25 +6185,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:
  */
@@ -6241,8 +6222,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))
@@ -6348,8 +6327,6 @@ pick_next_task(struct rq *rq, struct rq_
 		goto out_set_next;
 	}
 
-	prev_balance(rq, rf);
-
 	smt_mask = cpu_smt_mask(cpu);
 
 restart:
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -2705,7 +2705,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,
@@ -2724,8 +2724,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);
 }
 
 /*
@@ -2823,6 +2821,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;
@@ -3654,7 +3657,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/idle.c
+++ b/kernel/sched/idle.c
@@ -468,12 +468,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:
  */
@@ -594,7 +588,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);
 }
 
 /*
@@ -1718,14 +1716,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)
@@ -2616,7 +2615,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
@@ -2785,11 +2785,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)
 {
@@ -110,7 +104,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 v2 02/12] sched/core: Introduce rq_drop_lock()

[Kayra Cizmeci]

Hi Peter,

On Sat, Nov 07, 2026 at 12:59:15PM +0100, Peter Zijlstra wrote:
> Currently rq_unpin_lock() serves a dual purpose, it pairs with both
> rq_pin_lock() and rq_repin_lock(). That is:
> 
> 	rq_pin_lock()
> 	  rq_unpin_lock()
>           rq_repin_lock()
> 	rq_unpin_lock()
> 
> In prepration of making the temporary drop and the final drop

nit. 'prepration' should be 'preparation' :-).

> distinct, this needs another function. Introduce and use
> rq_drop_lock() for the temporary drop, like:
> 
> 	rq_pin_lock()
> 	  rq_drop_lock()
>           rq_repin_lock()
> 	rq_unpin_lock()
> 

Yeah well sadly I couldn't find anything aside from a nit :-(.

I couldn't even find 1 missed place that we didn't transfered to
rq_drop_lock().

Reviewed-by: Kayra Cizmeci <kayracizmeci@gmail.com>


Cheers.. Woo..

Kayra

Re: [PATCH v2 05/12] sched/core: Abstract/fix rq_clock_start_loop_update()

[Kayra Cizmeci]

Hi Peter!

I drank too much tea, I'm going to throw up. Bleah..

BTW this patch was *really* fun to review. But now it's 1 AM in the
night and I can't even type this sentence good.

On Sat, Nov 07, 2026 at 12:59:18PM +0100, Peter Zijlstra wrote:
> The existing rq_clock_start_loop_update() / rq_lock_stop_loop_update() almost
> do what we need rq_drop_lock() / rq_repin_lock() to do. They freeze time by
> setting ACT_SKIP.
> 
> Rename them rq_clock_freeze() / rq_clock_thaw() and have them operate on a new
> bit RQCF_FREEZE. This is such that schedule() can safely nest inside
> rq_clock_freeze() as is required for rq_drop_lock().
> 
> Sanitize the rq->clock_update_flags handling while adding the new bit --
> notably do away with the full shift left which makes a mess of things.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  kernel/sched/core.c  |    4 +--
>  kernel/sched/fair.c  |   15 +++++++-----
>  kernel/sched/sched.h |   63 +++++++++++++++++++++++++++++----------------------
>  3 files changed, 48 insertions(+), 34 deletions(-)


>  #define RQCF_REQ_SKIP		0x01
>  #define RQCF_ACT_SKIP		0x02
> -#define RQCF_UPDATED		0x04
> +#define RQCF_FREEZE		0x04
> +#define RQCF_UPDATED		0x08
>  
> -#define RQCF_NOP_MASK		(RQCF_ACT_SKIP | RQCF_UPDATED)
> +#define RQCF_NOP_MASK		(RQCF_ACT_SKIP | RQCF_FREEZE | RQCF_UPDATED)
>  
>  static inline void assert_clock_updated(struct rq *rq)
>  {
> @@ -1815,7 +1824,7 @@ static inline void assert_clock_updated(
>  	 * The only reason for not seeing a clock update since the
>  	 * last rq_pin_lock() is if we're currently skipping updates.
>  	 */
> -	WARN_ON_ONCE(rq->clock_update_flags < RQCF_ACT_SKIP);
> +	WARN_ON_ONCE(!(rq->clock_update_flags & RQCF_UPDATED));
>  }
>  
>  static inline u64 rq_clock(struct rq *rq)
> @@ -1851,25 +1860,27 @@ static inline void rq_clock_cancel_skipu
>  }
>  
>  /*
> - * During cpu offlining and rq wide unthrottling, we can trigger
> - * an update_rq_clock() for several cfs and rt runqueues (Typically
> - * when using list_for_each_entry_*)
> - * rq_clock_start_loop_update() can be called after updating the clock
> - * once and before iterating over the list to prevent multiple update.
> - * After the iterative traversal, we need to call rq_clock_stop_loop_update()
> - * to clear RQCF_ACT_SKIP of rq->clock_update_flags.
> + * FREEZE rq_clock. This is used in a number of places to
> + * ensure things appears to happen at the same time.
> + *
> + * Eg. during cpu offlining and rq wide unthrottling, we can trigger an
> + * update_rq_clock() for several cfs and rt runqueues (Typically when using
> + * list_for_each_entry_*)
>   */
> -static inline void rq_clock_start_loop_update(struct rq *rq)
> +static inline unsigned int rq_clock_freeze(struct rq *rq)
>  {
> +	unsigned int flags = rq->clock_update_flags;
>  	lockdep_assert_rq_held(rq);
> -	WARN_ON_ONCE(rq->clock_update_flags & RQCF_ACT_SKIP);
> -	rq->clock_update_flags |= RQCF_ACT_SKIP;
> +	assert_clock_updated(rq);
> +	rq->clock_update_flags |= RQCF_FREEZE;
> +	return flags;
>  }
>  
> -static inline void rq_clock_stop_loop_update(struct rq *rq)
> +static inline void rq_clock_thaw(struct rq *rq, bool flags)
>  {
>  	lockdep_assert_rq_held(rq);
> -	rq->clock_update_flags &= ~RQCF_ACT_SKIP;
> +	rq->clock_update_flags &= ~RQCF_FREEZE;
> +	rq->clock_update_flags |= RQCF_UPDATED | (flags & RQCF_FREEZE);
>  }
>  


So, these are getting called from rq_drop_lock() and rq_repin_lock():

 static inline void rq_drop_lock(struct rq *rq, struct rq_flags *rf)
 {
 	 rf->clock_update_flags = rq_clock_freeze(rq);
 	 scx_rq_clock_invalidate(rq);
 	 lockdep_unpin_lock(__rq_lockp(rq), rf->cookie);
 }

Here, rq_clock_freeze() returns unsigned int. While rq_clock_thaw()
converts it to bool.

So below,

 static inline void rq_clock_thaw(struct rq *rq, bool flags)
 {
 	 lockdep_assert_rq_held(rq);
 	 rq->clock_update_flags &= ~RQCF_FREEZE;
 	 rq->clock_update_flags |= RQCF_UPDATED | (flags & RQCF_FREEZE);
 }

flags could be only 1 or 0. both 1 & 4 and 0 & 4 is false. So this flags
& RQCF_FREEZE thing always returns false.

Shouldn't this needs to be unsigned int :-)?


Also look here:

 /*
  * In double_lock_balance()/double_rq_lock(), we use raw_spin_rq_lock() to
  * acquire rq lock instead of rq_lock(). So at the end of these two functions
  * we need to call double_rq_clock_clear_update() to clear RQCF_UPDATED of
  * rq->clock_update_flags to avoid the WARN_DOUBLE_CLOCK warning.
  */
 static inline void double_rq_clock_clear_update(struct rq *rq1, struct rq *rq2)
 {
 	 rq1->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
 	 rq2->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
 }

Right now this clears RQCF_FREEZE too. On every call. It's called
from double_rq_lock() and that gets called from _double_lock_balance()
and that gets called from double_lock_balance() and that gets called
from alots of places but for example:

* pull_dl_task() gets called from balance_dl(), we first call
rq_drop_lock() it calls rq_clock_freeze() and we set RQCF_FREEZE.

* Then we call pull_dl_task(), it calls double_lock_balance() and
RQCF_FREEZE gets cleared.


So to the period of double_lock_balance() call to rq_repin_lock()
RQCF_FREEZE remains unsetted.


So to fix both of these I would suggest something like so, not tested:


diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 339a77864ec8..a91d12640f37 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1911,7 +1911,7 @@ static inline unsigned int rq_clock_freeze(struct rq *rq)
 	return flags;
 }
 
-static inline void rq_clock_thaw(struct rq *rq, bool flags)
+static inline void rq_clock_thaw(struct rq *rq, unsigned int flags)
 {
 	lockdep_assert_rq_held(rq);
 	rq->clock_update_flags &= ~RQCF_FREEZE;
@@ -3357,8 +3357,8 @@ unsigned long arch_scale_freq_capacity(int cpu)
  */
 static inline void double_rq_clock_clear_update(struct rq *rq1, struct rq *rq2)
 {
-	rq1->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
-	rq2->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
+	rq1->clock_update_flags &= ~RQCF_UPDATED;
+	rq2->clock_update_flags &= ~RQCF_UPDATED;
 }
 
 #define DEFINE_LOCK_GUARD_2(name, type, _lock, _unlock, ...)				\




In a case like this:

CPU1 calls rq_drop_lock(), saves FREEZE = 0, and changes FREEZE to 1,
and drops the lock.

CPU2 gets the lock, saves FREEZE = 1 and changes FREEZE to 1, and then
drops the lock.

In this period CPU1 gets the lock again, makes FREEZE 0.

And later CPU2 comes and makes it FREEZE 1.


Not sure above could happen tho. Could it happen? I tried to fix this
assuming it could happen.

I'm not really happy with below diff, but I couldn't find a better way
neither, so please tell me that this case can't happen or there
is a better way to fix it :-(. This is on top of the current code BTW.
Only compile tested.

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 104c5333689a..9b3f92c03fea 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5385,7 +5385,6 @@ static inline void clear_tg_load_avg(struct cfs_rq *cfs_rq)
 static void __maybe_unused clear_tg_offline_cfs_rqs(struct rq *rq)
 {
 	struct task_group *tg;
-	unsigned int cuf;
 
 	lockdep_assert_rq_held(rq);
 
@@ -5394,7 +5393,7 @@ static void __maybe_unused clear_tg_offline_cfs_rqs(struct rq *rq)
 	 * set_rq_offline(), so we should skip updating
 	 * the rq clock again in unthrottle_cfs_rq().
 	 */
-	cuf = rq_clock_freeze(rq);
+	rq_clock_freeze(rq);
 
 	guard(rcu)();
 
@@ -5404,7 +5403,7 @@ static void __maybe_unused clear_tg_offline_cfs_rqs(struct rq *rq)
 		clear_tg_load_avg(cfs_rq);
 	}
 
-	rq_clock_thaw(rq, cuf);
+	rq_clock_thaw(rq);
 }
 
 /*
@@ -7280,7 +7279,6 @@ static void __cfsb_csd_unthrottle(void *arg)
 {
 	struct cfs_rq *cursor, *tmp;
 	struct rq *rq = arg;
-	unsigned int cuf;
 
 	guard(rq_lock)(rq);
 
@@ -7290,7 +7288,7 @@ static void __cfsb_csd_unthrottle(void *arg)
 	 * Do it once and skip the potential next ones.
 	 */
 	update_rq_clock(rq);
-	cuf = rq_clock_freeze(rq);
+	rq_clock_freeze(rq);
 
 	/*
 	 * Since we hold rq lock we're safe from concurrent manipulation of
@@ -7309,7 +7307,7 @@ static void __cfsb_csd_unthrottle(void *arg)
 			unthrottle_cfs_rq(cursor);
 	}
 
-	rq_clock_thaw(rq, cuf);
+	rq_clock_thaw(rq);
 }
 
 static inline void __unthrottle_cfs_rq_async(struct cfs_rq *cfs_rq)
@@ -7815,7 +7813,6 @@ static void __maybe_unused update_runtime_enabled(struct rq *rq)
 static void __maybe_unused unthrottle_offline_cfs_rqs(struct rq *rq)
 {
 	struct task_group *tg;
-	unsigned int cuf;
 
 	lockdep_assert_rq_held(rq);
 
@@ -7828,7 +7825,7 @@ static void __maybe_unused unthrottle_offline_cfs_rqs(struct rq *rq)
 	 * set_rq_offline(), so we should skip updating
 	 * the rq clock again in unthrottle_cfs_rq().
 	 */
-	cuf = rq_clock_freeze(rq);
+	rq_clock_freeze(rq);
 
 	guard(rcu)();
 
@@ -7855,7 +7852,7 @@ static void __maybe_unused unthrottle_offline_cfs_rqs(struct rq *rq)
 		unthrottle_cfs_rq(cfs_rq);
 	}
 
-	rq_clock_thaw(rq, cuf);
+	rq_clock_thaw(rq);
 }
 
 bool cfs_task_bw_constrained(struct task_struct *p)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 339a77864ec8..d8f1784d0640 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1281,6 +1281,7 @@ struct rq {
 	u64			clock;
 	unsigned long		lost_idle_time;
 	unsigned int		clock_update_flags;
+	unsigned int		clock_update_depth;
 	u64			clock_pelt_idle;
 	u64			clock_idle;
 
@@ -1902,31 +1903,28 @@ static inline void rq_clock_cancel_skipupdate(struct rq *rq)
  * update_rq_clock() for several cfs and rt runqueues (Typically when using
  * list_for_each_entry_*)
  */
-static inline unsigned int rq_clock_freeze(struct rq *rq)
+static inline void rq_clock_freeze(struct rq *rq)
 {
-	unsigned int flags = rq->clock_update_flags;
 	lockdep_assert_rq_held(rq);
 	assert_clock_updated(rq);
+	rq->clock_update_depth++;
 	rq->clock_update_flags |= RQCF_FREEZE;
-	return flags;
 }
 
-static inline void rq_clock_thaw(struct rq *rq, bool flags)
+static inline void rq_clock_thaw(struct rq *rq)
 {
 	lockdep_assert_rq_held(rq);
+	WARN_ON_ONCE(!rq->clock_update_depth);
+	rq->clock_update_flags |= RQCF_UPDATED;
+	rq->clock_update_depth--;
+	if (rq->clock_update_depth)
+		return;
 	rq->clock_update_flags &= ~RQCF_FREEZE;
-	rq->clock_update_flags |= RQCF_UPDATED | (flags & RQCF_FREEZE);
 }
 
 struct rq_flags {
 	unsigned long flags;
 	struct pin_cookie cookie;
-	/*
-	 * A copy of (rq::clock_update_flags & RQCF_UPDATED) for the
-	 * current pin context is stashed here in case it needs to be
-	 * restored in rq_repin_lock().
-	 */
-	unsigned int clock_update_flags;
 };
 
 extern struct balance_callback balance_push_callback;
@@ -2030,7 +2028,6 @@ static inline void rq_pin_lock(struct rq *rq, struct rq_flags *rf)
 	rf->cookie = lockdep_pin_lock(__rq_lockp(rq));
 
 	__rq_clock_clear(rq);
-	rf->clock_update_flags = 0;
 	assert_balance_callbacks_empty(rq);
 }
 
@@ -2043,7 +2040,7 @@ static inline void rq_unpin_lock(struct rq *rq, struct rq_flags *rf)
 
 static inline void rq_drop_lock(struct rq *rq, struct rq_flags *rf)
 {
-	rf->clock_update_flags = rq_clock_freeze(rq);
+	rq_clock_freeze(rq);
 	scx_rq_clock_invalidate(rq);
 	lockdep_unpin_lock(__rq_lockp(rq), rf->cookie);
 }
@@ -2051,7 +2048,7 @@ static inline void rq_drop_lock(struct rq *rq, struct rq_flags *rf)
 static inline void rq_repin_lock(struct rq *rq, struct rq_flags *rf)
 {
 	lockdep_repin_lock(__rq_lockp(rq), rf->cookie);
-	rq_clock_thaw(rq, rf->clock_update_flags);
+	rq_clock_thaw(rq);
 }
 
 #define __task_rq_lock(...) __acquire_ret(___task_rq_lock(__VA_ARGS__), __rq_lockp(__ret))



>  struct rq_flags {
> @@ -1938,14 +1949,14 @@ static inline void rq_pin_lock(struct rq
>  {
>  	rf->cookie = lockdep_pin_lock(__rq_lockp(rq));
>  
> -	rq->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
> +	rq->clock_update_flags &= ~RQCF_UPDATED;
>  	rf->clock_update_flags = 0;
>  	assert_balance_callbacks_empty(rq);
>  }

Ah you changed it here. So I guess you forgot to change it in
double_rq_clock_clear_update(), no? :-).

I'm gonna sleep now. Woo...

Cheers,

Kayra

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

[Tejun Heo]

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.

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.

[ ... ]

>  		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.

Thanks.

--
tejun

Re: [PATCH v2 04/12] sched/core: Make __update_rq_clock() skip updates on UPDATED

[Tejun Heo]

Hello, Peter.

On Sat, Nov 07, 2026 at 12:59:17PM +0100, Peter Zijlstra wrote:
>  static void __update_rq_clock(struct rq *rq)
>  {
> +	bool update;
> +
>  	lockdep_assert_rq_held(rq);
>  
> +	update = !(rq->clock_update_flags & RQCF_NOP_MASK);
>  	rq->clock_update_flags |= RQCF_UPDATED;
>  
> -	if (!(rq->clock_update_flags & RQCF_ACT_SKIP)) {
> +	if (update) {

RQCF_UPDATED is cleared only by rq_pin_lock() and
double_rq_clock_clear_update(), so a path which takes the rq lock with
raw_spin_rq_lock() inherits it from the previous holder and its
update_rq_clock() is now silently skipped.

Previously, you said to clear on unlock. Why not do that? That'd be less
confusing overall.

Thanks.

--
tejun

Re: [PATCH v2 05/12] sched/core: Abstract/fix rq_clock_start_loop_update()

[K Prateek Nayak]

Hello Kayre, Peter,

On 11/9/2026 3:56 AM, Kayra Cizmeci wrote:
> So to fix both of these I would suggest something like so, not tested:
> 
> 
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 339a77864ec8..a91d12640f37 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1911,7 +1911,7 @@ static inline unsigned int rq_clock_freeze(struct rq *rq)
>  	return flags;
>  }
>  
> -static inline void rq_clock_thaw(struct rq *rq, bool flags)
> +static inline void rq_clock_thaw(struct rq *rq, unsigned int flags)
>  {
>  	lockdep_assert_rq_held(rq);
>  	rq->clock_update_flags &= ~RQCF_FREEZE;
> @@ -3357,8 +3357,8 @@ unsigned long arch_scale_freq_capacity(int cpu)
>   */
>  static inline void double_rq_clock_clear_update(struct rq *rq1, struct rq *rq2)
>  {
> -	rq1->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
> -	rq2->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
> +	rq1->clock_update_flags &= ~RQCF_UPDATED;
> +	rq2->clock_update_flags &= ~RQCF_UPDATED;
>  }
>  
>  #define DEFINE_LOCK_GUARD_2(name, type, _lock, _unlock, ...)				\

So with this, I still get a clock_update_flags warning like:

    ------------[ cut here ]------------
    !(rq->clock_update_flags & RQCF_UPDATED)
    WARNING: kernel/sched/sched.h:1862 at set_next_task_idle+0xe1/0x100, CPU#262: sched-messaging/8375
    Modules linked in: ...
    CPU: 262 UID: 0 PID: 8375 Comm: sched-messaging Tainted: G        W           7.4.0-rc1-test+ #463 PREEMPT(lazy)
    Tainted: [W]=WARN
    Hardware name: AMD Corporation Titanite_4G/Titanite_4G, BIOS RTI100CC 03/28/2024
    RIP: 0010:set_next_task_idle+0xe1/0x100
    Code: ...
    RSP: 0018:ff919f922f4e7c80 EFLAGS: 00010046
    RAX: 0000000000000000 RBX: ff49b96ece1b33c0 RCX: 0000000000000007
    RDX: 000000128e40504c RSI: 0000000000000200 RDI: ff49b8b30f2d8000
    RBP: ff49b8b28ed84280 R08: 0000000000000200 R09: ff49b8b28ed84280
    R10: 0000000000000008 R11: 0000000000000008 R12: ff49b8b28ed84280
    R13: ffffffff86a043c0 R14: ff49b8b28dfca700 R15: 0000000000000200
    FS:  00007fddd81237c0(0000) GS:ff49b96f477af000(0000) knlGS:0000000000000000
    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 000055ce59162840 CR3: 00000001a1f11004 CR4: 0000000000f71ef0
    PKRU: 55555554
    Call Trace:
     <TASK>
     __schedule+0x93b/0x1ca0
     schedule+0x28/0xa0
     anon_pipe_read+0x302/0x410
     ? __pfx_autoremove_wake_function+0x10/0x10
     vfs_read+0x30b/0x340
     ksys_read+0xb8/0xe0
     do_syscall_64+0xce/0x550
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? do_syscall_64+0x35/0x550
     entry_SYSCALL_64_after_hwframe+0x76/0x7e
    RIP: 0033:0x7fddd6514932
    Code: ...
    RSP: 002b:00007fffd6dcde28 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
    RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fddd6514932
    RDX: 0000000000000064 RSI: 00007fffd6dcde40 RDI: 0000000000000007
    RBP: 00007fffd6dcdee0 R08: 0000000000000000 R09: 00007fddd81237c0
    R10: 00007fddd84ed0c8 R11: 0000000000000246 R12: 0000000000000064
    R13: 00007fffd6dcde40 R14: 00000000000001c7 R15: 000055ce88d5e840
     </TASK>
    ---[ end trace 0000000000000000 ]---

with:

  coresched new -t pid -- perf bench sched messaging -p -l 100000 -g 8

on queue:sched/hackery.

If I do:

diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 339a77864ec8..fd900d1419c6 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3357,8 +3357,8 @@ unsigned long arch_scale_freq_capacity(int cpu)
  */
 static inline void double_rq_clock_clear_update(struct rq *rq1, struct rq *rq2)
 {
-	rq1->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
-	rq2->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
+	__rq_clock_clear(rq1);
+	__rq_clock_clear(rq2);
 }
 
 #define DEFINE_LOCK_GUARD_2(name, type, _lock, _unlock, ...)				\
---

I don't see that warning anymore. Must be the "core_pick_in_flight"
check keeping everything sane.

> In a case like this:
> 
> CPU1 calls rq_drop_lock(), saves FREEZE = 0, and changes FREEZE to 1,
> and drops the lock.
> 
> CPU2 gets the lock, saves FREEZE = 1 and changes FREEZE to 1, and then
> drops the lock.
> 
> In this period CPU1 gets the lock again, makes FREEZE 0.
> 
> And later CPU2 comes and makes it FREEZE 1.
> 
> 
> Not sure above could happen tho. Could it happen?

I suppose it is possible.

Generally rq_lock is dropped in middle of __schedule() when NEED_RESCHED is still
set so ->task_woken() would generally bail out early for dl and rt (ext doesn't
seem to drop the rq_lock on that path).

That said, a concurrent wakeup can grab the rq_lock when another ->task_woken()
dropped the rq_lock or a __balance_callbacks() for setprio or sched_move_task()
can interleave with a ->task_woken() and lead to your scenario.

> I tried to fix this assuming it could happen.
> 
> I'm not really happy with below diff, but I couldn't find a better way
> neither, so please tell me that this case can't happen or there
> is a better way to fix it :-(. This is on top of the current code BTW.
> Only compile tested.

[..snip..]

>  bool cfs_task_bw_constrained(struct task_struct *p)
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 339a77864ec8..d8f1784d0640 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1281,6 +1281,7 @@ struct rq {
>  	u64			clock;
>  	unsigned long		lost_idle_time;
>  	unsigned int		clock_update_flags;
> +	unsigned int		clock_update_depth;

I think this can stand-in for the freeze flag too if you do
something like:

diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 339a77864ec8..49c732d3f3bb 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1280,7 +1280,18 @@ struct rq {
 	u64			clock_pelt;
 	u64			clock;
 	unsigned long		lost_idle_time;
-	unsigned int		clock_update_flags;
+	union {
+		struct {
+			s16	clock_update_flags;
+			u16	freeze_depth;
+		};
+		/*
+		 * Inspect both clock_update_flags and
+		 * freeze_depth together in
+		 * __update_rq_clock().
+		 */
+		u32		__clock_update_flags;
+	};
 	u64			clock_pelt_idle;
 	u64			clock_idle;
 
---

Probably makes it very ugly :-)

>  	u64			clock_pelt_idle;
>  	u64			clock_idle;
>  
> @@ -1902,31 +1903,28 @@ static inline void rq_clock_cancel_skipupdate(struct rq *rq)
>   * update_rq_clock() for several cfs and rt runqueues (Typically when using
>   * list_for_each_entry_*)
>   */
> -static inline unsigned int rq_clock_freeze(struct rq *rq)
> +static inline void rq_clock_freeze(struct rq *rq)
>  {
> -	unsigned int flags = rq->clock_update_flags;
>  	lockdep_assert_rq_held(rq);
>  	assert_clock_updated(rq);
> +	rq->clock_update_depth++;
>  	rq->clock_update_flags |= RQCF_FREEZE;
> -	return flags;
>  }
>  
> -static inline void rq_clock_thaw(struct rq *rq, bool flags)
> +static inline void rq_clock_thaw(struct rq *rq)
>  {
>  	lockdep_assert_rq_held(rq);
> +	WARN_ON_ONCE(!rq->clock_update_depth);
> +	rq->clock_update_flags |= RQCF_UPDATED;
> +	rq->clock_update_depth--;
> +	if (rq->clock_update_depth)
> +		return;
>  	rq->clock_update_flags &= ~RQCF_FREEZE;
> -	rq->clock_update_flags |= RQCF_UPDATED | (flags & RQCF_FREEZE);
>  }

This seems like a legit problem one can run into and
your solutions works well!

-- 
Thanks and Regards,
Prateek

Re: [PATCH v2 05/12] sched/core: Abstract/fix rq_clock_start_loop_update()

[Kayra Cizmeci]

Hi!

On Wed, Nov 11, 2026 at 02:40:36PM +0530, K Prateek Nayak wrote:
> Hello Kayre, Peter,
> 
> On 11/9/2026 3:56 AM, Kayra Cizmeci wrote:
> > So to fix both of these I would suggest something like so, not tested:
> > 
> > 
> > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> > index 339a77864ec8..a91d12640f37 100644
> > --- a/kernel/sched/sched.h
> > +++ b/kernel/sched/sched.h
> > @@ -1911,7 +1911,7 @@ static inline unsigned int rq_clock_freeze(struct rq *rq)
> >  	return flags;
> >  }
> >  
> > -static inline void rq_clock_thaw(struct rq *rq, bool flags)
> > +static inline void rq_clock_thaw(struct rq *rq, unsigned int flags)
> >  {
> >  	lockdep_assert_rq_held(rq);
> >  	rq->clock_update_flags &= ~RQCF_FREEZE;
> > @@ -3357,8 +3357,8 @@ unsigned long arch_scale_freq_capacity(int cpu)
> >   */
> >  static inline void double_rq_clock_clear_update(struct rq *rq1, struct rq *rq2)
> >  {
> > -	rq1->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
> > -	rq2->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
> > +	rq1->clock_update_flags &= ~RQCF_UPDATED;
> > +	rq2->clock_update_flags &= ~RQCF_UPDATED;
> >  }
> >  
> >  #define DEFINE_LOCK_GUARD_2(name, type, _lock, _unlock, ...)				\
> 
> So with this, I still get a clock_update_flags warning like:
> 
>     ------------[ cut here ]------------
>     !(rq->clock_update_flags & RQCF_UPDATED)
>     WARNING: kernel/sched/sched.h:1862 at set_next_task_idle+0xe1/0x100, CPU#262: sched-messaging/8375
>     Modules linked in: ...
>     CPU: 262 UID: 0 PID: 8375 Comm: sched-messaging Tainted: G        W           7.4.0-rc1-test+ #463 PREEMPT(lazy)
>     Tainted: [W]=WARN
>     Hardware name: AMD Corporation Titanite_4G/Titanite_4G, BIOS RTI100CC 03/28/2024
>     RIP: 0010:set_next_task_idle+0xe1/0x100
>     Code: ...
>     RSP: 0018:ff919f922f4e7c80 EFLAGS: 00010046
>     RAX: 0000000000000000 RBX: ff49b96ece1b33c0 RCX: 0000000000000007
>     RDX: 000000128e40504c RSI: 0000000000000200 RDI: ff49b8b30f2d8000
>     RBP: ff49b8b28ed84280 R08: 0000000000000200 R09: ff49b8b28ed84280
>     R10: 0000000000000008 R11: 0000000000000008 R12: ff49b8b28ed84280
>     R13: ffffffff86a043c0 R14: ff49b8b28dfca700 R15: 0000000000000200
>     FS:  00007fddd81237c0(0000) GS:ff49b96f477af000(0000) knlGS:0000000000000000
>     CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>     CR2: 000055ce59162840 CR3: 00000001a1f11004 CR4: 0000000000f71ef0
>     PKRU: 55555554
>     Call Trace:
>      <TASK>
>      __schedule+0x93b/0x1ca0
>      schedule+0x28/0xa0
>      anon_pipe_read+0x302/0x410
>      ? __pfx_autoremove_wake_function+0x10/0x10
>      vfs_read+0x30b/0x340
>      ksys_read+0xb8/0xe0
>      do_syscall_64+0xce/0x550
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? do_syscall_64+0x35/0x550
>      entry_SYSCALL_64_after_hwframe+0x76/0x7e
>     RIP: 0033:0x7fddd6514932
>     Code: ...
>     RSP: 002b:00007fffd6dcde28 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
>     RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fddd6514932
>     RDX: 0000000000000064 RSI: 00007fffd6dcde40 RDI: 0000000000000007
>     RBP: 00007fffd6dcdee0 R08: 0000000000000000 R09: 00007fddd81237c0
>     R10: 00007fddd84ed0c8 R11: 0000000000000246 R12: 0000000000000064
>     R13: 00007fffd6dcde40 R14: 00000000000001c7 R15: 000055ce88d5e840
>      </TASK>
>     ---[ end trace 0000000000000000 ]---
> 
> with:
> 
>   coresched new -t pid -- perf bench sched messaging -p -l 100000 -g 8
> 
> on queue:sched/hackery.
> 
> If I do:
> 
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 339a77864ec8..fd900d1419c6 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -3357,8 +3357,8 @@ unsigned long arch_scale_freq_capacity(int cpu)
>   */
>  static inline void double_rq_clock_clear_update(struct rq *rq1, struct rq *rq2)
>  {
> -	rq1->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
> -	rq2->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
> +	__rq_clock_clear(rq1);
> +	__rq_clock_clear(rq2);
>  }
>  
>  #define DEFINE_LOCK_GUARD_2(name, type, _lock, _unlock, ...)				\
> ---
> 
> I don't see that warning anymore. Must be the "core_pick_in_flight"
> check keeping everything sane.
> 

Thanks for testing! Well, I missed that :-(. Core Scheduling...

> > In a case like this:
> > 
> > CPU1 calls rq_drop_lock(), saves FREEZE = 0, and changes FREEZE to 1,
> > and drops the lock.
> > 
> > CPU2 gets the lock, saves FREEZE = 1 and changes FREEZE to 1, and then
> > drops the lock.
> > 
> > In this period CPU1 gets the lock again, makes FREEZE 0.
> > 
> > And later CPU2 comes and makes it FREEZE 1.
> > 
> > 
> > Not sure above could happen tho. Could it happen?
> 
> I suppose it is possible.
> 
> Generally rq_lock is dropped in middle of __schedule() when NEED_RESCHED is still
> set so ->task_woken() would generally bail out early for dl and rt (ext doesn't
> seem to drop the rq_lock on that path).
> 
> That said, a concurrent wakeup can grab the rq_lock when another ->task_woken()
> dropped the rq_lock or a __balance_callbacks() for setprio or sched_move_task()
> can interleave with a ->task_woken() and lead to your scenario.
> 
> > I tried to fix this assuming it could happen.
> > 
> > I'm not really happy with below diff, but I couldn't find a better way
> > neither, so please tell me that this case can't happen or there
> > is a better way to fix it :-(. This is on top of the current code BTW.
> > Only compile tested.
> 
> [..snip..]
> 
> >  bool cfs_task_bw_constrained(struct task_struct *p)
> > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> > index 339a77864ec8..d8f1784d0640 100644
> > --- a/kernel/sched/sched.h
> > +++ b/kernel/sched/sched.h
> > @@ -1281,6 +1281,7 @@ struct rq {
> >  	u64			clock;
> >  	unsigned long		lost_idle_time;
> >  	unsigned int		clock_update_flags;
> > +	unsigned int		clock_update_depth;
> 
> I think this can stand-in for the freeze flag too if you do
> something like:
> 
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 339a77864ec8..49c732d3f3bb 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1280,7 +1280,18 @@ struct rq {
>  	u64			clock_pelt;
>  	u64			clock;
>  	unsigned long		lost_idle_time;
> -	unsigned int		clock_update_flags;
> +	union {
> +		struct {
> +			s16	clock_update_flags;
> +			u16	freeze_depth;
> +		};
> +		/*
> +		 * Inspect both clock_update_flags and
> +		 * freeze_depth together in
> +		 * __update_rq_clock().
> +		 */
> +		u32		__clock_update_flags;
> +	};
>  	u64			clock_pelt_idle;
>  	u64			clock_idle;
>  
> ---
> 
> Probably makes it very ugly :-)
> 

TBH, I don't know which one is better. And if I were to decide,
I'd be wrong 50% of the time, so it's up to Peter :-).

> >  	u64			clock_pelt_idle;
> >  	u64			clock_idle;
> >  
> > @@ -1902,31 +1903,28 @@ static inline void rq_clock_cancel_skipupdate(struct rq *rq)
> >   * update_rq_clock() for several cfs and rt runqueues (Typically when using
> >   * list_for_each_entry_*)
> >   */
> > -static inline unsigned int rq_clock_freeze(struct rq *rq)
> > +static inline void rq_clock_freeze(struct rq *rq)
> >  {
> > -	unsigned int flags = rq->clock_update_flags;
> >  	lockdep_assert_rq_held(rq);
> >  	assert_clock_updated(rq);
> > +	rq->clock_update_depth++;
> >  	rq->clock_update_flags |= RQCF_FREEZE;
> > -	return flags;
> >  }
> >  
> > -static inline void rq_clock_thaw(struct rq *rq, bool flags)
> > +static inline void rq_clock_thaw(struct rq *rq)
> >  {
> >  	lockdep_assert_rq_held(rq);
> > +	WARN_ON_ONCE(!rq->clock_update_depth);
> > +	rq->clock_update_flags |= RQCF_UPDATED;
> > +	rq->clock_update_depth--;
> > +	if (rq->clock_update_depth)
> > +		return;
> >  	rq->clock_update_flags &= ~RQCF_FREEZE;
> > -	rq->clock_update_flags |= RQCF_UPDATED | (flags & RQCF_FREEZE);
> >  }
> 
> This seems like a legit problem one can run into and
> your solutions works well!

Thanks for having a look at this!

> 
> -- 
> Thanks and Regards,
> Prateek
> 

Cheers,

Kayra

Re: [PATCH v2 04/12] sched/core: Make __update_rq_clock() skip updates on UPDATED

[Peter Zijlstra]

On Tue, Nov 10, 2026 at 03:11:28PM -1000, Tejun Heo wrote:
> Hello, Peter.
> 
> On Sat, Nov 07, 2026 at 12:59:17PM +0100, Peter Zijlstra wrote:
> >  static void __update_rq_clock(struct rq *rq)
> >  {
> > +	bool update;
> > +
> >  	lockdep_assert_rq_held(rq);
> >  
> > +	update = !(rq->clock_update_flags & RQCF_NOP_MASK);
> >  	rq->clock_update_flags |= RQCF_UPDATED;
> >  
> > -	if (!(rq->clock_update_flags & RQCF_ACT_SKIP)) {
> > +	if (update) {
> 
> RQCF_UPDATED is cleared only by rq_pin_lock() and
> double_rq_clock_clear_update(), so a path which takes the rq lock with
> raw_spin_rq_lock() inherits it from the previous holder and its
> update_rq_clock() is now silently skipped.

Let me look at that specific case.

> Previously, you said to clear on unlock. Why not do that? That'd be less
> confusing overall.

Yes, that's what I tried, but it doesn't work for core-sched. What
happens is that SMT0 and SMT1 interleave and things go to hell, patch 6
has that picture.

That is, we can still clear on unlock too, but clear on unlock alone
isn't sufficient :/

Patch 6 even has the clear on unlock, but commented out.

Re: [PATCH v2 06/12] sched/core: New rq_{pin,drop,repin,unpin}_lock() semantics

[Tejun Heo]

Hello,

On Sat, Nov 07, 2026 at 12:59:19PM +0100, Peter Zijlstra wrote:
>  static inline void rq_drop_lock(struct rq *rq, struct rq_flags *rf)
>  {
> -	rq_unpin_lock(rq, rf);
> +	rf->clock_update_flags = rq_clock_freeze(rq);
> +	scx_rq_clock_invalidate(rq);
> +	lockdep_unpin_lock(__rq_lockp(rq), rf->cookie);
>  }

As 02/12 switched __balance_callbacks() to rq_drop_lock(), it now goes
through rq_clock_freeze() and its assert_clock_updated() whether a
callback is queued or not. The scx task iterator calls
__balance_callbacks() before each unlock and doesn't necessarily update
the clock in between, so loading a BPF scheduler triggers:

  WARNING: kernel/sched/sched.h:1862 at __balance_callbacks
   __scx_task_iter_rq_unlock
   scx_task_iter_unlock
   scx_root_enable_workfn

__balance_callbacks() could skip the drop when no callback is queued,
but that's getting too subtle. The scx side can manually update the
clock there too, but overall the whole thing is looking a bit too
brittle.

Thanks.

--
tejun

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

[Peter Zijlstra]

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

Re: [PATCH v2 02/12] sched/core: Introduce rq_drop_lock()

[Peter Zijlstra]

On Sun, Nov 08, 2026 at 12:25:10PM +0300, Kayra Cizmeci wrote:
> Hi Peter,
> 
> On Sat, Nov 07, 2026 at 12:59:15PM +0100, Peter Zijlstra wrote:
> > Currently rq_unpin_lock() serves a dual purpose, it pairs with both
> > rq_pin_lock() and rq_repin_lock(). That is:
> > 
> > 	rq_pin_lock()
> > 	  rq_unpin_lock()
> >           rq_repin_lock()
> > 	rq_unpin_lock()
> > 
> > In prepration of making the temporary drop and the final drop
> 
> nit. 'prepration' should be 'preparation' :-).

I shall forever be making typos :-) fixed.

Re: [PATCH v2 10/12] sched/fair: Reflow pick_task_fair() / newidle

[Tejun Heo]

Hello,

On Sat, Nov 07, 2026 at 12:59:23PM +0100, Peter Zijlstra wrote:
>  again:
> -	if (!cfs_rq->h_nr_queued)
> -		goto idle;
> +	if (unlikely(!cfs_rq->h_nr_queued)) {
> +		rq_modified_begin(rq, &fair_sched_class);
> +		sched_balance_newidle(rq, rf);

pick_next_task() clears rq->core->core_cookie at restart: and sets it
again only after all the siblings are picked. As newidle can now drop
the core lock in between, another sibling can see the temporary zero
and take the !need_sync fast path while a cookied task is still running
on the core. This is another SMT>2 problem:

  CPU3 is running Y, core_cookie == Y's cookie
  CPU0 pick_next_task(), core_cookie = 0
       pick_task(rq_i), newidle drops the core lock
  CPU1 pick_next_task(), core_cookie == 0 so !need_sync
       picks uncookied Z and switches to it
  CPU0 relocks and restarts as core_task_seq moved

Z and Y share the core until CPU0's selection completes and its IPIs
land. On an SMT4 VM, this triggers a couple of times per second under
core-sched churn. Maybe core_cookie can be left alone at restart: so
that it keeps its previous value until the new one is set after the
loop?

Also, core-sched churn now triggers assert_clock_updated() from
__schedule(), set_next_task_fair(), update_load_avg() and so on. While
the lock is dropped for newidle, another CPU's double_lock_balance() or
double_rq_lock() on one of the core's rqs goes through
double_rq_clock_clear_update(), which clears RQCF_UPDATED regardless of
core_pick_in_flight. If the selection doesn't restart, it goes on with
an rq which it already updated now marked as not updated. Maybe
double_rq_clock_clear_update() should follow __rq_clock_clear()?

Thanks.

--
tejun

Re: [PATCH v2 12/12] sched: Remove sched_class::balance()

[Tejun Heo]

Hello,

On Sat, Nov 07, 2026 at 12:59:25PM +0100, Peter Zijlstra wrote:
>  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;

This exposes an underlying problem on the DL side. rq->next_class is
raised only by wakeup_preempt(), but dl_server_start(), dl_task_timer()
and dl_server_timer() enqueue a DL entity and open-code resched_curr()
without going through it. prev_balance() ran before any class was
picked, so it didn't matter. Now, pick_task_dl() has already returned
NULL when balance_rt() drops the lock, and rq_modified_above() is the
only thing which can notice a DL entity queued in the meantime:

  CPU0 RT task blocks, pick_task_dl() returns NULL
       pick_task_rt(), balance_rt() drops the rq lock
  CPU1 wakes a fair task onto CPU0, dl_server_start()
  CPU0 relocks, rq_modified_above() is false
       picks an RT task with the fair server queued

resched_curr() doesn't help as __schedule() clears it on prev after the
pick. I can trigger this with NO_RT_PUSH_IPI and a delay added in the
lock drop. With RT_PUSH_IPI, pull_rt_task() doesn't drop the lock.

Maybe the DL sites should raise rq->next_class where they enqueue and
resched, so that they behave the same as regular wakeups?

Thanks.

--
tejun

Re: [PATCH v2 00/12] sched: core-sched fixes and balancing

[Tejun Heo]

Hello,

On Sat, Nov 07, 2026 at 12:59:13PM +0100, Peter Zijlstra wrote:
> The main difference is reworking of rq_clock due to TJ noticing the previous
> one got it all wrong. The new patches might seem a little aimless at times;
> this is because my initial idea for fixing the problem didn't pan out, but I
> kept some of the groundwork for it anyway, since it resulted in simpler (I
> hope) code.

I wonder whether the clock management is too subtle, and as Kayra
pointed out on 05/12, saving and restoring the flags doesn't work when
lock sections straddle each other, which core-sched makes common.
Stepping back, I think the clock needs only two things:

1. The wakeup path wants the following clock update to be skipped. This
   doesn't need the REQ/ACT handoff. It can be one bit which says
   "whoever updates the clock next, consider it updated without reading
   it", cleared at the end of __schedule() so that it doesn't leak past
   the reschedule that it was meant for.

2. The clock should stay put across a section including while the lock
   is dropped in the middle. This can be a per-rq depth count.
   rq_pin_lock() increments, rq_unpin_lock() decrements, and
   rq_drop_lock() and rq_repin_lock() don't touch it. update_rq_clock()
   reads the clock only if it hasn't since the count left zero, and
   that state is cleared when the count returns to zero, which is the
   clear on unlock. Whoever locks the rq while a section is dropped
   comes in at depth two and finds the clock already updated, so there's
   no separate freeze state and nothing to save and restore, and it
   doesn't matter in which order the sections end.

Raw lockers don't hold a count, so they'd update each time when no
section is open, and double_rq_clock_clear_update() can go away. For
core-sched, the core-wide selection updates every sibling's clock
anyway, so it can take a count on each sibling as it does that and drop
them when the selection ends.

What do you think?

Thanks.

--
tejun

Re: [PATCH v2 04/12] sched/core: Make __update_rq_clock() skip updates on UPDATED

[Peter Zijlstra]

On Wed, Nov 11, 2026 at 11:28:17AM +0100, Peter Zijlstra wrote:
> On Tue, Nov 10, 2026 at 03:11:28PM -1000, Tejun Heo wrote:
> > Hello, Peter.
> > 
> > On Sat, Nov 07, 2026 at 12:59:17PM +0100, Peter Zijlstra wrote:
> > >  static void __update_rq_clock(struct rq *rq)
> > >  {
> > > +	bool update;
> > > +
> > >  	lockdep_assert_rq_held(rq);
> > >  
> > > +	update = !(rq->clock_update_flags & RQCF_NOP_MASK);
> > >  	rq->clock_update_flags |= RQCF_UPDATED;
> > >  
> > > -	if (!(rq->clock_update_flags & RQCF_ACT_SKIP)) {
> > > +	if (update) {
> > 
> > RQCF_UPDATED is cleared only by rq_pin_lock() and
> > double_rq_clock_clear_update(), so a path which takes the rq lock with
> > raw_spin_rq_lock() inherits it from the previous holder and its
> > update_rq_clock() is now silently skipped.
> 
> Let me look at that specific case.
> 
> > Previously, you said to clear on unlock. Why not do that? That'd be less
> > confusing overall.
> 
> Yes, that's what I tried, but it doesn't work for core-sched. What
> happens is that SMT0 and SMT1 interleave and things go to hell, patch 6
> has that picture.
> 
> That is, we can still clear on unlock too, but clear on unlock alone
> isn't sufficient :/
> 
> Patch 6 even has the clear on unlock, but commented out.

Right, so the complication is that for core I want the rq-clock to be
'frozen' over the rq-lock drop.

Notably, I want dequeue and pick / set-next to have the same timestamp
such that the whole of schedule appears to happen at one instant in
time.

For ext you've explicitly chosen to not do so, per the placement of
scx_rq_clock_invalidate().

The consequence is that the raw_spin_rq_lock() functions must remain
independent of rq_clock management; otherwise we cannot drop rq-lock
while keeping time frozen.

This means the rq-clock management must live in {pin,drop,repin,unpin};
and at that point, I didn't really see an advantage in doing it at
unpin.

Re: [PATCH v2 10/12] sched/fair: Reflow pick_task_fair() / newidle

[Peter Zijlstra]

On Wed, Nov 11, 2026 at 01:02:45AM -1000, Tejun Heo wrote:

> an rq which it already updated now marked as not updated. Maybe
> double_rq_clock_clear_update() should follow __rq_clock_clear()?

Yes, definitely. I seem to have completely missed
double_rq_clock_clear_update() and somehow my test run; I promise, I
did one (although I didn't test sched_ext at all); didn't trigger
nothing :/

Re: [PATCH v2 12/12] sched: Remove sched_class::balance()

[Peter Zijlstra]

On Wed, Nov 11, 2026 at 01:18:42AM -1000, Tejun Heo wrote:
> Hello,
> 
> On Sat, Nov 07, 2026 at 12:59:25PM +0100, Peter Zijlstra wrote:
> >  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;
> 
> This exposes an underlying problem on the DL side. rq->next_class is
> raised only by wakeup_preempt(), but dl_server_start(), dl_task_timer()
> and dl_server_timer() enqueue a DL entity and open-code resched_curr()
> without going through it. prev_balance() ran before any class was
> picked, so it didn't matter. Now, pick_task_dl() has already returned
> NULL when balance_rt() drops the lock, and rq_modified_above() is the
> only thing which can notice a DL entity queued in the meantime:
> 
>   CPU0 RT task blocks, pick_task_dl() returns NULL
>        pick_task_rt(), balance_rt() drops the rq lock
>   CPU1 wakes a fair task onto CPU0, dl_server_start()
>   CPU0 relocks, rq_modified_above() is false
>        picks an RT task with the fair server queued

A very nice catch, but I'm not entirely convinced it wasn't a problem
before.

Similar argument could be made for FAIR vs EXT, and picking a FAIR task
while an EXT server is pickable.

> resched_curr() doesn't help as __schedule() clears it on prev after the
> pick. I can trigger this with NO_RT_PUSH_IPI and a delay added in the
> lock drop. With RT_PUSH_IPI, pull_rt_task() doesn't drop the lock.
> 
> Maybe the DL sites should raise rq->next_class where they enqueue and
> resched, so that they behave the same as regular wakeups?

Yes, something like that. I think the below captures all enqueues,
irrespective of rescheds. I'll add it as fix at the start of the series.

(although maybe I'll massage this a little more and create a helper,
doing the same thing over and over like this is daft)

Thanks!

---
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -715,8 +715,7 @@ static struct rq *dl_task_offline_migrat
 	return later_rq;
 }
 
-static void
-enqueue_dl_entity(struct sched_dl_entity *dl_se, int flags);
+static bool enqueue_dl_entity(struct sched_dl_entity *dl_se, int flags);
 static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags);
 static void dequeue_dl_entity(struct sched_dl_entity *dl_se, int flags);
 static void wakeup_preempt_dl(struct rq *rq, struct task_struct *p, int flags);
@@ -1184,7 +1183,8 @@ static enum hrtimer_restart dl_server_ti
 			dl_se->dl_defer_running = 1;
 		}
 
-		enqueue_dl_entity(dl_se, ENQUEUE_REPLENISH);
+		if (enqueue_dl_entity(dl_se, ENQUEUE_REPLENISH))
+			rq->next_class = &dl_sched_class;
 
 		if (!dl_task(dl_se->rq->curr) || dl_entity_preempt(dl_se, &dl_se->rq->curr->dl))
 			resched_curr(rq);
@@ -1520,7 +1520,8 @@ static void update_curr_dl_se(struct rq
 					replenish_dl_new_period(dl_se, rq);
 					start_dl_timer(dl_se);
 				} else {
-					enqueue_dl_entity(dl_se, ENQUEUE_REPLENISH);
+					if (enqueue_dl_entity(dl_se, ENQUEUE_REPLENISH))
+						rq->next_class = &dl_sched_class;
 				}
 			} else {
 				enqueue_task_dl(rq, dl_task_of(dl_se), ENQUEUE_REPLENISH);
@@ -1812,7 +1813,8 @@ void dl_server_start(struct sched_dl_ent
 
 	trace_sched_dl_server_start_tp(dl_se, cpu_of(rq), dl_get_type(dl_se, rq));
 	dl_se->dl_server_active = 1;
-	enqueue_dl_entity(dl_se, ENQUEUE_WAKEUP);
+	if (enqueue_dl_entity(dl_se, ENQUEUE_WAKEUP))
+		rq->next_class = &dl_sched_class;
 	if (!dl_task(dl_se->rq->curr) || dl_entity_preempt(dl_se, &rq->curr->dl))
 		resched_curr(dl_se->rq);
 }
@@ -2379,7 +2381,7 @@ static void __dequeue_dl_entity(struct s
 	dec_dl_tasks(dl_se, dl_rq);
 }
 
-static void
+static bool
 enqueue_dl_entity(struct sched_dl_entity *dl_se, int flags)
 {
 	WARN_ON_ONCE(on_dl_rq(dl_se));
@@ -2418,7 +2420,7 @@ enqueue_dl_entity(struct sched_dl_entity
 		if (flags & ENQUEUE_WAKEUP)
 			task_contending(dl_se, flags);
 
-		return;
+		return false;
 	}
 
 	/*
@@ -2442,7 +2444,7 @@ enqueue_dl_entity(struct sched_dl_entity
 	 * deferred task and still got to wait, don't enqueue.
 	 */
 	if (dl_se->dl_throttled && start_dl_timer(dl_se))
-		return;
+		return false;
 
 	/*
 	 * We're about to enqueue, make sure we're not ->dl_throttled!
@@ -2457,6 +2459,7 @@ enqueue_dl_entity(struct sched_dl_entity
 	}
 
 	__enqueue_dl_entity(dl_se);
+	return true;
 }
 
 static void dequeue_dl_entity(struct sched_dl_entity *dl_se, int flags)
@@ -2534,7 +2537,8 @@ static void enqueue_task_dl(struct rq *r
 	if (task_on_rq_migrating(p))
 		flags |= ENQUEUE_MIGRATING;
 
-	enqueue_dl_entity(dl_se, flags);
+	if (enqueue_dl_entity(dl_se, flags))
+		rq->next_class = &dl_sched_class;
 
 	if (dl_server(dl_se))
 		return;

Re: [PATCH v2 12/12] sched: Remove sched_class::balance()

[Peter Zijlstra]

On Wed, Nov 11, 2026 at 02:05:12PM +0100, Peter Zijlstra wrote:
> On Wed, Nov 11, 2026 at 01:18:42AM -1000, Tejun Heo wrote:
> > Hello,
> > 
> > On Sat, Nov 07, 2026 at 12:59:25PM +0100, Peter Zijlstra wrote:
> > >  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;
> > 
> > This exposes an underlying problem on the DL side. rq->next_class is
> > raised only by wakeup_preempt(), but dl_server_start(), dl_task_timer()
> > and dl_server_timer() enqueue a DL entity and open-code resched_curr()
> > without going through it. prev_balance() ran before any class was
> > picked, so it didn't matter. Now, pick_task_dl() has already returned
> > NULL when balance_rt() drops the lock, and rq_modified_above() is the
> > only thing which can notice a DL entity queued in the meantime:
> > 
> >   CPU0 RT task blocks, pick_task_dl() returns NULL
> >        pick_task_rt(), balance_rt() drops the rq lock
> >   CPU1 wakes a fair task onto CPU0, dl_server_start()
> >   CPU0 relocks, rq_modified_above() is false
> >        picks an RT task with the fair server queued
> 
> A very nice catch, but I'm not entirely convinced it wasn't a problem
> before.
> 
> Similar argument could be made for FAIR vs EXT, and picking a FAIR task
> while an EXT server is pickable.
> 
> > resched_curr() doesn't help as __schedule() clears it on prev after the
> > pick. I can trigger this with NO_RT_PUSH_IPI and a delay added in the
> > lock drop. With RT_PUSH_IPI, pull_rt_task() doesn't drop the lock.
> > 
> > Maybe the DL sites should raise rq->next_class where they enqueue and
> > resched, so that they behave the same as regular wakeups?
> 
> Yes, something like that. I think the below captures all enqueues,
> irrespective of rescheds. I'll add it as fix at the start of the series.
> 
> (although maybe I'll massage this a little more and create a helper,
> doing the same thing over and over like this is daft)
> 
> Thanks!

---
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -715,8 +715,7 @@ static struct rq *dl_task_offline_migrat
 	return later_rq;
 }
 
-static void
-enqueue_dl_entity(struct sched_dl_entity *dl_se, int flags);
+static void enqueue_dl_entity(struct rq *rq, struct sched_dl_entity *dl_se, int flags);
 static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags);
 static void dequeue_dl_entity(struct sched_dl_entity *dl_se, int flags);
 static void wakeup_preempt_dl(struct rq *rq, struct task_struct *p, int flags);
@@ -1184,7 +1183,7 @@ static enum hrtimer_restart dl_server_ti
 			dl_se->dl_defer_running = 1;
 		}
 
-		enqueue_dl_entity(dl_se, ENQUEUE_REPLENISH);
+		enqueue_dl_entity(rq, dl_se, ENQUEUE_REPLENISH);
 
 		if (!dl_task(dl_se->rq->curr) || dl_entity_preempt(dl_se, &dl_se->rq->curr->dl))
 			resched_curr(rq);
@@ -1520,7 +1519,7 @@ static void update_curr_dl_se(struct rq
 					replenish_dl_new_period(dl_se, rq);
 					start_dl_timer(dl_se);
 				} else {
-					enqueue_dl_entity(dl_se, ENQUEUE_REPLENISH);
+					enqueue_dl_entity(rq, dl_se, ENQUEUE_REPLENISH);
 				}
 			} else {
 				enqueue_task_dl(rq, dl_task_of(dl_se), ENQUEUE_REPLENISH);
@@ -1812,7 +1811,7 @@ void dl_server_start(struct sched_dl_ent
 
 	trace_sched_dl_server_start_tp(dl_se, cpu_of(rq), dl_get_type(dl_se, rq));
 	dl_se->dl_server_active = 1;
-	enqueue_dl_entity(dl_se, ENQUEUE_WAKEUP);
+	enqueue_dl_entity(rq, dl_se, ENQUEUE_WAKEUP);
 	if (!dl_task(dl_se->rq->curr) || dl_entity_preempt(dl_se, &rq->curr->dl))
 		resched_curr(dl_se->rq);
 }
@@ -2379,8 +2378,8 @@ static void __dequeue_dl_entity(struct s
 	dec_dl_tasks(dl_se, dl_rq);
 }
 
-static void
-enqueue_dl_entity(struct sched_dl_entity *dl_se, int flags)
+static bool
+_enqueue_dl_entity(struct sched_dl_entity *dl_se, int flags)
 {
 	WARN_ON_ONCE(on_dl_rq(dl_se));
 
@@ -2418,7 +2417,7 @@ enqueue_dl_entity(struct sched_dl_entity
 		if (flags & ENQUEUE_WAKEUP)
 			task_contending(dl_se, flags);
 
-		return;
+		return false;
 	}
 
 	/*
@@ -2442,7 +2441,7 @@ enqueue_dl_entity(struct sched_dl_entity
 	 * deferred task and still got to wait, don't enqueue.
 	 */
 	if (dl_se->dl_throttled && start_dl_timer(dl_se))
-		return;
+		return false;
 
 	/*
 	 * We're about to enqueue, make sure we're not ->dl_throttled!
@@ -2457,6 +2456,13 @@ enqueue_dl_entity(struct sched_dl_entity
 	}
 
 	__enqueue_dl_entity(dl_se);
+	return true;
+}
+
+static void enqueue_dl_entity(struct rq *rq, struct sched_dl_entity *dl_se, int flags)
+{
+	if (_enqueue_dl_entity(dl_se, flags))
+		rq->next_class = &dl_sched_class;
 }
 
 static void dequeue_dl_entity(struct sched_dl_entity *dl_se, int flags)
@@ -2534,7 +2540,7 @@ static void enqueue_task_dl(struct rq *r
 	if (task_on_rq_migrating(p))
 		flags |= ENQUEUE_MIGRATING;
 
-	enqueue_dl_entity(dl_se, flags);
+	enqueue_dl_entity(rq, dl_se, flags);
 
 	if (dl_server(dl_se))
 		return;

Re: [PATCH v2 00/12] sched: core-sched fixes and balancing

[Peter Zijlstra]

On Wed, Nov 11, 2026 at 01:46:16AM -1000, Tejun Heo wrote:
> Hello,
> 
> On Sat, Nov 07, 2026 at 12:59:13PM +0100, Peter Zijlstra wrote:
> > The main difference is reworking of rq_clock due to TJ noticing the previous
> > one got it all wrong. The new patches might seem a little aimless at times;
> > this is because my initial idea for fixing the problem didn't pan out, but I
> > kept some of the groundwork for it anyway, since it resulted in simpler (I
> > hope) code.
> 
> I wonder whether the clock management is too subtle, and as Kayra
> pointed out on 05/12, saving and restoring the flags doesn't work when
> lock sections straddle each other, which core-sched makes common.
> Stepping back, I think the clock needs only two things:
> 
> 1. The wakeup path wants the following clock update to be skipped. This
>    doesn't need the REQ/ACT handoff. It can be one bit which says
>    "whoever updates the clock next, consider it updated without reading
>    it", cleared at the end of __schedule() so that it doesn't leak past
>    the reschedule that it was meant for.
> 
> 2. The clock should stay put across a section including while the lock
>    is dropped in the middle. This can be a per-rq depth count.
>    rq_pin_lock() increments, rq_unpin_lock() decrements, and
>    rq_drop_lock() and rq_repin_lock() don't touch it. update_rq_clock()
>    reads the clock only if it hasn't since the count left zero, and
>    that state is cleared when the count returns to zero, which is the
>    clear on unlock. Whoever locks the rq while a section is dropped
>    comes in at depth two and finds the clock already updated, so there's
>    no separate freeze state and nothing to save and restore, and it
>    doesn't matter in which order the sections end.
> 
> Raw lockers don't hold a count, so they'd update each time when no
> section is open, and double_rq_clock_clear_update() can go away. For
> core-sched, the core-wide selection updates every sibling's clock
> anyway, so it can take a count on each sibling as it does that and drop
> them when the selection ends.
> 
> What do you think?

Yeah, let me to play around with all that.

Re: [PATCH v2 12/12] sched: Remove sched_class::balance()

[Kayra Cizmeci]

Hi Peter!

I was just going to sleep but I thought I'll finish reviewing this, then
sleep or maybe don't sleep.

Big mistake. Nevermind let me have a look...

On Wed, Nov 11, 2026 at 02:11:52PM +0100, Peter Zijlstra wrote:
> On Wed, Nov 11, 2026 at 02:05:12PM +0100, Peter Zijlstra wrote:
> > On Wed, Nov 11, 2026 at 01:18:42AM -1000, Tejun Heo wrote:
> > > Hello,
> > > 
> > > On Sat, Nov 07, 2026 at 12:59:25PM +0100, Peter Zijlstra wrote:
> > > >  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;
> > > 
> > > This exposes an underlying problem on the DL side. rq->next_class is
> > > raised only by wakeup_preempt(), but dl_server_start(), dl_task_timer()
> > > and dl_server_timer() enqueue a DL entity and open-code resched_curr()
> > > without going through it. prev_balance() ran before any class was
> > > picked, so it didn't matter. Now, pick_task_dl() has already returned
> > > NULL when balance_rt() drops the lock, and rq_modified_above() is the
> > > only thing which can notice a DL entity queued in the meantime:
> > > 
> > >   CPU0 RT task blocks, pick_task_dl() returns NULL
> > >        pick_task_rt(), balance_rt() drops the rq lock
> > >   CPU1 wakes a fair task onto CPU0, dl_server_start()
> > >   CPU0 relocks, rq_modified_above() is false
> > >        picks an RT task with the fair server queued
> > 
> > A very nice catch, but I'm not entirely convinced it wasn't a problem
> > before.
> > 
> > Similar argument could be made for FAIR vs EXT, and picking a FAIR task
> > while an EXT server is pickable.
> > 
> > > resched_curr() doesn't help as __schedule() clears it on prev after the
> > > pick. I can trigger this with NO_RT_PUSH_IPI and a delay added in the
> > > lock drop. With RT_PUSH_IPI, pull_rt_task() doesn't drop the lock.
> > > 
> > > Maybe the DL sites should raise rq->next_class where they enqueue and
> > > resched, so that they behave the same as regular wakeups?
> > 
> > Yes, something like that. I think the below captures all enqueues,
> > irrespective of rescheds. I'll add it as fix at the start of the series.
> > 
> > (although maybe I'll massage this a little more and create a helper,
> > doing the same thing over and over like this is daft)
> > 
> > Thanks!
> 
> ---
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -715,8 +715,7 @@ static struct rq *dl_task_offline_migrat
>  	return later_rq;
>  }
>  
> -static void
> -enqueue_dl_entity(struct sched_dl_entity *dl_se, int flags);
> +static void enqueue_dl_entity(struct rq *rq, struct sched_dl_entity *dl_se, int flags);
>  static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags);
>  static void dequeue_dl_entity(struct sched_dl_entity *dl_se, int flags);
>  static void wakeup_preempt_dl(struct rq *rq, struct task_struct *p, int flags);
> @@ -1184,7 +1183,7 @@ static enum hrtimer_restart dl_server_ti
>  			dl_se->dl_defer_running = 1;
>  		}
>  
> -		enqueue_dl_entity(dl_se, ENQUEUE_REPLENISH);
> +		enqueue_dl_entity(rq, dl_se, ENQUEUE_REPLENISH);
>  
>  		if (!dl_task(dl_se->rq->curr) || dl_entity_preempt(dl_se, &dl_se->rq->curr->dl))
>  			resched_curr(rq);
> @@ -1520,7 +1519,7 @@ static void update_curr_dl_se(struct rq
>  					replenish_dl_new_period(dl_se, rq);
>  					start_dl_timer(dl_se);
>  				} else {
> -					enqueue_dl_entity(dl_se, ENQUEUE_REPLENISH);
> +					enqueue_dl_entity(rq, dl_se, ENQUEUE_REPLENISH);
>  				}
>  			} else {
>  				enqueue_task_dl(rq, dl_task_of(dl_se), ENQUEUE_REPLENISH);
> @@ -1812,7 +1811,7 @@ void dl_server_start(struct sched_dl_ent
>  
>  	trace_sched_dl_server_start_tp(dl_se, cpu_of(rq), dl_get_type(dl_se, rq));
>  	dl_se->dl_server_active = 1;
> -	enqueue_dl_entity(dl_se, ENQUEUE_WAKEUP);
> +	enqueue_dl_entity(rq, dl_se, ENQUEUE_WAKEUP);
>  	if (!dl_task(dl_se->rq->curr) || dl_entity_preempt(dl_se, &rq->curr->dl))
>  		resched_curr(dl_se->rq);
>  }
> @@ -2379,8 +2378,8 @@ static void __dequeue_dl_entity(struct s
>  	dec_dl_tasks(dl_se, dl_rq);
>  }
>  
> -static void
> -enqueue_dl_entity(struct sched_dl_entity *dl_se, int flags)
> +static bool
> +_enqueue_dl_entity(struct sched_dl_entity *dl_se, int flags)
>  {
>  	WARN_ON_ONCE(on_dl_rq(dl_se));
>  
> @@ -2418,7 +2417,7 @@ enqueue_dl_entity(struct sched_dl_entity
>  		if (flags & ENQUEUE_WAKEUP)
>  			task_contending(dl_se, flags);
>  
> -		return;
> +		return false;
>  	}
>  
>  	/*
> @@ -2442,7 +2441,7 @@ enqueue_dl_entity(struct sched_dl_entity
>  	 * deferred task and still got to wait, don't enqueue.
>  	 */
>  	if (dl_se->dl_throttled && start_dl_timer(dl_se))
> -		return;
> +		return false;
>  
>  	/*
>  	 * We're about to enqueue, make sure we're not ->dl_throttled!
> @@ -2457,6 +2456,13 @@ enqueue_dl_entity(struct sched_dl_entity
>  	}
>  
>  	__enqueue_dl_entity(dl_se);
> +	return true;
> +}
> +
> +static void enqueue_dl_entity(struct rq *rq, struct sched_dl_entity *dl_se, int flags)
> +{
> +	if (_enqueue_dl_entity(dl_se, flags))
> +		rq->next_class = &dl_sched_class;
>  }
>  
>  static void dequeue_dl_entity(struct sched_dl_entity *dl_se, int flags)
> @@ -2534,7 +2540,7 @@ static void enqueue_task_dl(struct rq *r
>  	if (task_on_rq_migrating(p))
>  		flags |= ENQUEUE_MIGRATING;
>  
> -	enqueue_dl_entity(dl_se, flags);
> +	enqueue_dl_entity(rq, dl_se, flags);
>  
>  	if (dl_server(dl_se))
>  		return;

Let's say a fair task named TA is working in CPU1, a DL task named DA is
waking up. We enter the normal wakeup path, and call enqueue_task_dl()
and from there enqueue_dl_entity(). enqueue_dl_entity() sets next_class
to dl_sched_class. Later enqueue, when we call wakeup_preempt TA does
not gets preempted because:

void wakeup_preempt(struct rq *rq, struct task_struct *p, int flags)
{
	struct task_struct *donor = rq->donor;

	if (p->sched_class == rq->next_class) {
		rq->next_class->wakeup_preempt(rq, p, flags);

We already set next_class to DL, so we call wakeup_preempt_dl().

And it just early returns.

So, we have a fair task working while a new woken DL task just sits
around...

I also tested this:

On hackery, the latency of DA was 12 us, with hackery + above diff it
was 200 ms. On idle, it was 12 us to 3 seconds.

I'm not sure what is the cleanest fix tho. So this time there is no
diff.

Hope I'm not missing something :-).



Cheers,

Kayra

Re: [PATCH v2 12/12] sched: Remove sched_class::balance()

[Peter Zijlstra]

On Wed, Nov 11, 2026 at 05:57:01PM +0300, Kayra Cizmeci wrote:
> Hi Peter!

> Let's say a fair task named TA is working in CPU1, a DL task named DA is
> waking up. We enter the normal wakeup path, and call enqueue_task_dl()
> and from there enqueue_dl_entity(). enqueue_dl_entity() sets next_class
> to dl_sched_class. Later enqueue, when we call wakeup_preempt TA does
> not gets preempted because:
> 
> void wakeup_preempt(struct rq *rq, struct task_struct *p, int flags)
> {
> 	struct task_struct *donor = rq->donor;
> 
> 	if (p->sched_class == rq->next_class) {
> 		rq->next_class->wakeup_preempt(rq, p, flags);
> 

Gah, you're right. This don't work right. Let me try again.