mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error
@ 2026-11-06  3:08 David Woodhouse
  2026-11-06  3:08 ` [PATCH v2 1/6] timekeeping: Allow tick_length changes to apply mid-tick David Woodhouse
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: David Woodhouse @ 2026-11-06  3:08 UTC (permalink / raw)
  To: Thomas Gleixner, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Rodolfo Giometti,
	Richard Cochran, Jie Wang, Yuan Wang, Ryan Luu, Julien Ridoux,
	linux-kernel, nh-open-source, David Woodhouse

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

The timekeeper's ntp_error variable tracks the delta between the ideal
time that the kernel *should* be reporting as CLOCK_REALTIME, and the
time that is *actually* being reported.

A previous patch series in v7.3, up to commit 794ddd6e15cf ("ntp:
Remove tick_length_base, use tick_length directly"), fixed *errors* in
the accounting of ntp_error — cases where the reported time would
diverge from the ideal time *without* the divergence being correctly
tracked in ntp_error for subsequent compensation.

This series is the next step: it reduces the magnitude of the
excursions which are (now) *correctly* tracked in ntp_error.

The ntp_error mechanism works fine when it remains close to zero. The
actual clock is steered back towards the ideal by dithering between
mult and mult+1 for each cycle, changing between the two each time
ntp_error crosses zero.

This doesn't work as well when the value of ntp_error gets higher. In
some cases, 83µs of ntp_error was seen. Draining that through the mult
dithering, at typically less than 1ns/s, could take days. And while it
does, the precision of the frequency that can be effected by the
kernel's timekeeping is limited by the integer quantisation, because
it's permanently biased to either mult or mult+1 according to the
direction of the persistent ntp_error, and frequencies between the two
values cannot be achieved by the normal dithering. On top of which,
chrony was attempting to discipline the *reported* clock, even as the
kernel was attempting to steer it as hard as it could (albeit not very
hard) back to the kernel's ideal.

There are two cases where during frequency/phase adjustment, the
change was effectively applied to the 'ideal' vs. 'actual' rate at
different moments in time. The accounting for this in ntp_error was
self-consistent, but the better answer is for them to be conceptually
applied at the same moment, without contributing anything to ntp_error
at all. That also removes the step which repaying that ntp_error would
have caused; the rate simply pivots at the moment of the change.

This turns out not to be theoretical: Jie Wang reports¹ that since the
v7.3 accounting fixes, phc2sys takes minutes instead of seconds to
settle CLOCK_REALTIME after a step of the PHC it follows, because its
large frequency corrections are charged to ntp_error and repaid only
through the dithering. Patch 1 of this series is the fix for that:
Yuan Wang has measured it restoring their gPTP settling times (back to
32s from 200-400s or more) and withdrawn the RFC posted in that thread
in its favour.

While the timekeeping code is applying a phase correction, that
applies an additional bias to the per-cycle 'mult' values. If a
tickless kernel goes idle while this skew is being applied, it can
continue to apply that bias *long* after it was intended to finish,
overshooting the intended phase correction (by 500µs/s in the case of
adjtime). Fix this in timekeeping_max_deferment() by requesting a
wakeup at the top of the next second while phase skew is being
applied. (NB: only for the core timekeeper, not aux clocks which
remain as they were.)

Finally, if ntp_error ever does get excessive, we can deliberately
apply a phase skew to return it to zero faster than the normal
dithering would permit. An equivalent mechanism was removed in commit
dc491596f639 ("timekeeping: Rework frequency adjustments to work
better w/ nohz") but especially with the timekeeping_max_deferment()
change mentioned above, this now works effectively even with (and in
fact especially with) tickless idle. But again only for the core
timekeeper... unless we want to wake for aux clocks too?

Tests running at https://david.woodhou.se/ntptest-virt/ on baseline
vs. patched kernels, both tickless and periodic. The effect is seen
most clearly on the tickless kernel, where the baseline experiences
much higher jitter around corrections that should be smooth.

The final patch, correcting the output of ktime_get_snapshot_id() for
ntp_error at the moment of the snapshot, was marked DO NOT MERGE in v1.
It trades monotonicity (of the snapshot output, across read methods)
for accuracy, and Rodolfo wanted an explicit OK from the consumers of
the PPS_FETCH timestamps it changes; Miroslav has given chrony's². The
in-kernel snapshot users do want *accurate* time rather than a time we
*know* is microseconds wrong because the kernel happened to be idle
accumulating ntp_error right before they asked. So the tag is dropped
and the patch is now intended for merging with the rest.

v2:
 • Rebase to v7.4-rc1.
 • Pull the dividend/mult restructure up from the final patch into patch 1 so
   the old and new mult calculations match visually, and rewrite the
   comment explaining the mid-tick compensation (John).
 • Fix the ADJ_OFFSET_READONLY test in patch 3: that flag aliases
   ADJ_NANO and only has its read-only meaning in the ADJ_ADJTIME
   form. Also trigger the immediate update for ADJ_TIMECONST and
   ADJ_STATUS, mirroring process_adjtimex_modes(). (Found by Sashiko.)
 • Claim the do_timer duty when a slew starts (patch 3) or a drain
   engages (patch 4), so the per-second wakeup is honoured even if
   every other CPU is already sleeping on a timer programmed before
   the slew began.
 • Clamp the proportional drain in patch 4 at 50ppm instead of
   maxadj/4, and document the bounded frequency excursion (Miroslav).
   The clamp also bounds the extrapolation arithmetic in patch 6
   against overflow (found by Sashiko).
 • Smaller cleanups: an explicit negative-drain test, rounding on the
   insane-delta path, and honesty in the comments about the drain
   bias.
 • Pull the pps_get_ts() conversion to ktime_get_snapshot_id() into
   this series, ahead of the snapshot correction, so that the change
   to the PPS_FETCH timestamps is explicit and covers all PPS
   consumers (Rodolfo).
 • Drop DO NOT MERGE from the final patch.

¹ https://lore.kernel.org/all/20261105032150.1763572-1-yuan1.wang@intel.com/
² https://lore.kernel.org/all/atoVLqWfyJtCfci2@localhost/

David Woodhouse (6):
      timekeeping: Allow tick_length changes to apply mid-tick
      ntp: Recalculate skew_delta when the phase offset changes
      timekeeping: Bound idle sleep while a phase slew is in flight
      timekeeping: Reinstate proportional correction of ntp_error
      pps: Always use ktime_get_snapshot_id() for pps_get_ts()
      timekeeping: Apply extrapolated ntp_error to clock snapshots

 include/linux/pps_kernel.h          |   4 +-
 include/linux/timekeeper_internal.h |  11 +-
 kernel/time/ntp.c                   | 144 ++++++++++++++----------
 kernel/time/tick-internal.h         |  18 +++
 kernel/time/timekeeping.c           | 211 ++++++++++++++++++++++++++++++++++--
 5 files changed, 318 insertions(+), 70 deletions(-)

base-commit: 6628e7d4c6b2f582557a3437606eac34b1067b40
--
2.43.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/6] timekeeping: Allow tick_length changes to apply mid-tick
  2026-11-06  3:08 [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error David Woodhouse
@ 2026-11-06  3:08 ` David Woodhouse
  2026-11-06  3:08 ` [PATCH v2 2/6] ntp: Recalculate skew_delta when the phase offset changes David Woodhouse
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: David Woodhouse @ 2026-11-06  3:08 UTC (permalink / raw)
  To: Thomas Gleixner, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Rodolfo Giometti,
	Richard Cochran, Jie Wang, Yuan Wang, Ryan Luu, Julien Ridoux,
	linux-kernel, nh-open-source, David Woodhouse

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

When the NTP tick length changes, timekeeping_apply_adjustment() applies
the accounting for the new rate to ntp_error retrospectively for the
partial tick which is currently in progress. So the ideal line of the
clock conceptually changes from the moment of cycle_last (which can be
almost a full tick ago), and ntp_error grows to show the difference
between that and what userspace actually saw already.

Commit c2cda2a5bda9 ("timekeeping/ntp: Don't align NTP frequency
adjustments to ticks") removed the equivalent alignment term from the
old accounting in 2018, precisely so that a new frequency took effect
at the time of the update. Commit b7befd6d9120 ("timekeeping: Account
for monotonicity adjustment in ntp_error") effectively reinstated the
alignment, because *some* accounting is needed at that point — the
same retrospective mult change also carries the dither and skew
components, whose divergence genuinely does belong in ntp_error.

This can lead to a significant value (10s of microseconds) landing in
ntp_error which can take *days* to drain through the natural mult ±1
dithering.

The slow-draining ntp_error then applies a consistent bias to that ±1
dithering, meaning that the precision of the effective frequency
requested by userspace is limited to integer 'mult' values, since the
dithering is no longer effective to achieve the fractional parts.

Jie Wang reports that this makes phc2sys take minutes instead of
seconds to settle CLOCK_REALTIME after a step of the PHC it follows:
the servo slews at up to 10% and then backs off by several percent per
second, charging tens of microseconds to ntp_error on every update. On
a gPTP setup (ptp4l + phc2sys on back-to-back i226 NICs, PREEMPT_RT),
the time for the phc2sys offset to settle below 100ns after a 1s step
of the grandmaster's PHC rose from ~32s to 200-400s or more. Yuan Wang
measured this patch alone restoring it to 32s across three consecutive
steps, with the steady-state lock (~8ns rms) unchanged.

Fix this by allowing the rate change to be conceptually applied at
'offset' into the partial tick, rather than the start of the tick.
This means that the amount of delta which gets accumulated into
ntp_error remains low.

Rather than factoring this into timekeeping_apply_adjustment() which
already makes my eyes bleed, pre-correct it when the rate changes, to
adjust the 'intended' clock position according to the delta between
old and new tick lengths (but leaving skew as it should be).

Fixes: b7befd6d9120 ("timekeeping: Account for monotonicity adjustment in ntp_error")
Reported-by: Jie Wang <jie.wang@intel.com>
Closes: https://lore.kernel.org/all/20261105032150.1763572-1-yuan1.wang@intel.com/
Cc: stable@vger.kernel.org
Acked-by: John Stultz <jstultz@google.com>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
v2: Pull the dividend/mult restructure up from patch 6 so the old and
    new calculations match visually, and rewrite the comment explaining
    the mid-tick compensation (John). Add the commit archaeology, the
    gPTP symptom and measurements from Jie Wang's report, with
    Fixes/Cc: stable.

 kernel/time/timekeeping.c | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index dcf47f1ffdcf..118612c4a82e 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -2436,6 +2436,7 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
 {
 	u64 ntp_tl = ntp_tick_length(tk->id);
 	s64 skew = ntp_get_skew_delta(tk->id);
+	u64 dividend;
 	u32 mult;
 
 	/*
@@ -2447,6 +2448,8 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
 		/* Revert to the base mult rate. */
 		mult = tk->tkr_mono.mult - tk->ntp_err_mult;
 	} else {
+		u64 old_ntp_tick = tk->ntp_tick;
+
 		tk->ntp_tick = ntp_tl;
 		tk->skew_delta = skew;
 		/*
@@ -2454,8 +2457,35 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
 		 * scale it back up to the full per-tick rate for the mult bias.
 		 */
 		skew *= NTP_INTERVAL_FREQ;
-		mult = div64_u64((tk->ntp_tick + skew) >> tk->ntp_error_shift,
-				 tk->cycle_interval);
+		dividend = (tk->ntp_tick + skew) >> tk->ntp_error_shift;
+		mult = div64_u64(dividend, tk->cycle_interval);
+
+		/*
+		 * In timekeeping_apply_adjustment(), the mult_adj value is
+		 * retrospectively applied as if it had come into effect at
+		 * the *start* of the current tick.  Any discrepancy due to
+		 * the fact that time has *actually* been counted at the old
+		 * mult rate between the start of the tick and now ('offset'
+		 * cycles into that tick) is dumped into ntp_error.
+		 *
+		 * When tick_length is changed by a large amount, that can
+		 * lead to tens of microseconds of ntp_error being accrued.
+		 *
+		 * Fix this by conceptually applying the tick_length change
+		 * now (at 'offset'), rather than the start of the tick.
+		 *
+		 * That means calculating what 'mult' would have been with
+		 * the old ntp_tick length (but the *current* value of skew),
+		 * and pre-adjusting ntp_error to compensate.
+		 */
+		if (tk->ntp_tick != old_ntp_tick) {
+			u64 old_dividend = (old_ntp_tick + skew) >> tk->ntp_error_shift;
+			s64 old_base_mult = div64_u64(old_dividend, tk->cycle_interval);
+			s64 mult_delta = (s64)mult - old_base_mult;
+
+			tk->ntp_error -= ((s64)offset * mult_delta)
+					 << tk->ntp_error_shift;
+		}
 	}
 
 	/*
-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 2/6] ntp: Recalculate skew_delta when the phase offset changes
  2026-11-06  3:08 [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error David Woodhouse
  2026-11-06  3:08 ` [PATCH v2 1/6] timekeeping: Allow tick_length changes to apply mid-tick David Woodhouse
