[PATCH v4 0/4] Add ntp_error to clock snapshot, enable NTP_PPS on tickless kernel

[PATCH v4 0/4] Add ntp_error to clock snapshot, enable NTP_PPS on tickless kernel

[David Woodhouse]

The real time reported by the kernel's timekeeping sawtooths around the 
'ideal' time line that the kernel would like to report. Limited by the 
integer arithmetic, the kernel varies the 'mult' factor by ±1 each tick 
to achieve the correct rate on average over time. The reported time is 
further sanitized to ensure continuity when mult changes, even part way 
through a tick. The delta from the ideal to what is currently reported, 
is stored in tk->ntp_error. The sawtooth effect is more pronounced on 
tickless kernels, as the 'mult' value does not get adjusted each tick 
but only less frequently.

Both ktime_get_snapshot_id() and get_device_system_crosststamp() report 
the same sanitized time, but *every* user of those functions (PTP, PPS, 
KVM enlightenments) would be better served by the true ideal time.

Add snapshot_ntp_error() helper and use it from both of those functions 
to adjust the system time they report and return the more accurate 
result.

With this change, CONFIG_NTP_PPS works correctly on a tickless kernel;
enable it. And change the non-CONFIG_NTP_PPS code path in pps_get_ts()
to use ktime_get_snapshot_id() too, for the more accurate data.

Tested with a hack to make vmclock simulate a 1PPS signal, although there
are now better options for that. But it's enough to show that even the
tickless kernel converges to ±1ns of the PPS signal and remains there
(tested with a periodic PTP_SYS_OFFSET_EXTENDED to compare with the
vmclock reference).

[    0.631862] Run /init as init process
PPS: coarse-set CLOCK_REALTIME from vmclock
[    1.633957] pps pps0: bound kernel consumer: edge=0x1
PPS: enabled, bound to hardpps, STA_PPSTIME|STA_PPSFREQ set
PRECISE: dev-sys(adj)=+5565ns OK
EXT[ 0] diff=+5564ns
EXT[ 1] diff=+5562ns
[    2.696938] hardpps: PPSJITTER: jitter=5563, limit=0
EXT[ 2] diff=+5560ns
EXT[ 3] diff=+398ns
EXT[ 4] diff=+1ns
EXT[ 5] diff=+0ns
EXT[ 6] diff=+0ns
EXT[ 7] diff=+0ns
EXT[ 8] diff=+0ns
EXT[ 9] diff=+0ns
EXT[10] diff=+0ns



David Woodhouse (4):
      timekeeping: Apply extrapolated ntp_error to clock snapshots
      pps: Drop the !NO_HZ_COMMON dependency from NTP_PPS
      pps: Always use ktime_get_snapshot_id() for pps_get_ts()
      [DO NOT MERGE] ptp: ptp_vmclock: Add simulated 1PPS support

 drivers/pps/Kconfig                 |   3 -
 drivers/ptp/ptp_vmclock.c           | 196 ++++++++++++++++++++++++++++++++++--
 include/linux/pps_kernel.h          |   4 +-
 include/linux/timekeeper_internal.h |   6 ++
 kernel/time/timekeeping.c           |  70 ++++++++++++-
 5 files changed, 258 insertions(+), 21 deletions(-)

[PATCH v4 1/4] timekeeping: Apply extrapolated ntp_error to clock snapshots

[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.

This appears to be the reason why CONFIG_NTP_PPS is not enabled on
tickless kernels — because at that scale of precision, the realtime
snapshot at the time of the pulse bears little relation to the time the
kernel *actually* believes it to be, thus introducing random errors into
the PPS phase correction.

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 +1 mult dither
    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).

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.

Note that this *unconditionally* changes the ::systime returned by all
snapshot and cross timestamp consumers (PTP SYS_OFFSET_PRECISE/EXTENDED,
etc.): it is now the ideal NTP-disciplined time rather than the raw
accumulated clock.

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

diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index fe077d97b5f8..b17f7d54fb4d 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -97,6 +97,11 @@ 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
+ * @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. Used to
+ *				extrapolate @ntp_error to an arbitrary cycle count in
+ *				the lockless snapshot readers (ktime_get_snapshot_id).
  * @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
