[PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP

[David Woodhouse <dwmw2@infradead.org>]

From: David Woodhouse <dwmw@amazon.co.uk>

cycle_interval is an integer number of counter cycles per NTP interval,
so the real time it represents differs from the nominal
NTP_INTERVAL_LENGTH by up to half a counter period. For coarse
clocksources this is significant: the 3.579545 MHz ACPI PM timer at
HZ=1000 rounds 3579.545 cycles up to 3580, making each tick 1.000127 ms
(+127 PPM).

Commit a386b5af8edd ("time: Compensate for rounding on odd-frequency
clocksources") introduced xtime_remainder to compensate for exactly
this, citing the same 127 PPM ACPI PM example. The compensation is
correct and necessary, but it was applied inside the timekeeping
accumulation in timekeeping.c: subtracted in the mult computation in
timekeeping_adjust() and folded into the ntp_error update in
logarithmic_accumulation(). That keeps the base rate correct and leaves
NTP its full symmetric +/-MAXFREQ range rather than +373/-627 PPM, but
the NTP code in ntp.c never sees it: tick_length is computed without the
correction, so ntp.c's notion of how long a tick is disagrees with the
rate timekeeping actually produces.

Make the offset an explicit part of the NTP tick_length instead. Add
ntp_data::cs_tick_adj, a fixed per-second addend that
ntp_update_frequency() includes alongside ntp_tick_adj and time_freq.
tk_setup_internals() computes it from the difference between the real
cycle_interval duration and the nominal interval, stores it in the
timekeeper, and hands it to NTP through a new argument to ntp_clear() --
which already recomputes the frequency and is invoked after every
clocksource (re)configuration. timekeeping_init() now uses TK_UPDATE_ALL
for this; clearing NTP there is otherwise redundant since ntp_init() has
just initialised it.

ntp.c now computes the true tick rate, giving a single source of truth.
Like ntp_tick_adj, cs_tick_adj stays internal to the kernel: userspace
still sees the nominal 1.000000 ms tick via adjtimex and is unaware of
the addends. timekeeping_adjust() and logarithmic_accumulation() use
ntp_tick / xtime_interval directly, and xtime_remainder is removed.

The base-rate arithmetic is unchanged: ntp_tick becomes
xtime_interval << ntp_error_shift, so the mult division yields the same
base mult and the ntp_error accumulation still nets to zero per tick.

Beyond the cleanup of treating all the tick_length contributions
(nominal interval, ntp_tick_adj, cs_tick_adj, time_freq) consistently
as addends in one place, it also prepares for feed-forward discipline:
a future timekeeping_set_reference() will set tick_length to track an
absolute external reference such as a vmclock, and that path needs
ntp.c to own a tick_length that already reflects the clocksource
quantisation, with no hidden correction applied elsewhere.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.8
---
 include/linux/timekeeper_internal.h |  8 ++--
 kernel/time/ntp.c                   | 27 ++++++++++--
 kernel/time/ntp_internal.h          |  2 +-
 kernel/time/timekeeping.c           | 66 +++++++++++++++++++++--------
 4 files changed, 78 insertions(+), 25 deletions(-)

diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index e36d11e33e0c..ec81587a1400 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -84,8 +84,6 @@ struct tk_read_base {
  * @cycle_interval:		Number of clock cycles in one NTP interval
  * @xtime_interval:		Number of clock shifted nano seconds in one NTP
  *				interval.
- * @xtime_remainder:		Shifted nano seconds left over when rounding
- *				@cycle_interval
  * @raw_interval:		Shifted raw nano seconds accumulated per NTP interval.
  * @next_leap_ktime:		CLOCK_MONOTONIC time value of a pending leap-second
  * @ntp_tick:			The ntp_tick_length() value currently being
@@ -99,6 +97,10 @@ struct tk_read_base {
  * @ntp_error_shift:		Shift conversion between clock shifted nano seconds and
  *				ntp shifted nano seconds.
  * @ntp_err_mult:		Multiplication factor for scaled math conversion
+ * @cs_tick_adj:		Per-second adjustment handed to NTP via ntp_clear()
+ *				accounting for the difference between the nominal
+ *				NTP interval and the real time taken by the
+ *				clocksource's integer @cycle_interval (upscaled).
  * @skip_second_overflow:	Flag used to avoid updating NTP twice with same second
  * @tai_offset:			The current UTC to TAI offset in seconds
  *
@@ -178,7 +180,6 @@ struct timekeeper {
 
 	u64			cycle_interval;
 	u64			xtime_interval;
-	s64			xtime_remainder;
 	u64			raw_interval;
 
 	ktime_t			next_leap_ktime;
@@ -186,6 +187,7 @@ struct timekeeper {
 	s64			ntp_error;
 	u32			ntp_error_shift;
 	u32			ntp_err_mult;
+	s64			cs_tick_adj;
 	u32			skip_second_overflow;
 	s32			tai_offset;
 };
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 97fa99b96dd0..3fad82c47c4c 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -39,6 +39,10 @@
  * @time_reftime:	Time at last adjustment in seconds
  * @time_adjust:	Adjustment value
  * @ntp_tick_adj:	Constant boot-param configurable NTP tick adjustment (upscaled)
+ * @cs_tick_adj:	Fixed per-second adjustment compensating for the difference
+ *			between the nominal NTP interval and the real time taken
+ *			by the clocksource's integer @cycle_interval (upscaled).
+ *			Set by the timekeeping core via ntp_clear().
  * @ntp_next_leap_sec:	Second value of the next pending leapsecond, or TIME64_MAX if no leap
  *
  * @pps_valid:		PPS signal watchdog counter
@@ -70,6 +74,7 @@ struct ntp_data {
 	time64_t		time_reftime;
 	long			time_adjust;
 	s64			ntp_tick_adj;
+	s64			cs_tick_adj;
 	time64_t		ntp_next_leap_sec;
 #ifdef CONFIG_NTP_PPS
 	int			pps_valid;
@@ -255,6 +260,7 @@ static void ntp_update_frequency(struct ntp_data *ntpdata)
 	second_length		 = (u64)(tick_usec * NSEC_PER_USEC * USER_HZ) << NTP_SCALE_SHIFT;
 
 	second_length		+= ntpdata->ntp_tick_adj;
+	second_length		+= ntpdata->cs_tick_adj;
 	second_length		+= ntpdata->time_freq;
 
 	new_base		 = div_u64(second_length, NTP_INTERVAL_FREQ);
@@ -350,11 +356,26 @@ static void __ntp_clear(struct ntp_data *ntpdata)
 }
 
 /**
- * ntp_clear - Clears the NTP state variables
- * @tkid:	Timekeeper ID to be able to select proper ntp data array member
+ * ntp_clear - Clear NTP state and set the clocksource quantisation adjustment
+ * @tkid:		Timekeeper ID
+ * @cs_tick_adj:	Per-second adjustment in ns << NTP_SCALE_SHIFT
+ *
+ * The timekeeping core uses an integer number of cycles (@cycle_interval)
+ * per NTP interval, so the real time that interval represents differs from
+ * the nominal NTP_INTERVAL_LENGTH by up to half a counter period. Folding
+ * this fixed offset into @cs_tick_adj makes it an explicit part of the NTP
+ * tick_length computation in ntp.c, instead of being applied during
+ * timekeeping accumulation where the NTP code never saw it. Like
+ * @ntp_tick_adj it stays internal to the kernel; userspace still sees the
+ * nominal tick via adjtimex. NTP retains its full symmetric ±MAXFREQ range
+ * around the corrected base rate.
+ *
+ * Called whenever the clocksource is (re)configured, which is also when the
+ * rest of the NTP state must be cleared, so the two are done together.
  */
-void ntp_clear(unsigned int tkid)
+void ntp_clear(unsigned int tkid, s64 cs_tick_adj)
 {
+	tk_ntp_data[tkid].cs_tick_adj = cs_tick_adj;
 	__ntp_clear(&tk_ntp_data[tkid]);
 }
 
diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h
index 7084d839c207..598e5dd2fc5b 100644
--- a/kernel/time/ntp_internal.h
+++ b/kernel/time/ntp_internal.h
@@ -3,7 +3,7 @@
 #define _LINUX_NTP_INTERNAL_H
 
 extern void ntp_init(void);
-extern void ntp_clear(unsigned int tkid);
+extern void ntp_clear(unsigned int tkid, s64 cs_tick_adj);
 /* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
 extern u64 ntp_tick_length(unsigned int tkid);
 extern ktime_t ntp_get_next_leap(unsigned int tkid);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index d847bba0481b..53961a1fcf47 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -339,7 +339,6 @@ static inline void clocksource_enable_inline_read(void) { }
 static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
 {
 	u64 interval;
-	u64 tmp, ntpinterval;
 	struct clocksource *old_clock;
 
 	++tk->cs_was_changed_seq;
@@ -353,20 +352,16 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
 	tk->tkr_raw.cycle_last = tk->tkr_mono.cycle_last;
 
 	/* Do the ns -> cycle conversion first, using original mult */
-	tmp = NTP_INTERVAL_LENGTH;
-	tmp <<= clock->shift;
-	ntpinterval = tmp;
-	tmp += clock->mult/2;
-	do_div(tmp, clock->mult);
-	if (tmp == 0)
-		tmp = 1;
-
-	interval = (u64) tmp;
+	interval = NTP_INTERVAL_LENGTH << clock->shift;
+	interval += clock->mult/2;
+	do_div(interval, clock->mult);
+	if (interval == 0)
+		interval = 1;
+
 	tk->cycle_interval = interval;
 
 	/* Go back from cycles -> shifted ns */
 	tk->xtime_interval = interval * clock->mult;
-	tk->xtime_remainder = ntpinterval - tk->xtime_interval;
 	tk->raw_interval = interval * clock->mult;
 
 	 /* if changing clocks, convert xtime_nsec shift units */
@@ -386,7 +381,38 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
 
 	tk->ntp_error = 0;
 	tk->ntp_error_shift = NTP_SCALE_SHIFT - clock->shift;
-	tk->ntp_tick = ntpinterval << tk->ntp_error_shift;
+
+	/*
+	 * ntp_tick is the tick length that NTP disciplines (its ±500 PPM
+	 * scales only this part), in NTP-shifted ns: the real interval of
+	 * a whole number of counter cycles. Because cycle_interval is
+	 * rounded to an integer number of cycles, this ntp_tick differs
+	 * from the true intended 1/HZ tick length by up to half a cycle
+	 * period.
+	 */
+	tk->ntp_tick = (u64)tk->xtime_interval << tk->ntp_error_shift;
+
+	/*
+	 * cs_tick_adj is the constant difference between the disciplined
+	 * ntp_tick above and the true 1/HZ tick, expressed per-second to
+	 * match the ntp_update_frequency() addends and handed to NTP via
+	 * ntp_clear() to be explicitly included in its tick_length.
+	 *
+	 * Worked example: HZ=1000, ACPI PM timer at 3.579545 MHz, which
+	 * has 3579.545 cycles in 1ms, rounded to cycle_interval = 3580.
+	 *
+	 * So ntp_tick is actually 1.000127ms, as that is the amount of
+	 * time that 3580 cycles will take at the nominal frequency. This
+	 * is the part that NTP disciplines, causing each 3580 counts to
+	 * advance the clock by up to NTP's ±500PPM of that amount.
+	 *
+	 * The "extra" 127ns/tick is what's stored in cs_tick_adj and
+	 * applied as a constant correction by ntp_update_frequency() so
+	 * that NTP *believes* it's disciplining a 1ms tick.
+	 */
+	tk->cs_tick_adj = (s64)tk->ntp_tick -
+			  ((s64)NTP_INTERVAL_LENGTH << NTP_SCALE_SHIFT);
+	tk->cs_tick_adj *= NTP_INTERVAL_FREQ;
 
 	/*
 	 * The timekeeper keeps its own mult values for the currently
@@ -803,7 +829,7 @@ static void timekeeping_update_from_shadow(struct tk_data *tkd, unsigned int act
 
 	if (action & TK_CLEAR_NTP) {
 		tk->ntp_error = 0;
-		ntp_clear(tk->id);
+		ntp_clear(tk->id, tk->cs_tick_adj);
 	}
 
 	tk_update_leap_state(tk);
@@ -2075,7 +2101,12 @@ void __init timekeeping_init(void)
 
 	tk_set_wall_to_mono(tks, wall_to_mono);
 
-	timekeeping_update_from_shadow(&tk_core, TK_CLOCK_WAS_SET);
+	/*
+	 * Use TK_UPDATE_ALL so the NTP layer picks up the clocksource's
+	 * cs_tick_adj via ntp_clear(). Clearing NTP here is otherwise
+	 * redundant as ntp_init() already initialised it above.
+	 */
+	timekeeping_update_from_shadow(&tk_core, TK_UPDATE_ALL);
 }
 
 /* time in seconds when suspend began for persistent clock */
@@ -2424,8 +2455,8 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
 		mult = tk->tkr_mono.mult - tk->ntp_err_mult;
 	} else {
 		tk->ntp_tick = ntp_tl;
-		mult = div64_u64((tk->ntp_tick >> tk->ntp_error_shift) -
-				 tk->xtime_remainder, tk->cycle_interval);
+		mult = div64_u64(tk->ntp_tick >> tk->ntp_error_shift,
+				 tk->cycle_interval);
 	}
 
 	/*
@@ -2550,8 +2581,7 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
 
 	/* Accumulate error between NTP and clock interval */
 	tk->ntp_error += tk->ntp_tick << shift;
-	tk->ntp_error -= (tk->xtime_interval + tk->xtime_remainder) <<
-						(tk->ntp_error_shift + shift);
+	tk->ntp_error -= tk->xtime_interval << (tk->ntp_error_shift + shift);
 
 	return offset;
 }
-- 
2.54.0