@ 2026-11-06  3:08 ` David Woodhouse
  2026-11-06  3:08 ` [PATCH v2 3/6] timekeeping: Bound idle sleep while a phase slew is in flight David Woodhouse
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: David Woodhouse @ 2026-11-06  3:08 UTC (permalink / raw)
  To: Thomas Gleixner, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Rodolfo Giometti,
	Richard Cochran, Jie Wang, Yuan Wang, Ryan Luu, Julien Ridoux,
	linux-kernel, nh-open-source, David Woodhouse

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

The skew rate is recomputed only by second_overflow(), so when
time_offset or time_adjust changes mid-second (hardpps on each pulse,
adjtimex, adjtime()) the stale rate keeps delivering until the end of
the second, and the misdelivery lands in ntp_error. Pull the
computation out into ntp_update_skew_delta() and call it from the
writers. Settling of an opposing time_offset/time_adjust overlap stays
paced at one chunk per second, via the settle argument.

Acked-by: John Stultz <jstultz@google.com>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
v2: No change.

 kernel/time/ntp.c | 144 +++++++++++++++++++++++++++-------------------
 1 file changed, 85 insertions(+), 59 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index d22b532ec536..b8b0bf1d94e7 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -593,6 +593,82 @@ ktime_t ntp_get_next_leap(unsigned int tkid)
 	return KTIME_MAX;
 }
 
+/*
+ * ntp_update_skew_delta - Recompute the per-tick skew rate for the
+ * current second from the pending time_offset / time_adjust phase.
+ *
+ * The rate is in the same units as time_offset: (ns << NTP_SCALE_SHIFT)
+ * / HZ. If the result is so low that the skew imparted would round to
+ * zero, pass the bare minimum +-1 to ensure that it *does* actually
+ * drain completely to zero. It won't overshoot because
+ * logarithmic_accumulation() only drains what it can from time_offset
+ * or time_adjust, and the rest ends up in ntp_error which drives the
+ * selection of 'mult' immediately each tick.
+ *
+ * Called from second_overflow() at each second boundary (@settle =
+ * true), and directly by the writers of time_offset / time_adjust so a
+ * change takes effect at the next timekeeping advance rather than
+ * persisting a stale rate until the end of the second. @settle gates
+ * ntp_transfer_offset_adjust(): settling the opposing overlap is paced
+ * at one chunk per second by design and is the only mutation here, so
+ * with @settle false the call is a pure recompute, safe at any rate.
+ *
+ * The timekeeper lock is held on all paths, serializing ntp_data.
+ */
+static void ntp_update_skew_delta(struct ntp_data *ntpdata, bool settle)
+{
+	if (ntpdata->time_offset || ntpdata->time_adjust ||
+	    ntpdata->time_adjust_frac) {
+		s64 off_chunk = ntp_offset_chunk(ntpdata, ntpdata->time_offset);
+		s64 adj_chunk = 0, net;
+
+		/*
+		 * Once the exponential chunk rounds to zero, deliver the last
+		 * remaining offset this second so it converges to zero instead
+		 * of stalling just above it.
+		 */
+		if (!off_chunk)
+			off_chunk = ntpdata->time_offset;
+
+		if (ntpdata->time_adjust || ntpdata->time_adjust_frac) {
+			s64 adj;
+
+			if (ntpdata->time_adjust >= MAX_TICKADJ)
+				adj = MAX_TICKADJ * ONE_US_NS;
+			else if (ntpdata->time_adjust <= -MAX_TICKADJ)
+				adj = -MAX_TICKADJ * ONE_US_NS;
+			else
+				adj = ntpdata->time_adjust * ONE_US_NS +
+					ntpdata->time_adjust_frac;
+
+			adj_chunk = div_s64(adj, NTP_INTERVAL_FREQ);
+			if (!adj_chunk)
+				adj_chunk = signof(ntpdata->time_adjust_frac);
+		}
+
+		/*
+		 * If the two slews oppose, only their net would drive the
+		 * per-tick drain, so the cancelling part would never drain from
+		 * either tracker and an exact cancellation would stall both.
+		 * Settle that overlap directly between them (no clock motion).
+		 */
+		if (settle && off_chunk && adj_chunk &&
+		    signof(off_chunk) != signof(adj_chunk)) {
+			s64 conflict = min(abs(off_chunk), abs(adj_chunk));
+
+			ntp_transfer_offset_adjust(ntpdata, signof(off_chunk) * conflict);
+		}
+
+		/* Net is what the clock delivers; reduce to per-tick, then floor. */
+		net = off_chunk + adj_chunk;
+		ntpdata->skew_delta = div_s64(net, NTP_INTERVAL_FREQ);
+		if (!ntpdata->skew_delta && net)
+			ntpdata->skew_delta = signof(net);
+	} else {
+		ntpdata->skew_delta = 0;
+	}
+}
+
 /*
  * This routine handles the overflow of the microsecond field
  *
@@ -669,65 +745,8 @@ int second_overflow(unsigned int tkid, time64_t secs)
 	/* Check PPS signal */
 	pps_dec_valid(ntpdata);
 
