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

[Peter Zijlstra <peterz@infradead.org>]

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