sched/core: Handle pick_task() releasing the rq lock

Core scheduling's pick_next_task() breaks when a ->pick_task()
implementation can release the rq lock. The selection state derived on entry
is only valid while the lock is held continuously. Once a pick can drop the
lock, an interleaving selection can invalidate all of it: the single-CPU
fast path can commit an uncookied pick although the core went cookied during
the release, and forceidle committed by the interleaving selection skews the
restarted pass's accounting.

Fix it by restarting the whole selection when a pick returns RETRY_TASK
after releasing the lock: a single restart point above the state derivation
replaces the per-loop restart labels, so a retry picks up state committed by
interleaving selections and accounts and resets forceidle like a fresh
selection would.

need_sync and fi_before latch across retries. Clock validity can't be
re-derived - there is no program-ordered way to tell whether the own and
core rq clocks are still updated after the lock was released, as other
lockers' pin cycles may or may not have invalidated them. When restarting,
clear core_clock_updated so that the sibling loop re-updates the core rq,
and update the own rq clock if invalidated.

Fixes: 4c95380701f5 ("sched/ext: Fold balance_scx() into pick_task_scx()")
Cc: stable@vger.kernel.org # v6.19+
Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

1 files changed, 19 insertions, 8 deletions

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 3cc6fb1d2054..84ef83316562 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6217,7 +6217,7 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 	unsigned long cookie;
 	int i, cpu, occ = 0;
 	struct rq *rq_i;
-	bool need_sync;
+	bool need_sync = false;
 
 	if (!sched_core_enabled(rq))
 		return __pick_next_task(rq, rf);
@@ -6260,7 +6260,9 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 	prev_balance(rq, rf);
 
 	smt_mask = cpu_smt_mask(cpu);
-	need_sync = !!rq->core->core_cookie;
+
+restart:
+	need_sync |= !!rq->core->core_cookie;
 
 	/* reset state */
 	rq->core->core_cookie = 0UL;
@@ -6295,10 +6297,15 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 	 * and there are no cookied tasks running on siblings.
 	 */
 	if (!need_sync) {
-restart_single:
 		next = pick_task(rq, rf);
-		if (unlikely(next == RETRY_TASK))
-			goto restart_single;
+		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);
+			goto restart;
+		}
+
 		if (!next->core_cookie) {
 			rq->core_pick = NULL;
 			rq->core_dl_server = NULL;
@@ -6318,7 +6325,6 @@ restart_single:
 	 *
 	 * Tie-break prio towards the current CPU
 	 */
-restart_multi:
 	max = NULL;
 	for_each_cpu_wrap(i, smt_mask, cpu) {
 		rq_i = cpu_rq(i);
@@ -6332,8 +6338,13 @@ restart_multi:
 			update_rq_clock(rq_i);
 
 		p = pick_task(rq_i, rf);
-		if (unlikely(p == RETRY_TASK))
-			goto restart_multi;
+		if (unlikely(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);
+			goto restart;
+		}
 
 		rq_i->core_pick = p;
 		rq_i->core_dl_server = rq_i->dl_server;