[PATCH 2/2] timekeeping: Use inlined counter read in ktime_get_snapshot_id()

[David Woodhouse]

From: David Woodhouse <dwmw@xxxxxxxxxxxx>

Every instruction between a PPS/hwstamp caller's triggering event and
the counter read inside ktime_get_snapshot_id() displaces the returned
timestamp from that event. When the clocksource supports inlined
reads, read the counter directly in tk_clock_read_snapshot(), avoiding
the indirect call. The inlined read cannot provide a hardware
snapshot, so clear the chs struct after the read instead of
beforehand.

Also bypass the clock_id case statement for CLOCK_REALTIME, which is the
common and latency-critical caller.

Measured on a Cortex-A53 at 1.35GHz (12.5MHz arch counter), with
inlined clocksource reads enabled: the interval from a raw counter
read in the caller to the counter read inside the function drops from
49.9ns to 41.2ns with the inline read, and to 38.8ns with the dispatch
bias, mean of 1M iterations.

Assisted-by: LLM
Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
---
 kernel/time/timekeeping.c | 110 +++++++++++++++++++++++++-------------
 1 file changed, 73 insertions(+), 37 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index ea2e6e55f37b..f0768efb8cff 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -304,6 +304,30 @@ static __always_inline u64 tk_clock_read(const struct tk_read_base *tkr)
 	return clock->read(clock);
 }
 
+static inline u64 tk_clock_read_snapshot(const struct tk_read_base *tkr,
+					 struct clocksource_hw_snapshot *chs)
+{
+	struct clocksource *clock = READ_ONCE(tkr->clock);
+	u64 count;
+
+	if (static_branch_likely(&clocksource_read_inlined)) {
+		count = arch_inlined_clocksource_read(clock);
+	} else if (unlikely(clock->read_snapshot)) {
+		*chs = (struct clocksource_hw_snapshot){ };
+		return clock->read_snapshot(clock, chs);
+	} else {
+		count = clock->read(clock);
+	}
+
+	/*
+	 * Unless it's *used* by ->read_snapshot(), this can be cleared
+	 * *after* the clocksource is read, allowing the clocksource to
+	 * be sampled sooner.
+	 */
+	*chs = (struct clocksource_hw_snapshot){ };
+	return count;
+}
+
 static inline void clocksource_disable_inline_read(void)
 {
 	static_branch_disable(&clocksource_read_inlined);
@@ -321,6 +345,22 @@ static __always_inline u64 tk_clock_read(const struct tk_read_base *tkr)
 	return clock->read(clock);
 }
 
+static inline u64 tk_clock_read_snapshot(const struct tk_read_base *tkr,
+					 struct clocksource_hw_snapshot *chs)
+{
+	struct clocksource *clock = READ_ONCE(tkr->clock);
+	u64 count;
+
+	if (unlikely(clock->read_snapshot)) {
+		*chs = (struct clocksource_hw_snapshot){ };
+		return clock->read_snapshot(clock, chs);
+	}
+
+	count = clock->read(clock);
+	*chs = (struct clocksource_hw_snapshot){ };
+	return count;
+}
+
 static inline void clocksource_disable_inline_read(void) { }
 static inline void clocksource_enable_inline_read(void) { }
 #endif
@@ -1219,18 +1259,6 @@ noinstr time64_t __ktime_get_real_seconds(void)
 	return READ_ONCE(tk->xtime_sec);
 }
 
-static inline u64 tk_clock_read_snapshot(const struct tk_read_base *tkr,
-					 struct clocksource_hw_snapshot *chs)
-{
-	struct clocksource *clock = READ_ONCE(tkr->clock);
-
-	if (unlikely(clock->read_snapshot))
-		return clock->read_snapshot(clock, chs);
-
-	return clock->read(clock);
-}
-
-
 /**
  * ktime_get_snapshot_id -  Simultaneously snapshot a given clock ID with
  *			    the corresponding monotonic raw and the underlying
@@ -1263,36 +1291,41 @@ void ktime_get_snapshot_id(clockid_t clock_id, struct system_time_snapshot *syst
 	if (WARN_ON_ONCE(timekeeping_suspended))
 		return;
 
-	switch (clock_id) {
-	case CLOCK_REALTIME:
-		tkd = &tk_core;
-		offs = &tk_core.timekeeper.offs_real;
-		break;
-	/* Map RAW to MONOTONIC so the loop below is trivial */
-	case CLOCK_MONOTONIC_RAW:
-	case CLOCK_MONOTONIC:
-		tkd = &tk_core;
-		offs = &offs_zero;
-		break;
-	case CLOCK_BOOTTIME:
-		tkd = &tk_core;
-		offs = &tk_core.timekeeper.offs_boot;
-		break;
-	case CLOCK_AUX ... CLOCK_AUX_LAST:
-		tkd = aux_get_tk_data(clock_id);
-		if (!tkd)
+	/*
+	 * Bias the dispatch for CLOCK_REALTIME: it is the overwhelmingly
+	 * common caller (PPS, PTP cross-timestamps) and the most
+	 * latency-critical -- everything ahead of the counter read
+	 * displaces those callers' timestamps from their triggering event.
+	 */
+	tkd = &tk_core;
+	offs = &tk_core.timekeeper.offs_real;
+
+	if (unlikely(clock_id != CLOCK_REALTIME)) {
+		switch (clock_id) {
+		/* Map RAW to MONOTONIC so the loop below is trivial */
+		case CLOCK_MONOTONIC_RAW:
+		case CLOCK_MONOTONIC:
+			offs = &offs_zero;
+			break;
+		case CLOCK_BOOTTIME:
+			offs = &tk_core.timekeeper.offs_boot;
+			break;
+		case CLOCK_AUX ... CLOCK_AUX_LAST:
+			tkd = aux_get_tk_data(clock_id);
+			if (!tkd)
+				return;
+			offs = &tkd->timekeeper.offs_aux;
+			break;
+		default:
+			WARN_ON_ONCE(1);
 			return;
-		offs = &tkd->timekeeper.offs_aux;
-		break;
-	default:
-		WARN_ON_ONCE(1);
-		return;
+		}
 	}
 
 	tk = &tkd->timekeeper;
 
 	do {
-		struct clocksource_hw_snapshot chs = { };
+		struct clocksource_hw_snapshot chs;
 
 		seq = read_seqcount_begin(&tkd->seq);
 
@@ -1911,8 +1944,11 @@ int timekeeping_notify(struct clocksource *clock)
 	 * If the clocksource has been selected and supports inlined reads
 	 * enable the branch.
 	 */
-	if (tk->tkr_mono.clock == clock && clock->flags & CLOCK_SOURCE_CAN_INLINE_READ)
+	if (tk->tkr_mono.clock == clock && clock->flags & CLOCK_SOURCE_CAN_INLINE_READ) {
+		/* The inlined read cannot provide a hardware snapshot */
+		WARN_ON_ONCE(clock->read_snapshot);
 		clocksource_enable_inline_read();
+	}
 
 	tick_clock_notify();
 	return tk->tkr_mono.clock == clock ? 0 : -1;
-- 
2.43.0