@@ -187,6 +192,7 @@ struct timekeeper {
 	s64			ntp_error;
 	u32			ntp_error_shift;
 	u32			ntp_err_mult;
+	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 ea2e6e55f37b..5ef7eb1ab62b 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -422,6 +422,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->skip_second_overflow = 0;
 	tk->skew_delta = 0;
 
@@ -1230,6 +1231,51 @@ 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 +1 mult dither
+ *    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;
+
+	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
@@ -1253,6 +1299,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;
@@ -1315,10 +1362,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);
 
 	/*
@@ -1570,6 +1619,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;
@@ -1636,9 +1686,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);
 
 	/*
@@ -2433,6 +2484,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;
 
 	/*
@@ -2451,8 +2503,18 @@ 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);
+		/*
+		 * 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. The lockless snapshot readers use it to
+		 * extrapolate @ntp_error forward over the cycles accumulated
+		 * since the last tick (which on a NO_HZ kernel may be many
+		 * ticks' worth).
+		 */
+		dividend -= (u64)mult * tk->cycle_interval;
+		tk->ntp_err_frac = div64_u64(dividend << 32, tk->cycle_interval);
 	}
 
 	/*
-- 
2.55.0

[PATCH v4 2/4] pps: Drop the !NO_HZ_COMMON dependency from NTP_PPS

[David Woodhouse]

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

NTP_PPS has been disabled for tickless kernels since it was first
introduced in commit 025b40abe715d ("ntp: add hardpps implementation")
in 2011, with a comment that it "doesn't work on tickless kernels at the
moment".

Whatever the original reasons were, the only *remaining* reason seems to
have been that the accuracy of the time captured by pps_get_ts() was poor
on tickless kernels due to the kernel's per-tick timekeeping mechanism.

A recent change to ktime_get_snapshot_id() which is used by pps_get_ts()
has fixed that problem, by applying a correction to the ::systime field
so that it reports the ideal NTP-corrected time rather than the sanitized
and smoothed version which varies each tick.

Drop the no longer needed dependency.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.8
---
 drivers/pps/Kconfig | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
index e1651d51cfc9..3a2c457ed9d8 100644
--- a/drivers/pps/Kconfig
+++ b/drivers/pps/Kconfig
@@ -31,13 +31,10 @@ config PPS_DEBUG
 
 config NTP_PPS
 	bool "PPS kernel consumer support"
-	depends on !NO_HZ_COMMON
 	help
 	  This option adds support for direct in-kernel time
 	  synchronization using an external PPS signal.
 
-	  It doesn't work on tickless systems at the moment.
-
 source "drivers/pps/clients/Kconfig"
 
 source "drivers/pps/generators/Kconfig"
-- 
2.55.0

[PATCH v4 3/4] pps: Always use ktime_get_snapshot_id() for pps_get_ts()

[David Woodhouse]

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

A recent commit changed ktime_get_snapshot_id() to return a corrected
::systime value which takes into account the divergence of the normal
per-tick timekeeping from the ideal NTP-disciplined clock.

Rather than using that more accurate timestamp *only* in the case where
CONFIG_NTP_PPS is enabled, do so unconditionally.

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

[PATCH v4 4/4] [DO NOT MERGE] ptp: ptp_vmclock: Add simulated 1PPS support

[David Woodhouse]

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

Before the timekeeping_set_reference() work, the simplest way to
synchronise the kernel against vmclock was to simulate a 1PPS signal.
Restore that hack here, for testing CONFIG_NTP_PPS in NOHZ mode.

Set up an hrtimer to fire at each vmclock second boundary, and teach
vmclock_get_crosststamp() to return the cycle counter and the
corresponding { systime, monoraw } at the *start* of the current second,
because hardpps() expects the timestamps it is given for phase and
frequency adjustment to be the kernel's clock readings at the moment the
true time is at the top of a second (i.e. when the pulse arrives).

The timer feeds a PTP_CLOCK_PPSUSR event; with PTP_ENABLE_PPS, the PPS
source bound to the in-kernel hardpps() consumer and STA_PPSTIME/PPSFREQ
set, the kernel disciplines CLOCK_REALTIME directly from vmclock. The
second-boundary cycle is recovered from get_device_system_crosststamp()
using a history snapshot for interpolation.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.8
---
 drivers/ptp/ptp_vmclock.c | 196 +++++++++++++++++++++++++++++++++++---
 1 file changed, 185 insertions(+), 11 deletions(-)

diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c
index bb0e14bac9f2..44fb82f4054f 100644
--- a/drivers/ptp/ptp_vmclock.c
+++ b/drivers/ptp/ptp_vmclock.c
@@ -13,6 +13,7 @@
 #include <linux/err.h>
 #include <linux/file.h>
 #include <linux/fs.h>
+#include <linux/hrtimer.h>
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/interrupt.h>
@@ -51,6 +52,10 @@ struct vmclock_state {
 	enum clocksource_ids cs_id, sys_cs_id;
 	int index;
 	char *name;
+	struct hrtimer pps_timer;
+	bool pps_enabled;
+	struct system_time_snapshot history_snap;
+	bool history_valid;
 };
 
 #define VMCLOCK_MAX_WAIT ms_to_ktime(100)
@@ -98,10 +103,13 @@ static bool tai_adjust(struct vmclock_abi *clk, uint64_t *sec)
 static int vmclock_get_crosststamp(struct vmclock_state *st,
 				   struct ptp_system_timestamp *sts,
 				   struct system_counterval_t *system_counter,
-				   struct timespec64 *tspec)
+				   struct timespec64 *tspec,
+				   bool on_second)
 {
 	ktime_t deadline = ktime_add(ktime_get(), VMCLOCK_MAX_WAIT);
 	uint64_t cycle, delta, seq, frac_sec;
+	uint64_t period_frac_sec;
+	uint8_t period_shift;
 
 #ifdef CONFIG_X86
 	/*
@@ -154,11 +162,46 @@ static int vmclock_get_crosststamp(struct vmclock_state *st,
 
 		delta = cycle - le64_to_cpu(st->clk->counter_value);
 
+		period_frac_sec = le64_to_cpu(st->clk->counter_period_frac_sec);
+		period_shift = st->clk->counter_period_shift;
+
 		frac_sec = mul_u64_u64_shr_add_u64(&tspec->tv_sec, delta,
-						   le64_to_cpu(st->clk->counter_period_frac_sec),
-						   st->clk->counter_period_shift,
+						   period_frac_sec, period_shift,
 						   le64_to_cpu(st->clk->time_frac_sec));
-		tspec->tv_nsec = mul_u64_u64_shr(frac_sec, NSEC_PER_SEC, 64);
+
+		/* For simulated PPS, adjust to the most recent second boundary */
+		if (on_second) {
+			uint64_t delta_cycles;
+			int frac_shift, shift_remain;
+
+			if (tspec->tv_sec == 0)
+				return -EAGAIN;  /* No second boundary crossed yet */
+
+			/*
+			 * Roll the counter back to the top of the current second.
+			 * frac_sec == 0 means we are already exactly on the
+			 * boundary (and __builtin_clzll(0) is undefined).
+			 */
+			if (frac_sec) {
+				/* Shift frac_sec left until top bit is set */
+				frac_shift = __builtin_clzll(frac_sec);
+				frac_sec <<= frac_shift;
+
+				/* Shift period right by the remaining bits */
+				shift_remain = period_shift - frac_shift;
+				if (shift_remain > 0)
+					period_frac_sec >>= shift_remain;
+				else
+					frac_sec >>= -shift_remain;
+
+				delta_cycles = frac_sec / period_frac_sec;
+				cycle -= delta_cycles;
+			}
+			tspec->tv_nsec = 0;
+		} else {
+			tspec->tv_nsec = mul_u64_u64_shr(frac_sec, NSEC_PER_SEC, 64);
+		}
+
 		tspec->tv_sec += le64_to_cpu(st->clk->time_sec);
 
 		if (!tai_adjust(st->clk, &tspec->tv_sec))
