[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

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 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 --]

[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

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 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 --]

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