[RFC PATCH 0/4] timekeeping: add feed-forward clock discipline via vmclock

[RFC PATCH 0/4] timekeeping: add feed-forward clock discipline via vmclock

[David Woodhouse]

This proof-of-concept series adds support for feed-forward clock 
discipline, allowing a guest kernel to lock its system clock directly to 
a hypervisor-provided vmclock reference with sub-microsecond precision 
and no drift.

The vmclock device (https://uapi-group.org/specifications/specs/vmclock/)
provides a shared memory page containing a linear time function:
time = base + (counter - counter_value) × period. The guest can read
this at any time to determine the hypervisor's view of the current time,
without a VM exit.

The existing ptp_vmclock driver already exposes this as a PTP clock for
userspace consumers (phc2sys, chrony). This series adds kernel-internal
consumption: the tick mechanism can clamp directly to the vmclock
reference, eliminating the need for NTP to discipline the guest clock.

Patch 1 fixes a pre-existing bug in the ntp_error accumulator that
caused systematic drift when running at a fixed frequency (without NTP
continuously correcting). This is a standalone bugfix suitable for
stable backport.

Patch 2 adds /dev/vmclock_host, a miscdev that exports the host's
NTP-disciplined time as a vmclock page. This is what QEMU (or another
VMM) maps into the guest.

Patch 3 adds the timekeeping_set_reference() API and the feed-forward
clamping mechanism. When a reference is active, the dithering decision
(mult vs mult+1) uses an absolute comparison against the reference line
instead of the relative ntp_error accumulator. This also sets
time_offset for faster initial convergence.

Patch 4 wires ptp_vmclock to call timekeeping_set_reference() on probe
and on each notification from the hypervisor, enabling automatic
guest-to-host time synchronization.

Tested with QEMU passing through /dev/vmclock_host to a guest¹. The
guest clock converges to the host's vmclock reference within ~60 seconds
and remains locked to within ±500ns indefinitely, with no userspace NTP
daemon required.

TODO:
 • Graceful handling of clocksource switch (guest starts on kvmclock and
   only later switches to the final TSC). Although a modern guest on a
   modern kernel should *never* use kvmclock, and the guest was *told*
   in CPUID what the TSC frequency was, so I don't know why it was doing
   any of that.
 • Review accuracy of the base time reference exported from vmclock_host
   (I think I need to properly for ntp_error at the time of the snapshot,
   as my clocks sometimes converge on a point a few µs away from the
   host's CLOCK_REALTIME.

But generally as a proof of concept it seems to be working fairly well.
Especially after the fix in patch#1 which makes the kernel *actually*
run its clock at the time that adjtimex() tells it to.

In a dedicated hosting environment, the host userspace would probably
be disciplining the TSC against external NTP/1PPS and feeding that
*directly* to guests as as vmclock page, and would want to *also* feed
that into the kernel. We'll want a userspace API for that.

But let's get the part where Thomas calls me an idiot over with first :)

¹ https://git.infradead.org/?p=users/dwmw2/qemu.git;a=shortlog;h=refs/heads/vmclock-passthrough
                                                                                                                                                                                                
David Woodhouse (4):
      timekeeping: Remove xtime_remainder from ntp_error accumulation
      WIP: kernel/time: Add /dev/vmclock_host miscdev
      timekeeping: Add absolute reference for feed-forward clock discipline
      ptp_vmclock: Feed reference to timekeeping for feed-forward discipline

 drivers/ptp/ptp_vmclock.c                          |  26 ++
 include/linux/timekeeper_internal.h                |   2 -
 include/linux/timekeeping_reference.h              |  35 +++
 include/linux/vmclock_host.h                       |  17 ++
 kernel/time/Kconfig                                |   8 +
 kernel/time/Makefile                               |   1 +
 kernel/time/ntp.c                                  |  36 ++-
 kernel/time/ntp_internal.h                         |   3 +
 kernel/time/timekeeping.c                          |  89 +++++-
 kernel/time/vmclock_host.c                         | 319 +++++++++++++++++++++
 tools/testing/selftests/timers/vmclock_host_test.c | 171 +++++++++++
 11 files changed, 699 insertions(+), 8 deletions(-)

[RFC PATCH 1/4] timekeeping: Remove xtime_remainder from ntp_error accumulation

[David Woodhouse]

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

The ntp_error accumulator tracks the difference between intended and
actual clock advance. Each tick it adds ntp_tick (the intended advance)
and subtracts what the clock actually advanced.

The subtraction was (xtime_interval + xtime_remainder), but only
xtime_interval is actually added to xtime_nsec each tick.
xtime_remainder was a boot-time constant representing the rounding error
from converting the tick period to an integer number of counter cycles.
It was never added to xtime_nsec, so subtracting it from ntp_error
created a phantom credit that biased the dithering ratio.

The effect is a systematic drift whose magnitude depends on the value of
xtime_remainder and the NTP frequency correction. NTP masks this by
continuously adjusting the frequency to compensate, but with a fixed
frequency (or an external reference clock like vmclock), the drift is
exposed.

Also remove xtime_remainder from the mult computation in
timekeeping_adjust(), which used it to offset the division for the same
(incorrect) reason.

Fixes: a386b5af8edd ("time: Compensate for rounding on odd-frequency clocksources")
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 include/linux/timekeeper_internal.h | 2 --
 kernel/time/timekeeping.c           | 7 +++----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index e36d11e33e0c..2f4cfcfcaac0 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -84,7 +84,6 @@ struct tk_read_base {
  * @cycle_interval:		Number of clock cycles in one NTP interval
  * @xtime_interval:		Number of clock shifted nano seconds in one NTP
  *				interval.
- * @xtime_remainder:		Shifted nano seconds left over when rounding
  *				@cycle_interval
  * @raw_interval:		Shifted raw nano seconds accumulated per NTP interval.
  * @next_leap_ktime:		CLOCK_MONOTONIC time value of a pending leap-second
@@ -178,7 +177,6 @@ struct timekeeper {
 
 	u64			cycle_interval;
 	u64			xtime_interval;
-	s64			xtime_remainder;
 	u64			raw_interval;
 
 	ktime_t			next_leap_ktime;
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index c493a4010305..3da7167ceb0d 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -360,7 +360,6 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
 
 	/* Go back from cycles -> shifted ns */
 	tk->xtime_interval = interval * clock->mult;
-	tk->xtime_remainder = ntpinterval - tk->xtime_interval;
 	tk->raw_interval = interval * clock->mult;
 
 	 /* if changing clocks, convert xtime_nsec shift units */
@@ -2337,8 +2336,8 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
 		mult = tk->tkr_mono.mult - tk->ntp_err_mult;
 	} else {
 		tk->ntp_tick = ntp_tl;
-		mult = div64_u64((tk->ntp_tick >> tk->ntp_error_shift) -
-				 tk->xtime_remainder, tk->cycle_interval);
+		mult = div64_u64(tk->ntp_tick >> tk->ntp_error_shift,
+				 tk->cycle_interval);
 	}
 
 	/*
@@ -2463,7 +2462,7 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
 
 	/* Accumulate error between NTP and clock interval */
 	tk->ntp_error += tk->ntp_tick << shift;
-	tk->ntp_error -= (tk->xtime_interval + tk->xtime_remainder) <<
+	tk->ntp_error -= tk->xtime_interval <<
 						(tk->ntp_error_shift + shift);
 
 	return offset;
-- 
2.51.0

[RFC PATCH 2/4] WIP: kernel/time: Add /dev/vmclock_host miscdev

[David Woodhouse]

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

Expose the host's NTP-disciplined clock as a vmclock_abi page via
/dev/vmclock_host. A VMM can mmap or poll() this device to obtain
precision time parameters for relaying to guests.

The page is updated only when ntp_tick changes (i.e., when NTP
actually adjusts the frequency), not on every timekeeping tick.
This avoids the per-tick overhead of the existing pvclock_gtod
notifier while providing the same information.

Fields populated:
- counter_id: X86_TSC
- time_type: TAI
- counter_value: TSC at reference point
- time_sec/time_frac_sec: TAI at reference point
- counter_period_frac_sec: NTP-disciplined TSC period
- tai_offset_sec: current UTC-TAI offset

NOT YET DONE:
- Error bounds (esterror/maxerror)
- Leap second indicator
- Disruption marker (needs clocksource change hook)
- Selftest

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 include/linux/vmclock_host.h                  |  17 +
 kernel/time/Kconfig                           |   8 +
 kernel/time/Makefile                          |   1 +
 kernel/time/ntp.c                             |   3 +-
 kernel/time/ntp_internal.h                    |   1 +
 kernel/time/timekeeping.c                     |   6 +
 kernel/time/vmclock_host.c                    | 319 ++++++++++++++++++
 .../selftests/timers/vmclock_host_test.c      | 171 ++++++++++
 8 files changed, 525 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/vmclock_host.h
 create mode 100644 kernel/time/vmclock_host.c
 create mode 100644 tools/testing/selftests/timers/vmclock_host_test.c

diff --git a/include/linux/vmclock_host.h b/include/linux/vmclock_host.h
new file mode 100644
index 000000000000..388a5a1b470c
--- /dev/null
+++ b/include/linux/vmclock_host.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_VMCLOCK_HOST_H
+#define _LINUX_VMCLOCK_HOST_H
+
+struct timekeeper;
+
+extern void (*vmclock_host_update_fn)(struct timekeeper *tk);
+
+static inline void vmclock_host_update(struct timekeeper *tk)
+{
+	typeof(vmclock_host_update_fn) fn = READ_ONCE(vmclock_host_update_fn);
+
+	if (fn)
+		fn(tk);
+}
+
+#endif /* _LINUX_VMCLOCK_HOST_H */
diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index 02aac7c5aa76..493ffda434a8 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -221,4 +221,12 @@ config POSIX_AUX_CLOCKS
 	  and other clock domains, which are not correlated to the TAI/NTP
 	  notion of time.
 
+config VMCLOCK_HOST
+	tristate "VMClock host time provider (/dev/vmclock_host)"
+	depends on X86_TSC || ARM64
+	help
+	  Expose the host NTP-disciplined clock as a vmclock page via
+	  /dev/vmclock_host for VMMs to relay precision time to guests.
+
 endmenu
+
diff --git a/kernel/time/Makefile b/kernel/time/Makefile
index eaf290c972f9..549070254e3a 100644
--- a/kernel/time/Makefile
+++ b/kernel/time/Makefile
@@ -33,3 +33,4 @@ obj-$(CONFIG_TIME_NS)				+= namespace.o
 obj-$(CONFIG_TIME_NS_VDSO)			+= namespace_vdso.o
 obj-$(CONFIG_TEST_CLOCKSOURCE_WATCHDOG)		+= clocksource-wdtest.o
 obj-$(CONFIG_TIME_KUNIT_TEST)			+= time_test.o
+obj-$(CONFIG_VMCLOCK_HOST)	+= vmclock_host.o
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 97fa99b96dd0..3c318c96f35d 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -611,10 +611,11 @@ static inline int update_rtc(struct timespec64 *to_set, unsigned long *offset_ns
  * ntp_synced - Tells whether the NTP status is not UNSYNC
  * Returns:	true if not UNSYNC, false otherwise
  */
-static inline bool ntp_synced(void)
+bool ntp_synced(void)
 {
 	return !(tk_ntp_data[TIMEKEEPER_CORE].time_status & STA_UNSYNC);
 }
+EXPORT_SYMBOL_GPL(ntp_synced);
 
 /*
  * If we have an externally synchronized Linux clock, then update RTC clock
diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h
index 7084d839c207..b36a8090fc9c 100644
--- a/kernel/time/ntp_internal.h
+++ b/kernel/time/ntp_internal.h
@@ -3,6 +3,7 @@
 #define _LINUX_NTP_INTERNAL_H
 
 extern void ntp_init(void);
+extern bool ntp_synced(void);
 extern void ntp_clear(unsigned int tkid);
 /* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
 extern u64 ntp_tick_length(unsigned int tkid);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 3da7167ceb0d..1935881041d0 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -27,6 +27,10 @@
 #include "tick-internal.h"
 #include "timekeeping_internal.h"
 #include "ntp_internal.h"
+#include <linux/vmclock_host.h>
+
+void (*vmclock_host_update_fn)(struct timekeeper *tk);
+EXPORT_SYMBOL_GPL(vmclock_host_update_fn);
 
 #define TK_CLEAR_NTP		(1 << 0)
 #define TK_CLOCK_WAS_SET	(1 << 1)
@@ -2340,6 +2344,8 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
 				 tk->cycle_interval);
 	}
 
+	vmclock_host_update(tk);
+
 	/*
 	 * If the clock is behind the NTP time, increase the multiplier by 1
 	 * to catch up with it. If it's ahead and there was a remainder in the
diff --git a/kernel/time/vmclock_host.c b/kernel/time/vmclock_host.c
new file mode 100644
index 000000000000..f4baf9069e70
--- /dev/null
+++ b/kernel/time/vmclock_host.c
@@ -0,0 +1,319 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * /dev/vmclock_host - Expose host NTP-disciplined time as a vmclock page.
+ *
+ * This provides a vmclock_abi structure populated from the host's
+ * CLOCK_REALTIME (TAI), allowing a VMM to efficiently relay precision
+ * time to guests without per-tick overhead.
+ *
+ * The page is updated only when the NTP frequency (ntp_tick) changes
+ * or the clocksource changes — not on every timekeeping tick.
+ * Userspace can poll() for changes.
+ *
+ * Copyright © 2026 Amazon.com, Inc. or its affiliates.
+ */
+
+#include <linux/clocksource_ids.h>
+#include <linux/miscdevice.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/poll.h>
+#include <linux/slab.h>
+#include <linux/timekeeper_internal.h>
+#include <linux/wait.h>
+
+#include <uapi/linux/vmclock-abi.h>
+
+extern void (*vmclock_host_update_fn)(struct timekeeper *tk);
+extern bool ntp_synced(void);
+
+static struct vmclock_abi *vmclock_page;
+static DECLARE_WAIT_QUEUE_HEAD(vmclock_wait);
+static u64 vmclock_last_ntp_tick = 1; /* Sentinel: force first update */
+static enum clocksource_ids vmclock_last_cs_id;
+
+/*
+ * Compute counter_period_frac_sec from ntp_tick and cycle_interval.
+ *
+ * ntp_tick is ns_per_tick << 32.
+ * cycle_interval is counter cycles per tick.
+ *
+ * vmclock wants: period = frac_sec / 2^(64 + shift) in seconds.
+ *
+ * ns_per_cycle = ntp_tick / cycle_interval (in <<32 fixed point)
+/*
+ * Compute counter_period_frac_sec from ntp_tick and cycle_interval.
+ *
+ * period = ntp_tick / (cycle_interval * 10^9 * 2^32) seconds/cycle
+ * frac_sec = ntp_tick * 2^(32+shift) / (cycle_interval * 10^9)
+ *
+ * Use div64_u64 with maximum pre-shift for precision.
+ * The key: do TWO divisions to get 64 bits of quotient.
+ */
+static void vmclock_compute_period(struct timekeeper *tk,
+				   u64 *period_frac, u8 *period_shift)
+{
+	u64 ntp_tick = tk->ntp_tick;
+	u64 cycle_interval = tk->cycle_interval;
+	u64 divisor = cycle_interval * 1000000000ULL;
+	int headroom = __builtin_clzll(ntp_tick);
+	u64 rem, result;
+	int bits_so_far, need;
+
+	/*
+	 * Compute ntp_tick * 2^(headroom + N) / divisor with 64 bits
+	 * of precision, using iterative 32-bit chunk divisions.
+	 *
+	 * First division: ntp_tick << headroom / divisor
+	 */
+	result = div64_u64_rem(ntp_tick << headroom, divisor, &rem);
+	bits_so_far = 64 - __builtin_clzll(result ?: 1);
+
+	/* Fill remaining bits 32 at a time from the remainder */
+	while (bits_so_far < 64 && rem) {
+		int chunk = min(32, 64 - bits_so_far);
+		int rem_headroom = __builtin_clzll(rem);
+		u64 extra;
+
+		if (rem_headroom < chunk)
+			chunk = rem_headroom;
+
+		extra = div64_u64_rem(rem << chunk, divisor, &rem);
+		result = (result << chunk) | extra;
+		bits_so_far += chunk;
+		headroom += chunk;
+	}
+
+	/* Pad with zeros if we ran out of remainder */
+	if (bits_so_far < 64) {
+		result <<= (64 - bits_so_far);
+		headroom += (64 - bits_so_far);
+	}
+
+	/*
+	 * result = ntp_tick * 2^headroom / divisor
+	 *        = (ntp_tick / (cycle_interval * 10^9)) * 2^headroom
+	 *        = period_seconds * 2^32 * 2^headroom
+	 *        = period_seconds * 2^(32 + headroom)
+	 *
+	 * vmclock: frac_sec / 2^(64 + shift) = period_seconds
+	 * So: shift = 32 + headroom - 64 = headroom - 32
+	 */
+	*period_frac = result;
+	*period_shift = (u8)(headroom - 32);
+}
+
+
+static u8 vmclock_counter_id(struct timekeeper *tk)
+{
+	enum clocksource_ids id = tk->cs_id;
+
+	if (IS_ENABLED(CONFIG_X86) && id == CSID_X86_TSC)
+		return VMCLOCK_COUNTER_X86_TSC;
+	if (IS_ENABLED(CONFIG_ARM64) && id == CSID_ARM_ARCH_COUNTER)
+		return VMCLOCK_COUNTER_ARM_VCNT;
+	return VMCLOCK_COUNTER_INVALID;
+}
+
+/*
+ * Called from timekeeping_adjust() when ntp_tick changes.
+ * Also needs to be called on clocksource change.
+ */
+static void vmclock_host_do_update(struct timekeeper *tk)
+{
+	struct vmclock_abi *clk = vmclock_page;
+	u64 period_frac;
+	u8 period_shift, counter_id;
+
+	if (!clk)
+		return;
+
+	counter_id = vmclock_counter_id(tk);
+
+	/* Only do a full update when something meaningful changes */
+	if (tk->ntp_tick == vmclock_last_ntp_tick &&
+	    tk->cs_id == vmclock_last_cs_id)
+		return;
+
+	vmclock_last_ntp_tick = tk->ntp_tick;
+	vmclock_last_cs_id = tk->cs_id;
+
+	/* Increment seq_count to odd (update in progress) */
+	WRITE_ONCE(clk->seq_count, cpu_to_le32(le32_to_cpu(clk->seq_count) + 1));
+	smp_wmb();
+
+	clk->counter_id = counter_id;
+
+	if (counter_id != VMCLOCK_COUNTER_INVALID) {
+		u64 ns = tk->tkr_mono.xtime_nsec >> tk->tkr_mono.shift;
+		u64 hi, rem;
+
+		/* Adjust for ntp_error: represent where the clock is
+		 * converging TO, not where it is right now. */
+		ns += tk->ntp_error >> (tk->tkr_mono.shift + tk->ntp_error_shift);
+
+		clk->counter_value = cpu_to_le64(tk->tkr_mono.cycle_last);
+		clk->time_sec = cpu_to_le64(tk->xtime_sec + tk->tai_offset);
+
+		hi = div64_u64_rem(ns << 32, 1000000000ULL, &rem);
+		clk->time_frac_sec = cpu_to_le64(
+			(hi << 32) | div64_u64(rem << 32, 1000000000ULL));
+
+		vmclock_compute_period(tk,
+				       &period_frac, &period_shift);
+		clk->counter_period_frac_sec = cpu_to_le64(period_frac);
+		clk->counter_period_shift = period_shift;
+
+		clk->clock_status = ntp_synced() ?
+			VMCLOCK_STATUS_SYNCHRONIZED :
+			VMCLOCK_STATUS_FREERUNNING;
+	} else {
+		clk->clock_status = VMCLOCK_STATUS_UNKNOWN;
+	}
+
+	clk->tai_offset_sec = cpu_to_le16((s16)tk->tai_offset);
+	clk->flags = cpu_to_le64(VMCLOCK_FLAG_TAI_OFFSET_VALID |
+				 VMCLOCK_FLAG_TIME_MONOTONIC |
+				 VMCLOCK_FLAG_NOTIFICATION_PRESENT);
+
+	smp_wmb();
+	WRITE_ONCE(clk->seq_count, cpu_to_le32(le32_to_cpu(clk->seq_count) + 1));
+
+	wake_up_interruptible(&vmclock_wait);
+}
+
+/* File operations */
+
+struct vmclock_host_file {
+	u32 last_seq;
+};
+
+static int vmclock_host_open(struct inode *inode, struct file *fp)
+{
+	struct vmclock_host_file *fst;
+
+	fst = kzalloc(sizeof(*fst), GFP_KERNEL);
+	if (!fst)
+		return -ENOMEM;
+
+	fp->private_data = fst;
+	return 0;
+}
+
+static int vmclock_host_release(struct inode *inode, struct file *fp)
+{
+	kfree(fp->private_data);
+	return 0;
+}
+
+static int vmclock_host_mmap(struct file *fp, struct vm_area_struct *vma)
+{
+	if ((vma->vm_flags & (VM_READ | VM_WRITE)) != VM_READ)
+		return -EROFS;
+
+	if (vma->vm_end - vma->vm_start != PAGE_SIZE || vma->vm_pgoff)
+		return -EINVAL;
+
+	return remap_pfn_range(vma, vma->vm_start,
+			       virt_to_phys(vmclock_page) >> PAGE_SHIFT,
+			       PAGE_SIZE, vma->vm_page_prot);
+}
+
+static ssize_t vmclock_host_read(struct file *fp, char __user *buf,
+				 size_t count, loff_t *ppos)
+{
+	struct vmclock_host_file *fst = fp->private_data;
+	u32 seq;
+
+	if (*ppos >= PAGE_SIZE)
+		return 0;
+	if (count > PAGE_SIZE - *ppos)
+		count = PAGE_SIZE - *ppos;
+
+	do {
+		seq = le32_to_cpu(READ_ONCE(vmclock_page->seq_count));
+		if (seq & 1) {
+			cpu_relax();
+			continue;
+		}
+		smp_rmb();
+		if (copy_to_user(buf, (char *)vmclock_page + *ppos, count))
+			return -EFAULT;
+		smp_rmb();
+	} while (le32_to_cpu(READ_ONCE(vmclock_page->seq_count)) != seq);
+
+	fst->last_seq = seq;
+	*ppos += count;
+	return count;
+}
+
+static __poll_t vmclock_host_poll(struct file *fp, poll_table *wait)
+{
+	struct vmclock_host_file *fst = fp->private_data;
+	u32 seq;
+
+	poll_wait(fp, &vmclock_wait, wait);
+
+	seq = le32_to_cpu(READ_ONCE(vmclock_page->seq_count));
+	if (fst->last_seq != seq)
+		return EPOLLIN | EPOLLRDNORM;
+
+	return 0;
+}
+
+static const struct file_operations vmclock_host_fops = {
+	.owner = THIS_MODULE,
+	.open = vmclock_host_open,
+	.release = vmclock_host_release,
+	.mmap = vmclock_host_mmap,
+	.read = vmclock_host_read,
+	.poll = vmclock_host_poll,
+};
+
+static struct miscdevice vmclock_host_miscdev = {
+	.minor = MISC_DYNAMIC_MINOR,
+	.name = "vmclock_host",
+	.fops = &vmclock_host_fops,
+};
+
+static int __init vmclock_host_init(void)
+{
+	int ret;
+
+	vmclock_page = (struct vmclock_abi *)get_zeroed_page(GFP_KERNEL);
+	if (!vmclock_page)
+		return -ENOMEM;
+
+	/* Set constant fields */
+	vmclock_page->magic = cpu_to_le32(VMCLOCK_MAGIC);
+	vmclock_page->size = cpu_to_le32(PAGE_SIZE);
+	vmclock_page->version = cpu_to_le16(1);
+	vmclock_page->time_type = VMCLOCK_TIME_TAI;
+
+	ret = misc_register(&vmclock_host_miscdev);
+	if (ret) {
+		free_page((unsigned long)vmclock_page);
+		vmclock_page = NULL;
+		return ret;
+	}
+
+	WRITE_ONCE(vmclock_host_update_fn, vmclock_host_do_update);
+	pr_info("vmclock_host: registered /dev/vmclock_host\n");
+	return 0;
+}
+
+static void __exit vmclock_host_exit(void)
+{
+	WRITE_ONCE(vmclock_host_update_fn, NULL);
+	synchronize_rcu();
+	misc_deregister(&vmclock_host_miscdev);
+	free_page((unsigned long)vmclock_page);
+	vmclock_page = NULL;
+}
+
+module_init(vmclock_host_init);
+module_exit(vmclock_host_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("David Woodhouse <dwmw@amazon.co.uk>");
+MODULE_DESCRIPTION("VMClock host time provider");
diff --git a/tools/testing/selftests/timers/vmclock_host_test.c b/tools/testing/selftests/timers/vmclock_host_test.c
new file mode 100644
index 000000000000..c83cc7e6d404
--- /dev/null
+++ b/tools/testing/selftests/timers/vmclock_host_test.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test /dev/vmclock_host by comparing its time against CLOCK_TAI.
+ *
+ * Maps the vmclock page, reads time from it using the ABI formula,
+ * and compares with clock_gettime(CLOCK_TAI) using ABA timestamps
+ * to bound the uncertainty.
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <time.h>
+#include <unistd.h>
+
+#include <linux/vmclock-abi.h>
+
+#ifdef __x86_64__
+static inline uint64_t read_counter(void)
+{
+	unsigned int lo, hi;
+	asm volatile("rdtsc" : "=a"(lo), "=d"(hi));
+	return ((uint64_t)hi << 32) | lo;
+}
+#elif defined(__aarch64__)
+static inline uint64_t read_counter(void)
+{
+	uint64_t val;
+	asm volatile("mrs %0, cntvct_el0" : "=r"(val));
+	return val;
+}
+#else
+#error "Unsupported architecture"
+#endif
+
+/*
+ * Compute time from vmclock: T = time_sec + time_frac_sec/2^64 +
+ *   (counter_now - counter_value) * counter_period_frac_sec >> (64 + shift)
+ *
+ * Returns nanoseconds since epoch.
+ */
+static int64_t vmclock_read_ns(const volatile struct vmclock_abi *clk,
+			       uint64_t counter_now)
+{
+	uint64_t delta = counter_now - clk->counter_value;
+	uint64_t period = clk->counter_period_frac_sec;
+	uint8_t shift = clk->counter_period_shift;
+	__uint128_t ns128;
+
+	/* delta * period gives seconds in 0.(64+shift) fixed point */
+	ns128 = (__uint128_t)delta * period;
+	ns128 >>= shift;
+	/* Now ns128 is seconds in 0.64 fixed point. Add time_frac_sec */
+	ns128 += clk->time_frac_sec;
+	/* Top 64 bits are whole seconds of fractional part — but we
+	 * need to add time_sec for the full result */
+	uint64_t frac_sec = (uint64_t)(ns128 >> 64);
+	uint64_t sub_sec_ns = (uint64_t)(((ns128 & 0xFFFFFFFFFFFFFFFFULL) *
+					   1000000000ULL) >> 64);
+
+	return (int64_t)(clk->time_sec + frac_sec) * 1000000000LL + sub_sec_ns;
+}
+
+static int64_t clock_tai_ns(void)
+{
+	struct timespec ts;
+	clock_gettime(CLOCK_TAI, &ts);
+	return (int64_t)ts.tv_sec * 1000000000LL + ts.tv_nsec;
+}
+
+int main(void)
+{
+	int fd, ret = 0;
+	volatile struct vmclock_abi *clk;
+	int i, failures = 0;
+
+	fd = open("/dev/vmclock_host", O_RDONLY);
+	if (fd < 0) {
+		if (errno == ENOENT) {
+			printf("SKIP: /dev/vmclock_host not available\n");
+			return 4;
+		}
+		perror("open /dev/vmclock_host");
+		return 1;
+	}
+
+	clk = mmap(NULL, 4096, PROT_READ, MAP_SHARED, fd, 0);
+	if (clk == MAP_FAILED) {
+		perror("mmap");
+		close(fd);
+		return 1;
+	}
+
+	if (clk->magic != VMCLOCK_MAGIC) {
+		fprintf(stderr, "Bad magic: 0x%x\n", clk->magic);
+		ret = 1;
+		goto out;
+	}
+
+	if (clk->counter_id == VMCLOCK_COUNTER_INVALID) {
+		printf("SKIP: counter_id is INVALID (clocksource not TSC?)\n");
+		ret = 4;
+		goto out;
+	}
+
+	printf("vmclock_host: version=%u counter_id=%u time_type=%u status=%u\n",
+	       clk->version, clk->counter_id, clk->time_type, clk->clock_status);
+	printf("  tai_offset=%d\n", (int16_t)clk->tai_offset_sec);
+	printf("  counter_period_frac_sec=0x%" PRIx64 " shift=%u\n",
+	       (uint64_t)clk->counter_period_frac_sec, clk->counter_period_shift);
+
+	/* ABA comparison: read CLOCK_TAI, vmclock, CLOCK_TAI */
+	printf("\nABA comparison (vmclock vs CLOCK_TAI):\n");
+	for (i = 0; i < 10; i++) {
+		uint32_t seq;
+		int64_t tai_before, tai_after, vmclock_ns;
+		int64_t delta, window;
+
+		/* Read with seqcount retry */
+		do {
+			seq = clk->seq_count;
+			if (seq & 1) {
+				__asm__ volatile("pause" ::: "memory");
+				continue;
+			}
+			__asm__ volatile("" ::: "memory");
+
+			tai_before = clock_tai_ns();
+			uint64_t ctr = read_counter();
+			tai_after = clock_tai_ns();
+
+			__asm__ volatile("" ::: "memory");
+			if (clk->seq_count != seq)
+				continue;
+
+			vmclock_ns = vmclock_read_ns(clk, ctr);
+			break;
+		} while (1);
+
+		window = tai_after - tai_before;
+		/* vmclock should be between tai_before and tai_after */
+		delta = vmclock_ns - tai_before;
+
+		printf("  [%d] vmclock-tai_before=%+" PRId64 "ns window=%"
+		       PRId64 "ns", i, delta, window);
+
+		if (delta < -2000 || delta > window + 2000) {
+			printf(" FAIL (out of range)\n");
+			failures++;
+		} else {
+			printf(" OK\n");
+		}
+
+		usleep(100000); /* 100ms between samples */
+	}
+
+	if (failures) {
+		printf("\nFAIL: %d/%d samples out of range\n", failures, 10);
+		ret = 1;
+	} else {
+		printf("\nPASS: all samples within ABA window\n");
+	}
+
+out:
+	munmap((void *)clk, 4096);
+	close(fd);
+	return ret;
+}
-- 
2.51.0

[RFC PATCH 3/4] timekeeping: Add absolute reference for feed-forward clock discipline

[David Woodhouse]

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

Add timekeeping_set_reference() which allows an external clock source
(e.g. vmclock from a hypervisor) to provide an absolute time reference.
The reference defines a linear TSC-to-time mapping that the tick
mechanism clamps to, replacing the relative ntp_error accumulator for
the dithering decision.

When a reference is active:
 - ntp_tick is set to match the reference rate via the normal NTP path
 - The dithering decision (mult vs mult+1) uses an absolute comparison
   against the reference line instead of the ntp_error accumulator
 - timekeeping_apply_adjustment() still runs for vDSO monotonicity
 - adjtimex reads back the correct frequency

The reference is automatically cleared when:
 - adjtimex ADJ_FREQUENCY is called (NTP takes over)
 - The clocksource changes

This eliminates the ~26 PPB systematic drift caused by the interaction
between timekeeping_apply_adjustment()'s monotonicity correction and
the ntp_error accumulator, which depends on interrupt latency.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 include/linux/timekeeping_reference.h | 35 ++++++++++++
 kernel/time/ntp.c                     | 33 ++++++++++++
 kernel/time/ntp_internal.h            |  2 +
 kernel/time/timekeeping.c             | 76 ++++++++++++++++++++++++++-
 4 files changed, 145 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/timekeeping_reference.h

diff --git a/include/linux/timekeeping_reference.h b/include/linux/timekeeping_reference.h
new file mode 100644
index 000000000000..0cf248ace241
--- /dev/null
+++ b/include/linux/timekeeping_reference.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_TIMEKEEPING_REFERENCE_H
+#define _LINUX_TIMEKEEPING_REFERENCE_H
+
+#include <linux/clocksource_ids.h>
+#include <linux/types.h>
+
+struct timekeeper;
+
+/**
+ * struct tk_reference - Absolute time reference for feed-forward timekeeping
+ * @cs_id:		Clocksource counter this reference applies to
+ * @counter_value:	Counter reading at the reference point
+ * @cycle_interval:	Counter cycles per tick (for ntp_tick computation)
+ * @time_sec:		Seconds (UTC) at the reference point
+ * @time_frac_sec:	Fractional seconds (units of 1/2^64 second)
+ * @period_frac_sec:	Counter period (units of 1/2^(64+shift) seconds)
+ * @period_shift:	Additional shift for period fixed-point
+ */
+struct tk_reference {
+	enum clocksource_ids	cs_id;
+	u64			counter_value;
+	u64			cycle_interval;
+	u64			time_sec;
+	u64			time_frac_sec;
+	u64			period_frac_sec;
+	u8			period_shift;
+};
+
+int timekeeping_set_reference(const struct tk_reference *ref);
+bool timekeeping_has_reference(void);
+void timekeeping_clear_reference(void);
+bool timekeeping_ref_ahead(struct timekeeper *tk);
+
+#endif /* _LINUX_TIMEKEEPING_REFERENCE_H */
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 3c318c96f35d..cdd63589160d 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -21,6 +21,7 @@
 #include <linux/timekeeper_internal.h>
 
 #include "ntp_internal.h"
+#include <linux/timekeeping_reference.h>
 #include "timekeeping_internal.h"
 
 /**
@@ -364,6 +365,37 @@ u64 ntp_tick_length(unsigned int tkid)
 	return tk_ntp_data[tkid].tick_length;
 }
 
+void ntp_set_tick_length(unsigned int tkid, u64 tick_length)
+{
+	struct ntp_data *ntpdata = &tk_ntp_data[tkid];
+	u64 base;
+
+	/*
+	 * Reverse ntp_update_frequency() to find the time_freq that
+	 * produces this tick_length, keeping everything consistent.
+	 *
+	 * tick_length = ((tick_usec * 1000 * USER_HZ) << 32 +
+	 *                ntp_tick_adj + time_freq) / NTP_INTERVAL_FREQ
+	 *
+	 * time_freq = tick_length * NTP_INTERVAL_FREQ -
+	 *             (tick_usec * 1000 * USER_HZ) << 32 - ntp_tick_adj
+	 */
+	base = (u64)(ntpdata->tick_usec * NSEC_PER_USEC * USER_HZ) << NTP_SCALE_SHIFT;
+	base += ntpdata->ntp_tick_adj;
+
+	ntpdata->time_freq = (s64)(tick_length * NTP_INTERVAL_FREQ - base);
+	ntp_update_frequency(ntpdata);
+}
+
+void ntp_set_time_offset(unsigned int tkid, s64 offset_ns)
+{
+	struct ntp_data *ntpdata = &tk_ntp_data[tkid];
+
+	ntpdata->time_offset = div_s64((s64)offset_ns << NTP_SCALE_SHIFT,
+				       NTP_INTERVAL_FREQ);
+	ntpdata->time_adjust = 0;
+}
+
 /**
  * ntp_get_next_leap - Returns the next leapsecond in CLOCK_REALTIME ktime_t
  * @tkid:	Timekeeper ID
@@ -736,6 +768,7 @@ static inline void process_adjtimex_modes(struct ntp_data *ntpdata, const struct
 		ntpdata->time_freq = max(ntpdata->time_freq, -MAXFREQ_SCALED);
 		/* Update pps_freq */
 		pps_set_freq(ntpdata);
+		timekeeping_clear_reference();
 	}
 
 	if (txc->modes & ADJ_MAXERROR)
diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h
index b36a8090fc9c..4531c162d229 100644
--- a/kernel/time/ntp_internal.h
+++ b/kernel/time/ntp_internal.h
@@ -7,6 +7,8 @@ extern bool ntp_synced(void);
 extern void ntp_clear(unsigned int tkid);
 /* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
 extern u64 ntp_tick_length(unsigned int tkid);
+extern void ntp_set_tick_length(unsigned int tkid, u64 length);
+extern void ntp_set_time_offset(unsigned int tkid, s64 offset_ns);
 extern ktime_t ntp_get_next_leap(unsigned int tkid);
 extern int second_overflow(unsigned int tkid, time64_t secs);
 extern int ntp_adjtimex(unsigned int tkid, struct __kernel_timex *txc, const struct timespec64 *ts,
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 1935881041d0..1225efdf5dc0 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -27,6 +27,8 @@
 #include "tick-internal.h"
 #include "timekeeping_internal.h"
 #include "ntp_internal.h"
+#include <linux/timekeeping_reference.h>
+#include <linux/math64.h>
 #include <linux/vmclock_host.h>
 
 void (*vmclock_host_update_fn)(struct timekeeper *tk);
@@ -396,6 +398,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
 	tk->skip_second_overflow = 0;
 
 	tk->cs_id = clock->id;
+	timekeeping_clear_reference();
 
 	/* Coupled clockevent data */
 	if (IS_ENABLED(CONFIG_GENERIC_CLOCKEVENTS_COUPLED) &&
@@ -2323,9 +2326,77 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk,
 	tk->tkr_mono.xtime_nsec -= offset;
 }
 
+static struct tk_reference tk_ref;
+static bool tk_ref_valid;
+
+int timekeeping_set_reference(const struct tk_reference *ref)
+{
+	struct timekeeper *tk = &tk_core.timekeeper;
+	__uint128_t product;
+	u64 delta, ref_frac, ref_ns;
+	s64 offset_ns;
+
+	tk_ref = *ref;
+	if (!tk_ref.cycle_interval)
+		tk_ref.cycle_interval = tk->cycle_interval;
+
+	/* Reject if the clocksource doesn't match */
+	if (tk->cs_id != ref->cs_id)
+		return -ENODEV;
+
+	tk_ref_valid = true;
+	ntp_set_tick_length(TIMEKEEPER_CORE,
+		mul_u64_u64_shr(ref->period_frac_sec,
+				(u64)tk_ref.cycle_interval * NSEC_PER_SEC,
+				32 + ref->period_shift));
+
+	/* Compute phase offset: (reference_time - xtime) in ns */
+	delta = tk->tkr_mono.cycle_last - tk_ref.counter_value;
+	product = (__uint128_t)delta * tk_ref.period_frac_sec;
+	product >>= tk_ref.period_shift;
+	product += tk_ref.time_frac_sec;
+	ref_frac = (u64)product;
+	ref_ns = mul_u64_u64_shr(ref_frac, NSEC_PER_SEC, 64);
+
+	if (tk_ref.time_sec + (u64)(product >> 64) == tk->xtime_sec) {
+		offset_ns = (s64)ref_ns -
+			(s64)(tk->tkr_mono.xtime_nsec >> tk->tkr_mono.shift);
+		ntp_set_time_offset(TIMEKEEPER_CORE, offset_ns);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(timekeeping_set_reference);
+
+bool timekeeping_has_reference(void) { return tk_ref_valid; }
+void timekeeping_clear_reference(void) { tk_ref_valid = false; }
+
+bool timekeeping_ref_ahead(struct timekeeper *tk)
+{
+	u64 delta, ref_frac, ref_sec, ref_shifted_ns;
+	__uint128_t product;
+
+	if (tk->cs_id != tk_ref.cs_id)
+		return false;
+	delta = tk->tkr_mono.cycle_last - tk_ref.counter_value;
+	product = (__uint128_t)delta * tk_ref.period_frac_sec;
+	product >>= tk_ref.period_shift;
+	product += tk_ref.time_frac_sec;
+	ref_sec = tk_ref.time_sec + (u64)(product >> 64);
+	ref_frac = (u64)product;
+	ref_shifted_ns = mul_u64_u64_shr(ref_frac,
+				(u64)NSEC_PER_SEC << tk->tkr_mono.shift, 64);
+	if (tk->xtime_sec > ref_sec)
+		return true;
+	if (tk->xtime_sec == ref_sec &&
+	    tk->tkr_mono.xtime_nsec > ref_shifted_ns)
+		return true;
+	return false;
+}
 /*
  * Adjust the timekeeper's multiplier to the correct frequency
  * and also to reduce the accumulated error value.
+
  */
 static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
 {
@@ -2352,7 +2423,10 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
 	 * tick division, the clock will slow down. Otherwise it will stay
 	 * ahead until the tick length changes to a non-divisible value.
 	 */
-	tk->ntp_err_mult = tk->ntp_error > 0 ? 1 : 0;
+	if (timekeeping_has_reference())
+		tk->ntp_err_mult = timekeeping_ref_ahead(tk) ? 0 : 1;
+	else
+		tk->ntp_err_mult = tk->ntp_error > 0 ? 1 : 0;
 	mult += tk->ntp_err_mult;
 
 	timekeeping_apply_adjustment(tk, offset, mult - tk->tkr_mono.mult);
-- 
2.51.0

[RFC PATCH 4/4] ptp_vmclock: Feed reference to timekeeping for feed-forward discipline

[David Woodhouse]

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

When a vmclock device provides valid time, call timekeeping_set_reference()
to enable feed-forward clock discipline. This eliminates drift between the
system clock and the vmclock reference.

The reference is set at probe time (after PTP registration) and updated
on each notification from the hypervisor (ACPI or DT interrupt).

If cycle_interval is not provided (set to 0), timekeeping_set_reference()
fills it from the current timekeeper.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 drivers/ptp/ptp_vmclock.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c
index 8b630eb916b5..225cdc4366dc 100644
--- a/drivers/ptp/ptp_vmclock.c
+++ b/drivers/ptp/ptp_vmclock.c
@@ -27,6 +27,7 @@
 #include <uapi/linux/vmclock-abi.h>
 
 #include <linux/ptp_clock_kernel.h>
+#include <linux/timekeeping_reference.h>
 
 #ifdef CONFIG_X86
 #include <asm/pvclock.h>
@@ -334,6 +335,27 @@ static const struct ptp_clock_info ptp_vmclock_info = {
 	.getcrosststamp = ptp_vmclock_getcrosststamp,
 };
 
+static void vmclock_set_tk_reference(struct vmclock_state *st)
+{
+	struct vmclock_abi *clk = st->clk;
+	struct tk_reference ref = {
+		.cs_id = st->cs_id,
+		.counter_value = le64_to_cpu(clk->counter_value),
+		.time_sec = le64_to_cpu(clk->time_sec),
+		.time_frac_sec = le64_to_cpu(clk->time_frac_sec),
+		.period_frac_sec = le64_to_cpu(clk->counter_period_frac_sec),
+		.period_shift = clk->counter_period_shift,
+	};
+
+	/* Convert TAI to UTC for comparison with xtime_sec */
+	if (clk->time_type == VMCLOCK_TIME_TAI &&
+	    (le64_to_cpu(clk->flags) & VMCLOCK_FLAG_TAI_OFFSET_VALID))
+		ref.time_sec += (int16_t)le16_to_cpu(clk->tai_offset_sec);
+
+	if (clk->clock_status != VMCLOCK_STATUS_UNRELIABLE)
+		timekeeping_set_reference(&ref);
+}
+
 static struct ptp_clock *vmclock_ptp_register(struct device *dev,
 					      struct vmclock_state *st)
 {
@@ -525,6 +547,7 @@ vmclock_acpi_notification_handler(acpi_handle __always_unused handle,
 	struct device *device = dev;
 	struct vmclock_state *st = device->driver_data;
 
+	vmclock_set_tk_reference(st);
 	wake_up_interruptible(&st->disrupt_wait);
 }
 
@@ -580,6 +603,7 @@ static irqreturn_t vmclock_of_irq_handler(int __always_unused irq, void *_st)
 {
 	struct vmclock_state *st = _st;
 
+	vmclock_set_tk_reference(st);
 	wake_up_interruptible(&st->disrupt_wait);
 	return IRQ_HANDLED;
 }
@@ -751,6 +775,8 @@ static int vmclock_probe(struct platform_device *pdev)
 			st->ptp_clock = NULL;
 			return ret;
 		}
+		if (st->ptp_clock)
+			vmclock_set_tk_reference(st);
 	}
 
 	if (!st->miscdev.minor && !st->ptp_clock) {
-- 
2.51.0

Re: [RFC PATCH 1/4] timekeeping: Remove xtime_remainder from ntp_error accumulation

[John Stultz]

On Wed, May 13, 2026 at 2:02 PM David Woodhouse <dwmw2@infradead.org> wrote:
>
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> The ntp_error accumulator tracks the difference between intended and
> actual clock advance. Each tick it adds ntp_tick (the intended advance)
> and subtracts what the clock actually advanced.
>
> The subtraction was (xtime_interval + xtime_remainder), but only
> xtime_interval is actually added to xtime_nsec each tick.
> xtime_remainder was a boot-time constant representing the rounding error
> from converting the tick period to an integer number of counter cycles.
> It was never added to xtime_nsec, so subtracting it from ntp_error
> created a phantom credit that biased the dithering ratio.
>
> The effect is a systematic drift whose magnitude depends on the value of
> xtime_remainder and the NTP frequency correction. NTP masks this by
> continuously adjusting the frequency to compensate, but with a fixed
> frequency (or an external reference clock like vmclock), the drift is
> exposed.
>
> Also remove xtime_remainder from the mult computation in
> timekeeping_adjust(), which used it to offset the division for the same
> (incorrect) reason.
>
> Fixes: a386b5af8edd ("time: Compensate for rounding on odd-frequency clocksources")

Hey, thanks for sending this out!

Looks like it would be more accurate to say it is a revert then a fix?

Hrm. So this is maybe too far back for my goldfish brain to really remember.

My hazy sense of it was that for coarser-grained clocksources, there
will be an inherent granularity error for what we want to use as an
accumulation interval, and the hardware allows.

So if we want a 1ms ntp interval, taking the hardware freq and finding
cycles/ms, might be something like 10.4 (imaginary example here), so
that rounds 10 cycles per interval which is really gives us a 962us
interval.

Now, the 10.4 cycles/ms is still accurate, and the resulting
xtime_interval is still correct, but there is a resulting 38us
difference between the xtime_interval and the ntp_tick due to this
granularity error in the calculation., When we start accumulating
ntp_error, it will grow at that 38us/ms rate, causing the internal
logic to try to steer the clock faster to compensate.

Thus, instead of assuming the ntp-interval is actually 1ms, we want it
to match the actual accumulation interval, so we subtract that error
off of the ntp-interval.  That "phantom credit" is effectively
intended to address this inherent error (which is why it isn't applied
to the other side - since it's already there as part of the
calculation).

Now, again, it's been far too long, and I may be misremembering
details, and the correction may very well be unnecessary or otherwise
erronious! But I think it deserves a bit more evidence/rationale in
the commit message as to why.

thanks
-john

Re: [RFC PATCH 1/4] timekeeping: Remove xtime_remainder from ntp_error accumulation

[David Woodhouse]

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

On Fri, 2026-05-15 at 15:49 -0700, John Stultz wrote:
> On Wed, May 13, 2026 at 2:02 PM David Woodhouse <dwmw2@infradead.org> wrote:
> > 
> > From: David Woodhouse <dwmw@amazon.co.uk>
> > 
> > The ntp_error accumulator tracks the difference between intended and
> > actual clock advance. Each tick it adds ntp_tick (the intended advance)
> > and subtracts what the clock actually advanced.
> > 
> > The subtraction was (xtime_interval + xtime_remainder), but only
> > xtime_interval is actually added to xtime_nsec each tick.
> > xtime_remainder was a boot-time constant representing the rounding error
> > from converting the tick period to an integer number of counter cycles.
> > It was never added to xtime_nsec, so subtracting it from ntp_error
> > created a phantom credit that biased the dithering ratio.
> > 
> > The effect is a systematic drift whose magnitude depends on the value of
> > xtime_remainder and the NTP frequency correction. NTP masks this by
> > continuously adjusting the frequency to compensate, but with a fixed
> > frequency (or an external reference clock like vmclock), the drift is
> > exposed.
> > 
> > Also remove xtime_remainder from the mult computation in
> > timekeeping_adjust(), which used it to offset the division for the same
> > (incorrect) reason.
> > 
> > Fixes: a386b5af8edd ("time: Compensate for rounding on odd-frequency clocksources")
> 
> Hey, thanks for sending this out!
> 
> Looks like it would be more accurate to say it is a revert then a fix?
> 
> Hrm. So this is maybe too far back for my goldfish brain to really remember.
> 
> My hazy sense of it was that for coarser-grained clocksources, there
> will be an inherent granularity error for what we want to use as an
> accumulation interval, and the hardware allows.
> 
> So if we want a 1ms ntp interval, taking the hardware freq and finding
> cycles/ms, might be something like 10.4 (imaginary example here), so
> that rounds 10 cycles per interval which is really gives us a 962us
> interval.
> 
> Now, the 10.4 cycles/ms is still accurate, and the resulting
> xtime_interval is still correct, but there is a resulting 38us
> difference between the xtime_interval and the ntp_tick due to this
> granularity error in the calculation., When we start accumulating
> ntp_error, it will grow at that 38us/ms rate, causing the internal
> logic to try to steer the clock faster to compensate.
> 
> Thus, instead of assuming the ntp-interval is actually 1ms, we want it
> to match the actual accumulation interval, so we subtract that error
> off of the ntp-interval.  That "phantom credit" is effectively
> intended to address this inherent error (which is why it isn't applied
> to the other side - since it's already there as part of the
> calculation).
> 
> Now, again, it's been far too long, and I may be misremembering
> details, and the correction may very well be unnecessary or otherwise
> erronious! But I think it deserves a bit more evidence/rationale in
> the commit message as to why.

Thanks. This has been making my brain hurt for most of the last week,
but I think I finally have a handle on it.

It looks like we track three different times (ignoring units):

 • A: The xtime that we actually output to the vDSO/etc.

 • B: xtime+ntp_error is the time we *want* to be outputting right now,
      but the mult dithering and monotonicity clawback keep us from it.

 • C: xtime+ntp_error+time_offset is the time we *eventually* want to
      output, once we've finished skewing towards it.

On each tick:

 • xtime (A) advances by xtime_interval (and the clawback; there's
   another patch in my tree to account for that in ntp_error now).

 • (B) advances by whatever tick_length happens to be at the moment
   (adjusted by second_overflow to effect a skew).

 • (C) advances by the original tick_length_base actually set according
   to the adjtimex frequency.

So ntp_error, being the delta between (B) and (A), needs to advance by
tick_length - xtime_interval. Before this patch, xtime_remainder was
*also* being subtracted from the 'what xtime advanced' side, but it
isn't actually added to xtime; it *is* roughly the amount that needed
to be accumulated in ntp_error here (except for the fact that
xtime_remainder was calculated once at boot time and never updated).

I spent a while booting kernels in QEMU with a VMClock reference clock
precisely specifying the TSC to real-time relationship for (C), and
tracking the *nanosecond* delta of the output from what it was told.

I made it redundantly track (B) and (C) above as absolute values, so
that I could spot per-tick when the accounting of the existing *deltas*
in ntp_error and time_offset went astray (and compare C with the actual
refclock, to check that too).

In my test tree, xtime now correctly dithers precisely around the
desired time (B) and doesn't continually drift from it any more.

And I can even inject, say, a 10μs delta via time_offset and it'll skew
by *exactly* 10,000ns and stay there, still with with single digit
nanosecond divergence from where it's meant to be.

So now I don't *need* the external oracle to drive the dithering per
tick for the reference clock (as I had added in patch 3 in this
series), because the kernel can actually stay on the y=mx+c line
configured by tick_length and ntp_error/time_offset all by itself, so
all I have to do is set the *existing* parameters in
timekeeping_set_reference(). 

(Arithmetic precision would *eventually* catch up with it, of course,
but in reality it won't be following a hard-coded refclock and the
reference itself will be periodically adjusted as the real counter
varies, and reclamping will occur.)

I don't much like the complexity of the way that time_offset skews
ntp_tick, and it's somewhat redundant with the mult dithering that came
later. I'm playing with folding both time_offset and ntp_error into a
single 'phase_error' and doing the larger skew purely by adjusting
mult, like the dithering does... but by larger values. That's a WIP at
the top of my current working tree:

David Woodhouse (6):
      timekeeping: Remove xtime_remainder from ntp_error accumulation
      timekeeping: Account for clawback adjustment in ntp_error
      timekeeping: Clamp time_offset delta to prevent infinite tail
      timekeeping: Add absolute reference for feed-forward clock discipline
      ptp_vmclock: Feed reference to timekeeping for feed-forward discipline
      timekeeping: Unify ntp_error and time_offset into phase_error

https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/ffclock

I believe everything in there is correctly parameterised and nothing
makes assumptions that are invalid for different CONFIG_HZ and for much
lower-rate timers. I'm almost at the point of being happy enough with
the overall state that I'm ready to start testing those cases too...

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

Re: [RFC PATCH 1/4] timekeeping: Remove xtime_remainder from ntp_error accumulation

[John Stultz]

On Sat, May 16, 2026 at 1:25 AM David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Fri, 2026-05-15 at 15:49 -0700, John Stultz wrote:
> > On Wed, May 13, 2026 at 2:02 PM David Woodhouse <dwmw2@infradead.org> wrote:
> > >
> > > From: David Woodhouse <dwmw@amazon.co.uk>
> > >
> > > The ntp_error accumulator tracks the difference between intended and
> > > actual clock advance. Each tick it adds ntp_tick (the intended advance)
> > > and subtracts what the clock actually advanced.
> > >
> > > The subtraction was (xtime_interval + xtime_remainder), but only
> > > xtime_interval is actually added to xtime_nsec each tick.
> > > xtime_remainder was a boot-time constant representing the rounding error
> > > from converting the tick period to an integer number of counter cycles.
> > > It was never added to xtime_nsec, so subtracting it from ntp_error
> > > created a phantom credit that biased the dithering ratio.
> > >
> > > The effect is a systematic drift whose magnitude depends on the value of
> > > xtime_remainder and the NTP frequency correction. NTP masks this by
> > > continuously adjusting the frequency to compensate, but with a fixed
> > > frequency (or an external reference clock like vmclock), the drift is
> > > exposed.
> > >
> > > Also remove xtime_remainder from the mult computation in
> > > timekeeping_adjust(), which used it to offset the division for the same
> > > (incorrect) reason.
> > >
> > > Fixes: a386b5af8edd ("time: Compensate for rounding on odd-frequency clocksources")
> >
> > Hey, thanks for sending this out!
> >
> > Looks like it would be more accurate to say it is a revert then a fix?
> >
> > Hrm. So this is maybe too far back for my goldfish brain to really remember.
> >
> > My hazy sense of it was that for coarser-grained clocksources, there
> > will be an inherent granularity error for what we want to use as an
> > accumulation interval, and the hardware allows.
> >
> > So if we want a 1ms ntp interval, taking the hardware freq and finding
> > cycles/ms, might be something like 10.4 (imaginary example here), so
> > that rounds 10 cycles per interval which is really gives us a 962us
> > interval.
> >
> > Now, the 10.4 cycles/ms is still accurate, and the resulting
> > xtime_interval is still correct, but there is a resulting 38us
> > difference between the xtime_interval and the ntp_tick due to this
> > granularity error in the calculation., When we start accumulating
> > ntp_error, it will grow at that 38us/ms rate, causing the internal
> > logic to try to steer the clock faster to compensate.
> >
> > Thus, instead of assuming the ntp-interval is actually 1ms, we want it
> > to match the actual accumulation interval, so we subtract that error
> > off of the ntp-interval.  That "phantom credit" is effectively
> > intended to address this inherent error (which is why it isn't applied
> > to the other side - since it's already there as part of the
> > calculation).
> >
> > Now, again, it's been far too long, and I may be misremembering
> > details, and the correction may very well be unnecessary or otherwise
> > erronious! But I think it deserves a bit more evidence/rationale in
> > the commit message as to why.
>
> Thanks. This has been making my brain hurt for most of the last week,
> but I think I finally have a handle on it.
>
> It looks like we track three different times (ignoring units):
>
>  • A: The xtime that we actually output to the vDSO/etc.
>
>  • B: xtime+ntp_error is the time we *want* to be outputting right now,
>       but the mult dithering and monotonicity clawback keep us from it.

I think of B, or specifically ntp_error, as the delta between our time
and what *NTP* (well, the in-kernel ntp machinery) wants the system to
be right now.

As an aside, apologies if I'm asking obvious questions here,  some of
your terminology is unfamiliar. While it's not used around the code or
patches, I can understand the dithering metaphor for the long-term
error adjustments to effectively allow for sub-integer mult
adjustments over time (similar to b&w dithering to approximate levels
of grey), but there is also the common use of indecisively dithering
time away or the astrophotography sense of intentionlally adding
noise, which I worry might cause some confusion to others as to what
you mean.

Also I'm not sure its very clear what you mean by "monotonicity clawback".


>  • C: xtime+ntp_error+time_offset is the time we *eventually* want to
>       output, once we've finished skewing towards it.

And yeah, I'd say C is where the userland NTP wants it to slew towards.


> On each tick:
>
>  • xtime (A) advances by xtime_interval (and the clawback; there's
>    another patch in my tree to account for that in ntp_error now).
>
>  • (B) advances by whatever tick_length happens to be at the moment
>    (adjusted by second_overflow to effect a skew).
>
>  • (C) advances by the original tick_length_base actually set according
>    to the adjtimex frequency.
>
> So ntp_error, being the delta between (B) and (A), needs to advance by
> tick_length - xtime_interval. Before this patch, xtime_remainder was
> *also* being subtracted from the 'what xtime advanced' side, but it
> isn't actually added to xtime; it *is* roughly the amount that needed
> to be accumulated in ntp_error here (except for the fact that
> xtime_remainder was calculated once at boot time and never updated).

Again, I'm sure it could be miscalculated, or be misapplied, but as I
mentioned previously, the xtime_remainder is trying to address a
granularity error that is effectively baked into the delta between
xtime_interval and the initial ntp interval (essentially the initial
ntp_tick), which doesn't seem to be addressed here.

For fine-grained clocksources like the TSC its not likely a big issue,
but for coarser grained clocksources it seems like just removing this
would be a regression.


> I spent a while booting kernels in QEMU with a VMClock reference clock
> precisely specifying the TSC to real-time relationship for (C), and
> tracking the *nanosecond* delta of the output from what it was told.
>
> I made it redundantly track (B) and (C) above as absolute values, so
> that I could spot per-tick when the accounting of the existing *deltas*
> in ntp_error and time_offset went astray (and compare C with the actual
> refclock, to check that too).
>
> In my test tree, xtime now correctly dithers precisely around the
> desired time (B) and doesn't continually drift from it any more.
>
> And I can even inject, say, a 10μs delta via time_offset and it'll skew
> by *exactly* 10,000ns and stay there, still with with single digit
> nanosecond divergence from where it's meant to be.

Sounds like very nice results!

> So now I don't *need* the external oracle to drive the dithering per
> tick for the reference clock (as I had added in patch 3 in this
> series), because the kernel can actually stay on the y=mx+c line
> configured by tick_length and ntp_error/time_offset all by itself, so
> all I have to do is set the *existing* parameters in
> timekeeping_set_reference().
>
> (Arithmetic precision would *eventually* catch up with it, of course,
> but in reality it won't be following a hard-coded refclock and the
> reference itself will be periodically adjusted as the real counter
> varies, and reclamping will occur.)

I'm excited your efforts seem to be doing well! The ntp/timekeeping
core has been pretty static for awhile now, outside of some of the ptp
work, so its nice to see new major contributions. That said, it being
in place for so long, and apparently working for folks, means changes
do need close consideration to make sure we don't introduce subtle
regressions.

thanks
-john

Re: [RFC PATCH 1/4] timekeeping: Remove xtime_remainder from ntp_error accumulation

[David Woodhouse]

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

On Mon, 2026-05-18 at 18:37 -0700, John Stultz wrote:
> On Sat, May 16, 2026 at 1:25 AM David Woodhouse <dwmw2@infradead.org> wrote:
> > Thanks. This has been making my brain hurt for most of the last week,
> > but I think I finally have a handle on it.
> > 
> > It looks like we track three different times (ignoring units):
> > 
> >  • A: The xtime that we actually output to the vDSO/etc.
> > 
> >  • B: xtime+ntp_error is the time we *want* to be outputting right now,
> >       but the mult dithering and monotonicity clawback keep us from it.
> 
> I think of B, or specifically ntp_error, as the delta between our time
> and what *NTP* (well, the in-kernel ntp machinery) wants the system to
> be right now.

Right. I think we're entirely on the same page there. The "right now"
is the key.

> As an aside, apologies if I'm asking obvious questions here,  some of
> your terminology is unfamiliar. While it's not used around the code or
> patches, I can understand the dithering metaphor for the long-term
> error adjustments to effectively allow for sub-integer mult
> adjustments over time (similar to b&w dithering to approximate levels
> of grey), but there is also the common use of indecisively dithering
> time away or the astrophotography sense of intentionlally adding
> noise, which I worry might cause some confusion to others as to what
> you mean.

Indeed. The two adjacent values of 'mult' will effectively make xtime
proceed slightly faster than, or slightly slower than, the actual
desired rate. It is exactly 'dithering' in the sense of approximating
levels of grey, tracking ntp_error precisely in order to choose between
mult vs. mult+1 for the next tick.

> Also I'm not sure its very clear what you mean by "monotonicity clawback".

This is the offset applied by timekeeping_apply_adjustment() in order
to ensure that the observed xtime remains monotonic when the dithering
switches back from 'mult+1' to 'mult' and a consumer may have seen a
'later' time than it's about to set in {cycle_last,mult}.

(In fact it also prevents a forward jump, but IIUC that's less
critical.)

That offset was not being accounted in ntp_error, and was accumulating
over time in my tests.

> >  • C: xtime+ntp_error+time_offset is the time we *eventually* want to
> >       output, once we've finished skewing towards it.
> 
> And yeah, I'd say C is where the userland NTP wants it to slew towards.
> 
> 
> > On each tick:
> > 
> >  • xtime (A) advances by xtime_interval (and the clawback; there's
> >    another patch in my tree to account for that in ntp_error now).
> > 
> >  • (B) advances by whatever tick_length happens to be at the moment
> >    (adjusted by second_overflow to effect a skew).
> > 
> >  • (C) advances by the original tick_length_base actually set according
> >    to the adjtimex frequency.
> > 
> > So ntp_error, being the delta between (B) and (A), needs to advance by
> > tick_length - xtime_interval. Before this patch, xtime_remainder was
> > *also* being subtracted from the 'what xtime advanced' side, but it
> > isn't actually added to xtime; it *is* roughly the amount that needed
> > to be accumulated in ntp_error here (except for the fact that
> > xtime_remainder was calculated once at boot time and never updated).
> 
> Again, I'm sure it could be miscalculated, or be misapplied, but as I
> mentioned previously, the xtime_remainder is trying to address a
> granularity error that is effectively baked into the delta between
> xtime_interval and the initial ntp interval (essentially the initial
> ntp_tick), which doesn't seem to be addressed here.

My understanding is that xtime_remainder *is* the delta between the
initial xtime_interval and the initial ntp_interval. Calculated once at
boot and then permanently out of sync when mult and thus xtime_interval
actually change.

By simply adding ntp_interval and then subtracting the correct current
value of xtime_interval, logarithmic_accumulation doesn't *need* a
separate variable which tracks that delta. I don't quite understand why
it ever did add it in the first place.

> For fine-grained clocksources like the TSC its not likely a big issue,
> but for coarser grained clocksources it seems like just removing this
> would be a regression.

I think it should be fine with coarser grained clocksources. The
dithering sawtooth around the reference line will have a higher
amplitude, but adding ntp_interval and subtracting xtime_interval for
the mult value currently in effect is still the right thing to do.

> > I spent a while booting kernels in QEMU with a VMClock reference clock
> > precisely specifying the TSC to real-time relationship for (C), and
> > tracking the *nanosecond* delta of the output from what it was told.
> > 
> > I made it redundantly track (B) and (C) above as absolute values, so
> > that I could spot per-tick when the accounting of the existing *deltas*
> > in ntp_error and time_offset went astray (and compare C with the actual
> > refclock, to check that too).
> > 
> > In my test tree, xtime now correctly dithers precisely around the
> > desired time (B) and doesn't continually drift from it any more.
> > 
> > And I can even inject, say, a 10μs delta via time_offset and it'll skew
> > by *exactly* 10,000ns and stay there, still with with single digit
> > nanosecond divergence from where it's meant to be.
> 
> Sounds like very nice results!
> 
> > So now I don't *need* the external oracle to drive the dithering per
> > tick for the reference clock (as I had added in patch 3 in this
> > series), because the kernel can actually stay on the y=mx+c line
> > configured by tick_length and ntp_error/time_offset all by itself, so
> > all I have to do is set the *existing* parameters in
> > timekeeping_set_reference().
> > 
> > (Arithmetic precision would *eventually* catch up with it, of course,
> > but in reality it won't be following a hard-coded refclock and the
> > reference itself will be periodically adjusted as the real counter
> > varies, and reclamping will occur.)
> 
> I'm excited your efforts seem to be doing well! The ntp/timekeeping
> core has been pretty static for awhile now, outside of some of the ptp
> work, so its nice to see new major contributions. That said, it being
> in place for so long, and apparently working for folks, means changes
> do need close consideration to make sure we don't introduce subtle
> regressions.
> 

Absolutely. And I do definitely also need to do more testing with
coarser grained clock sources. I've obviously *tried* to ensure that
nothing I've done makes assumptions based around my test environment
which happens to be a 2400MHz TSC and CONFIG_HZ=1000, but we'll see how
it fares in actual testing. I'll also need to test a nohz setup.

As I see it, the timekeeping code already has a fairly clean split
between the core timekeeper and the ntp adjustment. The former is
*mostly* supposed to just keep the time progressing along a clean
y=mx+c relationship from counter to observed xtime, in the common case
where time_offset is zero. And setting time_offset should change the
value of 'c' in that formula and exponentially eventually converge to
*precisely* that delta away from the link it was previously keeping.

All I've done here, or so I believe, is make the timekeeping code
better at following that line it's given. The way that the NTP side
*gives* it that line to follow is fairly much untouched.


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

Re: [RFC PATCH 1/4] timekeeping: Remove xtime_remainder from ntp_error accumulation

[John Stultz]

On Tue, May 19, 2026 at 2:16 AM David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Mon, 2026-05-18 at 18:37 -0700, John Stultz wrote:
> > On Sat, May 16, 2026 at 1:25 AM David Woodhouse <dwmw2@infradead.org> wrote:
> > > Thanks. This has been making my brain hurt for most of the last week,
> > > but I think I finally have a handle on it.
> > >
> > > It looks like we track three different times (ignoring units):
> > >
> > >  • A: The xtime that we actually output to the vDSO/etc.
> > >
> > >  • B: xtime+ntp_error is the time we *want* to be outputting right now,
> > >       but the mult dithering and monotonicity clawback keep us from it.
> >
> > I think of B, or specifically ntp_error, as the delta between our time
> > and what *NTP* (well, the in-kernel ntp machinery) wants the system to
> > be right now.
>
> Right. I think we're entirely on the same page there. The "right now"
> is the key.
>
> > As an aside, apologies if I'm asking obvious questions here,  some of
> > your terminology is unfamiliar. While it's not used around the code or
> > patches, I can understand the dithering metaphor for the long-term
> > error adjustments to effectively allow for sub-integer mult
> > adjustments over time (similar to b&w dithering to approximate levels
> > of grey), but there is also the common use of indecisively dithering
> > time away or the astrophotography sense of intentionlally adding
> > noise, which I worry might cause some confusion to others as to what
> > you mean.
>
> Indeed. The two adjacent values of 'mult' will effectively make xtime
> proceed slightly faster than, or slightly slower than, the actual
> desired rate. It is exactly 'dithering' in the sense of approximating
> levels of grey, tracking ntp_error precisely in order to choose between
> mult vs. mult+1 for the next tick.

Its a nice analogy. Though if you're going to use it in your commit
messages, it might be good to also add a comment to the
timekeeping_adjustment() logic so there is an clear anchor to the code
for folks who might not be as familiar with the detail.


> > Also I'm not sure its very clear what you mean by "monotonicity clawback".
>
> This is the offset applied by timekeeping_apply_adjustment() in order
> to ensure that the observed xtime remains monotonic when the dithering
> switches back from 'mult+1' to 'mult' and a consumer may have seen a
> 'later' time than it's about to set in {cycle_last,mult}.
>

This one I find less clarifying, but I do recognize "adjustments to
the base xtime_nsec made when adjusting the multiplier due to
unaccumulated cycles" is a mouthful.

The "xtime_nsec adjustment in timekeeping_apply_adjustment()" is
probably easier/clearer?


> > > So ntp_error, being the delta between (B) and (A), needs to advance by
> > > tick_length - xtime_interval. Before this patch, xtime_remainder was
> > > *also* being subtracted from the 'what xtime advanced' side, but it
> > > isn't actually added to xtime; it *is* roughly the amount that needed
> > > to be accumulated in ntp_error here (except for the fact that
> > > xtime_remainder was calculated once at boot time and never updated).
> >
> > Again, I'm sure it could be miscalculated, or be misapplied, but as I
> > mentioned previously, the xtime_remainder is trying to address a
> > granularity error that is effectively baked into the delta between
> > xtime_interval and the initial ntp interval (essentially the initial
> > ntp_tick), which doesn't seem to be addressed here.
>
> My understanding is that xtime_remainder *is* the delta between the
> initial xtime_interval and the initial ntp_interval. Calculated once at
> boot and then permanently out of sync when mult and thus xtime_interval
> actually change.

Yes, it is the difference between the initial xtime_interval and the
initial ntp_interval, but maybe the key here is to ask "if there is no
NTP adjustment being made why is there a delta between these two
initial values?"

> By simply adding ntp_interval and then subtracting the correct current
> value of xtime_interval, logarithmic_accumulation doesn't *need* a
> separate variable which tracks that delta. I don't quite understand why
> it ever did add it in the first place.

Again, it's due to the error caused by the granularity of the
clocksource (against the HZ interval).  That initial delta will
otherwise cause an accurate clock to get erroniously steered either
faster or slower when no NTP adjustments being made.  The subtracting
of that remainder is just trying to compensate for this base error.

Now, it is imperfect because it's a constant adjustment and not
proportioned to the NTP adjustments that might be later made.

And clearly there is some issue as you're having problems with it
using a fine-grained clocksource like the TSC where it really
shouldn't be a major factor.

> > For fine-grained clocksources like the TSC its not likely a big issue,
> > but for coarser grained clocksources it seems like just removing this
> > would be a regression.
>
> I think it should be fine with coarser grained clocksources. The
> dithering sawtooth around the reference line will have a higher
> amplitude, but adding ntp_interval and subtracting xtime_interval for
> the mult value currently in effect is still the right thing to do.

I still worry ignoring it could cause behavior regressions on hardware
with coarse clocksources, and feels a little short sighted.

But again, I am excited for your work here and your deep investigation
of these behavioral issues you've found.

thanks
-john

Re: [RFC PATCH 1/4] timekeeping: Remove xtime_remainder from ntp_error accumulation

[David Woodhouse]

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

On Tue, 2026-05-19 at 14:54 -0700, John Stultz wrote:
> 
> > By simply adding ntp_interval and then subtracting the correct current
> > value of xtime_interval, logarithmic_accumulation doesn't *need* a
> > separate variable which tracks that delta. I don't quite understand why
> > it ever did add it in the first place.
> 
> Again, it's due to the error caused by the granularity of the
> clocksource (against the HZ interval).  That initial delta will
> otherwise cause an accurate clock to get erroniously steered either
> faster or slower when no NTP adjustments being made.  The subtracting
> of that remainder is just trying to compensate for this base error.

I must be confused. We *are* subtracting that remainder, not ignoring
it.

The value of xtime_remainder was (tick_length - xtime_interval). It's
the discrepancy between the distance we *want* the clock to move in a
single tick, and the distance it *does* move in a single tick at a
given value of 'mult'.

So we add tick_length, and we subtract xtime_interval. Thus adjusting
ntp_error by the difference between them.

I don't understand why the old code was then *also* subtracting a stale
value of xtime_remainder which had been calculated at boot time.

Am I missing something here? 

I've updated the commit message:
https://git.infradead.org/?p=users/dwmw2/linux.git;a=commitdiff;h=459ebaef612

> Now, it is imperfect because it's a constant adjustment and not
> proportioned to the NTP adjustments that might be later made.
> 
> And clearly there is some issue as you're having problems with it
> using a fine-grained clocksource like the TSC where it really
> shouldn't be a major factor.

None of it matters if you have NTP running and constantly re-adjusting
your clock. You can have a +50PPM drift through errors in the tracking,
and NTP will just assume your oscillator is running 50PPM fast and
adjust accordingly.

But when you set the kernel to follow a precise y=mx+c line for the TSC
to real time conversion and *expect* it to do as it's told while
tracking the divergence to the nanosecond... all counters are course-
grained :)



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

Re: [RFC PATCH 1/4] timekeeping: Remove xtime_remainder from ntp_error accumulation

[David Woodhouse]

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

On Tue, 2026-05-19 at 14:54 -0700, John Stultz wrote:
> 
> Its a nice analogy. Though if you're going to use it in your commit
> messages, it might be good to also add a comment to the
> timekeeping_adjustment() logic so there is an clear anchor to the code
> for folks who might not be as familiar with the detail.
> 
> 
> > > Also I'm not sure its very clear what you mean by "monotonicity clawback".
> > 
> > This is the offset applied by timekeeping_apply_adjustment() in order
> > to ensure that the observed xtime remains monotonic when the dithering
> > switches back from 'mult+1' to 'mult' and a consumer may have seen a
> > 'later' time than it's about to set in {cycle_last,mult}.
> > 
> 
> This one I find less clarifying, but I do recognize "adjustments to
> the base xtime_nsec made when adjusting the multiplier due to
> unaccumulated cycles" is a mouthful.
> 
> The "xtime_nsec adjustment in timekeeping_apply_adjustment()" is
> probably easier/clearer?

How's this...

# Linux Timekeeping: Tick-Based Clock Discipline

## Overview

The kernel exposes time to userspace via the vDSO as a linear function:
`time = base + (cycles_since_base * mult) >> shift`. Userspace reads the
cycle counter and computes the current time without a syscall.

Every tick (1ms at HZ=1000), the kernel updates this linear function: it
reads the counter, computes how much time elapsed since the last update,
advances `base` (called `xtime_nsec` internally), and publishes the new
parameters.

## mult, tick_length, and Dithering

`tick_length` is the intended duration of one tick — how much wall-clock
time should pass per tick. Again using HZ=1000 as the example, at boot
it's exactly 1ms (in shifted-ns: `1,000,000 << 32`). NTP adjusts it by
up to ±500 PPM to correct for clock drift: if the TSC runs 50 PPM slow,
NTP sets tick_length to 1.00005ms so each tick advances time slightly
more than 1ms.

`cycle_interval` is the number of counter cycles per tick — fixed at
boot from the initial TSC calibration (e.g. 2,400,000 for a 2.4 GHz TSC
at HZ=1000). This never changes.

`mult` converts cycles to time: `nanoseconds = cycles * mult >> shift`.
It's derived from `mult = tick_length / cycle_interval`. Since this
division rarely yields an integer, the kernel uses two adjacent values:

  - `mult_lo = floor(tick_length / cycle_interval)` — slightly too slow
  - `mult_hi = mult_lo + 1` — slightly too fast

The kernel alternates ("dithers") between these each tick so the
long-term average rate matches `tick_length` exactly. If the fractional
part of the division is 0.8, then 80% of ticks use mult_hi and 20% use
mult_lo.

There is a constraint: the vDSO must never return a time that goes
backwards. Between ticks, userspace interpolates using the published
mult. If the kernel switches from mult_hi to mult_lo, the new (slower)
interpolation line might be below where the old (faster) line already
reached. To prevent this, the kernel adjusts `base` forward when
lowering mult — setting it to where the old line would have been at the
current counter value. This adjustment is applied in
`timekeeping_apply_adjustment()`.

## The Three Reference Times

The timekeeping code tracks three distinct time values (ignoring units
for clarity):

 • **(A) xtime** — the time actually output to the vDSO and userspace.
   This is `xtime_nsec`, advanced each tick by `xtime_interval` (plus
   the monotonicity adjustment when mult changes).

 • **(B) xtime + ntp_error** — the time we *want* to be outputting
   right now. The mult dithering and monotonicity adjustment keep (A)
   from reaching (B) instantly, but the dithering steers (A) toward
   (B) over time.

 • **(C) xtime + ntp_error + time_offset** — the time we *eventually*
   want to output, once we've finished slewing toward it. This is the
   target set by `adjtimex()` or `timekeeping_set_reference()`.

On each tick:

 • (A) advances by `xtime_interval` (= `cycle_interval * mult`), plus
   the monotonicity adjustment when mult changes.

 • (B) advances by `tick_length + skew_delta` — the configured rate
   plus a per-tick bias that effects the slew toward (C).

 • (C) advances by `tick_length` — the rate corresponding to the
   `adjtimex` frequency (or the rate set by
   `timekeeping_set_reference()`).

So `ntp_error`, being the delta between (B) and (A), advances each tick
by `tick_length + skew_delta - xtime_interval`. The dithering oracle
uses the sign of `ntp_error` to choose mult_hi or mult_lo, steering (A)
toward (B).

And `time_offset`, being the delta between (C) and (B), is consumed
gradually as `skew_delta` biases (B) faster or slower than (C). Once
the slew completes, `time_offset` and `skew_delta` reach zero and
(B) = (C).

## Tick Accumulation and Leftover Cycles

On each timer interrupt (which may arrive late), the kernel reads the
cycle counter and computes the delta since `cycle_last`. It then
consumes as many full `cycle_interval` chunks as fit, advancing
`cycle_last` by `cycle_interval` for each and adding `xtime_interval` to
`xtime_nsec`.

After accumulation, `cycle_last` points to the last full-tick boundary
— NOT the actual counter value at interrupt time. The remaining cycles
between `cycle_last` and the actual counter reading are the `offset`:

    offset = counter_at_irq - cycle_last

These leftover cycles are NOT added to `xtime_nsec`. They remain as the
implicit gap that the vDSO interpolates: userspace computes
`time = xtime_nsec + (tsc_now - cycle_last) * mult >> shift`, so the
leftover is always accounted for at the current `mult`.

On the next interrupt, the new delta includes the previous leftover plus
new cycles. Full ticks are consumed, a new leftover remains.

This leftover is the `offset` parameter passed to `timekeeping_adjust()`
after accumulation completes. It represents cycles that the vDSO is
already interpolating at the current mult, but which haven't yet been
folded into `xtime_nsec`.

When `timekeeping_apply_adjustment()` changes mult, it must compensate:
the vDSO was interpolating those leftover cycles at the old mult, and
will now interpolate them at the new mult. To prevent a time
discontinuity visible to userspace, it adjusts:

    xtime_nsec -= offset    (for mult increasing by 1)
    xtime_nsec += offset    (for mult decreasing by 1)

This ensures `xtime_nsec + offset * mult` is the same before and after
the mult change — maintaining vDSO monotonicity.

## ntp_error: The Dithering Accumulator

`ntp_error` tracks the accumulated difference between the target rate
and the actual rate. Each tick:
```
ntp_error += ntp_tick                    (target advance)
ntp_error -= xtime_interval << shift     (actual advance)
```

When NTP adjusts the frequency, `ntp_tick` changes but
`xtime_interval` stays at the boot-time value. The per-tick residual
drives `ntp_error` positive or negative, triggering the dithering to
compensate.

The oracle: when `ntp_error > 0` (clock is behind), use mult_hi. When
`ntp_error ≤ 0` (clock is ahead), use mult_lo.

The monotonicity adjustment (`xtime_nsec -= offset`) moves time forward
without the dithering oracle knowing. To keep `ntp_error` accurate,
`timekeeping_apply_adjustment()` adds the same offset to ntp_error:

    ntp_error += offset << ntp_error_shift

This accounts for the fact that the adjustment advanced `xtime_nsec` by
`offset` shifted-ns, which the oracle must treat as actual progress.

## skew_delta and time_offset Slewing

When NTP (or `timekeeping_set_reference()`) sets a `time_offset`, the
NTP layer computes a `skew_delta` — a per-tick bias added to
`tick_length` that causes the clock to slew toward the target offset.
The slew rate is limited to ±500 µs/s (adjtime) or the full offset
divided over a decay period.

`timekeeping_adjust()` recomputes mult whenever `ntp_tick` or
`skew_delta` changes:

    mult = (ntp_tick + skew_delta) >> ntp_error_shift / cycle_interval

The `skew_delta` biases mult so that the ±1 dithering naturally delivers
the slew. As the offset is consumed, `skew_delta` decays toward zero.

The `ntp_error` accumulator accounts for `skew_delta` partially: it
credits only the portion of `skew_delta` actually delivered by the
current tick's mult choice, not the full requested amount. This allows
the dithering to converge `ntp_error` toward zero while simultaneously
delivering the slew.

## timekeeping_set_reference(): Feed-Forward Discipline

An external reference (e.g. vmclock from a host with NTP-disciplined
time) can set the clock's rate and phase directly via
`timekeeping_set_reference()`. This function:

1. **Validates the clocksource** — returns `-ENODEV` if the active
   clocksource doesn't match the reference's `cs_id`.

2. **Sets tick_length** — converts the reference's `period_frac_sec`
   (counter period in 1/2^64 seconds) to the kernel's `tick_length`
   format (shifted-ns per tick):

       tick_length = period_frac_sec * cycle_interval * NSEC_PER_SEC
                     >> (32 + period_shift)

3. **Computes the phase offset** — extrapolates the reference time to
   `cycle_last` and compares with the kernel's current `xtime_nsec`:

       ref_time_at_cycle_last = time_frac_sec +
           delta_cycles * period_frac_sec >> period_shift
       ref_err = ref_time_in_shifted_ns - xtime_nsec

4. **Sets time_offset** — programs the NTP slew machinery to converge
   toward the reference phase. The existing `skew_delta` mechanism
   delivers this offset over subsequent ticks.

5. **Clears ntp_error** — resets the dithering accumulator so it starts
   fresh relative to the new rate, avoiding stale bias from the
   previous discipline state.

All of this happens under `tk_core.lock`, making it atomic with respect
to tick processing. The function is fire-and-forget: it programs the
rate and phase, then the normal tick machinery delivers the correction
autonomously.


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

Re: [RFC PATCH 1/4] timekeeping: Remove xtime_remainder from ntp_error accumulation

[John Stultz]

On Tue, May 19, 2026 at 3:18 PM David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Tue, 2026-05-19 at 14:54 -0700, John Stultz wrote:
> >
> > > By simply adding ntp_interval and then subtracting the correct current
> > > value of xtime_interval, logarithmic_accumulation doesn't *need* a
> > > separate variable which tracks that delta. I don't quite understand why
> > > it ever did add it in the first place.
> >
> > Again, it's due to the error caused by the granularity of the
> > clocksource (against the HZ interval).  That initial delta will
> > otherwise cause an accurate clock to get erroniously steered either
> > faster or slower when no NTP adjustments being made.  The subtracting
> > of that remainder is just trying to compensate for this base error.
>
> I must be confused. We *are* subtracting that remainder, not ignoring
> it.
>
> The value of xtime_remainder was (tick_length - xtime_interval). It's
> the discrepancy between the distance we *want* the clock to move in a
> single tick, and the distance it *does* move in a single tick at a
> given value of 'mult'.
>
> So we add tick_length, and we subtract xtime_interval. Thus adjusting
> ntp_error by the difference between them.
>
> I don't understand why the old code was then *also* subtracting a stale
> value of xtime_remainder which had been calculated at boot time.
>
> Am I missing something here?

Hrm. Apologies, I fret I'm just repeating myself, and I'm not sure how
to reconceptualize my point to be more helpful. :/

But you skipped my question in my last mail, which I do think will help:
* If there is no adjustment being made, why would there be a
difference between the initial ntp_interval and the initial
xtime_interval?

* Why would it exist? And if it does exist, what does it represent?

* And if the frequency (cyc2ns) is correct, and no adjustment is being
made, is it not erroneous to be adjusting the clock to correct for
this accumulating delta?


Maybe does it help to thinking of it it as quantization error from a
single cycle length? And how it may not be able to match the
tick_length at various HZ?
But the quantization error doesn't have anything to do with the
accuracy of that cycle length.  But if not accounted for, that error
accumulates in ntp_error and results in unwanted frequency
adjustments.


> I've updated the commit message:
> https://git.infradead.org/?p=users/dwmw2/linux.git;a=commitdiff;h=459ebaef612
>
> > Now, it is imperfect because it's a constant adjustment and not
> > proportioned to the NTP adjustments that might be later made.
> >
> > And clearly there is some issue as you're having problems with it
> > using a fine-grained clocksource like the TSC where it really
> > shouldn't be a major factor.
>
> None of it matters if you have NTP running and constantly re-adjusting
> your clock. You can have a +50PPM drift through errors in the tracking,
> and NTP will just assume your oscillator is running 50PPM fast and
> adjust accordingly.
>
> But when you set the kernel to follow a precise y=mx+c line for the TSC
> to real time conversion and *expect* it to do as it's told while
> tracking the divergence to the nanosecond... all counters are course-
> grained :)

Something maybe as an alternative approach:
What if instead of tinkering with the xtime_remainder separately, we
just made the adjustment to the initial ntp_interval (propagated to
the tk->ntp_tick) to handle the granularity error? This would maybe
more intuitively address the conceptual "what the ntp machinery wants
time to be" issue?

Though we'd need to propagate that out to the ntp.c logic so it would
show up in ntp_tick_length() (and maybe even further up into
tick_usec). But then I worry the ntp userland might blindly overwrite
this on startup, so maybe this approach won't work.

Another thing to consider: we could maybe use some threshold for
applying the granularity error correction, so we only apply the
correction when the clocksource is coarse and the error is large
enough to warrant it?

thanks
-john

Re: [RFC PATCH 1/4] timekeeping: Remove xtime_remainder from ntp_error accumulation

[John Stultz]

On Tue, May 19, 2026 at 3:20 PM David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Tue, 2026-05-19 at 14:54 -0700, John Stultz wrote:
> >
> > Its a nice analogy. Though if you're going to use it in your commit
> > messages, it might be good to also add a comment to the
> > timekeeping_adjustment() logic so there is an clear anchor to the code
> > for folks who might not be as familiar with the detail.
> >
> >
> > > > Also I'm not sure its very clear what you mean by "monotonicity clawback".
> > >
> > > This is the offset applied by timekeeping_apply_adjustment() in order
> > > to ensure that the observed xtime remains monotonic when the dithering
> > > switches back from 'mult+1' to 'mult' and a consumer may have seen a
> > > 'later' time than it's about to set in {cycle_last,mult}.
> > >
> >
> > This one I find less clarifying, but I do recognize "adjustments to
> > the base xtime_nsec made when adjusting the multiplier due to
> > unaccumulated cycles" is a mouthful.
> >
> > The "xtime_nsec adjustment in timekeeping_apply_adjustment()" is
> > probably easier/clearer?
>
> How's this...
>
> # Linux Timekeeping: Tick-Based Clock Discipline

So, I'm guessing given how fast you were to create this and how
verbose it is, it was AI generated? (Apologies if I'm wrong here!)

If so, do remember to use the Assisted-by tag.

I'll try to find some time to carefully review it, but while I
appreciate the benefit of having a tool help write this, I'm not super
excited about externalizing the cost of the review onto others.
As careful review and feedback of an individuals work feels like
investing in that collaborative effort, but doing the same for AI
generated output just feels like a chore.

Though I also understand a similar frustration of having maintainers
gatekeeping their approval by asking for more (cleanups, docs, etc)
labor - so it's probably a fair turnabout.
But I've got to play the part here. :)

thanks
-john

Re: [RFC PATCH 1/4] timekeeping: Remove xtime_remainder from ntp_error accumulation

[David Woodhouse]

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

On Tue, 2026-05-19 at 17:32 -0700, John Stultz wrote:
> On Tue, May 19, 2026 at 3:20 PM David Woodhouse <dwmw2@infradead.org>
> wrote:
> > 
> > On Tue, 2026-05-19 at 14:54 -0700, John Stultz wrote:
> > > 
> > > Its a nice analogy. Though if you're going to use it in your
> > > commit
> > > messages, it might be good to also add a comment to the
> > > timekeeping_adjustment() logic so there is an clear anchor to the
> > > code
> > > for folks who might not be as familiar with the detail.
> > > 
> > > 
> > > > > Also I'm not sure its very clear what you mean by
> > > > > "monotonicity clawback".
> > > > 
> > > > This is the offset applied by timekeeping_apply_adjustment() in
> > > > order
> > > > to ensure that the observed xtime remains monotonic when the
> > > > dithering
> > > > switches back from 'mult+1' to 'mult' and a consumer may have
> > > > seen a
> > > > 'later' time than it's about to set in {cycle_last,mult}.
> > > > 
> > > 
> > > This one I find less clarifying, but I do recognize "adjustments
> > > to
> > > the base xtime_nsec made when adjusting the multiplier due to
> > > unaccumulated cycles" is a mouthful.
> > > 
> > > The "xtime_nsec adjustment in timekeeping_apply_adjustment()" is
> > > probably easier/clearer?
> > 
> > How's this...
> > 
> > # Linux Timekeeping: Tick-Based Clock Discipline
> 
> So, I'm guessing given how fast you were to create this and how
> verbose it is, it was AI generated? (Apologies if I'm wrong here!)

It was originally, yes. But it wasn't that fast; I've had it open in
emacs for a week or so now. I had the AI write it for me so that I
could sanity-check it and refine my own understanding.

I trust the AI about as far as I can throw it.

> If so, do remember to use the Assisted-by tag.

Ack.

> I'll try to find some time to carefully review it, but while I
> appreciate the benefit of having a tool help write this, I'm not super
> excited about externalizing the cost of the review onto others.
> As careful review and feedback of an individuals work feels like
> investing in that collaborative effort, but doing the same for AI
> generated output just feels like a chore.

Oh, absolutely! I have actually done the first week's worth of saying
"no, that's complete horseshit. Tell me the truth" to it, as well as
refining the flow to make it read more coherently :)


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

Re: [RFC PATCH 1/4] timekeeping: Remove xtime_remainder from ntp_error accumulation

[David Woodhouse]

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

On Tue, 2026-05-19 at 17:01 -0700, John Stultz wrote:
> On Tue, May 19, 2026 at 3:18 PM David Woodhouse <dwmw2@infradead.org> wrote:
> > 
> > On Tue, 2026-05-19 at 14:54 -0700, John Stultz wrote:
> > > 
> > > > By simply adding ntp_interval and then subtracting the correct current
> > > > value of xtime_interval, logarithmic_accumulation doesn't *need* a
> > > > separate variable which tracks that delta. I don't quite understand why
> > > > it ever did add it in the first place.
> > > 
> > > Again, it's due to the error caused by the granularity of the
> > > clocksource (against the HZ interval).  That initial delta will
> > > otherwise cause an accurate clock to get erroniously steered either
> > > faster or slower when no NTP adjustments being made.  The subtracting
> > > of that remainder is just trying to compensate for this base error.
> > 
> > I must be confused. We *are* subtracting that remainder, not ignoring
> > it.
> > 
> > The value of xtime_remainder was (tick_length - xtime_interval). It's
> > the discrepancy between the distance we *want* the clock to move in a
> > single tick, and the distance it *does* move in a single tick at a
> > given value of 'mult'.
> > 
> > So we add tick_length, and we subtract xtime_interval. Thus adjusting
> > ntp_error by the difference between them.
> > 
> > I don't understand why the old code was then *also* subtracting a stale
> > value of xtime_remainder which had been calculated at boot time.
> > 
> > Am I missing something here?
> 
> Hrm. Apologies, I fret I'm just repeating myself, and I'm not sure how
> to reconceptualize my point to be more helpful. :/
> 
> But you skipped my question in my last mail, which I do think will help:
> * If there is no adjustment being made, why would there be a
> difference between the initial ntp_interval and the initial
> xtime_interval?

Because the ntp_interval is not a precisely multiple of the cycle
period?

The example I've been working with all week is 1ms tick, 2400MHz TSC.

So that is 1000000<<24 "shifted nanoseconds", or 16777216000000,
divided by 2400000 to give a 'mult' value of 6990506.667.

Truncated to an integer, that's 6990506, giving an xtime_interval of 
16777214400000 and xtime_remainder of 1600000.

The dithering will do a duty cycle of one tick at 6990506 to two ticks
at 6990507, to achieve the overall desired rate.

> * Why would it exist? And if it does exist, what does it represent?

It represents the difference between the xtime *actually* added during
a tick (xtime_interval), and the time we *wanted* to add during a tick
(ntp_interval).

> * And if the frequency (cyc2ns) is correct, and no adjustment is being
> made, is it not erroneous to be adjusting the clock to correct for
> this accumulating delta?

I don't think it does accumulate in that state, does it? It sawtooths
around zero as we dither between mult and mult+1. And in my example,
over the course of *three* ticks it'll reach zero again, as
3*ntp_interval == 2*xtime_interval(mult+1) + 1*xtime_interval(mult)


> Maybe does it help to thinking of it it as quantization error from a
> single cycle length? And how it may not be able to match the
> tick_length at various HZ?

Right. Because it's all about the *fractional* part of 'mult'.

> But the quantization error doesn't have anything to do with the
> accuracy of that cycle length. 

Right. That part is purely about arithmetic precision.

>  But if not accounted for, that error accumulates in ntp_error and
> results in unwanted frequency adjustments.

Right. If ntp_error doesn't account for xtime_remainder each tick (or
more accurately, by +ntp_interval-xtime_interval), then errors will
accumulate and result in *effective* frequency adjustments because the
'mult' dithering will not follow the duty cycle that it should, and
xtime will not advance at the correct rate.

And since NTP works on a feedback loop, that in turn will result in
*actual* frequency adjustments (and alterations to tick_length) because
NTP will interpret the error-induced drift as the underlying oscillator
going faster/slower.

> 
> > I've updated the commit message:
> > https://git.infradead.org/?p=users/dwmw2/linux.git;a=commitdiff;h=459ebaef612
> > 
> > > Now, it is imperfect because it's a constant adjustment and not
> > > proportioned to the NTP adjustments that might be later made.
> > > 
> > > And clearly there is some issue as you're having problems with it
> > > using a fine-grained clocksource like the TSC where it really
> > > shouldn't be a major factor.
> > 
> > None of it matters if you have NTP running and constantly re-adjusting
> > your clock. You can have a +50PPM drift through errors in the tracking,
> > and NTP will just assume your oscillator is running 50PPM fast and
> > adjust accordingly.
> > 
> > But when you set the kernel to follow a precise y=mx+c line for the TSC
> > to real time conversion and *expect* it to do as it's told while
> > tracking the divergence to the nanosecond... all counters are course-
> > grained :)
> 
> Something maybe as an alternative approach:
> What if instead of tinkering with the xtime_remainder separately, we
> just made the adjustment to the initial ntp_interval (propagated to
> the tk->ntp_tick) to handle the granularity error? This would maybe
> more intuitively address the conceptual "what the ntp machinery wants
> time to be" issue?

Does that help? Aren't we continuously adjusting the tick_length
because that's how we accommodate actual frequency changes of the
counter?

> Though we'd need to propagate that out to the ntp.c logic so it would
> show up in ntp_tick_length() (and maybe even further up into
> tick_usec). But then I worry the ntp userland might blindly overwrite
> this on startup, so maybe this approach won't work.
> 
> Another thing to consider: we could maybe use some threshold for
> applying the granularity error correction, so we only apply the
> correction when the clocksource is coarse and the error is large
> enough to warrant it?

I'm still lost. What's the problem we're trying to solve here now?

AIUI the granularity error correction is simple; we add tick_length and
subtract xtime_interval, which is basically equivalent to adding the
*correct* value of xtime_remainder each tick. It's what this code was
always trying to do; it just got it wrong. Now it's fixed, why would we
only do it *sometimes*? 

This tracking is precisely what ensures that the dithering picks 'mult'
vs. 'mult+1' in the right duty cycle. It's not just for coarse
clocksources; even my 2400MHz TSC will lose ~95ns/s if we don't track
this.



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