@@ -193,7 +236,8 @@ static int vmclock_get_crosststamp(struct vmclock_state *st,
 static int vmclock_get_crosststamp_kvmclock(struct vmclock_state *st,
 					    struct ptp_system_timestamp *sts,
 					    struct system_counterval_t *system_counter,
-					    struct timespec64 *tspec)
+					    struct timespec64 *tspec,
+					    bool on_second)
 {
 	struct pvclock_vcpu_time_info *pvti = this_cpu_pvti();
 	unsigned int pvti_ver;
@@ -204,7 +248,7 @@ static int vmclock_get_crosststamp_kvmclock(struct vmclock_state *st,
 	do {
 		pvti_ver = pvclock_read_begin(pvti);
 
-		ret = vmclock_get_crosststamp(st, sts, system_counter, tspec);
+		ret = vmclock_get_crosststamp(st, sts, system_counter, tspec, on_second);
 		if (ret)
 			break;
 
@@ -240,10 +284,10 @@ static int ptp_vmclock_get_time_fn(ktime_t *device_time,
 #ifdef SUPPORT_KVMCLOCK
 	if (READ_ONCE(st->sys_cs_id) == CSID_X86_KVM_CLK)
 		ret = vmclock_get_crosststamp_kvmclock(st, NULL, system_counter,
-						       &tspec);
+						       &tspec, false);
 	else
 #endif
-		ret = vmclock_get_crosststamp(st, NULL, system_counter, &tspec);
+		ret = vmclock_get_crosststamp(st, NULL, system_counter, &tspec, false);
 
 	if (!ret)
 		*device_time = timespec64_to_ktime(tspec);
@@ -280,6 +324,98 @@ static int ptp_vmclock_getcrosststamp(struct ptp_clock_info *ptp,
 	return ret;
 }
 
+static int ptp_vmclock_get_time_fn_pps(ktime_t *device_time,
+				       struct system_counterval_t *system_counter,
+				       void *ctx)
+{
+	struct vmclock_state *st = ctx;
+	struct timespec64 tspec;
+	int ret;
+
+#ifdef SUPPORT_KVMCLOCK
+	if (st->history_valid && st->history_snap.cs_id == CSID_X86_KVM_CLK)
+		ret = vmclock_get_crosststamp_kvmclock(st, NULL, system_counter,
+						       &tspec, true);
+	else
+#endif
+		ret = vmclock_get_crosststamp(st, NULL, system_counter, &tspec, true);
+
+	if (!ret)
+		*device_time = timespec64_to_ktime(tspec);
+
+	return ret;
+}
+
+/*
+ * Generate simulated PPS events for feeding __hardpps(), which expects to be
+ * given both CLOCK_REALTIME and CLOCK_MONOTONIC_RAW values for when a 1PPS
+ * signal actually happened (i.e. at the top of a second).
+ *
+ * vmclock_get_crosststamp(..., on_second=true) reads the vmclock and both
+ * system clocks from the same TSC value, then rolls the TSC back to the value
+ * it would have had at the start of the current second so the timestamps line
+ * up with a real pulse. The hrtimer reschedules itself for the top of the next
+ * second according to *vmclock*, not necessarily CLOCK_REALTIME.
+ */
+static enum hrtimer_restart ptp_vmclock_pps_timer(struct hrtimer *timer)
+{
+	struct vmclock_state *st = container_of(timer, struct vmclock_state, pps_timer);
+	struct system_device_crosststamp xtstamp = { .clock_id = CLOCK_REALTIME };
+	struct ptp_clock_event event;
+	ktime_t next, now_rt;
+	s64 delta_ns;
+	int ret;
+
+	if (!st->pps_enabled)
+		return HRTIMER_NORESTART;
+
+	/* Only report PPS if we have a valid history snapshot to interpolate from */
+	ret = -EINVAL;
+	if (st->history_valid) {
+		ret = get_device_system_crosststamp(ptp_vmclock_get_time_fn_pps, st,
+						    &st->history_snap, &xtstamp);
+		if (!ret) {
+			event.type = PTP_CLOCK_PPSUSR;
+			event.pps_times.ts_real = ktime_to_timespec64(xtstamp.sys_systime);
+#ifdef CONFIG_NTP_PPS
+			event.pps_times.ts_raw = ktime_to_timespec64(xtstamp.sys_monoraw);
+#endif
+			ptp_clock_event(st->ptp_clock, &event);
+		}
+	}
+
+	/* Capture a snapshot to bound the next interpolation */
+	ktime_get_snapshot_id(CLOCK_REALTIME, &st->history_snap);
+	st->history_valid = true;
+
+	/*
+	 * Schedule the next timer for the top of the next second according to
+	 * vmclock. If we reported a PPS event, xtstamp.sys_systime is already
+	 * at the second boundary, so just add a second; otherwise read the
+	 * current vmclock time and work out when it next hits a boundary.
+	 */
+	if (!ret) {
+		next = ktime_add_ns(xtstamp.sys_systime, NSEC_PER_SEC);
+	} else {
+		struct timespec64 ts;
+
+		if (vmclock_get_crosststamp(st, NULL, NULL, &ts, false))
+			return HRTIMER_NORESTART;
+
+		delta_ns = NSEC_PER_SEC - ts.tv_nsec;
+		next = ktime_add_ns(st->history_snap.systime, delta_ns);
+	}
+
+	/* Never reschedule in the past, or the timer tight-loops */
+	now_rt = ktime_get_real();
+	if (ktime_compare(next, now_rt) <= 0)
+		next = ktime_add_ns(now_rt, NSEC_PER_SEC);
+
+	hrtimer_set_expires(timer, next);
+
+	return HRTIMER_RESTART;
+}
+
 /*
  * PTP clock operations
  */
@@ -306,12 +442,43 @@ static int ptp_vmclock_gettimex(struct ptp_clock_info *ptp, struct timespec64 *t
 	struct vmclock_state *st = container_of(ptp, struct vmclock_state,
 						ptp_clock_info);
 
-	return vmclock_get_crosststamp(st, sts, NULL, ts);
+	return vmclock_get_crosststamp(st, sts, NULL, ts, false);
 }
 
 static int ptp_vmclock_enable(struct ptp_clock_info *ptp,
 			  struct ptp_clock_request *rq, int on)
 {
+	struct vmclock_state *st = container_of(ptp, struct vmclock_state,
+						ptp_clock_info);
+
+	switch (rq->type) {
+	case PTP_CLK_REQ_PPS:
+		st->pps_enabled = !!on;
+		if (on) {
+			struct timespec64 ts;
+			s64 delta_ns;
+
+			/* Snapshot to bound the first interpolation */
+			ktime_get_snapshot_id(CLOCK_REALTIME, &st->history_snap);
+			st->history_valid = true;
+
+			if (vmclock_get_crosststamp(st, NULL, NULL, &ts, false))
+				return -EIO;
+
+			/* When will vmclock next reach a second boundary? */
+			delta_ns = NSEC_PER_SEC - ts.tv_nsec;
+
+			hrtimer_start(&st->pps_timer,
+				      ktime_add_ns(st->history_snap.systime, delta_ns),
+				      HRTIMER_MODE_ABS);
+		} else {
+			hrtimer_cancel(&st->pps_timer);
+		}
+		return 0;
+	default:
+		break;
+	}
+
 	return -EOPNOTSUPP;
 }
 
@@ -320,7 +487,7 @@ static const struct ptp_clock_info ptp_vmclock_info = {
 	.max_adj	= 0,
 	.n_ext_ts	= 0,
 	.n_pins		= 0,
-	.pps		= 0,
+	.pps		= 1,
 	.adjfine	= ptp_vmclock_adjfine,
 	.adjtime	= ptp_vmclock_adjtime,
 	.gettimex64	= ptp_vmclock_gettimex,
@@ -356,6 +523,10 @@ static struct ptp_clock *vmclock_ptp_register(struct device *dev,
 	st->ptp_clock_info = ptp_vmclock_info;
 	strscpy(st->ptp_clock_info.name, st->name);
 
+	hrtimer_setup(&st->pps_timer, ptp_vmclock_pps_timer, CLOCK_REALTIME,
+		      HRTIMER_MODE_ABS);
+	st->pps_enabled = false;
+
 	return ptp_clock_register(&st->ptp_clock_info, dev);
 }
 
@@ -643,8 +814,11 @@ static void vmclock_remove(void *data)
 					   vmclock_acpi_notification_handler);
 #endif
 
-	if (st->ptp_clock)
+	if (st->ptp_clock) {
+		st->pps_enabled = false;
+		hrtimer_cancel(&st->pps_timer);
 		ptp_clock_unregister(st->ptp_clock);
+	}
 
 	if (st->miscdev.minor != MISC_DYNAMIC_MINOR)
 		misc_deregister(&st->miscdev);
-- 
2.55.0

Re: [PATCH v4 0/4] Add ntp_error to clock snapshot, enable NTP_PPS on tickless kernel

[Rodolfo Giometti]

On Sat, 2026-08-29 at 21:56 +0100, David Woodhouse wrote:
> With this change, CONFIG_NTP_PPS works correctly on a tickless kernel;
> enable it. And change the non-CONFIG_NTP_PPS code path in pps_get_ts()
> to use ktime_get_snapshot_id() too, for the more accurate data.

Thanks for respinning. The idea is good, the !NO_HZ_COMMON dependency
has needed attention since 2011. Comments on 2/4 and 3/4 go in
separate mails. Here the general ones.

The series does not apply to mainline (v7.1-13176-g840ef6c78e6a). It
is written against some timekeeping rework that is not merged yet,
and there is no base-commit: and no word about which tree to use.
Please repost with "git format-patch --base=".

There is no changelog. Where are the v3 -> v4 notes? And why is this
now PATCH and no longer RFC?

3/4 does nothing at all without 1/4: it changes no timestamp value,
only the cost. So the series has to go through tip/timers as a unit,
not with the PPS bits going via Andrew separately.

1/4 itself is not mine to judge. It changes ::systime for every user of
ktime_get_snapshot_id() and get_device_system_crosststamp(), not only
PPS. Whether that is the right value for ptp4l and phc2sys is for
Richard and the PTP people to say. I raise it only because I want that
decision made explicitly, not inherited from a PPS series.

> Tested with a hack to make vmclock simulate a 1PPS signal, although there
> are now better options for that. But it's enough to show that even the
> tickless kernel converges to [...] the PPS signal and remains there
> (tested with a periodic PTP_SYS_OFFSET_EXTENDED to compare with the
> vmclock reference).

It is not enough. 4/4 takes the pulse from the same counter the
timekeeping reads, so there is no independent reference in the test at
all. Converging to +0ns against your own clock source proves very
little, and it says nothing about hardpps() driven by a real pulse.

You are asking me to undo something that has stood for fifteen years.
I am not going to ack that on this evidence. :) What I want to see
instead is in my reply to 2/4.

Rodolfo

Re: [PATCH v4 2/4] pps: Drop the !NO_HZ_COMMON dependency from NTP_PPS

[Rodolfo Giometti]

On Sat, 2026-08-29 at 21:57 +0100, David Woodhouse wrote:
> Whatever the original reasons were, the only *remaining* reason seems to
> have been that the accuracy of the time captured by pps_get_ts() was poor
> on tickless kernels due to the kernel's per-tick timekeeping mechanism.

"Whatever the original reasons were" and "seems to have been" is not
enough to drop a dependency that has been there for fifteen years. The
old comment is useless, I agree. But then we have to say what breaks
and what does not, not guess.

First a structural point. This is the only patch of the four that
applies to mainline, and it has no build dependency on the rest. It is
a three-line Kconfig delete that compiles on its own. That worries me:
a small "pps:" patch that applies cleanly is exactly what gets picked
up alone. Then NTP_PPS becomes selectable on tickless kernels without
1/4, and we are worse off than today. Reorder it last, or say in the
commit message that it must not be applied without 1/4.

> A recent change to ktime_get_snapshot_id() which is used by pps_get_ts()
> has fixed that problem, by applying a correction to the ::systime field

That "recent change" is 1/4 of this series, and it is in no tree yet.
Reading this, one assumes the groundwork already landed. Say "the
previous patch". Same wording in 3/4.

About the test. The pulse comes from 4/4, which derives it from the
same counter the timekeeping reads. No independent reference anywhere.

Before I ack this I want to see:

   - a real source, pps-gpio with a GPS receiver, where pulse and system
     clock are independent;
   - NO_HZ_FULL, not only NO_HZ_IDLE;
   - a run that goes through a long idle period, not just a busy system.

That is more work than a three-line delete suggests, I know. But those
three lines unlock a configuration people will run against real
receivers and then trust.

Rodolfo

Re: [PATCH v4 3/4] pps: Always use ktime_get_snapshot_id() for pps_get_ts()

[Rodolfo Giometti]

On Sat, 2026-08-29 at 21:57 +0100, David Woodhouse wrote:
> Rather than using that more accurate timestamp *only* in the case where
> CONFIG_NTP_PPS is enabled, do so unconditionally.
>
>  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
>  }

Why are you removing ktime_get_real_ts64()?

The commit message says you are using the more accurate timestamp
unconditionally. What the diff does is delete the !CONFIG_NTP_PPS
branch. Those are not the same thing, and the second one changes an
ABI: ts_real reaches userspace through PPS_FETCH on /dev/ppsN and
through /sys/class/pps/ppsX/assert, documented in
Documentation/ABI/testing/sysfs-pps and unchanged since 2008. Until
now it came from ktime_get_real_ts64(), the same clock userspace reads
with clock_gettime(CLOCK_REALTIME). After your patch it does not.

That needs a good reason and the commit message does not give one.
Give me that first. Until then the details do not matter.

Separately: are we sure that calling ktime_get_snapshot_id() does not
introduce much larger delays than ktime_get_real_ts64()? pps_get_ts()
runs in hard IRQ, and in pps-gpio it is the first statement of the
handler. Whatever it costs sits between the edge and the timestamp,
and that is the one thing PPS has to keep short.

I would like to see that measured on something other than an x86 VM
with a TSC. A small 32-bit ARM board is what I worry about.

Rodolfo

Re: [PATCH v4 0/4] Add ntp_error to clock snapshot, enable NTP_PPS on tickless kernel

[David Woodhouse]

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

On Tue, 2026-09-01 at 17:35 +0200, Rodolfo Giometti wrote:
> On Sat, 2026-08-29 at 21:56 +0100, David Woodhouse wrote:
> > With this change, CONFIG_NTP_PPS works correctly on a tickless kernel;
> > enable it. And change the non-CONFIG_NTP_PPS code path in pps_get_ts()
> > to use ktime_get_snapshot_id() too, for the more accurate data.
> 
> Thanks for respinning. The idea is good, the !NO_HZ_COMMON dependency
> has needed attention since 2011. Comments on 2/4 and 3/4 go in
> separate mails. Here the general ones.
> 
> The series does not apply to mainline (v7.1-13176-g840ef6c78e6a). It
> is written against some timekeeping rework that is not merged yet,
> and there is no base-commit: and no word about which tree to use.
> Please repost with "git format-patch --base=".

Oops, the dependencies actually landed in Linus's tree a lot more
recently than I thought; I thought it was so old that it wasn't worth
specifying a base. In fact although it *does* apply to mainline, it
needs a tree from mid-August or fresher (v7.2-1297-g3b4128b9f374).

> There is no changelog. Where are the v3 -> v4 notes? And why is this
> now PATCH and no longer RFC?

There were no substantive changes from RFC v3 to v4 (I rewrapped one
line which offended me while rebasing, I believe).

It's no longer RFC because nobody saw fit to comment :)

The evolution of this was in the timekeeping part, not on the PPS side.
If you really want to see it:

v1: https://lore.kernel.org/all/3616fc9718614bf11915569599038a5bcb268c02.camel@infradead.org/
v2: https://lore.kernel.org/all/3b10d2e91b18f49d8a3e6226b08ac8cd9cb49aa6.camel@infradead.org/
v3: https://lore.kernel.org/all/20260622211822.1056437-1-dwmw2@infradead.org/

> 3/4 does nothing at all without 1/4: it changes no timestamp value,
> only the cost. So the series has to go through tip/timers as a unit,
> not with the PPS bits going via Andrew separately.

Yes. Or we let the timekeeping part go through, and the PPS part can
come later. There's no rush.

> 1/4 itself is not mine to judge. It changes ::systime for every user of
> ktime_get_snapshot_id() and get_device_system_crosststamp(), not only
> PPS. Whether that is the right value for ptp4l and phc2sys is for
> Richard and the PTP people to say. I raise it only because I want that
> decision made explicitly, not inherited from a PPS series.
> 
> > Tested with a hack to make vmclock simulate a 1PPS signal, although there
> > are now better options for that. But it's enough to show that even the
> > tickless kernel converges to [...] the PPS signal and remains there
> > (tested with a periodic PTP_SYS_OFFSET_EXTENDED to compare with the
> > vmclock reference).
> 
> It is not enough. 4/4 takes the pulse from the same counter the
> timekeeping reads, so there is no independent reference in the test at
> all. Converging to +0ns against your own clock source proves very
> little, and it says nothing about hardpps() driven by a real pulse.

It's not about the counter being an independent reference. The point
here is that in a NO_HZ_FULL kernel, the CLOCK_REALTIME reading itself
sawtooths around where it should be, so even that perfect *dependent*
reference gives deltas of up to ±10ns for the same test.

You may be right that it doesn't actually test the pps_get_ts() path
though; the vmclock test builds its own pps_times. So it's proving the
*concept* but not the actual pps_get_ts() code path.

> You are asking me to undo something that has stood for fifteen years.
> I am not going to ack that on this evidence. :) What I want to see
> instead is in my reply to 2/4.

