[PATCH v2 3/6] timekeeping: Bound idle sleep while a phase slew is in flight

[David Woodhouse]

From: David Woodhouse <dwmw@xxxxxxxxxxxx>

Phase offset for adjtime() can be delivered at a rate of 500µs/s, and
for STA_PPSTIME potentially even more. The skew is recalculated in
second_overflow() each second, and if that's missed in a tickless
kernel, the skew continues to be applied for the whole of the idle
period. While skew is active, clamp timekeeping_max_deferment() to the
top of the current second to avoid the overshoot.

The bound is only evaluated when a CPU stops its tick, so it helps only
if some CPU does that after the slew starts. If the do_timer duty is
vacant and its last holder is already sleeping on a timer programmed
before the slew began, nothing enforces the wakeup until an unrelated
interrupt arrives. Claim the vacant duty for the CPU performing the
adjtimex() call, exactly as if its tick had fired: when it next stops
its tick it becomes the duty-last CPU and applies the bound.

Acked-by: John Stultz <jstultz@xxxxxxxxxx>
Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
---
v2: Fix the ADJ_OFFSET_READONLY test: that flag aliases ADJ_NANO and
    only has its read-only meaning in the ADJ_ADJTIME form, so the v1
    check skipped the immediate update for the standard
    ADJ_OFFSET|ADJ_NANO call. Also trigger it for ADJ_TIMECONST and
    ADJ_STATUS, which likewise invalidate the skew rate, mirroring the
    condition in process_adjtimex_modes(). (Found by Sashiko.)

    Claim the do_timer duty when starting a slew, so the deferment
    bound is honoured even when every other CPU is already asleep.
    The vacancy is real: in a 4-CPU qemu guest, a slew started from a
    non-timer wakeup found the duty vacant (and claimed it) on 3 out
    of 3 attempts. The overshoot itself is harder to demonstrate
    there, because ambient interrupts advance the timekeeper at
    better than 1Hz anyway.

 kernel/time/tick-internal.h | 18 +++++++++++++++
 kernel/time/timekeeping.c   | 44 +++++++++++++++++++++++++++++++++++--
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 65680db95053..9328582b1eb7 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -23,6 +23,23 @@ DECLARE_PER_CPU(struct tick_device, tick_cpu_device);
 extern ktime_t tick_next_period;
 extern int tick_do_timer_cpu __read_mostly;
 
+/*
+ * The bound on idle sleep imposed by timekeeping_max_deferment() is only
+ * evaluated by the do_timer duty CPU (or its last holder) when it stops
+ * its tick. If the duty is vacant and its last holder is already asleep
+ * on a timer programmed before the bound became necessary, nothing will
+ * honour it. Claim the vacant duty for the current CPU, exactly as if
+ * its tick had fired: when it next stops its tick, it becomes the
+ * duty-last CPU and applies the bound. Racing with a real tick taking
+ * the duty is benign, as in tick_sched_do_timer().
+ */
+static inline void tick_nohz_claim_do_timer(void)
+{
+	if (IS_ENABLED(CONFIG_NO_HZ_COMMON) && !tick_nohz_full_enabled() &&
+	    READ_ONCE(tick_do_timer_cpu) == TICK_DO_TIMER_NONE)
+		WRITE_ONCE(tick_do_timer_cpu, smp_processor_id());
+}
+
 extern void tick_setup_periodic(struct clock_event_device *dev, int broadcast);
 extern void tick_handle_periodic(struct clock_event_device *dev);
 extern void tick_check_new_device(struct clock_event_device *dev);
@@ -101,6 +118,7 @@ static inline void tick_set_periodic_handler(struct clock_event_device *dev, int
 #else /* !GENERIC_CLOCKEVENTS: */
 static inline void tick_suspend(void) { }
 static inline void tick_resume(void) { }
+static inline void tick_nohz_claim_do_timer(void) { }
 #endif /* !GENERIC_CLOCKEVENTS */
 
 /* Oneshot related functions */
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 118612c4a82e..667b4c2817b5 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1978,6 +1978,21 @@ u64 timekeeping_max_deferment(void)
 
 		ret = tk->tkr_mono.clock->max_idle_ns;
 
+		/*
+		 * Skew introduced for phase offset is intended to be
+		 * recalculated each second. Ensure a tickless kernel
+		 * wakes up for that and does not overshoot, while skew
+		 * is being applied.
+		 */
+		if (abs(tk->skew_delta) > 1) {
+			u64 nsec_in_sec = tk->tkr_mono.xtime_nsec >>
+					  tk->tkr_mono.shift;
+			u64 remaining = NSEC_PER_SEC - min_t(u64, nsec_in_sec,
+							     NSEC_PER_SEC);
+
+			ret = min_t(u64, ret, max_t(u64, remaining, TICK_NSEC));
+		}
+
 	} while (read_seqcount_retry(&tk_core.seq, seq));
 
 	return ret;
@@ -3062,9 +3077,34 @@ static int __do_adjtimex(struct tk_data *tkd, struct __kernel_timex *txc,
 		tk_update_leap_state_all(tkd);
 	}
 
-	/* Update the multiplier immediately if frequency was set directly */
-	if (txc->modes & (ADJ_FREQUENCY | ADJ_TICK))
+	/*
+	 * Update the multiplier immediately if the frequency was set
+	 * directly, and refresh the cached skew_delta for anything
+	 * which may have changed the skew rate — mirroring the
+	 * condition in process_adjtimex_modes() — so that it bounds
+	 * idle sleep while the slew is in flight.
+	 *
+	 * In the old-style ADJ_ADJTIME form the mode bits mean
+	 * different things (ADJ_OFFSET_READONLY aliases ADJ_NANO);
+	 * only the write form of adjtime() starts a slew.
+	 *
+	 * Also claim the do_timer duty if it is vacant, so that the
+	 * deferment bound is honoured even if every other CPU is
+	 * already sleeping on a timer programmed before the slew
+	 * began (not for the aux clocks, which are not bounded).
+	 */
+	if (txc->modes & ADJ_ADJTIME) {
+		if (!(txc->modes & ADJ_OFFSET_READONLY)) {
+			if (!aux_clock)
+				tick_nohz_claim_do_timer();
+			result->clock_set |= __timekeeping_advance(tkd, TK_ADV_FREQ);
+		}
+	} else if (txc->modes & (ADJ_FREQUENCY | ADJ_TICK | ADJ_OFFSET |
+				 ADJ_TIMECONST | ADJ_STATUS)) {
+		if (!aux_clock)
+			tick_nohz_claim_do_timer();
 		result->clock_set |= __timekeeping_advance(tkd, TK_ADV_FREQ);
+	}
 
 	return ret;
 }
-- 
2.43.0