-	/*
-	 * Set the per-tick skew rate for the next second. This is in
-	 * the same units as time_offset: (ns << NTP_SCALE_SHIFT) / HZ.
-	 * If the result is so low that the skew imparted would round
-	 * to zero, pass the bare minimum ±1 to ensure that it *does*
-	 * actually drain completely to zero. It won't overshoot because
-	 * logarithmic_accumulation() only drains what it can from
-	 * time_offset or time_adjust, and the rest ends up in ntp_error
-	 * which drives the selection of 'mult' immediately each tick.
-	 */
-	if (ntpdata->time_offset || ntpdata->time_adjust ||
-	    ntpdata->time_adjust_frac) {
-		s64 off_chunk = ntp_offset_chunk(ntpdata, ntpdata->time_offset);
-		s64 adj_chunk = 0, net;
-
-		/*
-		 * Once the exponential chunk rounds to zero, deliver the last
-		 * remaining offset this second so it converges to zero instead
-		 * of stalling just above it.
-		 */
-		if (!off_chunk)
-			off_chunk = ntpdata->time_offset;
-
-		if (ntpdata->time_adjust || ntpdata->time_adjust_frac) {
-			s64 adj;
-
-			if (ntpdata->time_adjust >= MAX_TICKADJ)
-				adj = MAX_TICKADJ * ONE_US_NS;
-			else if (ntpdata->time_adjust <= -MAX_TICKADJ)
-				adj = -MAX_TICKADJ * ONE_US_NS;
-			else
-				adj = ntpdata->time_adjust * ONE_US_NS +
-					ntpdata->time_adjust_frac;
-
-			adj_chunk = div_s64(adj, NTP_INTERVAL_FREQ);
-			if (!adj_chunk)
-				adj_chunk = signof(ntpdata->time_adjust_frac);
-		}
-
-		/*
-		 * If the two slews oppose, only their net would drive the
-		 * per-tick drain, so the cancelling part would never drain from
-		 * either tracker and an exact cancellation would stall both.
-		 * Settle that overlap directly between them (no clock motion).
-		 */
-		if (off_chunk && adj_chunk && signof(off_chunk) != signof(adj_chunk)) {
-			s64 conflict = min(abs(off_chunk), abs(adj_chunk));
-
-			ntp_transfer_offset_adjust(ntpdata, signof(off_chunk) * conflict);
-		}
-
-		/* Net is what the clock delivers; reduce to per-tick, then floor. */
-		net = off_chunk + adj_chunk;
-		ntpdata->skew_delta = div_s64(net, NTP_INTERVAL_FREQ);
-		if (!ntpdata->skew_delta && net)
-			ntpdata->skew_delta = signof(net);
-	} else {
-		ntpdata->skew_delta = 0;
-	}
+	/* Set the per-tick skew rate for the next second */
+	ntp_update_skew_delta(ntpdata, true);
 
 	return leap;
 }
@@ -1003,6 +1022,10 @@ static inline void process_adjtimex_modes(struct ntp_data *ntpdata, const struct
 
 	if (txc->modes & (ADJ_TICK|ADJ_FREQUENCY|ADJ_OFFSET))
 		ntp_update_frequency(ntpdata);
+
+	/* A changed phase target invalidates the current skew rate */
+	if (txc->modes & (ADJ_OFFSET | ADJ_TIMECONST | ADJ_STATUS))
+		ntp_update_skew_delta(ntpdata, false);
 }
 
 /*
@@ -1023,6 +1046,7 @@ int ntp_adjtimex(unsigned int tkid, struct __kernel_timex *txc, const struct tim
 			ntpdata->time_adjust = txc->offset;
 			ntpdata->time_adjust_frac = 0;
 			ntp_update_frequency(ntpdata);
+			ntp_update_skew_delta(ntpdata, false);
 
 			audit_ntp_set_old(ad, AUDIT_NTP_ADJUST,	save_adjust);
 			audit_ntp_set_new(ad, AUDIT_NTP_ADJUST,	ntpdata->time_adjust);
@@ -1264,6 +1288,8 @@ static void hardpps_update_phase(struct ntp_data *ntpdata, long error)
 		/* Cancel running adjtime() */
 		ntpdata->time_adjust = 0;
 		ntpdata->time_adjust_frac = 0;