Ack. qv.

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

Re: [PATCH v4 3/4] pps: Always use ktime_get_snapshot_id() for pps_get_ts()

[David Woodhouse]

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

On Tue, 2026-09-01 at 17:35 +0200, Rodolfo Giometti wrote:
> On Sat, 2026-08-29 at 21:57 +0100, David Woodhouse wrote:
> > Rather than using that more accurate timestamp *only* in the case where
> > CONFIG_NTP_PPS is enabled, do so unconditionally.
> > 
> >   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
> >   }
> 
> Why are you removing ktime_get_real_ts64()?

Because it's inaccurate.

> The commit message says you are using the more accurate timestamp
> unconditionally. What the diff does is delete the !CONFIG_NTP_PPS
> branch. Those are not the same thing, and the second one changes an
> ABI: ts_real reaches userspace through PPS_FETCH on /dev/ppsN and
> through /sys/class/pps/ppsX/assert, documented in
> Documentation/ABI/testing/sysfs-pps and unchanged since 2008. Until
> now it came from ktime_get_real_ts64(), the same clock userspace reads
> with clock_gettime(CLOCK_REALTIME). After your patch it does not.

Hm? After the patch it still reports CLOCK_REALTIME. Just more
accurately. Which is exactly what the commit message was trying to say.

> That needs a good reason and the commit message does not give one.
> Give me that first. Until then the details do not matter.

ktime_get_real_ts64() returns an approximate value of CLOCK_REALTIME
which is limited by the quantisation of the multiplier in the kernel's
core timekeeping. ktime_get_snapshot_id(CLOCK_REALTIME) returns a value
which is *corrected* for that drift.

> Separately: are we sure that calling ktime_get_snapshot_id() does not
> introduce much larger delays than ktime_get_real_ts64()? pps_get_ts()
> runs in hard IRQ, and in pps-gpio it is the first statement of the
> handler. Whatever it costs sits between the edge and the timestamp,
> and that is the one thing PPS has to keep short.

The critical measure here is the time from the edge to the actual
tk_clock_read() or tk_clock_read_snapshot() call which captures the
hardware counter value which is converted into the CLOCK_REALTIME
reading.

ktime_get_snapshot_id() is what's been called in the CONFIG_NTP_PPS
case for a very long time — we're just making it use that same path
unconditionally. The difference between that and ktime_get_real_ts64()
is tiny, but non-zero — it's about 2ns to the snapshot on my x86 test
host.

Even ktime_get_real_ts64() is doing things before the snapshot that
perhaps it needn't (setting ts->tv_sec), and could save us a couple of
cycles. If this is a real concern, we could certainly look at putting
the clock snapshot as early as possible and optimising the common case
of clock_id == CLOCK_REALTIME. 

> I would like to see that measured on something other than an x86 VM
> with a TSC. A small 32-bit ARM board is what I worry about.

Ack. I'll take a look.

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

Re: [PATCH v4 2/4] pps: Drop the !NO_HZ_COMMON dependency from NTP_PPS

[David Woodhouse]

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

On Tue, 2026-09-01 at 17:35 +0200, Rodolfo Giometti wrote:
> On Sat, 2026-08-29 at 21:57 +0100, David Woodhouse wrote:
> > Whatever the original reasons were, the only *remaining* reason seems to
> > have been that the accuracy of the time captured by pps_get_ts() was poor
> > on tickless kernels due to the kernel's per-tick timekeeping mechanism.
> 
> "Whatever the original reasons were" and "seems to have been" is not
> enough to drop a dependency that has been there for fifteen years. The
> old comment is useless, I agree. But then we have to say what breaks
> and what does not, not guess.

What breaks is this:

The kernel's core timekeeping keeps a 'mult' (multiplier) value which
it adjusts per tick. As it's an integer, it can either go slightly too
fast, or slightly too slow. The kernel dithers between adjacent values,
to achieve the correct overall rate for CLOCK_REALTIME.

The kernel *tracks* the actual error between what it's reporting in
CLOCK_REALTIME, and what it *should* be reporting, in order to choose
whether to use the high or low value for the next tick.

... in a *tickful* kernel, that is. In a tickful kernel, CLOCK_REALTIME
never really gets that far from where it should be, because the 'mult'
rate is adjusted every tick.