+		/* The previous correction's skew rate is stale; recompute */
+		ntp_update_skew_delta(ntpdata, false);
 	}
 	/* Update jitter */
 	ntpdata->pps_jitter += (jitter - ntpdata->pps_jitter) >> PPS_INTMIN;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 3/6] timekeeping: Bound idle sleep while a phase slew is in flight
  2026-11-06  3:08 [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error David Woodhouse
  2026-11-06  3:08 ` [PATCH v2 1/6] timekeeping: Allow tick_length changes to apply mid-tick David Woodhouse
  2026-11-06  3:08 ` [PATCH v2 2/6] ntp: Recalculate skew_delta when the phase offset changes David Woodhouse
@ 2026-11-06  3:08 ` David Woodhouse
  2026-11-06  3:08 ` [PATCH v2 4/6] timekeeping: Reinstate proportional correction of ntp_error David Woodhouse
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: David Woodhouse @ 2026-11-06  3:08 UTC (permalink / raw)
  To: Thomas Gleixner, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Rodolfo Giometti,
	Richard Cochran, Jie Wang, Yuan Wang, Ryan Luu, Julien Ridoux,
	linux-kernel, nh-open-source, David Woodhouse

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

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@google.com>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 4/6] timekeeping: Reinstate proportional correction of ntp_error
  2026-11-06  3:08 [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error David Woodhouse
                   ` (2 preceding siblings ...)
  2026-11-06  3:08 ` [PATCH v2 3/6] timekeeping: Bound idle sleep while a phase slew is in flight David Woodhouse
@ 2026-11-06  3:08 ` David Woodhouse
  2026-11-06  3:08 ` [PATCH v2 5/6] pps: Always use ktime_get_snapshot_id() for pps_get_ts() David Woodhouse
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: David Woodhouse @ 2026-11-06  3:08 UTC (permalink / raw)
  To: Thomas Gleixner, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Rodolfo Giometti,
	Richard Cochran, Jie Wang, Yuan Wang, Ryan Luu, Julien Ridoux,
	linux-kernel, nh-open-source, David Woodhouse

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

The +0/+1 mult dithering is sufficient to keep ntp_error at zero once
it's already there, but it doesn't do much to reduce it once any
significant ntp_error has accumulated (e.g. from frequency or phase
adjustments) — it can typically only drain single-digit nanoseconds
per second, only in one direction, and potentially not at all when the
mult division is exact. Commit dc491596f639 ("timekeeping: Rework
frequency adjustments to work better w/ nohz") removed a larger skew
because in a tickless kernel, it would remain in effect for a full
idle period and overshoot. Now that timekeeping_max_deferment()
ensures that the system wakes at the top of the second when skew is
active, the correction can be reinstated. If ntp_error exceeds an
amount that a single +1 change to mult can drain within a minute, add
an additional bias to mult to close the gap over ~10 seconds.

The deliberate frequency excursion this causes is bounded. In correct
operation the only remaining source of significant ntp_error is mult
quantisation drift across tickless idle, which accrues at well under
one mult-LSB-second per second and is bounded by the clocksource's
maximum idle deferment, so the drain engages rarely and at single-digit
ppm for ~10s; the worst error observed in the wild (83µs, from an
accounting bug since fixed) would drain at 8.3ppm. A clamp caps the
drain at 50ppm — an order of magnitude below the 500ppm adjtime()
slew rate — purely so that any future accounting bug results in a
slow, bounded drain rather than an unbounded frequency excursion
visible to NTP daemons measuring the clock; errors above 500µs simply
take proportionally longer than the 10 second horizon to repay.

The drain, like a slew, relies on the per-second wakeup from
timekeeping_max_deferment(). It is sized during accumulation, which may
be running from an idle CPU's interrupt with the do_timer duty vacant
and its last holder asleep on a long timer — so claim the vacant duty
there too, as __do_adjtimex() does when starting a slew.

Acked-by: John Stultz <jstultz@google.com>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
v2: Describe the dithering as +0/+1 rather than ±1 (Miroslav). Clamp
    the drain at 50ppm instead of maxadj/4, and document the bounded
    frequency excursion in the commit message; the clamp also bounds
    the extrapolation arithmetic in patch 6. Spell the negative-drain
    wakeup test explicitly instead of via a u32 cast (Sashiko). Claim
    the do_timer duty when the drain engages, as patch 3 now does for
    slews.

 include/linux/timekeeper_internal.h |  5 +-
 kernel/time/timekeeping.c           | 73 +++++++++++++++++++++++++++--
 2 files changed, 72 insertions(+), 6 deletions(-)

diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index fe077d97b5f8..85d123bcb270 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -97,6 +97,8 @@ 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
+ * @err_drain:			Extra mult bias repaying ntp_error beyond the
+ *				dither's reach; sized at second boundaries
  * @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
@@ -186,7 +188,8 @@ struct timekeeper {
 	u64			ntp_tick;
 	s64			ntp_error;
 	u32			ntp_error_shift;
-	u32			ntp_err_mult;
+	s32			ntp_err_mult;
+	s32			err_drain;
 	s64			cs_tick_adj;
 	u32			skip_second_overflow;
 	s64			skew_delta;
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 667b4c2817b5..1f3fde0c37b2 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -326,6 +326,22 @@ static inline void clocksource_disable_inline_read(void) { }
 static inline void clocksource_enable_inline_read(void) { }
 #endif
 
+/*
+ * The proportional ntp_error drain engages when the error exceeds
+ * TK_NTP_ERR_THRESH seconds' worth of the +0/+1 dither's delivery (so
+ * noise the dither handles alone never wakes the CPU), and then
+ * repays over ~TK_NTP_ERR_HORIZON seconds. The drain rate is capped
+ * at TK_NTP_ERR_MAX_PPM: in correct operation it is unreachable (the
+ * worst error ever observed, 83µs from a since-fixed accounting bug,
+ * drains at 8.3ppm), so it exists purely to bound the frequency
+ * excursion any future accounting bug could inflict on NTP daemons
+ * measuring the clock. Errors above 500µs just take proportionally
+ * longer than the usual horizon to repay.
+ */
+#define TK_NTP_ERR_THRESH	60
+#define TK_NTP_ERR_HORIZON	10
+#define TK_NTP_ERR_MAX_PPM	50
+
 /**
  * tk_setup_internals - Set up internals to use clocksource clock.
  *
@@ -423,6 +439,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
 	tk->tkr_mono.mult = clock->mult;
 	tk->tkr_raw.mult = clock->mult;
 	tk->ntp_err_mult = 0;
+	tk->err_drain = 0;
 	tk->skip_second_overflow = 0;
 	tk->skew_delta = 0;
 
@@ -831,6 +848,7 @@ static void timekeeping_update_from_shadow(struct tk_data *tkd, unsigned int act
 
 	if (action & TK_CLEAR_NTP) {
 		tk->ntp_error = 0;
+		tk->err_drain = 0;
 		ntp_clear(tk->id, tk->cs_tick_adj);
 	}
 
@@ -1980,11 +1998,12 @@ u64 timekeeping_max_deferment(void)
 
 		/*
 		 * 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.
+		 * recalculated each second, as is the proportional
+		 * ntp_error drain. Ensure a tickless kernel wakes up
+		 * for that and does not overshoot.
 		 */
-		if (abs(tk->skew_delta) > 1) {
+		if (abs(tk->skew_delta) > 1 ||
+		    tk->ntp_err_mult < 0 || tk->ntp_err_mult > 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,
@@ -2254,6 +2273,7 @@ void timekeeping_resume(void)
 	tks->tkr_raw.cycle_last  = cycle_now;
 
 	tks->ntp_error = 0;
+	tks->err_drain = 0;
 	timekeeping_suspended = 0;
 	timekeeping_update_from_shadow(&tk_core, TK_CLOCK_WAS_SET);
 	raw_spin_unlock_irqrestore(&tk_core.lock, flags);
@@ -2509,7 +2529,16 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
 	 * tick division, the clock will slow down. Otherwise it will stay
 	 * ahead until the tick length changes to a non-divisible value.
 	 */
-	tk->ntp_err_mult = tk->ntp_error > 0 ? 1 : 0;
+	/* Drop the ntp_error drain the moment it overshoots */
+	if (tk->err_drain &&
+	    (((s64)tk->err_drain ^ tk->ntp_error) < 0 || !tk->ntp_error))
+		tk->err_drain = 0;
+
+	if (tk->err_drain)
+		tk->ntp_err_mult = tk->err_drain;
+	else
+		tk->ntp_err_mult = tk->ntp_error > 0 ? 1 : 0;
+
 	mult += tk->ntp_err_mult;
 
 	timekeeping_apply_adjustment(tk, offset, mult - tk->tkr_mono.mult);
@@ -2552,6 +2581,7 @@ static inline unsigned int accumulate_nsecs_to_secs(struct timekeeper *tk)
 {
 	u64 nsecps = (u64)NSEC_PER_SEC << tk->tkr_mono.shift;
 	unsigned int clock_set = 0;
+	bool crossed = false;
 
 	while (tk->tkr_mono.xtime_nsec >= nsecps) {
 		int leap;
@@ -2584,6 +2614,39 @@ static inline unsigned int accumulate_nsecs_to_secs(struct timekeeper *tk)
 
 			clock_set = TK_CLOCK_WAS_SET;
 		}
+
+		crossed = true;
+	}
+
+	/*
+	 * Core timekeeper only; aux timekeepers are not covered by the
+	 * timekeeping_max_deferment() wakeup which re-sizes the drain
+	 * each second before it can overshoot.
+	 */
+	if (crossed && tk->id == TIMEKEEPER_CORE) {
+		/* Re-size the proportional ntp_error drain for this second */
+		s64 lsb_sec = ((s64)tk->cycle_interval <<
+			       tk->ntp_error_shift) * NTP_INTERVAL_FREQ;
+
+		tk->err_drain = 0;
+		if (abs(tk->ntp_error) > lsb_sec * TK_NTP_ERR_THRESH) {
+			s32 max_drain = (u64)tk->tkr_mono.clock->mult *
+					TK_NTP_ERR_MAX_PPM / 1000000;
+
+			tk->err_drain = clamp_t(s64,
+						div64_s64(tk->ntp_error,
+							  lsb_sec * TK_NTP_ERR_HORIZON),
+						-max_drain, max_drain);
+
+			/*
+			 * This accumulation may be running from an idle
+			 * CPU's interrupt (jiffies catch-up). Make sure
+			 * some CPU will honour the per-second deferment
+			 * bound while the drain is active.
+			 */
+			if (tk->err_drain)
+				tick_nohz_claim_do_timer();
+		}
 	}
 	return clock_set;
 }
-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 5/6] pps: Always use ktime_get_snapshot_id() for pps_get_ts()
  2026-11-06  3:08 [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error David Woodhouse
                   ` (3 preceding siblings ...)
  2026-11-06  3:08 ` [PATCH v2 4/6] timekeeping: Reinstate proportional correction of ntp_error David Woodhouse
@ 2026-11-06  3:08 ` David Woodhouse
  2026-11-06  8:44   ` Rodolfo Giometti
  2026-11-06  3:08 ` [PATCH v2 6/6] timekeeping: Apply extrapolated ntp_error to clock snapshots David Woodhouse
  2026-11-06  3:24 ` [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error David Woodhouse
  6 siblings, 1 reply; 12+ messages in thread
From: David Woodhouse @ 2026-11-06  3:08 UTC (permalink / raw)
  To: Thomas Gleixner, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Rodolfo Giometti,
	Richard Cochran, Jie Wang, Yuan Wang, Ryan Luu, Julien Ridoux,
	linux-kernel, nh-open-source, David Woodhouse

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

Without CONFIG_NTP_PPS, pps_get_ts() fills in ts_real with
ktime_get_real_ts64(). Use ktime_get_snapshot_id() unconditionally so
that PPS timestamps always come from the snapshot machinery; only the
raw timestamp remains conditional on CONFIG_NTP_PPS.

This is preparation for the next commit, which corrects the snapshot's
::systime for the accumulated ntp_error. That correction should reach
*all* PPS consumers, not only those on CONFIG_NTP_PPS kernels.

The cost of the snapshot on a small ARM board (Cortex-A53, 12.5MHz
arch counter) is 38.8ns from the pulse handler to the counter read,
against 23.4ns for ktime_get_real_ts64() — both small against the
~2.8µs measured from the pin edge to the handler.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
v2: New in this series, ahead of the snapshot correction so that its
    effect explicitly covers all PPS consumers.

 include/linux/pps_kernel.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index 00b840970d56..35d6ac06a0c0 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -100,14 +100,12 @@ static inline void timespec_to_pps_ktime(struct pps_ktime *kt,
 
 static inline void pps_get_ts(struct pps_event_time *ts)
 {
-#ifdef CONFIG_NTP_PPS
 	struct system_time_snapshot snap;
 
 	ktime_get_snapshot_id(CLOCK_REALTIME, &snap);
 	ts->ts_real = ktime_to_timespec64(snap.systime);
+#ifdef CONFIG_NTP_PPS
 	ts->ts_raw = ktime_to_timespec64(snap.monoraw);
-#else
-	ktime_get_real_ts64(&ts->ts_real);
 #endif
 }
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 6/6] timekeeping: Apply extrapolated ntp_error to clock snapshots
  2026-11-06  3:08 [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error David Woodhouse
                   ` (4 preceding siblings ...)
  2026-11-06  3:08 ` [PATCH v2 5/6] pps: Always use ktime_get_snapshot_id() for pps_get_ts() David Woodhouse
@ 2026-11-06  3:08 ` David Woodhouse
  2026-11-06  8:44   ` Rodolfo Giometti
  2026-11-06  3:24 ` [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error David Woodhouse
  6 siblings, 1 reply; 12+ messages in thread
From: David Woodhouse @ 2026-11-06  3:08 UTC (permalink / raw)
  To: Thomas Gleixner, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Rodolfo Giometti,
	Richard Cochran, Jie Wang, Yuan Wang, Ryan Luu, Julien Ridoux,
	linux-kernel, nh-open-source, David Woodhouse

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

The time reported in ::systime of a system_time_snapshot is known to be
slightly inaccurate because of the way that the reported realtime clock
sawtooths around the *intended* time series, limited by the integer mult
value used to calculate the inter-tick times, and designed to ensure
smoothness and monotonicity for its consumers.

It is particularly inaccurate in a tickless kernel, where ntp_err_mult
is not adjusted on each tick, allowing the reported clock to diverge
from the intended time for a large number of ticks before re-converging.

It would be better for callers of get_device_system_crosststamp() and
ktime_get_snapshot_id() to receive the *accurate* time, not the
sanitized version provided to gettimeofday().

Compute the deviation in snapshot_ntp_error() and add it to the returned
::systime so the snapshot lands on the ideal line. It sums four terms in
ns << NTP_SCALE_SHIFT before converting to signed ns:

  - tk->ntp_error, the deviation as of the last update;
  - (cycle_delta * ntp_err_frac), the fractional-mult drift accrued
    since then (cycle_delta is at most a tick on a tickful kernel, but
    many ticks' worth under NO_HZ);
  - (cycle_delta * ntp_err_mult), subtracting the applied mult bias (the
    +1 dither, or the err_drain bias while a drain is in progress) over
    the same span;
  - the sub-nanosecond fraction dropped when the read was truncated to
    whole ns (low shift bits, exact despite the multiply overflowing).

None of these terms can overflow: ntp_err_mult is clamped to 50ppm
(see "timekeeping: Reinstate proportional correction of ntp_error"),
and cycle_delta is bounded by the clocksource's max_cycles, so the
dither/drain term is at most 50ppm of the maximum idle period, in
ns << 32 — around 2^57 for a 500s TSC deferment. Deltas beyond
max_cycles (a marginal clocksource left unread too long) skip the
extrapolation entirely.

The helper uses the timekeeper selected for the requested clock id, so
all NTP-disciplined clocks are corrected, including the AUX clocks (each
has its own NTP instance); only CLOCK_MONOTONIC_RAW is undisciplined and
gets no correction. The residual is then a single clocksource cycle, the
same bound as a tickful kernel — except on the final tick of a phase
slew, where ntp_drain_skew() clamps to avoid overshooting zero and the
extrapolation can briefly overstate the correction by up to one tick's
worth of skew, repaid at the next update.

Note that this *unconditionally* changes the ::systime returned by all
snapshot and cross timestamp consumers (PTP SYS_OFFSET_PRECISE/EXTENDED,
and the timestamps which PPS_FETCH and /sys/class/pps report to
userspace, since the previous commit made pps_get_ts() use the snapshot
unconditionally): it is now the ideal NTP-disciplined time rather than
the raw accumulated clock.

For the PPS consumers that means ts_real no longer exactly matches what
clock_gettime(CLOCK_REALTIME) would have returned at the same moment,
but reports the time which the kernel is *intending* to steer
CLOCK_REALTIME to. The difference is the transient error which the
kernel is already committed to eliminating — reporting it to a PPS
consumer only feeds a disturbance into their measurements which will
have disappeared again by the time they react to it.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
v2: Shrunk by the dividend restructure moving to patch 1. Commit
    message now states the overflow bound on the extrapolation terms,
    which follows from the 50ppm clamp, and owns the transient
    over-extrapolation on the final tick of a phase slew. Round the
    insane-delta early return like the normal path, and mention the
    err_drain bias in the comments. (Overflow and the rest found by
    Sashiko.) Drop the speculative CONFIG_NTP_PPS paragraph and name
    the PPS_FETCH timestamps among the affected consumers (Rodolfo).
    Drop DO NOT MERGE.

 include/linux/timekeeper_internal.h |  6 +++
 kernel/time/timekeeping.c           | 68 ++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index 85d123bcb270..164672ccbd45 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -99,6 +99,11 @@ struct tk_read_base {
  * @ntp_err_mult:		Multiplication factor for scaled math conversion
  * @err_drain:			Extra mult bias repaying ntp_error beyond the
  *				dither's reach; sized at second boundaries
+ * @ntp_err_frac:		Fractional part of the per-cycle NTP-ideal mult that the
+ *				integer @mult truncates, as a fraction of 2^32 in
+ *				clock-shifted nanoseconds per cycle. Describes the
+ *				working point of the +0/+1 mult dither (or of the
+ *				@err_drain bias while a drain is in progress).
  * @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
@@ -190,6 +195,7 @@ struct timekeeper {
 	u32			ntp_error_shift;
 	s32			ntp_err_mult;
 	s32			err_drain;
+	u64			ntp_err_frac;
 	s64			cs_tick_adj;
 	u32			skip_second_overflow;
 	s64			skew_delta;
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 1f3fde0c37b2..1d8c0e7e057e 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -439,6 +439,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
 	tk->tkr_mono.mult = clock->mult;
 	tk->tkr_raw.mult = clock->mult;
 	tk->ntp_err_mult = 0;
+	tk->ntp_err_frac = 0;
 	tk->err_drain = 0;
 	tk->skip_second_overflow = 0;
 	tk->skew_delta = 0;
@@ -1251,6 +1252,57 @@ static inline u64 tk_clock_read_snapshot(const struct tk_read_base *tkr,
 	return clock->read(clock);
 }
 
+/*
+ * snapshot_ntp_error - record how far a snapshot's ::systime is from the
+ * ideal NTP-disciplined time at @now, in signed nanoseconds, so a caller
+ * can land exactly on the ideal line by adding it to ::systime.
+ *
+ * The value is summed in ns << NTP_SCALE_SHIFT from four parts:
+ *
+ *  - tk->ntp_error, the deviation accumulated as of the last timekeeping
+ *    update (tkr_mono.cycle_last);
+ *  - (cycle_delta * ntp_err_frac), the fractional-mult drift accrued over
+ *    the cycles read since then -- at most a tick on a tickful kernel, but
+ *    potentially many ticks' worth under NO_HZ;
+ *  - (cycle_delta * ntp_err_mult), subtracting the applied mult bias (the
+ *    +1 dither, or the larger signed err_drain bias while a drain is in
+ *    progress) over the same span;
+ *  - the sub-nanosecond fraction that ::systime dropped when the read was
+ *    truncated to whole ns (the low @shift bits, exact even though the
+ *    multiply overflows).
+ *
+ * CLOCK_MONOTONIC_RAW is not NTP-disciplined and carries no error. Every
+ * other clock id uses its own timekeeper @tk -- including the AUX clocks,
+ * which each have their own NTP instance.
+ */
+static s64 snapshot_ntp_error(const struct timekeeper *tk, clockid_t clock_id,
+			      u64 now)
+{
+	u64 cycle_delta;
+	u32 nes;
+	s64 tmp, err;
+
+	if (clock_id == CLOCK_MONOTONIC_RAW)
+		return 0;
+
+	cycle_delta = (now - tk->tkr_mono.cycle_last) & tk->tkr_mono.mask;
+	nes = tk->ntp_error_shift;
+
+	/* Insane deltas from marginal clocksources: skip the extrapolation */
+	if (unlikely(cycle_delta > tk->tkr_mono.clock->max_cycles))
+		return (tk->ntp_error + (1LL << (NTP_SCALE_SHIFT - 1))) >>
+			NTP_SCALE_SHIFT;
+
+	err = tk->ntp_error;
+	err += ((s64)mul_u64_u64_shr(cycle_delta, tk->ntp_err_frac, 32) -
+		(s64)cycle_delta * tk->ntp_err_mult) << nes;
+
+	tmp = (s64)(cycle_delta * tk->tkr_mono.mult + tk->tkr_mono.xtime_nsec);
+	tmp &= (1ULL << tk->tkr_mono.shift) - 1;
+	err += tmp << nes;
+
+	return (err + (1LL << (NTP_SCALE_SHIFT - 1))) >> NTP_SCALE_SHIFT;
+}
 
 /**
  * ktime_get_snapshot_id -  Simultaneously snapshot a given clock ID with
@@ -1274,6 +1326,7 @@ void ktime_get_snapshot_id(clockid_t clock_id, struct system_time_snapshot *syst
 {
 	ktime_t base_raw, base_sys, offs_sys, *offs, offs_zero = 0;
 	u64 nsec_raw, nsec_sys, now;
+	s64 ntp_error;
 	struct timekeeper *tk;
 	struct tk_data *tkd;
 	unsigned int seq;
@@ -1336,10 +1389,12 @@ void ktime_get_snapshot_id(clockid_t clock_id, struct system_time_snapshot *syst
 
 		nsec_sys = timekeeping_cycles_to_ns(&tk->tkr_mono, now);
 		nsec_raw = timekeeping_cycles_to_ns(&tk->tkr_raw, now);
+
+		ntp_error = snapshot_ntp_error(tk, clock_id, now);
 	} while (read_seqcount_retry(&tkd->seq, seq));
 
 	systime_snapshot->cycles = now;
-	systime_snapshot->systime = ktime_add_ns(base_sys, offs_sys + nsec_sys);
+	systime_snapshot->systime = ktime_add_ns(base_sys, offs_sys + nsec_sys) + ntp_error;
 	systime_snapshot->monoraw = ktime_add_ns(base_raw, nsec_raw);
 
 	/*
@@ -1591,6 +1646,7 @@ int get_device_system_crosststamp(int (*get_time_fn)
 	ktime_t base_sys, base_raw, *offs;
 	u32 clock_was_set_seq = 0;
 	u64 nsec_sys, nsec_raw;
+	s64 ntp_error;
 	u8 cs_was_changed_seq;
 	unsigned int seq;
 	bool do_interp;
@@ -1657,9 +1713,10 @@ int get_device_system_crosststamp(int (*get_time_fn)
 
 		nsec_sys = timekeeping_cycles_to_ns(&tk->tkr_mono, cycles);
 		nsec_raw = timekeeping_cycles_to_ns(&tk->tkr_raw, cycles);
+		ntp_error = snapshot_ntp_error(tk, xtstamp->clock_id, cycles);
 	} while (read_seqcount_retry(&tkd->seq, seq));
 
-	xtstamp->sys_systime = ktime_add_ns(base_sys, nsec_sys);
+	xtstamp->sys_systime = ktime_add_ns(base_sys, nsec_sys) + ntp_error;
 	xtstamp->sys_monoraw = ktime_add_ns(base_raw, nsec_raw);
 
 	/*
@@ -2494,6 +2551,13 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
 		skew *= NTP_INTERVAL_FREQ;
 		dividend = (tk->ntp_tick + skew) >> tk->ntp_error_shift;
 		mult = div64_u64(dividend, tk->cycle_interval);
+		/*
+		 * Stash the fractional part of the per-cycle ideal mult that
+		 * the integer @mult discards, scaled by 2^32, in clock-shifted
+		 * ns per cycle.
+		 */
+		dividend -= (u64)mult * tk->cycle_interval;
+		tk->ntp_err_frac = div64_u64(dividend << 32, tk->cycle_interval);
 
 		/*
 		 * In timekeeping_apply_adjustment(), the mult_adj value is
-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error
  2026-11-06  3:08 [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error David Woodhouse
                   ` (5 preceding siblings ...)
  2026-11-06  3:08 ` [PATCH v2 6/6] timekeeping: Apply extrapolated ntp_error to clock snapshots David Woodhouse
@ 2026-11-06  3:24 ` David Woodhouse
  2026-11-06  8:44   ` Rodolfo Giometti
  6 siblings, 1 reply; 12+ messages in thread
From: David Woodhouse @ 2026-11-06  3:24 UTC (permalink / raw)
  To: Thomas Gleixner, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Rodolfo Giometti,
	Richard Cochran, Jie Wang, Yuan Wang, Ryan Luu, Julien Ridoux,
	linux-kernel, nh-open-source

[-- Attachment #1: Type: text/plain, Size: 2188 bytes --]

On Thu, 2026-11-05 at 19:08 -0800, David Woodhouse wrote:
> 
> David Woodhouse (6):
>       timekeeping: Allow tick_length changes to apply mid-tick
>       ntp: Recalculate skew_delta when the phase offset changes
>       timekeeping: Bound idle sleep while a phase slew is in flight
>       timekeeping: Reinstate proportional correction of ntp_error
>       pps: Always use ktime_get_snapshot_id() for pps_get_ts()
>       timekeeping: Apply extrapolated ntp_error to clock snapshots
> 
>  include/linux/pps_kernel.h          |   4 +-
>  include/linux/timekeeper_internal.h |  11 +-
>  kernel/time/ntp.c                   | 144 ++++++++++++++----------
>  kernel/time/tick-internal.h         |  18 +++
>  kernel/time/timekeeping.c           | 211 ++++++++++++++++++++++++++++++++++--
>  5 files changed, 318 insertions(+), 70 deletions(-)

I think I reordered and reworded things as agreed with Rodolfo; please
shout if I screwed something up. What's sitting behind this in
https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/ffclock
is the timekeeping reference stuff for importing/exporting vmclock, and
preserving timekeeping across KHO:

The !NO_HZ_COMMON dependency can sit around for a little while longer
until the series above is actually applied, and it can note the
actually commit ID of its dependencies (which I think we agreed is just
patches 1 & 2 of the series above). I'll probably round it up with the
other PPS stuff I haven't quite got publishable yet, improving the
filtering and then working on counter-based capture and UAPI.

David Woodhouse (7):
      timekeeping: Use inlined counter read in ktime_get_snapshot_id()
      pps: Drop the !NO_HZ_COMMON dependency from NTP_PPS
      timekeeping: Add absolute reference for feed-forward clock discipline
      timekeeping: Hand clock reference over kexec via KHO
      ptp_vmclock: Feed reference to timekeeping for feed-forward discipline
      timekeeping: Provide a shared /sys/kernel/time kobject
      kernel/time: Export NTP-disciplined time as a vmclock page in sysfs

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error
  2026-11-06  3:24 ` [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error David Woodhouse
@ 2026-11-06  8:44   ` Rodolfo Giometti
  2026-11-06 18:05     ` David Woodhouse
  0 siblings, 1 reply; 12+ messages in thread
From: Rodolfo Giometti @ 2026-11-06  8:44 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Richard Cochran, Jie Wang,
	Yuan Wang, Ryan Luu, Julien Ridoux, linux-kernel, nh-open-source

On 06/11/2026 04:24, David Woodhouse wrote:
> I think I reordered and reworded things as agreed with Rodolfo; please
> shout if I screwed something up. What's sitting behind this in
> https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/ffclock
> is the timekeeping reference stuff for importing/exporting vmclock, and
> preserving timekeeping across KHO:

No, the 5/6 and 6/6 split is what we agreed, thanks. On 6/6 I'm still
missing the ntpd side, see my reply there.

> 
> The !NO_HZ_COMMON dependency can sit around for a little while longer
> until the series above is actually applied, and it can note the
> actually commit ID of its dependencies (which I think we agreed is just
> patches 1 & 2 of the series above). I'll probably round it up with the
> other PPS stuff I haven't quite got publishable yet, improving the
> filtering and then working on counter-based capture and UAPI.

AFAIK we agreed on patches 1-4: the tickless run you did for me
(rodolfo-14-tickless-1hz) had all four applied. If 1 and 2 alone are
enough, could you put numbers for that in the commit message too?

And for the UAPI, an RFC first please. :-)

Ciao,

Rodolfo

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 5/6] pps: Always use ktime_get_snapshot_id() for pps_get_ts()
  2026-11-06  3:08 ` [PATCH v2 5/6] pps: Always use ktime_get_snapshot_id() for pps_get_ts() David Woodhouse
@ 2026-11-06  8:44   ` Rodolfo Giometti
  0 siblings, 0 replies; 12+ messages in thread
From: Rodolfo Giometti @ 2026-11-06  8:44 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Richard Cochran, Jie Wang,
	Yuan Wang, Ryan Luu, Julien Ridoux, linux-kernel, nh-open-source

On 06/11/2026 04:08, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> Without CONFIG_NTP_PPS, pps_get_ts() fills in ts_real with
> ktime_get_real_ts64(). Use ktime_get_snapshot_id() unconditionally so
> that PPS timestamps always come from the snapshot machinery; only the
> raw timestamp remains conditional on CONFIG_NTP_PPS.
>
> This is preparation for the next commit, which corrects the snapshot's
> ::systime for the accumulated ntp_error. That correction should reach
> *all* PPS consumers, not only those on CONFIG_NTP_PPS kernels.
>
> The cost of the snapshot on a small ARM board (Cortex-A53, 12.5MHz
> arch counter) is 38.8ns from the pulse handler to the counter read,
> against 23.4ns for ktime_get_real_ts64() — both small against the
> ~2.8µs measured from the pin edge to the handler.
>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
> v2: New in this series, ahead of the snapshot correction so that its
>     effect explicitly covers all PPS consumers.
>
>  include/linux/pps_kernel.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
> index 00b840970d56..35d6ac06a0c0 100644
> --- a/include/linux/pps_kernel.h
> +++ b/include/linux/pps_kernel.h
> @@ -100,14 +100,12 @@ static inline void timespec_to_pps_ktime(struct pps_ktime *kt,
>
>  static inline void pps_get_ts(struct pps_event_time *ts)
>  {
> -#ifdef CONFIG_NTP_PPS
>  	struct system_time_snapshot snap;
>
>  	ktime_get_snapshot_id(CLOCK_REALTIME, &snap);
>  	ts->ts_real = ktime_to_timespec64(snap.systime);
> +#ifdef CONFIG_NTP_PPS
>  	ts->ts_raw = ktime_to_timespec64(snap.monoraw);
> -#else
> -	ktime_get_real_ts64(&ts->ts_real);
>  #endif
>  }
>

Acked-by: Rodolfo Giometti <giometti@enneenne.com>

Ciao,

Rodolfo

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 6/6] timekeeping: Apply extrapolated ntp_error to clock snapshots
  2026-11-06  3:08 ` [PATCH v2 6/6] timekeeping: Apply extrapolated ntp_error to clock snapshots David Woodhouse
@ 2026-11-06  8:44   ` Rodolfo Giometti
  0 siblings, 0 replies; 12+ messages in thread
From: Rodolfo Giometti @ 2026-11-06  8:44 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Richard Cochran, Jie Wang,
	Yuan Wang, Ryan Luu, Julien Ridoux, linux-kernel, nh-open-source

On 06/11/2026 04:08, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> The time reported in ::systime of a system_time_snapshot is known to be
> slightly inaccurate because of the way that the reported realtime clock
> sawtooths around the *intended* time series, limited by the integer mult
> value used to calculate the inter-tick times, and designed to ensure
> smoothness and monotonicity for its consumers.
>
> It is particularly inaccurate in a tickless kernel, where ntp_err_mult
> is not adjusted on each tick, allowing the reported clock to diverge
> from the intended time for a large number of ticks before re-converging.
>
> It would be better for callers of get_device_system_crosststamp() and
> ktime_get_snapshot_id() to receive the *accurate* time, not the
> sanitized version provided to gettimeofday().
>
> Compute the deviation in snapshot_ntp_error() and add it to the returned
> ::systime so the snapshot lands on the ideal line. It sums four terms in
> ns << NTP_SCALE_SHIFT before converting to signed ns:
>
>   - tk->ntp_error, the deviation as of the last update;
>   - (cycle_delta * ntp_err_frac), the fractional-mult drift accrued
>     since then (cycle_delta is at most a tick on a tickful kernel, but
>     many ticks' worth under NO_HZ);
>   - (cycle_delta * ntp_err_mult), subtracting the applied mult bias (the
>     +1 dither, or the err_drain bias while a drain is in progress) over
>     the same span;
>   - the sub-nanosecond fraction dropped when the read was truncated to
>     whole ns (low shift bits, exact despite the multiply overflowing).
>
> None of these terms can overflow: ntp_err_mult is clamped to 50ppm
> (see "timekeeping: Reinstate proportional correction of ntp_error"),
> and cycle_delta is bounded by the clocksource's max_cycles, so the
> dither/drain term is at most 50ppm of the maximum idle period, in
> ns << 32 — around 2^57 for a 500s TSC deferment. Deltas beyond
> max_cycles (a marginal clocksource left unread too long) skip the
> extrapolation entirely.
>
> The helper uses the timekeeper selected for the requested clock id, so
> all NTP-disciplined clocks are corrected, including the AUX clocks (each
> has its own NTP instance); only CLOCK_MONOTONIC_RAW is undisciplined and
> gets no correction. The residual is then a single clocksource cycle, the
> same bound as a tickful kernel — except on the final tick of a phase
> slew, where ntp_drain_skew() clamps to avoid overshooting zero and the
> extrapolation can briefly overstate the correction by up to one tick's
> worth of skew, repaid at the next update.
>
> Note that this *unconditionally* changes the ::systime returned by all
> snapshot and cross timestamp consumers (PTP SYS_OFFSET_PRECISE/EXTENDED,
> and the timestamps which PPS_FETCH and /sys/class/pps report to
> userspace, since the previous commit made pps_get_ts() use the snapshot
> unconditionally): it is now the ideal NTP-disciplined time rather than
> the raw accumulated clock.
>
> For the PPS consumers that means ts_real no longer exactly matches what
> clock_gettime(CLOCK_REALTIME) would have returned at the same moment,
> but reports the time which the kernel is *intending* to steer
> CLOCK_REALTIME to. The difference is the transient error which the
> kernel is already committed to eliminating — reporting it to a PPS
> consumer only feeds a disturbance into their measurements which will
> have disappeared again by the time they react to it.

As we agreed on v4 3/4, this is the patch the chrony and ntpd
maintainers should ack. Miroslav has given chrony's OK, but I see
nobody from ntpd in Cc: could you add them?

Also from that thread: could you say here that this relies on 1/6
("timekeeping: Allow tick_length changes to apply mid-tick"), and give
the order of magnitude of the difference from clock_gettime() (the
tens of ns you mentioned there)?

>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
> v2: Shrunk by the dividend restructure moving to patch 1. Commit
>     message now states the overflow bound on the extrapolation terms,
>     which follows from the 50ppm clamp, and owns the transient
>     over-extrapolation on the final tick of a phase slew. Round the
>     insane-delta early return like the normal path, and mention the
>     err_drain bias in the comments. (Overflow and the rest found by
>     Sashiko.) Drop the speculative CONFIG_NTP_PPS paragraph and name
>     the PPS_FETCH timestamps among the affected consumers (Rodolfo).
>     Drop DO NOT MERGE.
>
>  include/linux/timekeeper_internal.h |  6 +++
>  kernel/time/timekeeping.c           | 68 ++++++++++++++++++++++++++++-
>  2 files changed, 72 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
> index 85d123bcb270..164672ccbd45 100644
> --- a/include/linux/timekeeper_internal.h
> +++ b/include/linux/timekeeper_internal.h
> @@ -99,6 +99,11 @@ struct tk_read_base {
>   * @ntp_err_mult:		Multiplication factor for scaled math conversion
>   * @err_drain:			Extra mult bias repaying ntp_error beyond the
>   *				dither's reach; sized at second boundaries
> + * @ntp_err_frac:		Fractional part of the per-cycle NTP-ideal mult that the
> + *				integer @mult truncates, as a fraction of 2^32 in
> + *				clock-shifted nanoseconds per cycle. Describes the
> + *				working point of the +0/+1 mult dither (or of the
> + *				@err_drain bias while a drain is in progress).
>   * @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
> @@ -190,6 +195,7 @@ struct timekeeper {
>  	u32			ntp_error_shift;
>  	s32			ntp_err_mult;
>  	s32			err_drain;
> +	u64			ntp_err_frac;
>  	s64			cs_tick_adj;
>  	u32			skip_second_overflow;
>  	s64			skew_delta;
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 1f3fde0c37b2..1d8c0e7e057e 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -439,6 +439,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
>  	tk->tkr_mono.mult = clock->mult;
>  	tk->tkr_raw.mult = clock->mult;
>  	tk->ntp_err_mult = 0;
> +	tk->ntp_err_frac = 0;
>  	tk->err_drain = 0;
>  	tk->skip_second_overflow = 0;
>  	tk->skew_delta = 0;
> @@ -1251,6 +1252,57 @@ static inline u64 tk_clock_read_snapshot(const struct tk_read_base *tkr,
>  	return clock->read(clock);
>  }
>
> +/*
> + * snapshot_ntp_error - record how far a snapshot's ::systime is from the
> + * ideal NTP-disciplined time at @now, in signed nanoseconds, so a caller
> + * can land exactly on the ideal line by adding it to ::systime.
> + *
> + * The value is summed in ns << NTP_SCALE_SHIFT from four parts:
> + *
> + *  - tk->ntp_error, the deviation accumulated as of the last timekeeping
> + *    update (tkr_mono.cycle_last);
> + *  - (cycle_delta * ntp_err_frac), the fractional-mult drift accrued over
> + *    the cycles read since then -- at most a tick on a tickful kernel, but
> + *    potentially many ticks' worth under NO_HZ;
> + *  - (cycle_delta * ntp_err_mult), subtracting the applied mult bias (the
> + *    +1 dither, or the larger signed err_drain bias while a drain is in
> + *    progress) over the same span;
> + *  - the sub-nanosecond fraction that ::systime dropped when the read was
> + *    truncated to whole ns (the low @shift bits, exact even though the
> + *    multiply overflows).
> + *
> + * CLOCK_MONOTONIC_RAW is not NTP-disciplined and carries no error. Every
> + * other clock id uses its own timekeeper @tk -- including the AUX clocks,
> + * which each have their own NTP instance.
> + */
> +static s64 snapshot_ntp_error(const struct timekeeper *tk, clockid_t clock_id,
> +			      u64 now)
> +{
> +	u64 cycle_delta;
> +	u32 nes;
> +	s64 tmp, err;
> +
> +	if (clock_id == CLOCK_MONOTONIC_RAW)
> +		return 0;
> +
> +	cycle_delta = (now - tk->tkr_mono.cycle_last) & tk->tkr_mono.mask;
> +	nes = tk->ntp_error_shift;
> +
> +	/* Insane deltas from marginal clocksources: skip the extrapolation */
> +	if (unlikely(cycle_delta > tk->tkr_mono.clock->max_cycles))
> +		return (tk->ntp_error + (1LL << (NTP_SCALE_SHIFT - 1))) >>
> +			NTP_SCALE_SHIFT;
> +
> +	err = tk->ntp_error;
> +	err += ((s64)mul_u64_u64_shr(cycle_delta, tk->ntp_err_frac, 32) -
> +		(s64)cycle_delta * tk->ntp_err_mult) << nes;
> +
> +	tmp = (s64)(cycle_delta * tk->tkr_mono.mult + tk->tkr_mono.xtime_nsec);
> +	tmp &= (1ULL << tk->tkr_mono.shift) - 1;
> +	err += tmp << nes;
> +
> +	return (err + (1LL << (NTP_SCALE_SHIFT - 1))) >> NTP_SCALE_SHIFT;
> +}
>
>  /**
>   * ktime_get_snapshot_id -  Simultaneously snapshot a given clock ID with
> @@ -1274,6 +1326,7 @@ void ktime_get_snapshot_id(clockid_t clock_id, struct system_time_snapshot *syst
>  {
>  	ktime_t base_raw, base_sys, offs_sys, *offs, offs_zero = 0;
>  	u64 nsec_raw, nsec_sys, now;
> +	s64 ntp_error;
>  	struct timekeeper *tk;
>  	struct tk_data *tkd;
>  	unsigned int seq;
> @@ -1336,10 +1389,12 @@ void ktime_get_snapshot_id(clockid_t clock_id, struct system_time_snapshot *syst
>
>  		nsec_sys = timekeeping_cycles_to_ns(&tk->tkr_mono, now);
>  		nsec_raw = timekeeping_cycles_to_ns(&tk->tkr_raw, now);
> +
> +		ntp_error = snapshot_ntp_error(tk, clock_id, now);
>  	} while (read_seqcount_retry(&tkd->seq, seq));
>
>  	systime_snapshot->cycles = now;
> -	systime_snapshot->systime = ktime_add_ns(base_sys, offs_sys + nsec_sys);
> +	systime_snapshot->systime = ktime_add_ns(base_sys, offs_sys + nsec_sys) + ntp_error;
>  	systime_snapshot->monoraw = ktime_add_ns(base_raw, nsec_raw);
>
>  	/*
> @@ -1591,6 +1646,7 @@ int get_device_system_crosststamp(int (*get_time_fn)
>  	ktime_t base_sys, base_raw, *offs;
>  	u32 clock_was_set_seq = 0;
>  	u64 nsec_sys, nsec_raw;
> +	s64 ntp_error;
>  	u8 cs_was_changed_seq;
>  	unsigned int seq;
>  	bool do_interp;
> @@ -1657,9 +1713,10 @@ int get_device_system_crosststamp(int (*get_time_fn)
>
>  		nsec_sys = timekeeping_cycles_to_ns(&tk->tkr_mono, cycles);
>  		nsec_raw = timekeeping_cycles_to_ns(&tk->tkr_raw, cycles);
> +		ntp_error = snapshot_ntp_error(tk, xtstamp->clock_id, cycles);
>  	} while (read_seqcount_retry(&tkd->seq, seq));
>
> -	xtstamp->sys_systime = ktime_add_ns(base_sys, nsec_sys);
> +	xtstamp->sys_systime = ktime_add_ns(base_sys, nsec_sys) + ntp_error;
>  	xtstamp->sys_monoraw = ktime_add_ns(base_raw, nsec_raw);
>
>  	/*
> @@ -2494,6 +2551,13 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
>  		skew *= NTP_INTERVAL_FREQ;
>  		dividend = (tk->ntp_tick + skew) >> tk->ntp_error_shift;
>  		mult = div64_u64(dividend, tk->cycle_interval);
> +		/*
> +		 * Stash the fractional part of the per-cycle ideal mult that
> +		 * the integer @mult discards, scaled by 2^32, in clock-shifted
> +		 * ns per cycle.
> +		 */
> +		dividend -= (u64)mult * tk->cycle_interval;
> +		tk->ntp_err_frac = div64_u64(dividend << 32, tk->cycle_interval);
>
>  		/*
>  		 * In timekeeping_apply_adjustment(), the mult_adj value is

Ciao,

Rodolfo

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error
  2026-11-06  8:44   ` Rodolfo Giometti
@ 2026-11-06 18:05     ` David Woodhouse
  0 siblings, 0 replies; 12+ messages in thread
From: David Woodhouse @ 2026-11-06 18:05 UTC (permalink / raw)
  To: Rodolfo Giometti, Thomas Gleixner, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Richard Cochran, Jie Wang,
	Yuan Wang, Ryan Luu, Julien Ridoux, linux-kernel, nh-open-source

[-- Attachment #1: Type: text/plain, Size: 3268 bytes --]

On Fri, 2026-11-06 at 09:44 +0100, Rodolfo Giometti wrote:
> On 06/11/2026 04:24, David Woodhouse wrote:
> > I think I reordered and reworded things as agreed with Rodolfo; please
> > shout if I screwed something up. What's sitting behind this in
> > https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/ffclock
> > is the timekeeping reference stuff for importing/exporting vmclock, and
> > preserving timekeeping across KHO:
> 
> No, the 5/6 and 6/6 split is what we agreed, thanks. On 6/6 I'm still
> missing the ntpd side, see my reply there.

Ack, thanks. Do you know who we should talk to? Should we be adding
them to MAINTAINERS to be Cc'd, like we did Miroslav?

> > The !NO_HZ_COMMON dependency can sit around for a little while longer
> > until the series above is actually applied, and it can note the
> > actually commit ID of its dependencies (which I think we agreed is just
> > patches 1 & 2 of the series above). I'll probably round it up with the
> > other PPS stuff I haven't quite got publishable yet, improving the
> > filtering and then working on counter-based capture and UAPI.
> 
> AFAIK we agreed on patches 1-4: the tickless run you did for me
> (rodolfo-14-tickless-1hz) had all four applied. If 1 and 2 alone are
> enough, could you put numbers for that in the commit message too?

It's the same. Patches 1 and 2 eliminate the cases where I *saw*
ntp_error grow large, which means that the code to *eliminate* it
basically didn't get much exercise. If I'd seen anything else introduce
any significant value into ntp_error, I'd have fixed that too. The
draining is belt-and-braces, and should only really come into play if
there are long tickless periods.

I'm travelling ATM but can do the run again with just those patches
when I get home in a couple of weeks, just to be sure.

> And for the UAPI, an RFC first please. :-)

Sure. For your visibility, on my TODO list for PPS is the stuff that
nerd-sniped me as I was testing with hardpps:

 • tracepoints to export the pulse data (so captures from real hardware
   can be replayed offline to experiment with filters):
   https://git.infradead.org/users/dwmw2/linux.git/commitdiff/hardpps-trace
 • a simulator for evaluating filters against those captures:
   https://git.infradead.org/pps-lab.git
 • improving the filtering itself (I've been playing with a few things
   starting with actually implementing median-of-three, but then
   ditching it for better outlier rejection, and going down a rabbit
   hole of other things like a constant +120ns bias from hardpps which
   chrony doesn't show on the *same* pulse data...)
 • continuous frequency tracking.

And orthogonally the things I originally wanted to look at:

 • counter-based capture: report the (counter, CLOCK_REALTIME) pair to
   userspace instead of throwing the counter value away, and let
   hardpps consume it directly
 • support for hardware which latches the clocksource counter on the
   pulse edge (no interrupt latency in the timestamp at all; such
   hardware exists and currently gets shoehorned through PTP EXTTS)

I'll probably try to come up with a UAPI RFC in the next couple of
weeks after I get home from KVM Forum.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-11-06 18:05 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-06  3:08 [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error David Woodhouse
2026-11-06  3:08 ` [PATCH v2 1/6] timekeeping: Allow tick_length changes to apply mid-tick David Woodhouse
2026-11-06  3:08 ` [PATCH v2 2/6] ntp: Recalculate skew_delta when the phase offset changes David Woodhouse
2026-11-06  3:08 ` [PATCH v2 3/6] timekeeping: Bound idle sleep while a phase slew is in flight David Woodhouse
2026-11-06  3:08 ` [PATCH v2 4/6] timekeeping: Reinstate proportional correction of ntp_error David Woodhouse
2026-11-06  3:08 ` [PATCH v2 5/6] pps: Always use ktime_get_snapshot_id() for pps_get_ts() David Woodhouse
2026-11-06  8:44   ` Rodolfo Giometti
2026-11-06  3:08 ` [PATCH v2 6/6] timekeeping: Apply extrapolated ntp_error to clock snapshots David Woodhouse
2026-11-06  8:44   ` Rodolfo Giometti
2026-11-06  3:24 ` [PATCH v2 0/6] timekeeping: Reduce magnitude of ntp_error David Woodhouse
2026-11-06  8:44   ` Rodolfo Giometti
2026-11-06 18:05     ` David Woodhouse

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®