In a *tickless* kernel, it can go a *very* long time without adjusting
'mult', and thus the reported CLOCK_REALTIME can get a very long way
ahead of, or behind, what the kernel actually knows the time to be.

Since patch 1 of this series, ktime_get_snapshot_id() returns the
*corrected* time, while ktime_get_real_ts64() returns the sawtoothing
version.

That's why PPS wasn't viable in a NO_HZ_FULL kernel, and now is.

> First a structural point. This is the only patch of the four that
> applies to mainline, and it has no build dependency on the rest. It is
> a three-line Kconfig delete that compiles on its own. That worries me:
> a small "pps:" patch that applies cleanly is exactly what gets picked
> up alone. Then NTP_PPS becomes selectable on tickless kernels without
> 1/4, and we are worse off than today. Reorder it last, or say in the
> commit message that it must not be applied without 1/4.

Ack.

> > A recent change to ktime_get_snapshot_id() which is used by pps_get_ts()
> > has fixed that problem, by applying a correction to the ::systime field
> 
> That "recent change" is 1/4 of this series, and it is in no tree yet.
> Reading this, one assumes the groundwork already landed. Say "the
> previous patch". Same wording in 3/4.

I thought people hated 'the previous patch'. Probably better to let the
timekeeping patch hit tip, then reference it by commit id. As I said,
there's no rush for any of this. Hell, if you don't care, there's no
*need* for any of this. It's a cleanup that seemed worth doing while I
was fixing things in this area.

> About the test. The pulse comes from 4/4, which derives it from the
> same counter the timekeeping reads. No independent reference anywhere.

As noted elsewhere, that's still showing what it needs to show because
it's all about how we calculate CLOCK_REALTIME *from* that counter.
With the PPS changes and *not* the timekeeping fix, we see large skews
during idle. Fixing ktime_get_snapshot_id() in patch 1 brings it back
to where it should be.

> Before I ack this I want to see:
> 
>    - a real source, pps-gpio with a GPS receiver, where pulse and system
>      clock are independent;
>    - NO_HZ_FULL, not only NO_HZ_IDLE;
>    - a run that goes through a long idle period, not just a busy system.
> 
> That is more work than a three-line delete suggests, I know. But those
> three lines unlock a configuration people will run against real
> receivers and then trust.

Sure, happy to put that together. I don't have actual PPS hardware;
I'll have to see what I can come up with.

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

Re: [PATCH v4 3/4] pps: Always use ktime_get_snapshot_id() for pps_get_ts()

[David Woodhouse]

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

On Tue, 2026-09-01 at 17:35 +0200, Rodolfo Giometti wrote:
> 
> Separately: are we sure that calling ktime_get_snapshot_id() does not
> introduce much larger delays than ktime_get_real_ts64()? pps_get_ts()
> runs in hard IRQ, and in pps-gpio it is the first statement of the
> handler. Whatever it costs sits between the edge and the timestamp,
> and that is the one thing PPS has to keep short.
> 
> I would like to see that measured on something other than an x86 VM
> with a TSC. A small 32-bit ARM board is what I worry about.

Not 32-bit but I had a Banana Pi R64 lying around (Cortex A53, 12.5MHz
arch counter) and I bought it a GPS hat.

So yes, the specific code path you're looking at *does* get slightly
longer (50ns to the counter read instead of 30ns). Some of which is
easily reclaimable with some optimisations...

Firstly, why in $DEITY's name does the Arm kernel not set
CONFIG_ARCH_WANTS_CLOCKSOURCE_READ_INLINE? Setting that and using it
gains us 7-8ns back in both paths. I can get another 2-3ns back by
optimising the CLOCK_REALTIME path through ktime_get_snapshot_id() and
only hitting the case statement for other clock IDs:

                                    ┌───────────────┬─────────────────┐
                                    │  ktime_get_   │   ktime_get_    │
                                    │  real_ts64()  │  snapshot_id()  │
────────────────────────────────────┼───────────────┼─────────────────┤
 stock                              │    30.2 ns    │     49.9 ns     │
 + arm64 inlined clocksource read   │    23.4 ns    │     49.9 ns     │
 + inlined in get_snapshot_id() too │    23.2 ns    │     41.2 ns     │
 + CLOCK_REALTIME dispatch bias     │    23.4 ns    │     38.8 ns     │
────────────────────────────────────┴───────────────┴─────────────────┘

So I'll probably do those things because I've seen them now.

However, they are *entirely* in the noise, as there's about 600 ns of
hardware and 2-4 *microseconds* of software latency before we even get
there.

I was pondering an IRQF_HWTIMESTAMP which would sample the arch-defined
inline clocksource as early as possible and stash it in the pt_regs, so
I knocked up a proof of concept which just did that unconditionally in
kernel_entry in arch/arm64/kernel/entry.S. That's what gives me the
software latency I cited (~2.1µs from that stamp to the GPIO handler). 

To measure the hardware latency I removed the hat and looped the PPS
pin back from an output GPIO, then read the counter the instruction
before the MMIO write to trigger a rising edge. That gave me 0.6µs to
the counter read in entry.S.

 pin edge → exception entry stamp:   min 0.40µs  med 0.64µs  p90 0.72µs
 entry stamp → pps handler:          min 2.08µs  med 2.16µs  p90 2.56µs
 pin edge → pps handler (sum):       min 2.72µs  med 2.80µs  p90 3.12µs

The latency is higher from cold with an actual PPS signal, as opposed
to the warm soak test.

So no, I really don't care about the tiny cost of calling
ktime_get_snapshot_id() to get a more accurate timestamp; at least that
cost is *constant* unlike the sawtoothing ntp_error that it eliminates
from the readings. (Which is actually larger than it should be; I have
more fixes for ntp_error.)

I'm still interested in the IRQF_HWTIMESTAMP thing. As that sample
would be taken outside the tkd->seq count we'd need to use something
like get_system_device_crosststamp() to reliably interpret it, but I'm
going to have to implement that *anyway*. I want to add support for a
hardware module which *latches* the counter value on the pulse, instead
of waiting for the CPU to wake up and do so for itself. One of those
TimeCard devices with PCIe PTM could do that, for example.

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

Re: [PATCH v4 3/4] pps: Always use ktime_get_snapshot_id() for pps_get_ts()

[Rodolfo Giometti]

On Sat, 2026-09-26 at 21:38 +0100, David Woodhouse wrote:
> So yes, the specific code path you're looking at *does* get slightly
> longer (50ns to the counter read instead of 30ns).
[...]
> However, they are *entirely* in the noise, as there's about 600 ns of
> hardware and 2-4 *microseconds* of software latency before we even get
> there.

Thanks for measuring it, and on real hardware with a real edge. That
answers my concern: ~20 ns of constant cost against microseconds of
latency upstream of the handler is not something PPS can see, and
trading it for the removal of a non-constant error is the right
trade. It is arm64 rather than the 32-bit board I asked about, but I
accept the argument holds there too, since the software latency only
gets larger.

There is still one thing I want to be sure we agree on, about what
ts_real means for userspace.

Today, without CONFIG_NTP_PPS, ts_real comes from ktime_get_real_ts64(),
which is the same value userspace reads with
clock_gettime(CLOCK_REALTIME). A PPS timestamp and a clock_gettime()
reading are identical by construction: both are the sanitized clock.
With CONFIG_NTP_PPS the same held so far, since ktime_get_snapshot_id()
also returned the sanitized value.

After 1/4 and this patch that is no longer true. Your 1/4 says it
explicitly: callers of ktime_get_snapshot_id() now receive the ideal
time, "not the sanitized version provided to gettimeofday()". So ts_real
becomes the ideal line, while clock_gettime(CLOCK_REALTIME) keeps
returning the sanitized one, and the two differ by ntp_error at the
instant of the edge.

For hardpps this is clearly what we want, and your 1/4 and 2/4 make
that case. For userspace I am less sure. chrony and ntpd compare PPS
timestamps against other sources whose timestamps come from
clock_gettime(), i.e. from the sanitized clock, so after this series
the two would be referenced to different lines, ntp_error apart.
Miroslav, is that something chrony would notice? And David, how large
can that difference get in practice? Your 1/4 says the divergence can
span many ticks under NO_HZ.

Since the main users of ts_real are chrony and ntpd, not the kernel,
I would like an Acked-by from their maintainers before I ack this
patch. They are the ones who will have to live with the new semantics,
so they should know what is coming and agree with it.

So for v5 please put this in the commit message, not only in the
thread: why ktime_get_real_ts64() is inaccurate (the quantisation of
the multiplier), that ts_real is now the ideal NTP-disciplined time
rather than what clock_gettime() returns, the order of magnitude of
the difference, and a short summary of the latency numbers above.
Whoever runs git blame on pps_get_ts() in a few years should not have
to find this thread.

Ciao,

Rodolfo

Re: [PATCH v4 3/4] pps: Always use ktime_get_snapshot_id() for pps_get_ts()

[David Woodhouse]

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

On Mon, 2026-09-28 at 09:58 +0200, Rodolfo Giometti wrote:
> On Sat, 2026-09-26 at 21:38 +0100, David Woodhouse wrote:
> > So yes, the specific code path you're looking at *does* get slightly
> > longer (50ns to the counter read instead of 30ns).
> [...]
> > However, they are *entirely* in the noise, as there's about 600 ns of
> > hardware and 2-4 *microseconds* of software latency before we even get
> > there.
> 
> Thanks for measuring it, and on real hardware with a real edge. That
> answers my concern: ~20 ns of constant cost against microseconds of
> latency upstream of the handler is not something PPS can see, and
> trading it for the removal of a non-constant error is the right
> trade. It is arm64 rather than the 32-bit board I asked about, but I
> accept the argument holds there too, since the software latency only
> gets larger.
> 
> There is still one thing I want to be sure we agree on, about what
> ts_real means for userspace.
> 
> Today, without CONFIG_NTP_PPS, ts_real comes from ktime_get_real_ts64(),
> which is the same value userspace reads with
> clock_gettime(CLOCK_REALTIME). A PPS timestamp and a clock_gettime()
> reading are identical by construction: both are the sanitized clock.
> With CONFIG_NTP_PPS the same held so far, since ktime_get_snapshot_id()
> also returned the sanitized value.
> 
> After 1/4 and this patch that is no longer true. Your 1/4 says it
> explicitly: callers of ktime_get_snapshot_id() now receive the ideal
> time, "not the sanitized version provided to gettimeofday()". So ts_real
> becomes the ideal line, while clock_gettime(CLOCK_REALTIME) keeps
> returning the sanitized one, and the two differ by ntp_error at the
> instant of the edge.
> 
> For hardpps this is clearly what we want, and your 1/4 and 2/4 make
> that case. For userspace I am less sure. chrony and ntpd compare PPS
> timestamps against other sources whose timestamps come from
> clock_gettime(), i.e. from the sanitized clock, so after this series
> the two would be referenced to different lines, ntp_error apart.
> Miroslav, is that something chrony would notice? And David, how large
> can that difference get in practice? Your 1/4 says the divergence can
> span many ticks under NO_HZ.

Theoretically, absent other bugs (qv), ntp_error should rarely be more
than a few tens of nanoseconds and even that is the extreme case.

It swings slightly positive and negative as the reported CLOCK_REALTIME
goes slightly behind, and ahead of, the ideal time.

This happens because of the integer quantisation of the counter period.
The kernel spends a while running with the base 'mult' and getting
behind, and then switches to 'mult+1' to catch up. On a *tickful*
kernel, it flips between them as soon as ntp_error goes
positive/negative each tick.

On the board I'm testing with, that ±1 difference equates to about
0.745ns/s. Right now, its "ideal" mult calibrated against the PPS
signal is about in the middle (1342189164.4992), so that means it has a
choice of running 0.372ns/s slow, or 0.373ns/s fast. Worst case, the
value could come out very close to an integer, and the ±1 choices could
approximate the full 0.745 in one direction, and almost negligible in
the other.

So on a *tickless* kernel when it doesn't course correct every tick, it
could get set to gain 0.745ns/s and then go to sleep for, say, ten
minutes, and accumulate... half a microsecond.

I don't think ten minutes is really that realistic, of course. And in
practice I definitely can't make nohz_full actually contribute a
meaningful amount to ntp_error while *also* waking it up every second
(or even every 5 seconds) to process a pulse. 

However, I *have* seen (and fixed) the tick_length changes at chrony
startup introducing 83µs into ntp_error, which would take *days* to
drain through the normal ±1 dithering, and would screw up the actual
frequency settings while it was draining.

I've pushed out my current WIP to my timekeeping branch¹. The fix for
the 83µs ntp_error is the first² in the series — "timekeeping: Allow
tick_length changes to apply mid-tick".

> Since the main users of ts_real are chrony and ntpd, not the kernel,
> I would like an Acked-by from their maintainers before I ack this
> patch. They are the ones who will have to live with the new semantics,
> so they should know what is coming and agree with it.

Absolutely. I've called this out explicitly in the patch which makes
ktime_get_snapshot_id() do that correction. That's the second patch³ in
my branch — "timekeeping: Apply extrapolated ntp_error to clock
snapshots".

I wonder if we should switch PPS to using ktime_get_snapshot_id() in an
*earlier* patch, which wouldn't then include the behavioural change.
Then the note in the 'Apply extrapolated error' patch can then cover
PPS and we consider them all together.

The PPS change does stand alone anyway — regardless of the snapshot
*corrections*, I want PPS using snapshots so that it can report the
actual *counter* values to userspace, like PTP is going to be able to.
Then userspace can choose to discipline the actual *counter* without
worrying about the feedback loop of the kernel's own timekeeping at
all.

¹ https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/timekeeping
² https://git.infradead.org/?p=users/dwmw2/linux.git;a=commitdiff;h=3ff34be9c943
³ https://git.infradead.org/?p=users/dwmw2/linux.git;a=commitdiff;h=bac732b0bf23

> So for v5 please put this in the commit message, not only in the
> thread: why ktime_get_real_ts64() is inaccurate (the quantisation of
> the multiplier), that ts_real is now the ideal NTP-disciplined time
> rather than what clock_gettime() returns, the order of magnitude of
> the difference, and a short summary of the latency numbers above.
> Whoever runs git blame on pps_get_ts() in a few years should not have
> to find this thread.

Ack. I discussed the order of magnitude above. Let's see how my
proposed optimisations land, and I'll update the comments on latency
too.

For the "ts_real is suddenly something different" aspect, I'll defer to
Miroslav et al but I do believe that this is either in the noise at
most levels of precision, or a *correction* where it's even noticeable.

The use cases which consume a snapshot are the ones which return a
tuple of that timestamp against something simultaneous — either the raw
counter value, a PPS pulse which is known to be at the top of a second,
a PTP reading from an external clock. (Worst case, PTP sandwiches two
local timestamps around an external clock reading).

Those use cases run within an actual system call (not vDSO), and are
never about the time "now", per se — they're all about the pairing.

They shouldn't be compared directly with a clock_gettime() where
userspace... is preempted and... calls into the vDSO to get the time...
is preempted again and... eventually does something with that timestamp
which it considers to be current, or worse paired with whatever happens
before or after it.

So I'm not too worried about the fact that the two different mechanisms
return time in a *slightly* different form, because the normal
userspace path chooses speed and monotonicity over accuracy.

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

Re: [PATCH v4 2/4] pps: Drop the !NO_HZ_COMMON dependency from NTP_PPS

[David Woodhouse]

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

On Tue, 2026-09-01 at 17:35 +0200, Rodolfo Giometti wrote:
> About the test. The pulse comes from 4/4, which derives it from the
> same counter the timekeeping reads. No independent reference
> anywhere.

That was intentional, btw. I'm trying to eliminate the ever-smaller
errors in the kernel's timekeeping, and at this point they are fairly
much in the noise compared with the real-world measurement against
*real* time, on any test platform I can currently lay my hands on.

I'm not yet able to test on hardware which actually snapshots the CPU
counter at the moment of the PPS signal.

But I can *simulate* it, by running a virtual machine with vmclock and
knowing the precise relationship of counter to real time that I'm
*telling* it to discipline against — then checking how it did. Your "no
independent reference anywhere" is my "I'm actually testing the part I
want to test" :)



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

Re: [PATCH v4 3/4] pps: Always use ktime_get_snapshot_id() for pps_get_ts()

[Rodolfo Giometti]

On Mon, 2026-09-28 at 13:59 +0100, David Woodhouse wrote:
> Theoretically, absent other bugs (qv), ntp_error should rarely be more
> than a few tens of nanoseconds and even that is the extreme case.
[...]
> However, I *have* seen (and fixed) the tick_length changes at chrony
> startup introducing 83µs into ntp_error, which would take *days* to
> drain through the normal ±1 dithering, and would screw up the actual
> frequency settings while it was draining.

Thanks, that gives the order of magnitude I was asking for. Tens of
nanoseconds is below what chrony or ntpd can resolve from a PPS
source, so in the normal case the two lines are indistinguishable.

I think the "Allow tick_length changes to apply mid-tick" fix should
land before (or together with) the patch applying ntp_error to the
snapshots, and the commit message of the latter should say so.

> I wonder if we should switch PPS to using ktime_get_snapshot_id() in an
> *earlier* patch, which wouldn't then include the behavioural change.
> Then the note in the 'Apply extrapolated error' patch can then cover
> PPS and we consider them all together.

Yes, please. That split works well for me:

- the patch switching pps_get_ts() to ktime_get_snapshot_id() is pure
   plumbing: ts_real keeps its current meaning, and its cost is the
   one you measured and I already accepted. I can ack that one;

- the semantic change then lives in "timekeeping: Apply extrapolated
   ntp_error to clock snapshots", covering PPS and the other snapshot
   users together. That is a timekeeping decision, and it is the patch
   where the chrony and ntpd maintainers should be Cc'ed and ack.

As a bonus the two changes can be bisected and reverted independently,
which helps if userspace does notice something.

> The PPS change does stand alone anyway — regardless of the snapshot
> *corrections*, I want PPS using snapshots so that it can report the
> actual *counter* values to userspace, like PTP is going to be able to.

That is new ABI for the PPS subsystem, so please post it as a separate
series, and I would like to see the proposed interface before the
code. Things I would want settled there: how userspace learns which
counter the value refers to (and what happens when the clocksource
changes), and that the existing ioctls and struct pps_ktime stay
unchanged for current users.

Please also keep RFC 2783 in mind: the PPS API is defined there and
LinuxPPS has to stay compliant with it, so the counter values should
come as an extension on top of it that RFC-based users (e.g.
time_pps_fetch() via timepps.h) can simply ignore.

> They shouldn't be compared directly with a clock_gettime() where
> userspace... is preempted and... calls into the vDSO to get the time...
> is preempted again and... eventually does something with that timestamp
> which it considers to be current, or worse paired with whatever happens
> before or after it.

Agreed for a single reading. My concern is systematic rather than
per-sample: chrony and ntpd do compare PPS timestamps with timestamps
taken from the sanitized clock (e.g. NTP packet timestamps), and a
slowly varying offset between the two lines does not average out the
way preemption jitter does. With ntp_error in the tens of nanoseconds
it is irrelevant; I just want the larger cases fixed first and
documented. I'll wait for Miroslav's opinion on the userspace side.

Ciao,

Rodolfo

Re: [PATCH v4 2/4] pps: Drop the !NO_HZ_COMMON dependency from NTP_PPS

[Rodolfo Giometti]

On Mon, 2026-09-28 at 14:37 +0100, David Woodhouse wrote:
> But I can *simulate* it, by running a virtual machine with vmclock and
> knowing the precise relationship of counter to real time that I'm
> *telling* it to discipline against — then checking how it did. Your "no
> independent reference anywhere" is my "I'm actually testing the part I
> want to test" :)

What it cannot show is the thing this patch actually enables: hardpps
on a tickless kernel driven by a real PPS source, with its jitter, its
IRQ latency, and the CPU going idle between pulses. If you (or someone
on Cc) can run it for a while with a GPS PPS on a GPIO and report how
the offset converges compared with a tickful kernel, that is the test
I would like to see mentioned in the commit message. If not, please at
least state there that the validation was done with the simulated 4/4
pulse only.

Ciao,

Rodolfo