* [PATCH v7 1/7] MAINTAINERS: Add Miroslav as timekeeping reviewer
2026-06-21 21:53 [PATCH v7 0/7] timekeeping/ntp: Fix drift tracking precision David Woodhouse
@ 2026-06-21 21:53 ` David Woodhouse
2026-07-07 22:22 ` [tip: timers/core] " tip-bot2 for David Woodhouse
2026-06-21 21:53 ` [PATCH v7 2/7] timekeeping: Account for monotonicity adjustment in ntp_error David Woodhouse
` (5 subsequent siblings)
6 siblings, 1 reply; 30+ messages in thread
From: David Woodhouse @ 2026-06-21 21:53 UTC (permalink / raw)
To: Richard Cochran, Wen Gu, David Woodhouse, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John Stultz, Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Peter Zijlstra,
Thomas Weißschuh, Arnd Bergmann, Miroslav Lichvar,
Julien Ridoux, Ryan Luu, linux-kernel
From: David Woodhouse <dwmw@amazon.co.uk>
If Thomas is going to nudge me on IRC to add Miroslav to Cc on
timekeeping patches, then he might as well actually be listed in the
MAINTAINERS file.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Acked-by: John Stultz <jstultz@google.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 882214b0e7db..de39cc3e7e68 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26877,6 +26877,7 @@ TIMEKEEPING, CLOCKSOURCE CORE, NTP, ALARMTIMER
M: John Stultz <jstultz@google.com>
M: Thomas Gleixner <tglx@kernel.org>
R: Stephen Boyd <sboyd@kernel.org>
+R: Miroslav Lichvar <mlichvar@redhat.com>
L: linux-kernel@vger.kernel.org
S: Supported
T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers/core
--
2.54.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [tip: timers/core] MAINTAINERS: Add Miroslav as timekeeping reviewer
2026-06-21 21:53 ` [PATCH v7 1/7] MAINTAINERS: Add Miroslav as timekeeping reviewer David Woodhouse
@ 2026-07-07 22:22 ` tip-bot2 for David Woodhouse
0 siblings, 0 replies; 30+ messages in thread
From: tip-bot2 for David Woodhouse @ 2026-07-07 22:22 UTC (permalink / raw)
To: linux-tip-commits
Cc: David Woodhouse, Thomas Gleixner, John Stultz, x86, linux-kernel
The following commit has been merged into the timers/core branch of tip:
Commit-ID: 79b8bd857bd7f5a8c970fc50c611062b96fe56e5
Gitweb: https://git.kernel.org/tip/79b8bd857bd7f5a8c970fc50c611062b96fe56e5
Author: David Woodhouse <dwmw@amazon.co.uk>
AuthorDate: Sun, 21 Jun 2026 22:53:54 +01:00
Committer: Thomas Gleixner <tglx@kernel.org>
CommitterDate: Wed, 08 Jul 2026 00:17:26 +02:00
MAINTAINERS: Add Miroslav as timekeeping reviewer
If Thomas is going to nudge me on IRC to add Miroslav to Cc on
timekeeping patches, then he might as well actually be listed in the
MAINTAINERS file.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Acked-by: John Stultz <jstultz@google.com>
Link: https://patch.msgid.link/20260621220051.1030462-2-dwmw2@infradead.org
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 15011f5..ff01c39 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27199,6 +27199,7 @@ TIMEKEEPING, CLOCKSOURCE CORE, NTP, ALARMTIMER
M: John Stultz <jstultz@google.com>
M: Thomas Gleixner <tglx@kernel.org>
R: Stephen Boyd <sboyd@kernel.org>
+R: Miroslav Lichvar <mlichvar@redhat.com>
L: linux-kernel@vger.kernel.org
S: Supported
T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers/core
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v7 2/7] timekeeping: Account for monotonicity adjustment in ntp_error
2026-06-21 21:53 [PATCH v7 0/7] timekeeping/ntp: Fix drift tracking precision David Woodhouse
2026-06-21 21:53 ` [PATCH v7 1/7] MAINTAINERS: Add Miroslav as timekeeping reviewer David Woodhouse
@ 2026-06-21 21:53 ` David Woodhouse
2026-07-07 22:22 ` [tip: timers/core] " tip-bot2 for David Woodhouse
2026-06-21 21:53 ` [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP David Woodhouse
` (4 subsequent siblings)
6 siblings, 1 reply; 30+ messages in thread
From: David Woodhouse @ 2026-06-21 21:53 UTC (permalink / raw)
To: Richard Cochran, Wen Gu, David Woodhouse, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John Stultz, Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Peter Zijlstra,
Thomas Weißschuh, Arnd Bergmann, Miroslav Lichvar,
Julien Ridoux, Ryan Luu, linux-kernel
From: David Woodhouse <dwmw@amazon.co.uk>
timekeeping_apply_adjustment() modifies xtime_nsec to ensure monotonicity
when mult changes:
xtime_nsec -= offset
This ensures that the time reported to userspace does not jump when the
multiplier is adjusted from one tick to the next. However, the ntp_error
accumulator which tracks the difference between intended and actual
clock position was not being updated to reflect this additional
discrepancy.
An earlier attempt at this compensation existed as:
ntp_error -= (interval - offset) << ntp_error_shift
but was removed in commit c2cda2a5bda9 ("timekeeping/ntp: Don't align
NTP frequency adjustments to ticks") because it was a major source of
NTP error. That's because (interval - offset) was wrong: the subtraction
of "interval" prematurely accounted for the changed xtime_interval of
the next tick, which would be correctly accounted in the next
accumulation anyway — a double subtraction.
What is actually needed is just the "offset" part: ntp_error must be
told that xtime_nsec moved by "offset" without a corresponding change
in the intended position. For the normal ±1 mult dithering this is
negligible (the adjustments cancel over time), but for larger mult
changes — such as when an external reference clock sets a new
frequency — the one-time uncompensated offset is significant.
Fix by adjusting ntp_error by the correct amount:
ntp_error += offset << ntp_error_shift
This keeps ntp_error consistent with the actual xtime_nsec position
after the adjustment, and ensures the discrepancy is correctly smoothed
away over time and the clock returns to where it should have been.
Fixes: c2cda2a5bda9 ("timekeeping/ntp: Don't align NTP frequency adjustments to ticks")
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.6-1m
Acked-by: John Stultz <jstultz@google.com>
---
kernel/time/timekeeping.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 0d5b67f609bb..d847bba0481b 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -2389,6 +2389,11 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk,
* xtime_nsec_2 = xtime_nsec_1 - offset
* Which simplifies to:
* xtime_nsec -= offset
+ *
+ * When subtracting offset from xtime_nsec, the same amount
+ * (in appropriate units) has to be added to ntp_error, in
+ * order to correctly track the delta between the time
+ * reported in xtime_nsec, and the intended time.
*/
if ((mult_adj > 0) && (tk->tkr_mono.mult + mult_adj < mult_adj)) {
/* NTP adjustment caused clocksource mult overflow */
@@ -2399,6 +2404,7 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk,
tk->tkr_mono.mult += mult_adj;
tk->xtime_interval += interval;
tk->tkr_mono.xtime_nsec -= offset;
+ tk->ntp_error += offset << tk->ntp_error_shift;
}
/*
--
2.54.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [tip: timers/core] timekeeping: Account for monotonicity adjustment in ntp_error
2026-06-21 21:53 ` [PATCH v7 2/7] timekeeping: Account for monotonicity adjustment in ntp_error David Woodhouse
@ 2026-07-07 22:22 ` tip-bot2 for David Woodhouse
0 siblings, 0 replies; 30+ messages in thread
From: tip-bot2 for David Woodhouse @ 2026-07-07 22:22 UTC (permalink / raw)
To: linux-tip-commits
Cc: David Woodhouse, Thomas Gleixner, John Stultz, x86, linux-kernel
The following commit has been merged into the timers/core branch of tip:
Commit-ID: b7befd6d91207cf3f4cecd68fea0c212093906cf
Gitweb: https://git.kernel.org/tip/b7befd6d91207cf3f4cecd68fea0c212093906cf
Author: David Woodhouse <dwmw@amazon.co.uk>
AuthorDate: Sun, 21 Jun 2026 22:53:55 +01:00
Committer: Thomas Gleixner <tglx@kernel.org>
CommitterDate: Wed, 08 Jul 2026 00:17:26 +02:00
timekeeping: Account for monotonicity adjustment in ntp_error
timekeeping_apply_adjustment() modifies xtime_nsec to ensure monotonicity
when mult changes:
xtime_nsec -= offset
This ensures that the time reported to userspace does not jump when the
multiplier is adjusted from one tick to the next. However, the ntp_error
accumulator which tracks the difference between intended and actual
clock position was not being updated to reflect this additional
discrepancy.
An earlier attempt at this compensation existed as:
ntp_error -= (interval - offset) << ntp_error_shift
but was removed in commit c2cda2a5bda9 ("timekeeping/ntp: Don't align
NTP frequency adjustments to ticks") because it was a major source of
NTP error. That's because (interval - offset) was wrong: the subtraction
of "interval" prematurely accounted for the changed xtime_interval of
the next tick, which would be correctly accounted in the next
accumulation anyway — a double subtraction.
What is actually needed is just the "offset" part: ntp_error must be
told that xtime_nsec moved by "offset" without a corresponding change
in the intended position. For the normal ±1 mult dithering this is
negligible (the adjustments cancel over time), but for larger mult
changes — such as when an external reference clock sets a new
frequency — the one-time uncompensated offset is significant.
Fix by adjusting ntp_error by the correct amount:
ntp_error += offset << ntp_error_shift
This keeps ntp_error consistent with the actual xtime_nsec position
after the adjustment, and ensures the discrepancy is correctly smoothed
away over time and the clock returns to where it should have been.
Fixes: c2cda2a5bda9 ("timekeeping/ntp: Don't align NTP frequency adjustments to ticks")
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Assisted-by: Kiro:claude-opus-4.6-1m
Acked-by: John Stultz <jstultz@google.com>
Link: https://patch.msgid.link/20260621220051.1030462-3-dwmw2@infradead.org
---
kernel/time/timekeeping.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 2527a48..9e1cbc5 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -2404,6 +2404,11 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk,
* xtime_nsec_2 = xtime_nsec_1 - offset
* Which simplifies to:
* xtime_nsec -= offset
+ *
+ * When subtracting offset from xtime_nsec, the same amount
+ * (in appropriate units) has to be added to ntp_error, in
+ * order to correctly track the delta between the time
+ * reported in xtime_nsec, and the intended time.
*/
if ((mult_adj > 0) && (tk->tkr_mono.mult + mult_adj < mult_adj)) {
/* NTP adjustment caused clocksource mult overflow */
@@ -2414,6 +2419,7 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk,
tk->tkr_mono.mult += mult_adj;
tk->xtime_interval += interval;
tk->tkr_mono.xtime_nsec -= offset;
+ tk->ntp_error += offset << tk->ntp_error_shift;
}
/*
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP
2026-06-21 21:53 [PATCH v7 0/7] timekeeping/ntp: Fix drift tracking precision David Woodhouse
2026-06-21 21:53 ` [PATCH v7 1/7] MAINTAINERS: Add Miroslav as timekeeping reviewer David Woodhouse
2026-06-21 21:53 ` [PATCH v7 2/7] timekeeping: Account for monotonicity adjustment in ntp_error David Woodhouse
@ 2026-06-21 21:53 ` David Woodhouse
2026-06-22 17:52 ` John Stultz
` (3 more replies)
2026-06-21 21:53 ` [PATCH v7 4/7] timekeeping: Drive time_offset skew via per-tick ntp_error transfer David Woodhouse
` (3 subsequent siblings)
6 siblings, 4 replies; 30+ messages in thread
From: David Woodhouse @ 2026-06-21 21:53 UTC (permalink / raw)
To: Richard Cochran, Wen Gu, David Woodhouse, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John Stultz, Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Peter Zijlstra,
Thomas Weißschuh, Arnd Bergmann, Miroslav Lichvar,
Julien Ridoux, Ryan Luu, linux-kernel
From: David Woodhouse <dwmw@amazon.co.uk>
cycle_interval is an integer number of counter cycles per NTP interval,
so the real time it represents differs from the nominal
NTP_INTERVAL_LENGTH by up to half a counter period. For coarse
clocksources this is significant: the 3.579545 MHz ACPI PM timer at
HZ=1000 rounds 3579.545 cycles up to 3580, making each tick 1.000127 ms
(+127 PPM).
Commit a386b5af8edd ("time: Compensate for rounding on odd-frequency
clocksources") introduced xtime_remainder to compensate for exactly
this, citing the same 127 PPM ACPI PM example. The compensation is
correct and necessary, but it was applied inside the timekeeping
accumulation in timekeeping.c: subtracted in the mult computation in
timekeeping_adjust() and folded into the ntp_error update in
logarithmic_accumulation(). That keeps the base rate correct and leaves
NTP its full symmetric +/-MAXFREQ range rather than +373/-627 PPM, but
the NTP code in ntp.c never sees it: tick_length is computed without the
correction, so ntp.c's notion of how long a tick is disagrees with the
rate timekeeping actually produces.
Make the offset an explicit part of the NTP tick_length instead. Add
ntp_data::cs_tick_adj, a fixed per-second addend that
ntp_update_frequency() includes alongside ntp_tick_adj and time_freq.
tk_setup_internals() computes it from the difference between the real
cycle_interval duration and the nominal interval, stores it in the
timekeeper, and hands it to NTP through a new argument to ntp_clear() --
which already recomputes the frequency and is invoked after every
clocksource (re)configuration. timekeeping_init() now uses TK_UPDATE_ALL
for this; clearing NTP there is otherwise redundant since ntp_init() has
just initialised it.
ntp.c now computes the true tick rate, giving a single source of truth.
Like ntp_tick_adj, cs_tick_adj stays internal to the kernel: userspace
still sees the nominal 1.000000 ms tick via adjtimex and is unaware of
the addends. timekeeping_adjust() and logarithmic_accumulation() use
ntp_tick / xtime_interval directly, and xtime_remainder is removed.
The base-rate arithmetic is unchanged: ntp_tick becomes
xtime_interval << ntp_error_shift, so the mult division yields the same
base mult and the ntp_error accumulation still nets to zero per tick.
Beyond the cleanup of treating all the tick_length contributions
(nominal interval, ntp_tick_adj, cs_tick_adj, time_freq) consistently
as addends in one place, it also prepares for feed-forward discipline:
a future timekeeping_set_reference() will set tick_length to track an
absolute external reference such as a vmclock, and that path needs
ntp.c to own a tick_length that already reflects the clocksource
quantisation, with no hidden correction applied elsewhere.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.8
---
include/linux/timekeeper_internal.h | 8 ++--
kernel/time/ntp.c | 27 ++++++++++--
kernel/time/ntp_internal.h | 2 +-
kernel/time/timekeeping.c | 66 +++++++++++++++++++++--------
4 files changed, 78 insertions(+), 25 deletions(-)
diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index e36d11e33e0c..ec81587a1400 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -84,8 +84,6 @@ struct tk_read_base {
* @cycle_interval: Number of clock cycles in one NTP interval
* @xtime_interval: Number of clock shifted nano seconds in one NTP
* interval.
- * @xtime_remainder: Shifted nano seconds left over when rounding
- * @cycle_interval
* @raw_interval: Shifted raw nano seconds accumulated per NTP interval.
* @next_leap_ktime: CLOCK_MONOTONIC time value of a pending leap-second
* @ntp_tick: The ntp_tick_length() value currently being
@@ -99,6 +97,10 @@ struct tk_read_base {
* @ntp_error_shift: Shift conversion between clock shifted nano seconds and
* ntp shifted nano seconds.
* @ntp_err_mult: Multiplication factor for scaled math conversion
+ * @cs_tick_adj: Per-second adjustment handed to NTP via ntp_clear()
+ * accounting for the difference between the nominal
+ * NTP interval and the real time taken by the
+ * clocksource's integer @cycle_interval (upscaled).
* @skip_second_overflow: Flag used to avoid updating NTP twice with same second
* @tai_offset: The current UTC to TAI offset in seconds
*
@@ -178,7 +180,6 @@ struct timekeeper {
u64 cycle_interval;
u64 xtime_interval;
- s64 xtime_remainder;
u64 raw_interval;
ktime_t next_leap_ktime;
@@ -186,6 +187,7 @@ struct timekeeper {
s64 ntp_error;
u32 ntp_error_shift;
u32 ntp_err_mult;
+ s64 cs_tick_adj;
u32 skip_second_overflow;
s32 tai_offset;
};
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 97fa99b96dd0..3fad82c47c4c 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -39,6 +39,10 @@
* @time_reftime: Time at last adjustment in seconds
* @time_adjust: Adjustment value
* @ntp_tick_adj: Constant boot-param configurable NTP tick adjustment (upscaled)
+ * @cs_tick_adj: Fixed per-second adjustment compensating for the difference
+ * between the nominal NTP interval and the real time taken
+ * by the clocksource's integer @cycle_interval (upscaled).
+ * Set by the timekeeping core via ntp_clear().
* @ntp_next_leap_sec: Second value of the next pending leapsecond, or TIME64_MAX if no leap
*
* @pps_valid: PPS signal watchdog counter
@@ -70,6 +74,7 @@ struct ntp_data {
time64_t time_reftime;
long time_adjust;
s64 ntp_tick_adj;
+ s64 cs_tick_adj;
time64_t ntp_next_leap_sec;
#ifdef CONFIG_NTP_PPS
int pps_valid;
@@ -255,6 +260,7 @@ static void ntp_update_frequency(struct ntp_data *ntpdata)
second_length = (u64)(tick_usec * NSEC_PER_USEC * USER_HZ) << NTP_SCALE_SHIFT;
second_length += ntpdata->ntp_tick_adj;
+ second_length += ntpdata->cs_tick_adj;
second_length += ntpdata->time_freq;
new_base = div_u64(second_length, NTP_INTERVAL_FREQ);
@@ -350,11 +356,26 @@ static void __ntp_clear(struct ntp_data *ntpdata)
}
/**
- * ntp_clear - Clears the NTP state variables
- * @tkid: Timekeeper ID to be able to select proper ntp data array member
+ * ntp_clear - Clear NTP state and set the clocksource quantisation adjustment
+ * @tkid: Timekeeper ID
+ * @cs_tick_adj: Per-second adjustment in ns << NTP_SCALE_SHIFT
+ *
+ * The timekeeping core uses an integer number of cycles (@cycle_interval)
+ * per NTP interval, so the real time that interval represents differs from
+ * the nominal NTP_INTERVAL_LENGTH by up to half a counter period. Folding
+ * this fixed offset into @cs_tick_adj makes it an explicit part of the NTP
+ * tick_length computation in ntp.c, instead of being applied during
+ * timekeeping accumulation where the NTP code never saw it. Like
+ * @ntp_tick_adj it stays internal to the kernel; userspace still sees the
+ * nominal tick via adjtimex. NTP retains its full symmetric ±MAXFREQ range
+ * around the corrected base rate.
+ *
+ * Called whenever the clocksource is (re)configured, which is also when the
+ * rest of the NTP state must be cleared, so the two are done together.
*/
-void ntp_clear(unsigned int tkid)
+void ntp_clear(unsigned int tkid, s64 cs_tick_adj)
{
+ tk_ntp_data[tkid].cs_tick_adj = cs_tick_adj;
__ntp_clear(&tk_ntp_data[tkid]);
}
diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h
index 7084d839c207..598e5dd2fc5b 100644
--- a/kernel/time/ntp_internal.h
+++ b/kernel/time/ntp_internal.h
@@ -3,7 +3,7 @@
#define _LINUX_NTP_INTERNAL_H
extern void ntp_init(void);
-extern void ntp_clear(unsigned int tkid);
+extern void ntp_clear(unsigned int tkid, s64 cs_tick_adj);
/* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
extern u64 ntp_tick_length(unsigned int tkid);
extern ktime_t ntp_get_next_leap(unsigned int tkid);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index d847bba0481b..53961a1fcf47 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -339,7 +339,6 @@ static inline void clocksource_enable_inline_read(void) { }
static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
{
u64 interval;
- u64 tmp, ntpinterval;
struct clocksource *old_clock;
++tk->cs_was_changed_seq;
@@ -353,20 +352,16 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
tk->tkr_raw.cycle_last = tk->tkr_mono.cycle_last;
/* Do the ns -> cycle conversion first, using original mult */
- tmp = NTP_INTERVAL_LENGTH;
- tmp <<= clock->shift;
- ntpinterval = tmp;
- tmp += clock->mult/2;
- do_div(tmp, clock->mult);
- if (tmp == 0)
- tmp = 1;
-
- interval = (u64) tmp;
+ interval = NTP_INTERVAL_LENGTH << clock->shift;
+ interval += clock->mult/2;
+ do_div(interval, clock->mult);
+ if (interval == 0)
+ interval = 1;
+
tk->cycle_interval = interval;
/* Go back from cycles -> shifted ns */
tk->xtime_interval = interval * clock->mult;
- tk->xtime_remainder = ntpinterval - tk->xtime_interval;
tk->raw_interval = interval * clock->mult;
/* if changing clocks, convert xtime_nsec shift units */
@@ -386,7 +381,38 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
tk->ntp_error = 0;
tk->ntp_error_shift = NTP_SCALE_SHIFT - clock->shift;
- tk->ntp_tick = ntpinterval << tk->ntp_error_shift;
+
+ /*
+ * ntp_tick is the tick length that NTP disciplines (its ±500 PPM
+ * scales only this part), in NTP-shifted ns: the real interval of
+ * a whole number of counter cycles. Because cycle_interval is
+ * rounded to an integer number of cycles, this ntp_tick differs
+ * from the true intended 1/HZ tick length by up to half a cycle
+ * period.
+ */
+ tk->ntp_tick = (u64)tk->xtime_interval << tk->ntp_error_shift;
+
+ /*
+ * cs_tick_adj is the constant difference between the disciplined
+ * ntp_tick above and the true 1/HZ tick, expressed per-second to
+ * match the ntp_update_frequency() addends and handed to NTP via
+ * ntp_clear() to be explicitly included in its tick_length.
+ *
+ * Worked example: HZ=1000, ACPI PM timer at 3.579545 MHz, which
+ * has 3579.545 cycles in 1ms, rounded to cycle_interval = 3580.
+ *
+ * So ntp_tick is actually 1.000127ms, as that is the amount of
+ * time that 3580 cycles will take at the nominal frequency. This
+ * is the part that NTP disciplines, causing each 3580 counts to
+ * advance the clock by up to NTP's ±500PPM of that amount.
+ *
+ * The "extra" 127ns/tick is what's stored in cs_tick_adj and
+ * applied as a constant correction by ntp_update_frequency() so
+ * that NTP *believes* it's disciplining a 1ms tick.
+ */
+ tk->cs_tick_adj = (s64)tk->ntp_tick -
+ ((s64)NTP_INTERVAL_LENGTH << NTP_SCALE_SHIFT);
+ tk->cs_tick_adj *= NTP_INTERVAL_FREQ;
/*
* The timekeeper keeps its own mult values for the currently
@@ -803,7 +829,7 @@ static void timekeeping_update_from_shadow(struct tk_data *tkd, unsigned int act
if (action & TK_CLEAR_NTP) {
tk->ntp_error = 0;
- ntp_clear(tk->id);
+ ntp_clear(tk->id, tk->cs_tick_adj);
}
tk_update_leap_state(tk);
@@ -2075,7 +2101,12 @@ void __init timekeeping_init(void)
tk_set_wall_to_mono(tks, wall_to_mono);
- timekeeping_update_from_shadow(&tk_core, TK_CLOCK_WAS_SET);
+ /*
+ * Use TK_UPDATE_ALL so the NTP layer picks up the clocksource's
+ * cs_tick_adj via ntp_clear(). Clearing NTP here is otherwise
+ * redundant as ntp_init() already initialised it above.
+ */
+ timekeeping_update_from_shadow(&tk_core, TK_UPDATE_ALL);
}
/* time in seconds when suspend began for persistent clock */
@@ -2424,8 +2455,8 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
mult = tk->tkr_mono.mult - tk->ntp_err_mult;
} else {
tk->ntp_tick = ntp_tl;
- mult = div64_u64((tk->ntp_tick >> tk->ntp_error_shift) -
- tk->xtime_remainder, tk->cycle_interval);
+ mult = div64_u64(tk->ntp_tick >> tk->ntp_error_shift,
+ tk->cycle_interval);
}
/*
@@ -2550,8 +2581,7 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
/* Accumulate error between NTP and clock interval */
tk->ntp_error += tk->ntp_tick << shift;
- tk->ntp_error -= (tk->xtime_interval + tk->xtime_remainder) <<
- (tk->ntp_error_shift + shift);
+ tk->ntp_error -= tk->xtime_interval << (tk->ntp_error_shift + shift);
return offset;
}
--
2.54.0
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP
2026-06-21 21:53 ` [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP David Woodhouse
@ 2026-06-22 17:52 ` John Stultz
2026-07-07 19:27 ` David Woodhouse
2026-07-07 22:22 ` [tip: timers/core] " tip-bot2 for David Woodhouse
` (2 subsequent siblings)
3 siblings, 1 reply; 30+ messages in thread
From: John Stultz @ 2026-06-22 17:52 UTC (permalink / raw)
To: David Woodhouse
Cc: Richard Cochran, Wen Gu, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Thomas Gleixner,
Stephen Boyd, Anna-Maria Behnsen, Frederic Weisbecker,
Shuah Khan, Peter Zijlstra, Thomas Weißschuh, Arnd Bergmann,
Miroslav Lichvar, Julien Ridoux, Ryan Luu, linux-kernel
On Sun, Jun 21, 2026 at 3:00 PM David Woodhouse <dwmw2@infradead.org> wrote:
>
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> cycle_interval is an integer number of counter cycles per NTP interval,
> so the real time it represents differs from the nominal
> NTP_INTERVAL_LENGTH by up to half a counter period. For coarse
> clocksources this is significant: the 3.579545 MHz ACPI PM timer at
> HZ=1000 rounds 3579.545 cycles up to 3580, making each tick 1.000127 ms
> (+127 PPM).
>
> Commit a386b5af8edd ("time: Compensate for rounding on odd-frequency
> clocksources") introduced xtime_remainder to compensate for exactly
> this, citing the same 127 PPM ACPI PM example. The compensation is
> correct and necessary, but it was applied inside the timekeeping
> accumulation in timekeeping.c: subtracted in the mult computation in
> timekeeping_adjust() and folded into the ntp_error update in
> logarithmic_accumulation(). That keeps the base rate correct and leaves
> NTP its full symmetric +/-MAXFREQ range rather than +373/-627 PPM, but
> the NTP code in ntp.c never sees it: tick_length is computed without the
> correction, so ntp.c's notion of how long a tick is disagrees with the
> rate timekeeping actually produces.
>
> Make the offset an explicit part of the NTP tick_length instead. Add
> ntp_data::cs_tick_adj, a fixed per-second addend that
> ntp_update_frequency() includes alongside ntp_tick_adj and time_freq.
> tk_setup_internals() computes it from the difference between the real
> cycle_interval duration and the nominal interval, stores it in the
> timekeeper, and hands it to NTP through a new argument to ntp_clear() --
> which already recomputes the frequency and is invoked after every
> clocksource (re)configuration. timekeeping_init() now uses TK_UPDATE_ALL
> for this; clearing NTP there is otherwise redundant since ntp_init() has
> just initialised it.
>
> ntp.c now computes the true tick rate, giving a single source of truth.
> Like ntp_tick_adj, cs_tick_adj stays internal to the kernel: userspace
> still sees the nominal 1.000000 ms tick via adjtimex and is unaware of
> the addends. timekeeping_adjust() and logarithmic_accumulation() use
> ntp_tick / xtime_interval directly, and xtime_remainder is removed.
>
> The base-rate arithmetic is unchanged: ntp_tick becomes
> xtime_interval << ntp_error_shift, so the mult division yields the same
> base mult and the ntp_error accumulation still nets to zero per tick.
>
> Beyond the cleanup of treating all the tick_length contributions
> (nominal interval, ntp_tick_adj, cs_tick_adj, time_freq) consistently
> as addends in one place, it also prepares for feed-forward discipline:
> a future timekeeping_set_reference() will set tick_length to track an
> absolute external reference such as a vmclock, and that path needs
> ntp.c to own a tick_length that already reflects the clocksource
> quantisation, with no hidden correction applied elsewhere.
>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> Assisted-by: Kiro:claude-opus-4.8
Looks good! Thanks for the laborious iteations here, I appreciate your
work on this!
Acked-by: John Stultz <jstultz@google.com>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP
2026-06-22 17:52 ` John Stultz
@ 2026-07-07 19:27 ` David Woodhouse
0 siblings, 0 replies; 30+ messages in thread
From: David Woodhouse @ 2026-07-07 19:27 UTC (permalink / raw)
To: John Stultz
Cc: Richard Cochran, Wen Gu, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Thomas Gleixner,
Stephen Boyd, Anna-Maria Behnsen, Frederic Weisbecker,
Shuah Khan, Peter Zijlstra, Thomas Weißschuh, Arnd Bergmann,
Miroslav Lichvar, Julien Ridoux, Ryan Luu, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 258 bytes --]
On Mon, 2026-06-22 at 10:52 -0700, John Stultz wrote:
> Looks good! Thanks for the laborious iteations here, I appreciate your
> work on this!
> Acked-by: John Stultz <jstultz@google.com>
Thanks, John.
Thomas, are you happy to pick this series up?
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [tip: timers/core] timekeeping: Account for clocksource tick quantisation via NTP
2026-06-21 21:53 ` [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP David Woodhouse
2026-06-22 17:52 ` John Stultz
@ 2026-07-07 22:22 ` tip-bot2 for David Woodhouse
[not found] ` <CGME20260709125630eucas1p2d1acb4893a1c72dfba5312453c0971f2@eucas1p2.samsung.com>
2026-07-10 7:25 ` [tip: timers/core] " tip-bot2 for David Woodhouse
3 siblings, 0 replies; 30+ messages in thread
From: tip-bot2 for David Woodhouse @ 2026-07-07 22:22 UTC (permalink / raw)
To: linux-tip-commits
Cc: David Woodhouse, Thomas Gleixner, John Stultz, x86, linux-kernel
The following commit has been merged into the timers/core branch of tip:
Commit-ID: e8bf3955dc2f363b3a0badae28ee0f136ac82b9a
Gitweb: https://git.kernel.org/tip/e8bf3955dc2f363b3a0badae28ee0f136ac82b9a
Author: David Woodhouse <dwmw@amazon.co.uk>
AuthorDate: Sun, 21 Jun 2026 22:53:56 +01:00
Committer: Thomas Gleixner <tglx@kernel.org>
CommitterDate: Wed, 08 Jul 2026 00:17:26 +02:00
timekeeping: Account for clocksource tick quantisation via NTP
cycle_interval is an integer number of counter cycles per NTP interval,
so the real time it represents differs from the nominal
NTP_INTERVAL_LENGTH by up to half a counter period. For coarse
clocksources this is significant: the 3.579545 MHz ACPI PM timer at
HZ=1000 rounds 3579.545 cycles up to 3580, making each tick 1.000127 ms
(+127 PPM).
Commit a386b5af8edd ("time: Compensate for rounding on odd-frequency
clocksources") introduced xtime_remainder to compensate for exactly
this, citing the same 127 PPM ACPI PM example. The compensation is
correct and necessary, but it was applied inside the timekeeping
accumulation in timekeeping.c: subtracted in the mult computation in
timekeeping_adjust() and folded into the ntp_error update in
logarithmic_accumulation(). That keeps the base rate correct and leaves
NTP its full symmetric +/-MAXFREQ range rather than +373/-627 PPM, but
the NTP code in ntp.c never sees it: tick_length is computed without the
correction, so ntp.c's notion of how long a tick is disagrees with the
rate timekeeping actually produces.
Make the offset an explicit part of the NTP tick_length instead. Add
ntp_data::cs_tick_adj, a fixed per-second addend that
ntp_update_frequency() includes alongside ntp_tick_adj and time_freq.
tk_setup_internals() computes it from the difference between the real
cycle_interval duration and the nominal interval, stores it in the
timekeeper, and hands it to NTP through a new argument to ntp_clear() --
which already recomputes the frequency and is invoked after every
clocksource (re)configuration. timekeeping_init() now uses TK_UPDATE_ALL
for this; clearing NTP there is otherwise redundant since ntp_init() has
just initialised it.
ntp.c now computes the true tick rate, giving a single source of truth.
Like ntp_tick_adj, cs_tick_adj stays internal to the kernel: userspace
still sees the nominal 1.000000 ms tick via adjtimex and is unaware of
the addends. timekeeping_adjust() and logarithmic_accumulation() use
ntp_tick / xtime_interval directly, and xtime_remainder is removed.
The base-rate arithmetic is unchanged: ntp_tick becomes
xtime_interval << ntp_error_shift, so the mult division yields the same
base mult and the ntp_error accumulation still nets to zero per tick.
Beyond the cleanup of treating all the tick_length contributions
(nominal interval, ntp_tick_adj, cs_tick_adj, time_freq) consistently
as addends in one place, it also prepares for feed-forward discipline:
a future timekeeping_set_reference() will set tick_length to track an
absolute external reference such as a vmclock, and that path needs
ntp.c to own a tick_length that already reflects the clocksource
quantisation, with no hidden correction applied elsewhere.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Assisted-by: Kiro:claude-opus-4.8
Acked-by: John Stultz <jstultz@google.com>
Link: https://patch.msgid.link/20260621220051.1030462-4-dwmw2@infradead.org
---
include/linux/timekeeper_internal.h | 8 +--
kernel/time/ntp.c | 27 +++++++++--
kernel/time/ntp_internal.h | 2 +-
kernel/time/timekeeping.c | 66 ++++++++++++++++++++--------
4 files changed, 78 insertions(+), 25 deletions(-)
diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index 264db7c..9c53f44 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -84,8 +84,6 @@ struct tk_read_base {
* @cycle_interval: Number of clock cycles in one NTP interval
* @xtime_interval: Number of clock shifted nano seconds in one NTP
* interval.
- * @xtime_remainder: Shifted nano seconds left over when rounding
- * @cycle_interval
* @raw_interval: Shifted raw nano seconds accumulated per NTP interval.
* @next_leap_ktime: CLOCK_MONOTONIC time value of a pending leap-second
* @ntp_tick: The ntp_tick_length() value currently being
@@ -99,6 +97,10 @@ struct tk_read_base {
* @ntp_error_shift: Shift conversion between clock shifted nano seconds and
* ntp shifted nano seconds.
* @ntp_err_mult: Multiplication factor for scaled math conversion
+ * @cs_tick_adj: Per-second adjustment handed to NTP via ntp_clear()
+ * accounting for the difference between the nominal
+ * NTP interval and the real time taken by the
+ * clocksource's integer @cycle_interval (upscaled).
* @skip_second_overflow: Flag used to avoid updating NTP twice with same second
* @tai_offset: The current UTC to TAI offset in seconds
*
@@ -178,7 +180,6 @@ struct timekeeper {
u64 cycle_interval;
u64 xtime_interval;
- s64 xtime_remainder;
u64 raw_interval;
ktime_t next_leap_ktime;
@@ -186,6 +187,7 @@ struct timekeeper {
s64 ntp_error;
u32 ntp_error_shift;
u32 ntp_err_mult;
+ s64 cs_tick_adj;
u32 skip_second_overflow;
s32 tai_offset;
};
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 97fa99b..3fad82c 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -39,6 +39,10 @@
* @time_reftime: Time at last adjustment in seconds
* @time_adjust: Adjustment value
* @ntp_tick_adj: Constant boot-param configurable NTP tick adjustment (upscaled)
+ * @cs_tick_adj: Fixed per-second adjustment compensating for the difference
+ * between the nominal NTP interval and the real time taken
+ * by the clocksource's integer @cycle_interval (upscaled).
+ * Set by the timekeeping core via ntp_clear().
* @ntp_next_leap_sec: Second value of the next pending leapsecond, or TIME64_MAX if no leap
*
* @pps_valid: PPS signal watchdog counter
@@ -70,6 +74,7 @@ struct ntp_data {
time64_t time_reftime;
long time_adjust;
s64 ntp_tick_adj;
+ s64 cs_tick_adj;
time64_t ntp_next_leap_sec;
#ifdef CONFIG_NTP_PPS
int pps_valid;
@@ -255,6 +260,7 @@ static void ntp_update_frequency(struct ntp_data *ntpdata)
second_length = (u64)(tick_usec * NSEC_PER_USEC * USER_HZ) << NTP_SCALE_SHIFT;
second_length += ntpdata->ntp_tick_adj;
+ second_length += ntpdata->cs_tick_adj;
second_length += ntpdata->time_freq;
new_base = div_u64(second_length, NTP_INTERVAL_FREQ);
@@ -350,11 +356,26 @@ static void __ntp_clear(struct ntp_data *ntpdata)
}
/**
- * ntp_clear - Clears the NTP state variables
- * @tkid: Timekeeper ID to be able to select proper ntp data array member
+ * ntp_clear - Clear NTP state and set the clocksource quantisation adjustment
+ * @tkid: Timekeeper ID
+ * @cs_tick_adj: Per-second adjustment in ns << NTP_SCALE_SHIFT
+ *
+ * The timekeeping core uses an integer number of cycles (@cycle_interval)
+ * per NTP interval, so the real time that interval represents differs from
+ * the nominal NTP_INTERVAL_LENGTH by up to half a counter period. Folding
+ * this fixed offset into @cs_tick_adj makes it an explicit part of the NTP
+ * tick_length computation in ntp.c, instead of being applied during
+ * timekeeping accumulation where the NTP code never saw it. Like
+ * @ntp_tick_adj it stays internal to the kernel; userspace still sees the
+ * nominal tick via adjtimex. NTP retains its full symmetric ±MAXFREQ range
+ * around the corrected base rate.
+ *
+ * Called whenever the clocksource is (re)configured, which is also when the
+ * rest of the NTP state must be cleared, so the two are done together.
*/
-void ntp_clear(unsigned int tkid)
+void ntp_clear(unsigned int tkid, s64 cs_tick_adj)
{
+ tk_ntp_data[tkid].cs_tick_adj = cs_tick_adj;
__ntp_clear(&tk_ntp_data[tkid]);
}
diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h
index 7084d83..598e5dd 100644
--- a/kernel/time/ntp_internal.h
+++ b/kernel/time/ntp_internal.h
@@ -3,7 +3,7 @@
#define _LINUX_NTP_INTERNAL_H
extern void ntp_init(void);
-extern void ntp_clear(unsigned int tkid);
+extern void ntp_clear(unsigned int tkid, s64 cs_tick_adj);
/* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
extern u64 ntp_tick_length(unsigned int tkid);
extern ktime_t ntp_get_next_leap(unsigned int tkid);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 9e1cbc5..e9aff97 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -339,7 +339,6 @@ static inline void clocksource_enable_inline_read(void) { }
static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
{
u64 interval;
- u64 tmp, ntpinterval;
struct clocksource *old_clock;
++tk->cs_was_changed_seq;
@@ -353,20 +352,16 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
tk->tkr_raw.cycle_last = tk->tkr_mono.cycle_last;
/* Do the ns -> cycle conversion first, using original mult */
- tmp = NTP_INTERVAL_LENGTH;
- tmp <<= clock->shift;
- ntpinterval = tmp;
- tmp += clock->mult/2;
- do_div(tmp, clock->mult);
- if (tmp == 0)
- tmp = 1;
-
- interval = (u64) tmp;
+ interval = NTP_INTERVAL_LENGTH << clock->shift;
+ interval += clock->mult/2;
+ do_div(interval, clock->mult);
+ if (interval == 0)
+ interval = 1;
+
tk->cycle_interval = interval;
/* Go back from cycles -> shifted ns */
tk->xtime_interval = interval * clock->mult;
- tk->xtime_remainder = ntpinterval - tk->xtime_interval;
tk->raw_interval = interval * clock->mult;
/* if changing clocks, convert xtime_nsec shift units */
@@ -386,7 +381,38 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
tk->ntp_error = 0;
tk->ntp_error_shift = NTP_SCALE_SHIFT - clock->shift;
- tk->ntp_tick = ntpinterval << tk->ntp_error_shift;
+
+ /*
+ * ntp_tick is the tick length that NTP disciplines (its ±500 PPM
+ * scales only this part), in NTP-shifted ns: the real interval of
+ * a whole number of counter cycles. Because cycle_interval is
+ * rounded to an integer number of cycles, this ntp_tick differs
+ * from the true intended 1/HZ tick length by up to half a cycle
+ * period.
+ */
+ tk->ntp_tick = (u64)tk->xtime_interval << tk->ntp_error_shift;
+
+ /*
+ * cs_tick_adj is the constant difference between the disciplined
+ * ntp_tick above and the true 1/HZ tick, expressed per-second to
+ * match the ntp_update_frequency() addends and handed to NTP via
+ * ntp_clear() to be explicitly included in its tick_length.
+ *
+ * Worked example: HZ=1000, ACPI PM timer at 3.579545 MHz, which
+ * has 3579.545 cycles in 1ms, rounded to cycle_interval = 3580.
+ *
+ * So ntp_tick is actually 1.000127ms, as that is the amount of
+ * time that 3580 cycles will take at the nominal frequency. This
+ * is the part that NTP disciplines, causing each 3580 counts to
+ * advance the clock by up to NTP's ±500PPM of that amount.
+ *
+ * The "extra" 127ns/tick is what's stored in cs_tick_adj and
+ * applied as a constant correction by ntp_update_frequency() so
+ * that NTP *believes* it's disciplining a 1ms tick.
+ */
+ tk->cs_tick_adj = (s64)tk->ntp_tick -
+ ((s64)NTP_INTERVAL_LENGTH << NTP_SCALE_SHIFT);
+ tk->cs_tick_adj *= NTP_INTERVAL_FREQ;
/*
* The timekeeper keeps its own mult values for the currently
@@ -803,7 +829,7 @@ static void timekeeping_update_from_shadow(struct tk_data *tkd, unsigned int act
if (action & TK_CLEAR_NTP) {
tk->ntp_error = 0;
- ntp_clear(tk->id);
+ ntp_clear(tk->id, tk->cs_tick_adj);
}
tk_update_leap_state(tk);
@@ -2090,7 +2116,12 @@ void __init timekeeping_init(void)
tk_set_wall_to_mono(tks, wall_to_mono);
- timekeeping_update_from_shadow(&tk_core, TK_CLOCK_WAS_SET);
+ /*
+ * Use TK_UPDATE_ALL so the NTP layer picks up the clocksource's
+ * cs_tick_adj via ntp_clear(). Clearing NTP here is otherwise
+ * redundant as ntp_init() already initialised it above.
+ */
+ timekeeping_update_from_shadow(&tk_core, TK_UPDATE_ALL);
}
/* time in seconds when suspend began for persistent clock */
@@ -2439,8 +2470,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);
}
/*
@@ -2565,8 +2596,7 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
/* Accumulate error between NTP and clock interval */
tk->ntp_error += tk->ntp_tick << shift;
- tk->ntp_error -= (tk->xtime_interval + tk->xtime_remainder) <<
- (tk->ntp_error_shift + shift);
+ tk->ntp_error -= tk->xtime_interval << (tk->ntp_error_shift + shift);
return offset;
}
^ permalink raw reply [flat|nested] 30+ messages in thread[parent not found: <CGME20260709125630eucas1p2d1acb4893a1c72dfba5312453c0971f2@eucas1p2.samsung.com>]
* Re: [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP
[not found] ` <CGME20260709125630eucas1p2d1acb4893a1c72dfba5312453c0971f2@eucas1p2.samsung.com>
@ 2026-07-09 12:56 ` Marek Szyprowski
2026-07-09 13:19 ` Arnd Bergmann
` (2 more replies)
0 siblings, 3 replies; 30+ messages in thread
From: Marek Szyprowski @ 2026-07-09 12:56 UTC (permalink / raw)
To: David Woodhouse, Richard Cochran, Wen Gu, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John Stultz, Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Peter Zijlstra,
Thomas Weißschuh, Arnd Bergmann, Miroslav Lichvar,
Julien Ridoux, Ryan Luu, linux-kernel
On 21.06.2026 23:53, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> cycle_interval is an integer number of counter cycles per NTP interval,
> so the real time it represents differs from the nominal
> NTP_INTERVAL_LENGTH by up to half a counter period. For coarse
> clocksources this is significant: the 3.579545 MHz ACPI PM timer at
> HZ=1000 rounds 3579.545 cycles up to 3580, making each tick 1.000127 ms
> (+127 PPM).
>
> Commit a386b5af8edd ("time: Compensate for rounding on odd-frequency
> clocksources") introduced xtime_remainder to compensate for exactly
> this, citing the same 127 PPM ACPI PM example. The compensation is
> correct and necessary, but it was applied inside the timekeeping
> accumulation in timekeeping.c: subtracted in the mult computation in
> timekeeping_adjust() and folded into the ntp_error update in
> logarithmic_accumulation(). That keeps the base rate correct and leaves
> NTP its full symmetric +/-MAXFREQ range rather than +373/-627 PPM, but
> the NTP code in ntp.c never sees it: tick_length is computed without the
> correction, so ntp.c's notion of how long a tick is disagrees with the
> rate timekeeping actually produces.
>
> Make the offset an explicit part of the NTP tick_length instead. Add
> ntp_data::cs_tick_adj, a fixed per-second addend that
> ntp_update_frequency() includes alongside ntp_tick_adj and time_freq.
> tk_setup_internals() computes it from the difference between the real
> cycle_interval duration and the nominal interval, stores it in the
> timekeeper, and hands it to NTP through a new argument to ntp_clear() --
> which already recomputes the frequency and is invoked after every
> clocksource (re)configuration. timekeeping_init() now uses TK_UPDATE_ALL
> for this; clearing NTP there is otherwise redundant since ntp_init() has
> just initialised it.
>
> ntp.c now computes the true tick rate, giving a single source of truth.
> Like ntp_tick_adj, cs_tick_adj stays internal to the kernel: userspace
> still sees the nominal 1.000000 ms tick via adjtimex and is unaware of
> the addends. timekeeping_adjust() and logarithmic_accumulation() use
> ntp_tick / xtime_interval directly, and xtime_remainder is removed.
>
> The base-rate arithmetic is unchanged: ntp_tick becomes
> xtime_interval << ntp_error_shift, so the mult division yields the same
> base mult and the ntp_error accumulation still nets to zero per tick.
>
> Beyond the cleanup of treating all the tick_length contributions
> (nominal interval, ntp_tick_adj, cs_tick_adj, time_freq) consistently
> as addends in one place, it also prepares for feed-forward discipline:
> a future timekeeping_set_reference() will set tick_length to track an
> absolute external reference such as a vmclock, and that path needs
> ntp.c to own a tick_length that already reflects the clocksource
> quantisation, with no hidden correction applied elsewhere.
>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> Assisted-by: Kiro:claude-opus-4.8
This patch landed yesterday in linux-next as commit e8bf3955dc2f
("timekeeping: Account for clocksource tick quantisation via NTP").
In my tests I found that it breaks booting Raspberry Pi 3B and Pi 4B
boards in ARM 32bit mode with quite old (Debian 10) userspace. Booting
freezes just before getting a getty login prompt and no more kernel
or userspace messages are observed. If I start with init=/bin/bash in
kernel cmdline and then wait a while and do "exec /sbin/init" I get
the following additional kernel message:
[ 186.867605] Adjusting arch_sys_counter more than 11% (12136 vs 344864995)
then booting freezes again before getting getty prompt.
I can do some more tests, just let me know what will help debugging this
issue. Other ARM 32bit machines I have work fine with this patch.
Reverting subject together with its dependencies on top of linux-next
fixes the issue.
> ---
> include/linux/timekeeper_internal.h | 8 ++--
> kernel/time/ntp.c | 27 ++++++++++--
> kernel/time/ntp_internal.h | 2 +-
> kernel/time/timekeeping.c | 66 +++++++++++++++++++++--------
> 4 files changed, 78 insertions(+), 25 deletions(-)
>
> diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
> index e36d11e33e0c..ec81587a1400 100644
> --- a/include/linux/timekeeper_internal.h
> +++ b/include/linux/timekeeper_internal.h
> @@ -84,8 +84,6 @@ struct tk_read_base {
> * @cycle_interval: Number of clock cycles in one NTP interval
> * @xtime_interval: Number of clock shifted nano seconds in one NTP
> * interval.
> - * @xtime_remainder: Shifted nano seconds left over when rounding
> - * @cycle_interval
> * @raw_interval: Shifted raw nano seconds accumulated per NTP interval.
> * @next_leap_ktime: CLOCK_MONOTONIC time value of a pending leap-second
> * @ntp_tick: The ntp_tick_length() value currently being
> @@ -99,6 +97,10 @@ struct tk_read_base {
> * @ntp_error_shift: Shift conversion between clock shifted nano seconds and
> * ntp shifted nano seconds.
> * @ntp_err_mult: Multiplication factor for scaled math conversion
> + * @cs_tick_adj: Per-second adjustment handed to NTP via ntp_clear()
> + * accounting for the difference between the nominal
> + * NTP interval and the real time taken by the
> + * clocksource's integer @cycle_interval (upscaled).
> * @skip_second_overflow: Flag used to avoid updating NTP twice with same second
> * @tai_offset: The current UTC to TAI offset in seconds
> *
> @@ -178,7 +180,6 @@ struct timekeeper {
>
> u64 cycle_interval;
> u64 xtime_interval;
> - s64 xtime_remainder;
> u64 raw_interval;
>
> ktime_t next_leap_ktime;
> @@ -186,6 +187,7 @@ struct timekeeper {
> s64 ntp_error;
> u32 ntp_error_shift;
> u32 ntp_err_mult;
> + s64 cs_tick_adj;
> u32 skip_second_overflow;
> s32 tai_offset;
> };
> diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
> index 97fa99b96dd0..3fad82c47c4c 100644
> --- a/kernel/time/ntp.c
> +++ b/kernel/time/ntp.c
> @@ -39,6 +39,10 @@
> * @time_reftime: Time at last adjustment in seconds
> * @time_adjust: Adjustment value
> * @ntp_tick_adj: Constant boot-param configurable NTP tick adjustment (upscaled)
> + * @cs_tick_adj: Fixed per-second adjustment compensating for the difference
> + * between the nominal NTP interval and the real time taken
> + * by the clocksource's integer @cycle_interval (upscaled).
> + * Set by the timekeeping core via ntp_clear().
> * @ntp_next_leap_sec: Second value of the next pending leapsecond, or TIME64_MAX if no leap
> *
> * @pps_valid: PPS signal watchdog counter
> @@ -70,6 +74,7 @@ struct ntp_data {
> time64_t time_reftime;
> long time_adjust;
> s64 ntp_tick_adj;
> + s64 cs_tick_adj;
> time64_t ntp_next_leap_sec;
> #ifdef CONFIG_NTP_PPS
> int pps_valid;
> @@ -255,6 +260,7 @@ static void ntp_update_frequency(struct ntp_data *ntpdata)
> second_length = (u64)(tick_usec * NSEC_PER_USEC * USER_HZ) << NTP_SCALE_SHIFT;
>
> second_length += ntpdata->ntp_tick_adj;
> + second_length += ntpdata->cs_tick_adj;
> second_length += ntpdata->time_freq;
>
> new_base = div_u64(second_length, NTP_INTERVAL_FREQ);
> @@ -350,11 +356,26 @@ static void __ntp_clear(struct ntp_data *ntpdata)
> }
>
> /**
> - * ntp_clear - Clears the NTP state variables
> - * @tkid: Timekeeper ID to be able to select proper ntp data array member
> + * ntp_clear - Clear NTP state and set the clocksource quantisation adjustment
> + * @tkid: Timekeeper ID
> + * @cs_tick_adj: Per-second adjustment in ns << NTP_SCALE_SHIFT
> + *
> + * The timekeeping core uses an integer number of cycles (@cycle_interval)
> + * per NTP interval, so the real time that interval represents differs from
> + * the nominal NTP_INTERVAL_LENGTH by up to half a counter period. Folding
> + * this fixed offset into @cs_tick_adj makes it an explicit part of the NTP
> + * tick_length computation in ntp.c, instead of being applied during
> + * timekeeping accumulation where the NTP code never saw it. Like
> + * @ntp_tick_adj it stays internal to the kernel; userspace still sees the
> + * nominal tick via adjtimex. NTP retains its full symmetric ±MAXFREQ range
> + * around the corrected base rate.
> + *
> + * Called whenever the clocksource is (re)configured, which is also when the
> + * rest of the NTP state must be cleared, so the two are done together.
> */
> -void ntp_clear(unsigned int tkid)
> +void ntp_clear(unsigned int tkid, s64 cs_tick_adj)
> {
> + tk_ntp_data[tkid].cs_tick_adj = cs_tick_adj;
> __ntp_clear(&tk_ntp_data[tkid]);
> }
>
> diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h
> index 7084d839c207..598e5dd2fc5b 100644
> --- a/kernel/time/ntp_internal.h
> +++ b/kernel/time/ntp_internal.h
> @@ -3,7 +3,7 @@
> #define _LINUX_NTP_INTERNAL_H
>
> extern void ntp_init(void);
> -extern void ntp_clear(unsigned int tkid);
> +extern void ntp_clear(unsigned int tkid, s64 cs_tick_adj);
> /* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
> extern u64 ntp_tick_length(unsigned int tkid);
> extern ktime_t ntp_get_next_leap(unsigned int tkid);
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index d847bba0481b..53961a1fcf47 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -339,7 +339,6 @@ static inline void clocksource_enable_inline_read(void) { }
> static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
> {
> u64 interval;
> - u64 tmp, ntpinterval;
> struct clocksource *old_clock;
>
> ++tk->cs_was_changed_seq;
> @@ -353,20 +352,16 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
> tk->tkr_raw.cycle_last = tk->tkr_mono.cycle_last;
>
> /* Do the ns -> cycle conversion first, using original mult */
> - tmp = NTP_INTERVAL_LENGTH;
> - tmp <<= clock->shift;
> - ntpinterval = tmp;
> - tmp += clock->mult/2;
> - do_div(tmp, clock->mult);
> - if (tmp == 0)
> - tmp = 1;
> -
> - interval = (u64) tmp;
> + interval = NTP_INTERVAL_LENGTH << clock->shift;
> + interval += clock->mult/2;
> + do_div(interval, clock->mult);
> + if (interval == 0)
> + interval = 1;
> +
> tk->cycle_interval = interval;
>
> /* Go back from cycles -> shifted ns */
> tk->xtime_interval = interval * clock->mult;
> - tk->xtime_remainder = ntpinterval - tk->xtime_interval;
> tk->raw_interval = interval * clock->mult;
>
> /* if changing clocks, convert xtime_nsec shift units */
> @@ -386,7 +381,38 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
>
> tk->ntp_error = 0;
> tk->ntp_error_shift = NTP_SCALE_SHIFT - clock->shift;
> - tk->ntp_tick = ntpinterval << tk->ntp_error_shift;
> +
> + /*
> + * ntp_tick is the tick length that NTP disciplines (its ±500 PPM
> + * scales only this part), in NTP-shifted ns: the real interval of
> + * a whole number of counter cycles. Because cycle_interval is
> + * rounded to an integer number of cycles, this ntp_tick differs
> + * from the true intended 1/HZ tick length by up to half a cycle
> + * period.
> + */
> + tk->ntp_tick = (u64)tk->xtime_interval << tk->ntp_error_shift;
> +
> + /*
> + * cs_tick_adj is the constant difference between the disciplined
> + * ntp_tick above and the true 1/HZ tick, expressed per-second to
> + * match the ntp_update_frequency() addends and handed to NTP via
> + * ntp_clear() to be explicitly included in its tick_length.
> + *
> + * Worked example: HZ=1000, ACPI PM timer at 3.579545 MHz, which
> + * has 3579.545 cycles in 1ms, rounded to cycle_interval = 3580.
> + *
> + * So ntp_tick is actually 1.000127ms, as that is the amount of
> + * time that 3580 cycles will take at the nominal frequency. This
> + * is the part that NTP disciplines, causing each 3580 counts to
> + * advance the clock by up to NTP's ±500PPM of that amount.
> + *
> + * The "extra" 127ns/tick is what's stored in cs_tick_adj and
> + * applied as a constant correction by ntp_update_frequency() so
> + * that NTP *believes* it's disciplining a 1ms tick.
> + */
> + tk->cs_tick_adj = (s64)tk->ntp_tick -
> + ((s64)NTP_INTERVAL_LENGTH << NTP_SCALE_SHIFT);
> + tk->cs_tick_adj *= NTP_INTERVAL_FREQ;
>
> /*
> * The timekeeper keeps its own mult values for the currently
> @@ -803,7 +829,7 @@ static void timekeeping_update_from_shadow(struct tk_data *tkd, unsigned int act
>
> if (action & TK_CLEAR_NTP) {
> tk->ntp_error = 0;
> - ntp_clear(tk->id);
> + ntp_clear(tk->id, tk->cs_tick_adj);
> }
>
> tk_update_leap_state(tk);
> @@ -2075,7 +2101,12 @@ void __init timekeeping_init(void)
>
> tk_set_wall_to_mono(tks, wall_to_mono);
>
> - timekeeping_update_from_shadow(&tk_core, TK_CLOCK_WAS_SET);
> + /*
> + * Use TK_UPDATE_ALL so the NTP layer picks up the clocksource's
> + * cs_tick_adj via ntp_clear(). Clearing NTP here is otherwise
> + * redundant as ntp_init() already initialised it above.
> + */
> + timekeeping_update_from_shadow(&tk_core, TK_UPDATE_ALL);
> }
>
> /* time in seconds when suspend began for persistent clock */
> @@ -2424,8 +2455,8 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
> mult = tk->tkr_mono.mult - tk->ntp_err_mult;
> } else {
> tk->ntp_tick = ntp_tl;
> - mult = div64_u64((tk->ntp_tick >> tk->ntp_error_shift) -
> - tk->xtime_remainder, tk->cycle_interval);
> + mult = div64_u64(tk->ntp_tick >> tk->ntp_error_shift,
> + tk->cycle_interval);
> }
>
> /*
> @@ -2550,8 +2581,7 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
>
> /* Accumulate error between NTP and clock interval */
> tk->ntp_error += tk->ntp_tick << shift;
> - tk->ntp_error -= (tk->xtime_interval + tk->xtime_remainder) <<
> - (tk->ntp_error_shift + shift);
> + tk->ntp_error -= tk->xtime_interval << (tk->ntp_error_shift + shift);
>
> return offset;
> }
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP
2026-07-09 12:56 ` [PATCH v7 3/7] " Marek Szyprowski
@ 2026-07-09 13:19 ` Arnd Bergmann
2026-07-09 14:10 ` Marek Szyprowski
2026-07-09 13:38 ` David Woodhouse
2026-07-09 16:08 ` David Woodhouse
2 siblings, 1 reply; 30+ messages in thread
From: Arnd Bergmann @ 2026-07-09 13:19 UTC (permalink / raw)
To: Marek Szyprowski, David Woodhouse, Richard Cochran, Wen Gu,
Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, John Stultz, Thomas Gleixner, Stephen Boyd,
Anna-Maria Gleixner, Frederic Weisbecker, shuah, Peter Zijlstra,
Thomas Weißschuh, Miroslav Lichvar, Julien Ridoux, Ryan Luu,
linux-kernel
On Thu, Jul 9, 2026, at 14:56, Marek Szyprowski wrote:
> On 21.06.2026 23:53, David Woodhouse wrote:
>
> This patch landed yesterday in linux-next as commit e8bf3955dc2f
> ("timekeeping: Account for clocksource tick quantisation via NTP").
> In my tests I found that it breaks booting Raspberry Pi 3B and Pi 4B
> boards in ARM 32bit mode with quite old (Debian 10) userspace.
Hi Marek,
Are you running a 32-bit or 64-bit kernel with your 32-bit userspace?
> Booting
> freezes just before getting a getty login prompt and no more kernel
> or userspace messages are observed. If I start with init=/bin/bash in
> kernel cmdline and then wait a while and do "exec /sbin/init" I get
> the following additional kernel message:
>
> [ 186.867605] Adjusting arch_sys_counter more than 11% (12136 vs 344864995)
>
> then booting freezes again before getting getty prompt.
>
> I can do some more tests, just let me know what will help debugging this
> issue. Other ARM 32bit machines I have work fine with this patch.
>
> Reverting subject together with its dependencies on top of linux-next
> fixes the issue.
IIRC, there are some variants of these machines with broken
arch timer. Can you check on the working kernel whether
the vdso clock_gettime64 helper is enabled, and whether the
"arm,cpu-registers-not-fw-configured" property is set on
the arm,armv7-timer node?
If it's not that, the difference between the working and
non-working systems could be the clock frequency behind
the arch timer, which differs wildly between systems,
you should see a "cp15 timer running at ... MHz" message
that tells you what is is
Arnd
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP
2026-07-09 13:19 ` Arnd Bergmann
@ 2026-07-09 14:10 ` Marek Szyprowski
0 siblings, 0 replies; 30+ messages in thread
From: Marek Szyprowski @ 2026-07-09 14:10 UTC (permalink / raw)
To: Arnd Bergmann, David Woodhouse, Richard Cochran, Wen Gu,
Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, John Stultz, Thomas Gleixner, Stephen Boyd,
Anna-Maria Gleixner, Frederic Weisbecker, shuah, Peter Zijlstra,
Thomas Weißschuh, Miroslav Lichvar, Julien Ridoux, Ryan Luu,
linux-kernel
On 09.07.2026 15:19, Arnd Bergmann wrote:
> On Thu, Jul 9, 2026, at 14:56, Marek Szyprowski wrote:
>> On 21.06.2026 23:53, David Woodhouse wrote:
>>
>> This patch landed yesterday in linux-next as commit e8bf3955dc2f
>> ("timekeeping: Account for clocksource tick quantisation via NTP").
>> In my tests I found that it breaks booting Raspberry Pi 3B and Pi 4B
>> boards in ARM 32bit mode with quite old (Debian 10) userspace.
> Are you running a 32-bit or 64-bit kernel with your 32-bit userspace?
32bit kernel and 32bit userspace.
>> Booting
>> freezes just before getting a getty login prompt and no more kernel
>> or userspace messages are observed. If I start with init=/bin/bash in
>> kernel cmdline and then wait a while and do "exec /sbin/init" I get
>> the following additional kernel message:
>>
>> [ 186.867605] Adjusting arch_sys_counter more than 11% (12136 vs 344864995)
>>
>> then booting freezes again before getting getty prompt.
>>
>> I can do some more tests, just let me know what will help debugging this
>> issue. Other ARM 32bit machines I have work fine with this patch.
>>
>> Reverting subject together with its dependencies on top of linux-next
>> fixes the issue.
> IIRC, there are some variants of these machines with broken
> arch timer. Can you check on the working kernel whether
> the vdso clock_gettime64 helper is enabled, and whether the
> "arm,cpu-registers-not-fw-configured" property is set on
> the arm,armv7-timer node?
It is not set. I use vanilla dtb from mainline.
> If it's not that, the difference between the working and
> non-working systems could be the clock frequency behind
> the arch timer, which differs wildly between systems,
> you should see a "cp15 timer running at ... MHz" message
> that tells you what is is
I use multi_v7_defconfig from arch/arm, it has CONFIG_VDSO=y.
Relevant lines from kernel log (RPi4 board, $subject patch reverted):
[ 0.000000] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
[ 0.000001] sched_clock: 32 bits at 1000kHz, resolution 1000ns, wraps every 2147483647500ns
[ 0.008453] clocksource: timer: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 1911260446275 ns
[ 0.017911] bcm2835: system timer (irq = 25)
[ 0.025858] arch_timer: cp15 timer running at 54.00MHz (sec-phys).
[ 0.032113] clocksource: arch_sys_counter: mask: 0xffffffffffffff max_cycles: 0xc743ce346, max_idle_ns: 440795203123 ns
[ 0.043023] sched_clock: 56 bits at 54MHz, resolution 18ns, wraps every 4398046511102ns
[ 0.051121] Switching to timer-based delay loop, resolution 18ns
root@target:~# grep -a "" /sys/firmware/devicetree/base/timer/compatible
arm,armv8-timer
root@target:~# ls -la /sys/firmware/devicetree/base/timer/
total 0
drwxr-xr-x 2 root root 0 Jan 1 00:00 .
drwxr-xr-x 27 root root 0 Jan 1 00:00 ..
-r--r--r-- 1 root root 16 Jan 1 00:00 compatible
-r--r--r-- 1 root root 48 Jan 1 00:00 interrupts
-r--r--r-- 1 root root 6 Jan 1 00:00 name
With CONFIG_VDSO disabled same issue happens.
Adding "arm,cpu-registers-not-fw-configured" property to arch_timer
doesn't change anything.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP
2026-07-09 12:56 ` [PATCH v7 3/7] " Marek Szyprowski
2026-07-09 13:19 ` Arnd Bergmann
@ 2026-07-09 13:38 ` David Woodhouse
2026-07-09 14:42 ` Marek Szyprowski
2026-07-09 16:08 ` David Woodhouse
2 siblings, 1 reply; 30+ messages in thread
From: David Woodhouse @ 2026-07-09 13:38 UTC (permalink / raw)
To: Marek Szyprowski, Richard Cochran, Wen Gu, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John Stultz, Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Peter Zijlstra,
Thomas Weißschuh, Arnd Bergmann, Miroslav Lichvar,
Julien Ridoux, Ryan Luu, linux-kernel
On Thu, 2026-07-09 at 14:56 +0200, Marek Szyprowski wrote:
>
> This patch landed yesterday in linux-next as commit e8bf3955dc2f
> ("timekeeping: Account for clocksource tick quantisation via NTP").
> In my tests I found that it breaks booting Raspberry Pi 3B and Pi 4B
> boards in ARM 32bit mode with quite old (Debian 10) userspace. Booting
> freezes just before getting a getty login prompt and no more kernel
> or userspace messages are observed. If I start with init=/bin/bash in
> kernel cmdline and then wait a while and do "exec /sbin/init" I get
> the following additional kernel message:
>
> [ 186.867605] Adjusting arch_sys_counter more than 11% (12136 vs 344864995)
>
> then booting freezes again before getting getty prompt.
>
> I can do some more tests, just let me know what will help debugging this
> issue. Other ARM 32bit machines I have work fine with this patch.
>
> Reverting subject together with its dependencies on top of linux-next
> fixes the issue.
Thank you.
I'm confused at this, because it's basically adding the *same* delta to
the tick, just in a different place.
While I go and frown at the maths a bit more... would you mind testing
if *just* the change from TK_CLOCK_WAS_SET to TK_UPDATE_ALL in
timekeeping_init() is the culprit?
Could you add some pr_info() of xtime_remainder and cs_tick_adj (in the
before vs. after cases)?
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP
2026-07-09 13:38 ` David Woodhouse
@ 2026-07-09 14:42 ` Marek Szyprowski
0 siblings, 0 replies; 30+ messages in thread
From: Marek Szyprowski @ 2026-07-09 14:42 UTC (permalink / raw)
To: David Woodhouse, Richard Cochran, Wen Gu, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John Stultz, Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Peter Zijlstra,
Thomas Weißschuh, Arnd Bergmann, Miroslav Lichvar,
Julien Ridoux, Ryan Luu, linux-kernel
On 09.07.2026 15:38, David Woodhouse wrote:
> On Thu, 2026-07-09 at 14:56 +0200, Marek Szyprowski wrote:
>> This patch landed yesterday in linux-next as commit e8bf3955dc2f
>> ("timekeeping: Account for clocksource tick quantisation via NTP").
>> In my tests I found that it breaks booting Raspberry Pi 3B and Pi 4B
>> boards in ARM 32bit mode with quite old (Debian 10) userspace. Booting
>> freezes just before getting a getty login prompt and no more kernel
>> or userspace messages are observed. If I start with init=/bin/bash in
>> kernel cmdline and then wait a while and do "exec /sbin/init" I get
>> the following additional kernel message:
>>
>> [ 186.867605] Adjusting arch_sys_counter more than 11% (12136 vs 344864995)
>>
>> then booting freezes again before getting getty prompt.
>>
>> I can do some more tests, just let me know what will help debugging this
>> issue. Other ARM 32bit machines I have work fine with this patch.
>>
>> Reverting subject together with its dependencies on top of linux-next
>> fixes the issue.
> Thank you.
>
> I'm confused at this, because it's basically adding the *same* delta to
> the tick, just in a different place.
>
> While I go and frown at the maths a bit more... would you mind testing
> if *just* the change from TK_CLOCK_WAS_SET to TK_UPDATE_ALL in
> timekeeping_init() is the culprit?
>
> Could you add some pr_info() of xtime_remainder and cs_tick_adj (in the
> before vs. after cases)?
I'm not sure if I got it right. Here are the results and diffs I've applied:
before:
[ 0.458729] cs_tick_adj 100000 186a0
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 9e1cbc590988..1aef9434b5a3 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -369,6 +369,8 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
tk->xtime_remainder = ntpinterval - tk->xtime_interval;
tk->raw_interval = interval * clock->mult;
+ pr_info("cs_tick_adj %llu %llx\n", (unsigned long long)tk->xtime_remainder, (unsigned long long)tk->xtime_remainder);
+
/* if changing clocks, convert xtime_nsec shift units */
if (old_clock) {
int shift_change = clock->shift - old_clock->shift;
after:
[ 0.458546] cs_tick_adj 14151718768290390016 c465013da12d6000 [ 0.464393] clocksource: Switched to clocksource arch_sys_counter
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index e9aff9752d55..c93f2e5e72a6 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -414,6 +414,8 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock) ((s64)NTP_INTERVAL_LENGTH << NTP_SCALE_SHIFT); tk->cs_tick_adj *= NTP_INTERVAL_FREQ; + pr_info("cs_tick_adj %llu %llx\n", (unsigned long long)tk->cs_tick_adj, (unsigned long long)tk->cs_tick_adj); + /* * The timekeeper keeps its own mult values for the currently * active clocksource. These value will be adjusted via NTP
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP
2026-07-09 12:56 ` [PATCH v7 3/7] " Marek Szyprowski
2026-07-09 13:19 ` Arnd Bergmann
2026-07-09 13:38 ` David Woodhouse
@ 2026-07-09 16:08 ` David Woodhouse
2026-07-09 16:21 ` Marek Szyprowski
2 siblings, 1 reply; 30+ messages in thread
From: David Woodhouse @ 2026-07-09 16:08 UTC (permalink / raw)
To: Marek Szyprowski, Richard Cochran, Wen Gu, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John Stultz, Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Peter Zijlstra,
Thomas Weißschuh, Arnd Bergmann, Miroslav Lichvar,
Julien Ridoux, Ryan Luu, linux-kernel
On Thu, 2026-07-09 at 14:56 +0200, Marek Szyprowski wrote:
>
> This patch landed yesterday in linux-next as commit e8bf3955dc2f
> ("timekeeping: Account for clocksource tick quantisation via NTP").
> In my tests I found that it breaks booting Raspberry Pi 3B and Pi 4B
> boards in ARM 32bit mode with quite old (Debian 10) userspace. Booting
> freezes just before getting a getty login prompt and no more kernel
> or userspace messages are observed. If I start with init=/bin/bash in
> kernel cmdline and then wait a while and do "exec /sbin/init" I get
> the following additional kernel message:
>
> [ 186.867605] Adjusting arch_sys_counter more than 11% (12136 vs 344864995)
>
> then booting freezes again before getting getty prompt.
>
> I can do some more tests, just let me know what will help debugging this
> issue. Other ARM 32bit machines I have work fine with this patch.
>
> Reverting subject together with its dependencies on top of linux-next
> fixes the issue.
Thanks for the report, and the debugging. Does this fix it?
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -352,7 +352,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
tk->tkr_raw.cycle_last = tk->tkr_mono.cycle_last;
/* Do the ns -> cycle conversion first, using original mult */
- interval = NTP_INTERVAL_LENGTH << clock->shift;
+ interval = (u64)NTP_INTERVAL_LENGTH << clock->shift;
interval += clock->mult/2;
do_div(interval, clock->mult);
if (interval == 0)
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP
2026-07-09 16:08 ` David Woodhouse
@ 2026-07-09 16:21 ` Marek Szyprowski
2026-07-09 16:24 ` David Woodhouse
0 siblings, 1 reply; 30+ messages in thread
From: Marek Szyprowski @ 2026-07-09 16:21 UTC (permalink / raw)
To: David Woodhouse, Richard Cochran, Wen Gu, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John Stultz, Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Peter Zijlstra,
Thomas Weißschuh, Arnd Bergmann, Miroslav Lichvar,
Julien Ridoux, Ryan Luu, linux-kernel
On 09.07.2026 18:08, David Woodhouse wrote:
> On Thu, 2026-07-09 at 14:56 +0200, Marek Szyprowski wrote:
>> This patch landed yesterday in linux-next as commit e8bf3955dc2f
>> ("timekeeping: Account for clocksource tick quantisation via NTP").
>> In my tests I found that it breaks booting Raspberry Pi 3B and Pi 4B
>> boards in ARM 32bit mode with quite old (Debian 10) userspace. Booting
>> freezes just before getting a getty login prompt and no more kernel
>> or userspace messages are observed. If I start with init=/bin/bash in
>> kernel cmdline and then wait a while and do "exec /sbin/init" I get
>> the following additional kernel message:
>>
>> [ 186.867605] Adjusting arch_sys_counter more than 11% (12136 vs 344864995)
>>
>> then booting freezes again before getting getty prompt.
>>
>> I can do some more tests, just let me know what will help debugging this
>> issue. Other ARM 32bit machines I have work fine with this patch.
>>
>> Reverting subject together with its dependencies on top of linux-next
>> fixes the issue.
> Thanks for the report, and the debugging. Does this fix it?
Yes! Feel free to add:
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -352,7 +352,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
> tk->tkr_raw.cycle_last = tk->tkr_mono.cycle_last;
>
> /* Do the ns -> cycle conversion first, using original mult */
> - interval = NTP_INTERVAL_LENGTH << clock->shift;
> + interval = (u64)NTP_INTERVAL_LENGTH << clock->shift;
> interval += clock->mult/2;
> do_div(interval, clock->mult);
> if (interval == 0)
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP
2026-07-09 16:21 ` Marek Szyprowski
@ 2026-07-09 16:24 ` David Woodhouse
0 siblings, 0 replies; 30+ messages in thread
From: David Woodhouse @ 2026-07-09 16:24 UTC (permalink / raw)
To: Marek Szyprowski, Richard Cochran, Wen Gu, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John Stultz, Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Peter Zijlstra,
Thomas Weißschuh, Arnd Bergmann, Miroslav Lichvar,
Julien Ridoux, Ryan Luu, linux-kernel
On Thu, 2026-07-09 at 18:21 +0200, Marek Szyprowski wrote:
>
> Yes! Feel free to add:
>
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Thank you. I guess I could throw a
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
in too, just in case (as I'm going to be away for the next week or so,
starting about now as I hear footsteps approaching the study).
Thomas, do you want an incremental patch or is it OK to fold the one-
liner back into the offending original?
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -352,7 +352,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
> tk->tkr_raw.cycle_last = tk->tkr_mono.cycle_last;
>
> /* Do the ns -> cycle conversion first, using original mult */
> - interval = NTP_INTERVAL_LENGTH << clock->shift;
> + interval = (u64)NTP_INTERVAL_LENGTH << clock->shift;
> interval += clock->mult/2;
> do_div(interval, clock->mult);
> if (interval == 0)
^ permalink raw reply [flat|nested] 30+ messages in thread
* [tip: timers/core] timekeeping: Account for clocksource tick quantisation via NTP
2026-06-21 21:53 ` [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP David Woodhouse
` (2 preceding siblings ...)
[not found] ` <CGME20260709125630eucas1p2d1acb4893a1c72dfba5312453c0971f2@eucas1p2.samsung.com>
@ 2026-07-10 7:25 ` tip-bot2 for David Woodhouse
3 siblings, 0 replies; 30+ messages in thread
From: tip-bot2 for David Woodhouse @ 2026-07-10 7:25 UTC (permalink / raw)
To: linux-tip-commits
Cc: David Woodhouse, Thomas Gleixner, John Stultz, x86, linux-kernel
The following commit has been merged into the timers/core branch of tip:
Commit-ID: 869a55e662a080a5c6618b68ae320231c720eeee
Gitweb: https://git.kernel.org/tip/869a55e662a080a5c6618b68ae320231c720eeee
Author: David Woodhouse <dwmw@amazon.co.uk>
AuthorDate: Sun, 21 Jun 2026 22:53:56 +01:00
Committer: Thomas Gleixner <tglx@kernel.org>
CommitterDate: Fri, 10 Jul 2026 09:20:54 +02:00
timekeeping: Account for clocksource tick quantisation via NTP
cycle_interval is an integer number of counter cycles per NTP interval,
so the real time it represents differs from the nominal
NTP_INTERVAL_LENGTH by up to half a counter period. For coarse
clocksources this is significant: the 3.579545 MHz ACPI PM timer at
HZ=1000 rounds 3579.545 cycles up to 3580, making each tick 1.000127 ms
(+127 PPM).
Commit a386b5af8edd ("time: Compensate for rounding on odd-frequency
clocksources") introduced xtime_remainder to compensate for exactly
this, citing the same 127 PPM ACPI PM example. The compensation is
correct and necessary, but it was applied inside the timekeeping
accumulation in timekeeping.c: subtracted in the mult computation in
timekeeping_adjust() and folded into the ntp_error update in
logarithmic_accumulation(). That keeps the base rate correct and leaves
NTP its full symmetric +/-MAXFREQ range rather than +373/-627 PPM, but
the NTP code in ntp.c never sees it: tick_length is computed without the
correction, so ntp.c's notion of how long a tick is disagrees with the
rate timekeeping actually produces.
Make the offset an explicit part of the NTP tick_length instead. Add
ntp_data::cs_tick_adj, a fixed per-second addend that
ntp_update_frequency() includes alongside ntp_tick_adj and time_freq.
tk_setup_internals() computes it from the difference between the real
cycle_interval duration and the nominal interval, stores it in the
timekeeper, and hands it to NTP through a new argument to ntp_clear() --
which already recomputes the frequency and is invoked after every
clocksource (re)configuration. timekeeping_init() now uses TK_UPDATE_ALL
for this; clearing NTP there is otherwise redundant since ntp_init() has
just initialised it.
ntp.c now computes the true tick rate, giving a single source of truth.
Like ntp_tick_adj, cs_tick_adj stays internal to the kernel: userspace
still sees the nominal 1.000000 ms tick via adjtimex and is unaware of
the addends. timekeeping_adjust() and logarithmic_accumulation() use
ntp_tick / xtime_interval directly, and xtime_remainder is removed.
The base-rate arithmetic is unchanged: ntp_tick becomes
xtime_interval << ntp_error_shift, so the mult division yields the same
base mult and the ntp_error accumulation still nets to zero per tick.
Beyond the cleanup of treating all the tick_length contributions
(nominal interval, ntp_tick_adj, cs_tick_adj, time_freq) consistently
as addends in one place, it also prepares for feed-forward discipline:
a future timekeeping_set_reference() will set tick_length to track an
absolute external reference such as a vmclock, and that path needs
ntp.c to own a tick_length that already reflects the clocksource
quantisation, with no hidden correction applied elsewhere.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Assisted-by: Kiro:claude-opus-4.8
Acked-by: John Stultz <jstultz@google.com>
Link: https://patch.msgid.link/20260621220051.1030462-4-dwmw2@infradead.org
---
include/linux/timekeeper_internal.h | 8 +--
kernel/time/ntp.c | 27 +++++++++--
kernel/time/ntp_internal.h | 2 +-
kernel/time/timekeeping.c | 66 ++++++++++++++++++++--------
4 files changed, 78 insertions(+), 25 deletions(-)
diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index 264db7c..9c53f44 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -84,8 +84,6 @@ struct tk_read_base {
* @cycle_interval: Number of clock cycles in one NTP interval
* @xtime_interval: Number of clock shifted nano seconds in one NTP
* interval.
- * @xtime_remainder: Shifted nano seconds left over when rounding
- * @cycle_interval
* @raw_interval: Shifted raw nano seconds accumulated per NTP interval.
* @next_leap_ktime: CLOCK_MONOTONIC time value of a pending leap-second
* @ntp_tick: The ntp_tick_length() value currently being
@@ -99,6 +97,10 @@ struct tk_read_base {
* @ntp_error_shift: Shift conversion between clock shifted nano seconds and
* ntp shifted nano seconds.
* @ntp_err_mult: Multiplication factor for scaled math conversion
+ * @cs_tick_adj: Per-second adjustment handed to NTP via ntp_clear()
+ * accounting for the difference between the nominal
+ * NTP interval and the real time taken by the
+ * clocksource's integer @cycle_interval (upscaled).
* @skip_second_overflow: Flag used to avoid updating NTP twice with same second
* @tai_offset: The current UTC to TAI offset in seconds
*
@@ -178,7 +180,6 @@ struct timekeeper {
u64 cycle_interval;
u64 xtime_interval;
- s64 xtime_remainder;
u64 raw_interval;
ktime_t next_leap_ktime;
@@ -186,6 +187,7 @@ struct timekeeper {
s64 ntp_error;
u32 ntp_error_shift;
u32 ntp_err_mult;
+ s64 cs_tick_adj;
u32 skip_second_overflow;
s32 tai_offset;
};
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 97fa99b..3fad82c 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -39,6 +39,10 @@
* @time_reftime: Time at last adjustment in seconds
* @time_adjust: Adjustment value
* @ntp_tick_adj: Constant boot-param configurable NTP tick adjustment (upscaled)
+ * @cs_tick_adj: Fixed per-second adjustment compensating for the difference
+ * between the nominal NTP interval and the real time taken
+ * by the clocksource's integer @cycle_interval (upscaled).
+ * Set by the timekeeping core via ntp_clear().
* @ntp_next_leap_sec: Second value of the next pending leapsecond, or TIME64_MAX if no leap
*
* @pps_valid: PPS signal watchdog counter
@@ -70,6 +74,7 @@ struct ntp_data {
time64_t time_reftime;
long time_adjust;
s64 ntp_tick_adj;
+ s64 cs_tick_adj;
time64_t ntp_next_leap_sec;
#ifdef CONFIG_NTP_PPS
int pps_valid;
@@ -255,6 +260,7 @@ static void ntp_update_frequency(struct ntp_data *ntpdata)
second_length = (u64)(tick_usec * NSEC_PER_USEC * USER_HZ) << NTP_SCALE_SHIFT;
second_length += ntpdata->ntp_tick_adj;
+ second_length += ntpdata->cs_tick_adj;
second_length += ntpdata->time_freq;
new_base = div_u64(second_length, NTP_INTERVAL_FREQ);
@@ -350,11 +356,26 @@ static void __ntp_clear(struct ntp_data *ntpdata)
}
/**
- * ntp_clear - Clears the NTP state variables
- * @tkid: Timekeeper ID to be able to select proper ntp data array member
+ * ntp_clear - Clear NTP state and set the clocksource quantisation adjustment
+ * @tkid: Timekeeper ID
+ * @cs_tick_adj: Per-second adjustment in ns << NTP_SCALE_SHIFT
+ *
+ * The timekeeping core uses an integer number of cycles (@cycle_interval)
+ * per NTP interval, so the real time that interval represents differs from
+ * the nominal NTP_INTERVAL_LENGTH by up to half a counter period. Folding
+ * this fixed offset into @cs_tick_adj makes it an explicit part of the NTP
+ * tick_length computation in ntp.c, instead of being applied during
+ * timekeeping accumulation where the NTP code never saw it. Like
+ * @ntp_tick_adj it stays internal to the kernel; userspace still sees the
+ * nominal tick via adjtimex. NTP retains its full symmetric ±MAXFREQ range
+ * around the corrected base rate.
+ *
+ * Called whenever the clocksource is (re)configured, which is also when the
+ * rest of the NTP state must be cleared, so the two are done together.
*/
-void ntp_clear(unsigned int tkid)
+void ntp_clear(unsigned int tkid, s64 cs_tick_adj)
{
+ tk_ntp_data[tkid].cs_tick_adj = cs_tick_adj;
__ntp_clear(&tk_ntp_data[tkid]);
}
diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h
index 7084d83..598e5dd 100644
--- a/kernel/time/ntp_internal.h
+++ b/kernel/time/ntp_internal.h
@@ -3,7 +3,7 @@
#define _LINUX_NTP_INTERNAL_H
extern void ntp_init(void);
-extern void ntp_clear(unsigned int tkid);
+extern void ntp_clear(unsigned int tkid, s64 cs_tick_adj);
/* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
extern u64 ntp_tick_length(unsigned int tkid);
extern ktime_t ntp_get_next_leap(unsigned int tkid);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 9e1cbc5..e5e4cb5 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -339,7 +339,6 @@ static inline void clocksource_enable_inline_read(void) { }
static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
{
u64 interval;
- u64 tmp, ntpinterval;
struct clocksource *old_clock;
++tk->cs_was_changed_seq;
@@ -353,20 +352,16 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
tk->tkr_raw.cycle_last = tk->tkr_mono.cycle_last;
/* Do the ns -> cycle conversion first, using original mult */
- tmp = NTP_INTERVAL_LENGTH;
- tmp <<= clock->shift;
- ntpinterval = tmp;
- tmp += clock->mult/2;
- do_div(tmp, clock->mult);
- if (tmp == 0)
- tmp = 1;
-
- interval = (u64) tmp;
+ interval = (u64)NTP_INTERVAL_LENGTH << clock->shift;
+ interval += clock->mult / 2;
+ do_div(interval, clock->mult);
+ if (interval == 0)
+ interval = 1;
+
tk->cycle_interval = interval;
/* Go back from cycles -> shifted ns */
tk->xtime_interval = interval * clock->mult;
- tk->xtime_remainder = ntpinterval - tk->xtime_interval;
tk->raw_interval = interval * clock->mult;
/* if changing clocks, convert xtime_nsec shift units */
@@ -386,7 +381,38 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
tk->ntp_error = 0;
tk->ntp_error_shift = NTP_SCALE_SHIFT - clock->shift;
- tk->ntp_tick = ntpinterval << tk->ntp_error_shift;
+
+ /*
+ * ntp_tick is the tick length that NTP disciplines (its ±500 PPM
+ * scales only this part), in NTP-shifted ns: the real interval of
+ * a whole number of counter cycles. Because cycle_interval is
+ * rounded to an integer number of cycles, this ntp_tick differs
+ * from the true intended 1/HZ tick length by up to half a cycle
+ * period.
+ */
+ tk->ntp_tick = (u64)tk->xtime_interval << tk->ntp_error_shift;
+
+ /*
+ * cs_tick_adj is the constant difference between the disciplined
+ * ntp_tick above and the true 1/HZ tick, expressed per-second to
+ * match the ntp_update_frequency() addends and handed to NTP via
+ * ntp_clear() to be explicitly included in its tick_length.
+ *
+ * Worked example: HZ=1000, ACPI PM timer at 3.579545 MHz, which
+ * has 3579.545 cycles in 1ms, rounded to cycle_interval = 3580.
+ *
+ * So ntp_tick is actually 1.000127ms, as that is the amount of
+ * time that 3580 cycles will take at the nominal frequency. This
+ * is the part that NTP disciplines, causing each 3580 counts to
+ * advance the clock by up to NTP's ±500PPM of that amount.
+ *
+ * The "extra" 127ns/tick is what's stored in cs_tick_adj and
+ * applied as a constant correction by ntp_update_frequency() so
+ * that NTP *believes* it's disciplining a 1ms tick.
+ */
+ tk->cs_tick_adj = (s64)tk->ntp_tick -
+ ((s64)NTP_INTERVAL_LENGTH << NTP_SCALE_SHIFT);
+ tk->cs_tick_adj *= NTP_INTERVAL_FREQ;
/*
* The timekeeper keeps its own mult values for the currently
@@ -803,7 +829,7 @@ static void timekeeping_update_from_shadow(struct tk_data *tkd, unsigned int act
if (action & TK_CLEAR_NTP) {
tk->ntp_error = 0;
- ntp_clear(tk->id);
+ ntp_clear(tk->id, tk->cs_tick_adj);
}
tk_update_leap_state(tk);
@@ -2090,7 +2116,12 @@ void __init timekeeping_init(void)
tk_set_wall_to_mono(tks, wall_to_mono);
- timekeeping_update_from_shadow(&tk_core, TK_CLOCK_WAS_SET);
+ /*
+ * Use TK_UPDATE_ALL so the NTP layer picks up the clocksource's
+ * cs_tick_adj via ntp_clear(). Clearing NTP here is otherwise
+ * redundant as ntp_init() already initialised it above.
+ */
+ timekeeping_update_from_shadow(&tk_core, TK_UPDATE_ALL);
}
/* time in seconds when suspend began for persistent clock */
@@ -2439,8 +2470,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);
}
/*
@@ -2565,8 +2596,7 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
/* Accumulate error between NTP and clock interval */
tk->ntp_error += tk->ntp_tick << shift;
- tk->ntp_error -= (tk->xtime_interval + tk->xtime_remainder) <<
- (tk->ntp_error_shift + shift);
+ tk->ntp_error -= tk->xtime_interval << (tk->ntp_error_shift + shift);
return offset;
}
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v7 4/7] timekeeping: Drive time_offset skew via per-tick ntp_error transfer
2026-06-21 21:53 [PATCH v7 0/7] timekeeping/ntp: Fix drift tracking precision David Woodhouse
` (2 preceding siblings ...)
2026-06-21 21:53 ` [PATCH v7 3/7] timekeeping: Account for clocksource tick quantisation via NTP David Woodhouse
@ 2026-06-21 21:53 ` David Woodhouse
2026-07-07 22:22 ` [tip: timers/core] " tip-bot2 for David Woodhouse
2026-07-10 7:25 ` tip-bot2 for David Woodhouse
2026-06-21 21:53 ` [PATCH v7 5/7] timekeeping: Drive time_adjust " David Woodhouse
` (2 subsequent siblings)
6 siblings, 2 replies; 30+ messages in thread
From: David Woodhouse @ 2026-06-21 21:53 UTC (permalink / raw)
To: Richard Cochran, Wen Gu, David Woodhouse, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John Stultz, Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Peter Zijlstra,
Thomas Weißschuh, Arnd Bergmann, Miroslav Lichvar,
Julien Ridoux, Ryan Luu, linux-kernel
From: David Woodhouse <dwmw@amazon.co.uk>
Currently, the phase offset of time_offset and time_adjust is delivered
by adjusting tick_length in second_overflow(), and immediately draining
time_offset/time_adjust by the amount that the tick_length adjustment is
*estimated* to cause. This is fairly approximate, in part because it is
not always correct to assume that precisely NTP_INTERVAL_FREQ ticks will
occur between one call to second_overflow() and the next. It could also
over and under-run in the final second of delivery.
Instead of inflating tick_length, transfer the intended skew directly
into ntp_error each tick to achieve the desired rate.
In second_overflow(), calculate skew_delta which is the per-tick slew
rate, in the same units as time_offset: (ns << NTP_SCALE_SHIFT) / HZ.
In logarithmic_accumulation(), drain up to 'skew_delta' time units from
time_offset into ntp_error to drive the overall effective rate. The new
ntp_drain_skew() function returns the amount which is actually 'claimed'
by time_offset (and in a future patch, time_adjust). Any overrun which
is delivered by the changed 'mult' (as described below) but not claimed
by ntp_drain_skew() will remain in ntp_error to be corrected away in
subsequent ticks.
Simply transferring the precise amount from time_offset to ntp_error
would be sufficent to make the time *eventually* converge, however the
skew delivered is limited by the choice of { mult, mult+1 } each tick
and thus the convergence would be extremely slow.
In theory we could inflate ntp_err_mult with the magnitude of ntp_error
in the general case — but that would cause overcorrection in a tickless
kernel. Instead, in timekeeping_adjust(), take skew_delta into account
when calculating 'mult', such that the available {mult, mult+1} choices
bracket the overall effective rate *including* the skew, to avoid the
delta just building up in ntp_error.
The effect is that the inflated 'mult' causes ntp_error to grow because
xtime_interval is (e.g.) longer than the true tick_length. But then the
same delta is removed again as it's drained from time_offset.
This gives behaviour equivalent to the old tick_length += delta approach
but with exact per-tick accounting of the time_offset actually imparted
to the clock, and no overrun.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.8
---
include/linux/timekeeper_internal.h | 1 +
kernel/time/ntp.c | 88 +++++++++++++++++++++++++++--
kernel/time/ntp_internal.h | 2 +
kernel/time/timekeeping.c | 36 ++++++++++--
4 files changed, 118 insertions(+), 9 deletions(-)
diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index ec81587a1400..fb37a736ec1c 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -189,6 +189,7 @@ struct timekeeper {
u32 ntp_err_mult;
s64 cs_tick_adj;
u32 skip_second_overflow;
+ s64 skew_delta;
s32 tai_offset;
};
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 3fad82c47c4c..064e68e7a77c 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -31,6 +31,9 @@
* @time_state: State of the clock synchronization
* @time_status: Clock status bits
* @time_offset: Time adjustment in nanoseconds
+ * @skew_delta: Per-tick phase slew rate for the coming second, in
+ * @time_offset units (shifted-ns / HZ). Set by
+ * second_overflow().
* @time_constant: PLL time constant
* @time_maxerror: Maximum error in microseconds holding the NTP sync distance
* (NTP dispersion + delay / 2)
@@ -67,6 +70,7 @@ struct ntp_data {
int time_state;
int time_status;
s64 time_offset;
+ s64 skew_delta;
long time_constant;
long time_maxerror;
long time_esterror;
@@ -349,6 +353,7 @@ static void __ntp_clear(struct ntp_data *ntpdata)
ntpdata->tick_length = ntpdata->tick_length_base;
ntpdata->time_offset = 0;
+ ntpdata->skew_delta = 0;
ntpdata->ntp_next_leap_sec = TIME64_MAX;
/* Clear PPS state variables */
@@ -385,6 +390,55 @@ u64 ntp_tick_length(unsigned int tkid)
return tk_ntp_data[tkid].tick_length;
}
+s64 ntp_get_skew_delta(unsigned int tkid)
+{
+ return tk_ntp_data[tkid].skew_delta;
+}
+
+/* Sign of @x as +1 or -1 (zero counts as positive; callers pass nonzero). */
+static inline int signof(s64 x)
+{
+ return x < 0 ? -1 : 1;
+}
+
+static s64 ntp_drain_time_offset(unsigned int tkid, s64 amount)
+{
+ struct ntp_data *ntpdata = &tk_ntp_data[tkid];
+
+ /* Only drain if amount and time_offset have the same sign */
+ if (!amount || signof(amount) != signof(ntpdata->time_offset))
+ return amount;
+
+ /* Clamp: don't overshoot zero */
+ if (abs(amount) > abs(ntpdata->time_offset)) {
+ s64 undrained = amount - ntpdata->time_offset;
+
+ ntpdata->time_offset = 0;
+ return undrained;
+ }
+
+ ntpdata->time_offset -= amount;
+ return 0;
+}
+
+/*
+ * Drain one accumulation's worth of intentional skew as it is delivered.
+ *
+ * @amount is the total intentional per-tick skew for this accumulation
+ * (skew_delta << shift), in time_offset units (shifted_ns / HZ). Returns
+ * the amount actually claimed (same ÷HZ units).
+ */
+s64 ntp_drain_skew(unsigned int tkid, s64 amount, unsigned int shift)
+{
+ s64 unclaimed = ntp_drain_time_offset(tkid, amount);
+
+ /*
+ * Return the amount actually drained from the intentional
+ * phase offset in time_offset.
+ */
+ return amount - unclaimed;
+}
+
/**
* ntp_get_next_leap - Returns the next leapsecond in CLOCK_REALTIME ktime_t
* @tkid: Timekeeper ID
@@ -419,7 +473,6 @@ ktime_t ntp_get_next_leap(unsigned int tkid)
int second_overflow(unsigned int tkid, time64_t secs)
{
struct ntp_data *ntpdata = &tk_ntp_data[tkid];
- s64 delta;
int leap = 0;
s32 rem;
@@ -481,13 +534,38 @@ int second_overflow(unsigned int tkid, time64_t secs)
/* Compute the phase adjustment for the next second */
ntpdata->tick_length = ntpdata->tick_length_base;
- delta = ntp_offset_chunk(ntpdata, ntpdata->time_offset);
- ntpdata->time_offset -= delta;
- ntpdata->tick_length += delta;
-
/* Check PPS signal */
pps_dec_valid(ntpdata);
+ /*
+ * Set the per-tick skew rate for the next second. This is in
+ * the same units as time_offset: (ns << NTP_SCALE_SHIFT) / HZ.
+ * If the result is so low that the skew imparted would round
+ * to zero, pass the bare minimum ±1 to ensure that it *does*
+ * actually drain completely to zero. It won't overshoot because
+ * logarithmic_accumulation() only drains what it can from
+ * time_offset and the rest ends up in ntp_error which drives
+ * the selection of 'mult' immediately each tick.
+ */
+ if (ntpdata->time_offset) {
+ s64 off_chunk = ntp_offset_chunk(ntpdata, ntpdata->time_offset);
+
+ /*
+ * Once the exponential chunk rounds to zero, deliver the last
+ * remaining offset this second so it converges to zero instead
+ * of stalling just above it.
+ */
+ if (!off_chunk)
+ off_chunk = ntpdata->time_offset;
+
+ /* Reduce to per-tick, then floor. */
+ ntpdata->skew_delta = div_s64(off_chunk, NTP_INTERVAL_FREQ);
+ if (!ntpdata->skew_delta)
+ ntpdata->skew_delta = signof(off_chunk);
+ } else {
+ ntpdata->skew_delta = 0;
+ }
+
if (!ntpdata->time_adjust)
goto out;
diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h
index 598e5dd2fc5b..0474a761bafc 100644
--- a/kernel/time/ntp_internal.h
+++ b/kernel/time/ntp_internal.h
@@ -6,6 +6,8 @@ extern void ntp_init(void);
extern void ntp_clear(unsigned int tkid, s64 cs_tick_adj);
/* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
extern u64 ntp_tick_length(unsigned int tkid);
+extern s64 ntp_get_skew_delta(unsigned int tkid);
+extern s64 ntp_drain_skew(unsigned int tkid, s64 amount, unsigned int shift);
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 53961a1fcf47..89b417e22990 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -423,6 +423,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
tk->tkr_raw.mult = clock->mult;
tk->ntp_err_mult = 0;
tk->skip_second_overflow = 0;
+ tk->skew_delta = 0;
tk->cs_id = clock->id;
@@ -2445,17 +2446,26 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk,
static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
{
u64 ntp_tl = ntp_tick_length(tk->id);
+ s64 skew = ntp_get_skew_delta(tk->id);
u32 mult;
/*
- * Determine the multiplier from the current NTP tick length.
- * Avoid expensive division when the tick length doesn't change.
+ * Determine the multiplier from the current NTP tick length plus
+ * skew_delta. The skew biases mult so that ±1 dithering can deliver
+ * the time_offset slew rate. Recompute when either changes.
*/
- if (likely(tk->ntp_tick == ntp_tl)) {
+ if (likely(tk->ntp_tick == ntp_tl && tk->skew_delta == skew)) {
+ /* Revert to the base mult rate. */
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->skew_delta = skew;
+ /*
+ * skew_delta is stored pre-divided by HZ (matching time_offset);
+ * scale it back up to the full per-tick rate for the mult bias.
+ */
+ skew *= NTP_INTERVAL_FREQ;
+ mult = div64_u64((tk->ntp_tick + skew) >> tk->ntp_error_shift,
tk->cycle_interval);
}
@@ -2583,6 +2593,24 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
tk->ntp_error += tk->ntp_tick << shift;
tk->ntp_error -= tk->xtime_interval << (tk->ntp_error_shift + shift);
+ /*
+ * When skewing, do so by adjusting ntp_error to impart an extra
+ * target delta into ntp_error per tick, limited to what can be
+ * drained from time_offset to avoid overshoot.
+ *
+ * The base 'mult' value was calculated with the skew taken into
+ * account, such that the per-tick choice of 'mult' vs. 'mult+1'
+ * allows for the desired effective rate and ntp_error does not
+ * grow unbounded.
+ *
+ * Once the full desired phase offset is delivered, any remaining
+ * skew imparted by the adjusted 'mult', accounted above, remains
+ * in ntp_error and will be compensated by the dithering over time.
+ */
+ if (tk->skew_delta)
+ tk->ntp_error += ntp_drain_skew(tk->id, tk->skew_delta << shift,
+ shift) * NTP_INTERVAL_FREQ;
+
return offset;
}
--
2.54.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [tip: timers/core] timekeeping: Drive time_offset skew via per-tick ntp_error transfer
2026-06-21 21:53 ` [PATCH v7 4/7] timekeeping: Drive time_offset skew via per-tick ntp_error transfer David Woodhouse
@ 2026-07-07 22:22 ` tip-bot2 for David Woodhouse
2026-07-10 7:25 ` tip-bot2 for David Woodhouse
1 sibling, 0 replies; 30+ messages in thread
From: tip-bot2 for David Woodhouse @ 2026-07-07 22:22 UTC (permalink / raw)
To: linux-tip-commits; +Cc: David Woodhouse, Thomas Gleixner, x86, linux-kernel
The following commit has been merged into the timers/core branch of tip:
Commit-ID: 5f6668bf5b06fb569950a966443606fe764cbd3a
Gitweb: https://git.kernel.org/tip/5f6668bf5b06fb569950a966443606fe764cbd3a
Author: David Woodhouse <dwmw@amazon.co.uk>
AuthorDate: Sun, 21 Jun 2026 22:53:57 +01:00
Committer: Thomas Gleixner <tglx@kernel.org>
CommitterDate: Wed, 08 Jul 2026 00:17:27 +02:00
timekeeping: Drive time_offset skew via per-tick ntp_error transfer
Currently, the phase offset of time_offset and time_adjust is delivered
by adjusting tick_length in second_overflow(), and immediately draining
time_offset/time_adjust by the amount that the tick_length adjustment is
*estimated* to cause. This is fairly approximate, in part because it is
not always correct to assume that precisely NTP_INTERVAL_FREQ ticks will
occur between one call to second_overflow() and the next. It could also
over and under-run in the final second of delivery.
Instead of inflating tick_length, transfer the intended skew directly
into ntp_error each tick to achieve the desired rate.
In second_overflow(), calculate skew_delta which is the per-tick slew
rate, in the same units as time_offset: (ns << NTP_SCALE_SHIFT) / HZ.
In logarithmic_accumulation(), drain up to 'skew_delta' time units from
time_offset into ntp_error to drive the overall effective rate. The new
ntp_drain_skew() function returns the amount which is actually 'claimed'
by time_offset (and in a future patch, time_adjust). Any overrun which
is delivered by the changed 'mult' (as described below) but not claimed
by ntp_drain_skew() will remain in ntp_error to be corrected away in
subsequent ticks.
Simply transferring the precise amount from time_offset to ntp_error
would be sufficent to make the time *eventually* converge, however the
skew delivered is limited by the choice of { mult, mult+1 } each tick
and thus the convergence would be extremely slow.
In theory we could inflate ntp_err_mult with the magnitude of ntp_error
in the general case — but that would cause overcorrection in a tickless
kernel. Instead, in timekeeping_adjust(), take skew_delta into account
when calculating 'mult', such that the available {mult, mult+1} choices
bracket the overall effective rate *including* the skew, to avoid the
delta just building up in ntp_error.
The effect is that the inflated 'mult' causes ntp_error to grow because
xtime_interval is (e.g.) longer than the true tick_length. But then the
same delta is removed again as it's drained from time_offset.
This gives behaviour equivalent to the old tick_length += delta approach
but with exact per-tick accounting of the time_offset actually imparted
to the clock, and no overrun.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Assisted-by: Kiro:claude-opus-4.8
Link: https://patch.msgid.link/20260621220051.1030462-5-dwmw2@infradead.org
---
include/linux/timekeeper_internal.h | 1 +-
kernel/time/ntp.c | 88 ++++++++++++++++++++++++++--
kernel/time/ntp_internal.h | 2 +-
kernel/time/timekeeping.c | 36 +++++++++--
4 files changed, 118 insertions(+), 9 deletions(-)
diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index 9c53f44..9c198f6 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -189,6 +189,7 @@ struct timekeeper {
u32 ntp_err_mult;
s64 cs_tick_adj;
u32 skip_second_overflow;
+ s64 skew_delta;
s32 tai_offset;
};
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 3fad82c..064e68e 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -31,6 +31,9 @@
* @time_state: State of the clock synchronization
* @time_status: Clock status bits
* @time_offset: Time adjustment in nanoseconds
+ * @skew_delta: Per-tick phase slew rate for the coming second, in
+ * @time_offset units (shifted-ns / HZ). Set by
+ * second_overflow().
* @time_constant: PLL time constant
* @time_maxerror: Maximum error in microseconds holding the NTP sync distance
* (NTP dispersion + delay / 2)
@@ -67,6 +70,7 @@ struct ntp_data {
int time_state;
int time_status;
s64 time_offset;
+ s64 skew_delta;
long time_constant;
long time_maxerror;
long time_esterror;
@@ -349,6 +353,7 @@ static void __ntp_clear(struct ntp_data *ntpdata)
ntpdata->tick_length = ntpdata->tick_length_base;
ntpdata->time_offset = 0;
+ ntpdata->skew_delta = 0;
ntpdata->ntp_next_leap_sec = TIME64_MAX;
/* Clear PPS state variables */
@@ -385,6 +390,55 @@ u64 ntp_tick_length(unsigned int tkid)
return tk_ntp_data[tkid].tick_length;
}
+s64 ntp_get_skew_delta(unsigned int tkid)
+{
+ return tk_ntp_data[tkid].skew_delta;
+}
+
+/* Sign of @x as +1 or -1 (zero counts as positive; callers pass nonzero). */
+static inline int signof(s64 x)
+{
+ return x < 0 ? -1 : 1;
+}
+
+static s64 ntp_drain_time_offset(unsigned int tkid, s64 amount)
+{
+ struct ntp_data *ntpdata = &tk_ntp_data[tkid];
+
+ /* Only drain if amount and time_offset have the same sign */
+ if (!amount || signof(amount) != signof(ntpdata->time_offset))
+ return amount;
+
+ /* Clamp: don't overshoot zero */
+ if (abs(amount) > abs(ntpdata->time_offset)) {
+ s64 undrained = amount - ntpdata->time_offset;
+
+ ntpdata->time_offset = 0;
+ return undrained;
+ }
+
+ ntpdata->time_offset -= amount;
+ return 0;
+}
+
+/*
+ * Drain one accumulation's worth of intentional skew as it is delivered.
+ *
+ * @amount is the total intentional per-tick skew for this accumulation
+ * (skew_delta << shift), in time_offset units (shifted_ns / HZ). Returns
+ * the amount actually claimed (same ÷HZ units).
+ */
+s64 ntp_drain_skew(unsigned int tkid, s64 amount, unsigned int shift)
+{
+ s64 unclaimed = ntp_drain_time_offset(tkid, amount);
+
+ /*
+ * Return the amount actually drained from the intentional
+ * phase offset in time_offset.
+ */
+ return amount - unclaimed;
+}
+
/**
* ntp_get_next_leap - Returns the next leapsecond in CLOCK_REALTIME ktime_t
* @tkid: Timekeeper ID
@@ -419,7 +473,6 @@ ktime_t ntp_get_next_leap(unsigned int tkid)
int second_overflow(unsigned int tkid, time64_t secs)
{
struct ntp_data *ntpdata = &tk_ntp_data[tkid];
- s64 delta;
int leap = 0;
s32 rem;
@@ -481,13 +534,38 @@ int second_overflow(unsigned int tkid, time64_t secs)
/* Compute the phase adjustment for the next second */
ntpdata->tick_length = ntpdata->tick_length_base;
- delta = ntp_offset_chunk(ntpdata, ntpdata->time_offset);
- ntpdata->time_offset -= delta;
- ntpdata->tick_length += delta;
-
/* Check PPS signal */
pps_dec_valid(ntpdata);
+ /*
+ * Set the per-tick skew rate for the next second. This is in
+ * the same units as time_offset: (ns << NTP_SCALE_SHIFT) / HZ.
+ * If the result is so low that the skew imparted would round
+ * to zero, pass the bare minimum ±1 to ensure that it *does*
+ * actually drain completely to zero. It won't overshoot because
+ * logarithmic_accumulation() only drains what it can from
+ * time_offset and the rest ends up in ntp_error which drives
+ * the selection of 'mult' immediately each tick.
+ */
+ if (ntpdata->time_offset) {
+ s64 off_chunk = ntp_offset_chunk(ntpdata, ntpdata->time_offset);
+
+ /*
+ * Once the exponential chunk rounds to zero, deliver the last
+ * remaining offset this second so it converges to zero instead
+ * of stalling just above it.
+ */
+ if (!off_chunk)
+ off_chunk = ntpdata->time_offset;
+
+ /* Reduce to per-tick, then floor. */
+ ntpdata->skew_delta = div_s64(off_chunk, NTP_INTERVAL_FREQ);
+ if (!ntpdata->skew_delta)
+ ntpdata->skew_delta = signof(off_chunk);
+ } else {
+ ntpdata->skew_delta = 0;
+ }
+
if (!ntpdata->time_adjust)
goto out;
diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h
index 598e5dd..0474a76 100644
--- a/kernel/time/ntp_internal.h
+++ b/kernel/time/ntp_internal.h
@@ -6,6 +6,8 @@ extern void ntp_init(void);
extern void ntp_clear(unsigned int tkid, s64 cs_tick_adj);
/* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
extern u64 ntp_tick_length(unsigned int tkid);
+extern s64 ntp_get_skew_delta(unsigned int tkid);
+extern s64 ntp_drain_skew(unsigned int tkid, s64 amount, unsigned int shift);
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 e9aff97..bf96c97 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -423,6 +423,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
tk->tkr_raw.mult = clock->mult;
tk->ntp_err_mult = 0;
tk->skip_second_overflow = 0;
+ tk->skew_delta = 0;
tk->cs_id = clock->id;
@@ -2460,17 +2461,26 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk,
static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
{
u64 ntp_tl = ntp_tick_length(tk->id);
+ s64 skew = ntp_get_skew_delta(tk->id);
u32 mult;
/*
- * Determine the multiplier from the current NTP tick length.
- * Avoid expensive division when the tick length doesn't change.
+ * Determine the multiplier from the current NTP tick length plus
+ * skew_delta. The skew biases mult so that ±1 dithering can deliver
+ * the time_offset slew rate. Recompute when either changes.
*/
- if (likely(tk->ntp_tick == ntp_tl)) {
+ if (likely(tk->ntp_tick == ntp_tl && tk->skew_delta == skew)) {
+ /* Revert to the base mult rate. */
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->skew_delta = skew;
+ /*
+ * skew_delta is stored pre-divided by HZ (matching time_offset);
+ * scale it back up to the full per-tick rate for the mult bias.
+ */
+ skew *= NTP_INTERVAL_FREQ;
+ mult = div64_u64((tk->ntp_tick + skew) >> tk->ntp_error_shift,
tk->cycle_interval);
}
@@ -2598,6 +2608,24 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
tk->ntp_error += tk->ntp_tick << shift;
tk->ntp_error -= tk->xtime_interval << (tk->ntp_error_shift + shift);
+ /*
+ * When skewing, do so by adjusting ntp_error to impart an extra
+ * target delta into ntp_error per tick, limited to what can be
+ * drained from time_offset to avoid overshoot.
+ *
+ * The base 'mult' value was calculated with the skew taken into
+ * account, such that the per-tick choice of 'mult' vs. 'mult+1'
+ * allows for the desired effective rate and ntp_error does not
+ * grow unbounded.
+ *
+ * Once the full desired phase offset is delivered, any remaining
+ * skew imparted by the adjusted 'mult', accounted above, remains
+ * in ntp_error and will be compensated by the dithering over time.
+ */
+ if (tk->skew_delta)
+ tk->ntp_error += ntp_drain_skew(tk->id, tk->skew_delta << shift,
+ shift) * NTP_INTERVAL_FREQ;
+
return offset;
}
^ permalink raw reply [flat|nested] 30+ messages in thread* [tip: timers/core] timekeeping: Drive time_offset skew via per-tick ntp_error transfer
2026-06-21 21:53 ` [PATCH v7 4/7] timekeeping: Drive time_offset skew via per-tick ntp_error transfer David Woodhouse
2026-07-07 22:22 ` [tip: timers/core] " tip-bot2 for David Woodhouse
@ 2026-07-10 7:25 ` tip-bot2 for David Woodhouse
1 sibling, 0 replies; 30+ messages in thread
From: tip-bot2 for David Woodhouse @ 2026-07-10 7:25 UTC (permalink / raw)
To: linux-tip-commits; +Cc: David Woodhouse, Thomas Gleixner, x86, linux-kernel
The following commit has been merged into the timers/core branch of tip:
Commit-ID: d375af58990902e73dd62bd7c049e759bcff92a5
Gitweb: https://git.kernel.org/tip/d375af58990902e73dd62bd7c049e759bcff92a5
Author: David Woodhouse <dwmw@amazon.co.uk>
AuthorDate: Sun, 21 Jun 2026 22:53:57 +01:00
Committer: Thomas Gleixner <tglx@kernel.org>
CommitterDate: Fri, 10 Jul 2026 09:20:54 +02:00
timekeeping: Drive time_offset skew via per-tick ntp_error transfer
Currently, the phase offset of time_offset and time_adjust is delivered
by adjusting tick_length in second_overflow(), and immediately draining
time_offset/time_adjust by the amount that the tick_length adjustment is
*estimated* to cause. This is fairly approximate, in part because it is
not always correct to assume that precisely NTP_INTERVAL_FREQ ticks will
occur between one call to second_overflow() and the next. It could also
over and under-run in the final second of delivery.
Instead of inflating tick_length, transfer the intended skew directly
into ntp_error each tick to achieve the desired rate.
In second_overflow(), calculate skew_delta which is the per-tick slew
rate, in the same units as time_offset: (ns << NTP_SCALE_SHIFT) / HZ.
In logarithmic_accumulation(), drain up to 'skew_delta' time units from
time_offset into ntp_error to drive the overall effective rate. The new
ntp_drain_skew() function returns the amount which is actually 'claimed'
by time_offset (and in a future patch, time_adjust). Any overrun which
is delivered by the changed 'mult' (as described below) but not claimed
by ntp_drain_skew() will remain in ntp_error to be corrected away in
subsequent ticks.
Simply transferring the precise amount from time_offset to ntp_error
would be sufficent to make the time *eventually* converge, however the
skew delivered is limited by the choice of { mult, mult+1 } each tick
and thus the convergence would be extremely slow.
In theory we could inflate ntp_err_mult with the magnitude of ntp_error
in the general case — but that would cause overcorrection in a tickless
kernel. Instead, in timekeeping_adjust(), take skew_delta into account
when calculating 'mult', such that the available {mult, mult+1} choices
bracket the overall effective rate *including* the skew, to avoid the
delta just building up in ntp_error.
The effect is that the inflated 'mult' causes ntp_error to grow because
xtime_interval is (e.g.) longer than the true tick_length. But then the
same delta is removed again as it's drained from time_offset.
This gives behaviour equivalent to the old tick_length += delta approach
but with exact per-tick accounting of the time_offset actually imparted
to the clock, and no overrun.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Assisted-by: Kiro:claude-opus-4.8
Link: https://patch.msgid.link/20260621220051.1030462-5-dwmw2@infradead.org
---
include/linux/timekeeper_internal.h | 1 +-
kernel/time/ntp.c | 88 ++++++++++++++++++++++++++--
kernel/time/ntp_internal.h | 2 +-
kernel/time/timekeeping.c | 36 +++++++++--
4 files changed, 118 insertions(+), 9 deletions(-)
diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index 9c53f44..9c198f6 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -189,6 +189,7 @@ struct timekeeper {
u32 ntp_err_mult;
s64 cs_tick_adj;
u32 skip_second_overflow;
+ s64 skew_delta;
s32 tai_offset;
};
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 3fad82c..064e68e 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -31,6 +31,9 @@
* @time_state: State of the clock synchronization
* @time_status: Clock status bits
* @time_offset: Time adjustment in nanoseconds
+ * @skew_delta: Per-tick phase slew rate for the coming second, in
+ * @time_offset units (shifted-ns / HZ). Set by
+ * second_overflow().
* @time_constant: PLL time constant
* @time_maxerror: Maximum error in microseconds holding the NTP sync distance
* (NTP dispersion + delay / 2)
@@ -67,6 +70,7 @@ struct ntp_data {
int time_state;
int time_status;
s64 time_offset;
+ s64 skew_delta;
long time_constant;
long time_maxerror;
long time_esterror;
@@ -349,6 +353,7 @@ static void __ntp_clear(struct ntp_data *ntpdata)
ntpdata->tick_length = ntpdata->tick_length_base;
ntpdata->time_offset = 0;
+ ntpdata->skew_delta = 0;
ntpdata->ntp_next_leap_sec = TIME64_MAX;
/* Clear PPS state variables */
@@ -385,6 +390,55 @@ u64 ntp_tick_length(unsigned int tkid)
return tk_ntp_data[tkid].tick_length;
}
+s64 ntp_get_skew_delta(unsigned int tkid)
+{
+ return tk_ntp_data[tkid].skew_delta;
+}
+
+/* Sign of @x as +1 or -1 (zero counts as positive; callers pass nonzero). */
+static inline int signof(s64 x)
+{
+ return x < 0 ? -1 : 1;
+}
+
+static s64 ntp_drain_time_offset(unsigned int tkid, s64 amount)
+{
+ struct ntp_data *ntpdata = &tk_ntp_data[tkid];
+
+ /* Only drain if amount and time_offset have the same sign */
+ if (!amount || signof(amount) != signof(ntpdata->time_offset))
+ return amount;
+
+ /* Clamp: don't overshoot zero */
+ if (abs(amount) > abs(ntpdata->time_offset)) {
+ s64 undrained = amount - ntpdata->time_offset;
+
+ ntpdata->time_offset = 0;
+ return undrained;
+ }
+
+ ntpdata->time_offset -= amount;
+ return 0;
+}
+
+/*
+ * Drain one accumulation's worth of intentional skew as it is delivered.
+ *
+ * @amount is the total intentional per-tick skew for this accumulation
+ * (skew_delta << shift), in time_offset units (shifted_ns / HZ). Returns
+ * the amount actually claimed (same ÷HZ units).
+ */
+s64 ntp_drain_skew(unsigned int tkid, s64 amount, unsigned int shift)
+{
+ s64 unclaimed = ntp_drain_time_offset(tkid, amount);
+
+ /*
+ * Return the amount actually drained from the intentional
+ * phase offset in time_offset.
+ */
+ return amount - unclaimed;
+}
+
/**
* ntp_get_next_leap - Returns the next leapsecond in CLOCK_REALTIME ktime_t
* @tkid: Timekeeper ID
@@ -419,7 +473,6 @@ ktime_t ntp_get_next_leap(unsigned int tkid)
int second_overflow(unsigned int tkid, time64_t secs)
{
struct ntp_data *ntpdata = &tk_ntp_data[tkid];
- s64 delta;
int leap = 0;
s32 rem;
@@ -481,13 +534,38 @@ int second_overflow(unsigned int tkid, time64_t secs)
/* Compute the phase adjustment for the next second */
ntpdata->tick_length = ntpdata->tick_length_base;
- delta = ntp_offset_chunk(ntpdata, ntpdata->time_offset);
- ntpdata->time_offset -= delta;
- ntpdata->tick_length += delta;
-
/* Check PPS signal */
pps_dec_valid(ntpdata);
+ /*
+ * Set the per-tick skew rate for the next second. This is in
+ * the same units as time_offset: (ns << NTP_SCALE_SHIFT) / HZ.
+ * If the result is so low that the skew imparted would round
+ * to zero, pass the bare minimum ±1 to ensure that it *does*
+ * actually drain completely to zero. It won't overshoot because
+ * logarithmic_accumulation() only drains what it can from
+ * time_offset and the rest ends up in ntp_error which drives
+ * the selection of 'mult' immediately each tick.
+ */
+ if (ntpdata->time_offset) {
+ s64 off_chunk = ntp_offset_chunk(ntpdata, ntpdata->time_offset);
+
+ /*
+ * Once the exponential chunk rounds to zero, deliver the last
+ * remaining offset this second so it converges to zero instead
+ * of stalling just above it.
+ */
+ if (!off_chunk)
+ off_chunk = ntpdata->time_offset;
+
+ /* Reduce to per-tick, then floor. */
+ ntpdata->skew_delta = div_s64(off_chunk, NTP_INTERVAL_FREQ);
+ if (!ntpdata->skew_delta)
+ ntpdata->skew_delta = signof(off_chunk);
+ } else {
+ ntpdata->skew_delta = 0;
+ }
+
if (!ntpdata->time_adjust)
goto out;
diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h
index 598e5dd..0474a76 100644
--- a/kernel/time/ntp_internal.h
+++ b/kernel/time/ntp_internal.h
@@ -6,6 +6,8 @@ extern void ntp_init(void);
extern void ntp_clear(unsigned int tkid, s64 cs_tick_adj);
/* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
extern u64 ntp_tick_length(unsigned int tkid);
+extern s64 ntp_get_skew_delta(unsigned int tkid);
+extern s64 ntp_drain_skew(unsigned int tkid, s64 amount, unsigned int shift);
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 e5e4cb5..0e646e9 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -423,6 +423,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
tk->tkr_raw.mult = clock->mult;
tk->ntp_err_mult = 0;
tk->skip_second_overflow = 0;
+ tk->skew_delta = 0;
tk->cs_id = clock->id;
@@ -2460,17 +2461,26 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk,
static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
{
u64 ntp_tl = ntp_tick_length(tk->id);
+ s64 skew = ntp_get_skew_delta(tk->id);
u32 mult;
/*
- * Determine the multiplier from the current NTP tick length.
- * Avoid expensive division when the tick length doesn't change.
+ * Determine the multiplier from the current NTP tick length plus
+ * skew_delta. The skew biases mult so that ±1 dithering can deliver
+ * the time_offset slew rate. Recompute when either changes.
*/
- if (likely(tk->ntp_tick == ntp_tl)) {
+ if (likely(tk->ntp_tick == ntp_tl && tk->skew_delta == skew)) {
+ /* Revert to the base mult rate. */
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->skew_delta = skew;
+ /*
+ * skew_delta is stored pre-divided by HZ (matching time_offset);
+ * scale it back up to the full per-tick rate for the mult bias.
+ */
+ skew *= NTP_INTERVAL_FREQ;
+ mult = div64_u64((tk->ntp_tick + skew) >> tk->ntp_error_shift,
tk->cycle_interval);
}
@@ -2598,6 +2608,24 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
tk->ntp_error += tk->ntp_tick << shift;
tk->ntp_error -= tk->xtime_interval << (tk->ntp_error_shift + shift);
+ /*
+ * When skewing, do so by adjusting ntp_error to impart an extra
+ * target delta into ntp_error per tick, limited to what can be
+ * drained from time_offset to avoid overshoot.
+ *
+ * The base 'mult' value was calculated with the skew taken into
+ * account, such that the per-tick choice of 'mult' vs. 'mult+1'
+ * allows for the desired effective rate and ntp_error does not
+ * grow unbounded.
+ *
+ * Once the full desired phase offset is delivered, any remaining
+ * skew imparted by the adjusted 'mult', accounted above, remains
+ * in ntp_error and will be compensated by the dithering over time.
+ */
+ if (tk->skew_delta)
+ tk->ntp_error += ntp_drain_skew(tk->id, tk->skew_delta << shift,
+ shift) * NTP_INTERVAL_FREQ;
+
return offset;
}
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v7 5/7] timekeeping: Drive time_adjust skew via per-tick ntp_error transfer
2026-06-21 21:53 [PATCH v7 0/7] timekeeping/ntp: Fix drift tracking precision David Woodhouse
` (3 preceding siblings ...)
2026-06-21 21:53 ` [PATCH v7 4/7] timekeeping: Drive time_offset skew via per-tick ntp_error transfer David Woodhouse
@ 2026-06-21 21:53 ` David Woodhouse
2026-07-07 22:22 ` [tip: timers/core] " tip-bot2 for David Woodhouse
2026-07-10 7:25 ` tip-bot2 for David Woodhouse
2026-06-21 21:53 ` [PATCH v7 6/7] timekeeping: Settle competing time_offset and time_adjust skew David Woodhouse
2026-06-21 21:54 ` [PATCH v7 7/7] ntp: Remove tick_length_base, use tick_length directly David Woodhouse
6 siblings, 2 replies; 30+ messages in thread
From: David Woodhouse @ 2026-06-21 21:53 UTC (permalink / raw)
To: Richard Cochran, Wen Gu, David Woodhouse, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John Stultz, Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Peter Zijlstra,
Thomas Weißschuh, Arnd Bergmann, Miroslav Lichvar,
Julien Ridoux, Ryan Luu, linux-kernel
From: David Woodhouse <dwmw@amazon.co.uk>
The legacy adjtime() slew (ADJ_OFFSET_SINGLESHOT) was the last user of
tick_length != tick_length_base: it slewed the clock by inflating
tick_length directly, which delivered the correction imprecisely (e.g.
delivering only 4997.5µs when asked for a 5ms skew).
Deliver it accurately through the same per-tick mechanism that is now
used for time_offset, allowing it to contribute to skew_delta and thus
drive the delivery through ntp_error and mult selection.
To allow for accurate accounting, store the sub-microsecond part of
time_adjust is separately, while keeping time_adjust in microseconds
as that's the external API.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.8
---
kernel/time/ntp.c | 136 +++++++++++++++++++++++++++++---------
kernel/time/timekeeping.c | 2 +-
2 files changed, 107 insertions(+), 31 deletions(-)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 064e68e7a77c..20d211b27908 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -41,6 +41,8 @@
* @time_freq: Frequency offset scaled nsecs/secs
* @time_reftime: Time at last adjustment in seconds
* @time_adjust: Adjustment value
+ * @time_adjust_frac: Sub-microsecond remainder of @time_adjust being
+ * delivered, in ns << NTP_SCALE_SHIFT (not divided by HZ).
* @ntp_tick_adj: Constant boot-param configurable NTP tick adjustment (upscaled)
* @cs_tick_adj: Fixed per-second adjustment compensating for the difference
* between the nominal NTP interval and the real time taken
@@ -77,6 +79,7 @@ struct ntp_data {
s64 time_freq;
time64_t time_reftime;
long time_adjust;
+ s64 time_adjust_frac;
s64 ntp_tick_adj;
s64 cs_tick_adj;
time64_t ntp_next_leap_sec;
@@ -110,6 +113,9 @@ static struct ntp_data tk_ntp_data[TIMEKEEPERS_MAX] = {
#define SECS_PER_DAY 86400
#define MAX_TICKADJ 500LL /* usecs */
+/* One microsecond of phase, in plain shifted-ns (ns << NTP_SCALE_SHIFT) */
+#define ONE_US_NS ((s64)NSEC_PER_USEC << NTP_SCALE_SHIFT)
+/* Per-tick MAX_TICKADJ slew, in plain shifted-ns */
#define MAX_TICKADJ_SCALED \
(((MAX_TICKADJ * NSEC_PER_USEC) << NTP_SCALE_SHIFT) / NTP_INTERVAL_FREQ)
#define MAX_TAI_OFFSET 100000
@@ -345,6 +351,7 @@ static void __ntp_clear(struct ntp_data *ntpdata)
{
/* Stop active adjtime() */
ntpdata->time_adjust = 0;
+ ntpdata->time_adjust_frac = 0;
ntpdata->time_status |= STA_UNSYNC;
ntpdata->time_maxerror = NTP_PHASE_LIMIT;
ntpdata->time_esterror = NTP_PHASE_LIMIT;
@@ -421,20 +428,88 @@ static s64 ntp_drain_time_offset(unsigned int tkid, s64 amount)
return 0;
}
+/*
+ * Drain the legacy adjtime() correction (time_adjust) as it is delivered.
+ *
+ * @amount is the total intentional per-tick skew for this accumulation
+ * (skew_delta << shift), in time_offset units (shifted_ns / HZ); it covers
+ * both the exponential time_offset slew and the linear adjtime slew. This
+ * function claims only the adjtime share — capped at the MAX_TICKADJ rate —
+ * and returns the remainder for ntp_drain_time_offset().
+ *
+ * time_adjust is in whole µs. The sub-µs remainder being delivered lives in
+ * time_adjust_frac (plain shifted-ns, i.e. ns << NTP_SCALE_SHIFT -- unlike
+ * time_offset these are NOT pre-divided by HZ); we top it up by borrowing
+ * whole microseconds from time_adjust as the drain consumes it.
+ */
+static s64 ntp_drain_time_adjust(unsigned int tkid, s64 amount, unsigned int shift)
+{
+ struct ntp_data *ntpdata = &tk_ntp_data[tkid];
+ /* Sign reference: time_adjust if any whole us remain, else the drawer */
+ s64 ref = ntpdata->time_adjust ? (s64)ntpdata->time_adjust
+ : ntpdata->time_adjust_frac;
+ s64 deliver, deficit, claimed;
+
+ if (!amount || !ref || signof(amount) != signof(ref))
+ return amount;
+
+ /*
+ * Phase to deliver this accumulation, in plain shifted-ns. The drain
+ * @amount is in ÷HZ units, so multiply by HZ first, then clamp to the
+ * MAX_TICKADJ rate (MAX_TICKADJ_SCALED is the per-tick slew in
+ * shifted-ns). Multiply-then-clamp avoids an s64 divide for the cap.
+ */
+ deliver = min(abs(amount) * NTP_INTERVAL_FREQ,
+ (s64)MAX_TICKADJ_SCALED << shift);
+
+ /* Top up the sub-µs drawer from whole-µs time_adjust as needed */
+ deficit = deliver - abs(ntpdata->time_adjust_frac);
+ if (deficit > 0 && ntpdata->time_adjust) {
+ long borrow = div64_u64(deficit + ONE_US_NS - 1, ONE_US_NS);
+
+ if (ntpdata->time_adjust > 0) {
+ borrow = min(borrow, ntpdata->time_adjust);
+ ntpdata->time_adjust -= borrow;
+ ntpdata->time_adjust_frac += (s64)borrow * ONE_US_NS;
+ } else {
+ /* Clamp without negating time_adjust (UB for LONG_MIN) */
+ if (ntpdata->time_adjust > -borrow)
+ borrow = -ntpdata->time_adjust;
+ ntpdata->time_adjust += borrow;
+ ntpdata->time_adjust_frac -= (s64)borrow * ONE_US_NS;
+ }
+ }
+
+ /* Never deliver more than the drawer holds */
+ deliver = min(deliver, abs(ntpdata->time_adjust_frac));
+ if (ntpdata->time_adjust_frac > 0)
+ ntpdata->time_adjust_frac -= deliver;
+ else
+ ntpdata->time_adjust_frac += deliver;
+
+ /* Return the unclaimed remainder in ÷HZ drain units for time_offset */
+ claimed = div_s64(deliver, NTP_INTERVAL_FREQ);
+ return amount - signof(amount) * claimed;
+}
+
/*
* Drain one accumulation's worth of intentional skew as it is delivered.
*
* @amount is the total intentional per-tick skew for this accumulation
- * (skew_delta << shift), in time_offset units (shifted_ns / HZ). Returns
- * the amount actually claimed (same ÷HZ units).
+ * (skew_delta << shift), in time_offset units (shifted_ns / HZ). The
+ * adjtime() linear share is taken from time_adjust first (capped at the
+ * MAX_TICKADJ rate, hence @shift), then the exponential remainder from
+ * time_offset. Returns the amount actually claimed (same ÷HZ units).
*/
s64 ntp_drain_skew(unsigned int tkid, s64 amount, unsigned int shift)
{
- s64 unclaimed = ntp_drain_time_offset(tkid, amount);
+ s64 unclaimed = ntp_drain_time_adjust(tkid, amount, shift);
+
+ unclaimed = ntp_drain_time_offset(tkid, unclaimed);
/*
* Return the amount actually drained from the intentional
- * phase offset in time_offset.
+ * phase offset in time_offset and/or time_adjust.
*/
return amount - unclaimed;
}
@@ -544,11 +619,13 @@ int second_overflow(unsigned int tkid, time64_t secs)
* to zero, pass the bare minimum ±1 to ensure that it *does*
* actually drain completely to zero. It won't overshoot because
* logarithmic_accumulation() only drains what it can from
- * time_offset and the rest ends up in ntp_error which drives
- * the selection of 'mult' immediately each tick.
+ * time_offset or time_adjust, and the rest ends up in ntp_error
+ * which drives the selection of 'mult' immediately each tick.
*/
- if (ntpdata->time_offset) {
+ if (ntpdata->time_offset || ntpdata->time_adjust ||
+ ntpdata->time_adjust_frac) {
s64 off_chunk = ntp_offset_chunk(ntpdata, ntpdata->time_offset);
+ s64 adj_chunk = 0, net;
/*
* Once the exponential chunk rounds to zero, deliver the last
@@ -558,34 +635,31 @@ int second_overflow(unsigned int tkid, time64_t secs)
if (!off_chunk)
off_chunk = ntpdata->time_offset;
- /* Reduce to per-tick, then floor. */
- ntpdata->skew_delta = div_s64(off_chunk, NTP_INTERVAL_FREQ);
- if (!ntpdata->skew_delta)
- ntpdata->skew_delta = signof(off_chunk);
- } else {
- ntpdata->skew_delta = 0;
- }
+ if (ntpdata->time_adjust || ntpdata->time_adjust_frac) {
+ s64 adj;
- if (!ntpdata->time_adjust)
- goto out;
+ if (ntpdata->time_adjust >= MAX_TICKADJ)
+ adj = MAX_TICKADJ * ONE_US_NS;
+ else if (ntpdata->time_adjust <= -MAX_TICKADJ)
+ adj = -MAX_TICKADJ * ONE_US_NS;
+ else
+ adj = ntpdata->time_adjust * ONE_US_NS +
+ ntpdata->time_adjust_frac;
- if (ntpdata->time_adjust > MAX_TICKADJ) {
- ntpdata->time_adjust -= MAX_TICKADJ;
- ntpdata->tick_length += MAX_TICKADJ_SCALED;
- goto out;
- }
+ adj_chunk = div_s64(adj, NTP_INTERVAL_FREQ);
+ if (!adj_chunk)
+ adj_chunk = signof(ntpdata->time_adjust_frac);
+ }
- if (ntpdata->time_adjust < -MAX_TICKADJ) {
- ntpdata->time_adjust += MAX_TICKADJ;
- ntpdata->tick_length -= MAX_TICKADJ_SCALED;
- goto out;
+ /* Net is what the clock delivers; reduce to per-tick, then floor. */
+ net = off_chunk + adj_chunk;
+ ntpdata->skew_delta = div_s64(net, NTP_INTERVAL_FREQ);
+ if (!ntpdata->skew_delta && net)
+ ntpdata->skew_delta = signof(net);
+ } else {
+ ntpdata->skew_delta = 0;
}
- ntpdata->tick_length += (s64)(ntpdata->time_adjust * NSEC_PER_USEC / NTP_INTERVAL_FREQ)
- << NTP_SCALE_SHIFT;
- ntpdata->time_adjust = 0;
-
-out:
return leap;
}
@@ -878,6 +952,7 @@ int ntp_adjtimex(unsigned int tkid, struct __kernel_timex *txc, const struct tim
if (!(txc->modes & ADJ_OFFSET_READONLY)) {
/* adjtime() is independent from ntp_adjtime() */
ntpdata->time_adjust = txc->offset;
+ ntpdata->time_adjust_frac = 0;
ntp_update_frequency(ntpdata);
audit_ntp_set_old(ad, AUDIT_NTP_ADJUST, save_adjust);
@@ -1119,6 +1194,7 @@ static void hardpps_update_phase(struct ntp_data *ntpdata, long error)
NTP_INTERVAL_FREQ);
/* Cancel running adjtime() */
ntpdata->time_adjust = 0;
+ ntpdata->time_adjust_frac = 0;
}
/* Update jitter */
ntpdata->pps_jitter += (jitter - ntpdata->pps_jitter) >> PPS_INTMIN;
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 89b417e22990..eb94ec99d503 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -2596,7 +2596,7 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
/*
* When skewing, do so by adjusting ntp_error to impart an extra
* target delta into ntp_error per tick, limited to what can be
- * drained from time_offset to avoid overshoot.
+ * drained from time_offset / time_adjust to avoid overshoot.
*
* The base 'mult' value was calculated with the skew taken into
* account, such that the per-tick choice of 'mult' vs. 'mult+1'
--
2.54.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [tip: timers/core] timekeeping: Drive time_adjust skew via per-tick ntp_error transfer
2026-06-21 21:53 ` [PATCH v7 5/7] timekeeping: Drive time_adjust " David Woodhouse
@ 2026-07-07 22:22 ` tip-bot2 for David Woodhouse
2026-07-10 7:25 ` tip-bot2 for David Woodhouse
1 sibling, 0 replies; 30+ messages in thread
From: tip-bot2 for David Woodhouse @ 2026-07-07 22:22 UTC (permalink / raw)
To: linux-tip-commits; +Cc: David Woodhouse, Thomas Gleixner, x86, linux-kernel
The following commit has been merged into the timers/core branch of tip:
Commit-ID: f5adb6779968f22ed0410940faa9d572597a110c
Gitweb: https://git.kernel.org/tip/f5adb6779968f22ed0410940faa9d572597a110c
Author: David Woodhouse <dwmw@amazon.co.uk>
AuthorDate: Sun, 21 Jun 2026 22:53:58 +01:00
Committer: Thomas Gleixner <tglx@kernel.org>
CommitterDate: Wed, 08 Jul 2026 00:17:27 +02:00
timekeeping: Drive time_adjust skew via per-tick ntp_error transfer
The legacy adjtime() slew (ADJ_OFFSET_SINGLESHOT) was the last user of
tick_length != tick_length_base: it slewed the clock by inflating
tick_length directly, which delivered the correction imprecisely (e.g.
delivering only 4997.5µs when asked for a 5ms skew).
Deliver it accurately through the same per-tick mechanism that is now
used for time_offset, allowing it to contribute to skew_delta and thus
drive the delivery through ntp_error and mult selection.
To allow for accurate accounting, store the sub-microsecond part of
time_adjust is separately, while keeping time_adjust in microseconds
as that's the external API.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Assisted-by: Kiro:claude-opus-4.8
Link: https://patch.msgid.link/20260621220051.1030462-6-dwmw2@infradead.org
---
kernel/time/ntp.c | 136 ++++++++++++++++++++++++++++---------
kernel/time/timekeeping.c | 2 +-
2 files changed, 107 insertions(+), 31 deletions(-)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 064e68e..20d211b 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -41,6 +41,8 @@
* @time_freq: Frequency offset scaled nsecs/secs
* @time_reftime: Time at last adjustment in seconds
* @time_adjust: Adjustment value
+ * @time_adjust_frac: Sub-microsecond remainder of @time_adjust being
+ * delivered, in ns << NTP_SCALE_SHIFT (not divided by HZ).
* @ntp_tick_adj: Constant boot-param configurable NTP tick adjustment (upscaled)
* @cs_tick_adj: Fixed per-second adjustment compensating for the difference
* between the nominal NTP interval and the real time taken
@@ -77,6 +79,7 @@ struct ntp_data {
s64 time_freq;
time64_t time_reftime;
long time_adjust;
+ s64 time_adjust_frac;
s64 ntp_tick_adj;
s64 cs_tick_adj;
time64_t ntp_next_leap_sec;
@@ -110,6 +113,9 @@ static struct ntp_data tk_ntp_data[TIMEKEEPERS_MAX] = {
#define SECS_PER_DAY 86400
#define MAX_TICKADJ 500LL /* usecs */
+/* One microsecond of phase, in plain shifted-ns (ns << NTP_SCALE_SHIFT) */
+#define ONE_US_NS ((s64)NSEC_PER_USEC << NTP_SCALE_SHIFT)
+/* Per-tick MAX_TICKADJ slew, in plain shifted-ns */
#define MAX_TICKADJ_SCALED \
(((MAX_TICKADJ * NSEC_PER_USEC) << NTP_SCALE_SHIFT) / NTP_INTERVAL_FREQ)
#define MAX_TAI_OFFSET 100000
@@ -345,6 +351,7 @@ static void __ntp_clear(struct ntp_data *ntpdata)
{
/* Stop active adjtime() */
ntpdata->time_adjust = 0;
+ ntpdata->time_adjust_frac = 0;
ntpdata->time_status |= STA_UNSYNC;
ntpdata->time_maxerror = NTP_PHASE_LIMIT;
ntpdata->time_esterror = NTP_PHASE_LIMIT;
@@ -422,19 +429,87 @@ static s64 ntp_drain_time_offset(unsigned int tkid, s64 amount)
}
/*
+ * Drain the legacy adjtime() correction (time_adjust) as it is delivered.
+ *
+ * @amount is the total intentional per-tick skew for this accumulation
+ * (skew_delta << shift), in time_offset units (shifted_ns / HZ); it covers
+ * both the exponential time_offset slew and the linear adjtime slew. This
+ * function claims only the adjtime share — capped at the MAX_TICKADJ rate —
+ * and returns the remainder for ntp_drain_time_offset().
+ *
+ * time_adjust is in whole µs. The sub-µs remainder being delivered lives in
+ * time_adjust_frac (plain shifted-ns, i.e. ns << NTP_SCALE_SHIFT -- unlike
+ * time_offset these are NOT pre-divided by HZ); we top it up by borrowing
+ * whole microseconds from time_adjust as the drain consumes it.
+ */
+static s64 ntp_drain_time_adjust(unsigned int tkid, s64 amount, unsigned int shift)
+{
+ struct ntp_data *ntpdata = &tk_ntp_data[tkid];
+ /* Sign reference: time_adjust if any whole us remain, else the drawer */
+ s64 ref = ntpdata->time_adjust ? (s64)ntpdata->time_adjust
+ : ntpdata->time_adjust_frac;
+ s64 deliver, deficit, claimed;
+
+ if (!amount || !ref || signof(amount) != signof(ref))
+ return amount;
+
+ /*
+ * Phase to deliver this accumulation, in plain shifted-ns. The drain
+ * @amount is in ÷HZ units, so multiply by HZ first, then clamp to the
+ * MAX_TICKADJ rate (MAX_TICKADJ_SCALED is the per-tick slew in
+ * shifted-ns). Multiply-then-clamp avoids an s64 divide for the cap.
+ */
+ deliver = min(abs(amount) * NTP_INTERVAL_FREQ,
+ (s64)MAX_TICKADJ_SCALED << shift);
+
+ /* Top up the sub-µs drawer from whole-µs time_adjust as needed */
+ deficit = deliver - abs(ntpdata->time_adjust_frac);
+ if (deficit > 0 && ntpdata->time_adjust) {
+ long borrow = div64_u64(deficit + ONE_US_NS - 1, ONE_US_NS);
+
+ if (ntpdata->time_adjust > 0) {
+ borrow = min(borrow, ntpdata->time_adjust);
+ ntpdata->time_adjust -= borrow;
+ ntpdata->time_adjust_frac += (s64)borrow * ONE_US_NS;
+ } else {
+ /* Clamp without negating time_adjust (UB for LONG_MIN) */
+ if (ntpdata->time_adjust > -borrow)
+ borrow = -ntpdata->time_adjust;
+ ntpdata->time_adjust += borrow;
+ ntpdata->time_adjust_frac -= (s64)borrow * ONE_US_NS;
+ }
+ }
+
+ /* Never deliver more than the drawer holds */
+ deliver = min(deliver, abs(ntpdata->time_adjust_frac));
+ if (ntpdata->time_adjust_frac > 0)
+ ntpdata->time_adjust_frac -= deliver;
+ else
+ ntpdata->time_adjust_frac += deliver;
+
+ /* Return the unclaimed remainder in ÷HZ drain units for time_offset */
+ claimed = div_s64(deliver, NTP_INTERVAL_FREQ);
+ return amount - signof(amount) * claimed;
+}
+
+/*
* Drain one accumulation's worth of intentional skew as it is delivered.
*
* @amount is the total intentional per-tick skew for this accumulation
- * (skew_delta << shift), in time_offset units (shifted_ns / HZ). Returns
- * the amount actually claimed (same ÷HZ units).
+ * (skew_delta << shift), in time_offset units (shifted_ns / HZ). The
+ * adjtime() linear share is taken from time_adjust first (capped at the
+ * MAX_TICKADJ rate, hence @shift), then the exponential remainder from
+ * time_offset. Returns the amount actually claimed (same ÷HZ units).
*/
s64 ntp_drain_skew(unsigned int tkid, s64 amount, unsigned int shift)
{
- s64 unclaimed = ntp_drain_time_offset(tkid, amount);
+ s64 unclaimed = ntp_drain_time_adjust(tkid, amount, shift);
+
+ unclaimed = ntp_drain_time_offset(tkid, unclaimed);
/*
* Return the amount actually drained from the intentional
- * phase offset in time_offset.
+ * phase offset in time_offset and/or time_adjust.
*/
return amount - unclaimed;
}
@@ -544,11 +619,13 @@ int second_overflow(unsigned int tkid, time64_t secs)
* to zero, pass the bare minimum ±1 to ensure that it *does*
* actually drain completely to zero. It won't overshoot because
* logarithmic_accumulation() only drains what it can from
- * time_offset and the rest ends up in ntp_error which drives
- * the selection of 'mult' immediately each tick.
+ * time_offset or time_adjust, and the rest ends up in ntp_error
+ * which drives the selection of 'mult' immediately each tick.
*/
- if (ntpdata->time_offset) {
+ if (ntpdata->time_offset || ntpdata->time_adjust ||
+ ntpdata->time_adjust_frac) {
s64 off_chunk = ntp_offset_chunk(ntpdata, ntpdata->time_offset);
+ s64 adj_chunk = 0, net;
/*
* Once the exponential chunk rounds to zero, deliver the last
@@ -558,34 +635,31 @@ int second_overflow(unsigned int tkid, time64_t secs)
if (!off_chunk)
off_chunk = ntpdata->time_offset;
- /* Reduce to per-tick, then floor. */
- ntpdata->skew_delta = div_s64(off_chunk, NTP_INTERVAL_FREQ);
- if (!ntpdata->skew_delta)
- ntpdata->skew_delta = signof(off_chunk);
- } else {
- ntpdata->skew_delta = 0;
- }
+ if (ntpdata->time_adjust || ntpdata->time_adjust_frac) {
+ s64 adj;
- if (!ntpdata->time_adjust)
- goto out;
+ if (ntpdata->time_adjust >= MAX_TICKADJ)
+ adj = MAX_TICKADJ * ONE_US_NS;
+ else if (ntpdata->time_adjust <= -MAX_TICKADJ)
+ adj = -MAX_TICKADJ * ONE_US_NS;
+ else
+ adj = ntpdata->time_adjust * ONE_US_NS +
+ ntpdata->time_adjust_frac;
- if (ntpdata->time_adjust > MAX_TICKADJ) {
- ntpdata->time_adjust -= MAX_TICKADJ;
- ntpdata->tick_length += MAX_TICKADJ_SCALED;
- goto out;
- }
+ adj_chunk = div_s64(adj, NTP_INTERVAL_FREQ);
+ if (!adj_chunk)
+ adj_chunk = signof(ntpdata->time_adjust_frac);
+ }
- if (ntpdata->time_adjust < -MAX_TICKADJ) {
- ntpdata->time_adjust += MAX_TICKADJ;
- ntpdata->tick_length -= MAX_TICKADJ_SCALED;
- goto out;
+ /* Net is what the clock delivers; reduce to per-tick, then floor. */
+ net = off_chunk + adj_chunk;
+ ntpdata->skew_delta = div_s64(net, NTP_INTERVAL_FREQ);
+ if (!ntpdata->skew_delta && net)
+ ntpdata->skew_delta = signof(net);
+ } else {
+ ntpdata->skew_delta = 0;
}
- ntpdata->tick_length += (s64)(ntpdata->time_adjust * NSEC_PER_USEC / NTP_INTERVAL_FREQ)
- << NTP_SCALE_SHIFT;
- ntpdata->time_adjust = 0;
-
-out:
return leap;
}
@@ -878,6 +952,7 @@ int ntp_adjtimex(unsigned int tkid, struct __kernel_timex *txc, const struct tim
if (!(txc->modes & ADJ_OFFSET_READONLY)) {
/* adjtime() is independent from ntp_adjtime() */
ntpdata->time_adjust = txc->offset;
+ ntpdata->time_adjust_frac = 0;
ntp_update_frequency(ntpdata);
audit_ntp_set_old(ad, AUDIT_NTP_ADJUST, save_adjust);
@@ -1119,6 +1194,7 @@ static void hardpps_update_phase(struct ntp_data *ntpdata, long error)
NTP_INTERVAL_FREQ);
/* Cancel running adjtime() */
ntpdata->time_adjust = 0;
+ ntpdata->time_adjust_frac = 0;
}
/* Update jitter */
ntpdata->pps_jitter += (jitter - ntpdata->pps_jitter) >> PPS_INTMIN;
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index bf96c97..6e95bd4 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -2611,7 +2611,7 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
/*
* When skewing, do so by adjusting ntp_error to impart an extra
* target delta into ntp_error per tick, limited to what can be
- * drained from time_offset to avoid overshoot.
+ * drained from time_offset / time_adjust to avoid overshoot.
*
* The base 'mult' value was calculated with the skew taken into
* account, such that the per-tick choice of 'mult' vs. 'mult+1'
^ permalink raw reply [flat|nested] 30+ messages in thread* [tip: timers/core] timekeeping: Drive time_adjust skew via per-tick ntp_error transfer
2026-06-21 21:53 ` [PATCH v7 5/7] timekeeping: Drive time_adjust " David Woodhouse
2026-07-07 22:22 ` [tip: timers/core] " tip-bot2 for David Woodhouse
@ 2026-07-10 7:25 ` tip-bot2 for David Woodhouse
1 sibling, 0 replies; 30+ messages in thread
From: tip-bot2 for David Woodhouse @ 2026-07-10 7:25 UTC (permalink / raw)
To: linux-tip-commits; +Cc: David Woodhouse, Thomas Gleixner, x86, linux-kernel
The following commit has been merged into the timers/core branch of tip:
Commit-ID: 289d1759494f4a97b0ca4565f3eed00964b952c4
Gitweb: https://git.kernel.org/tip/289d1759494f4a97b0ca4565f3eed00964b952c4
Author: David Woodhouse <dwmw@amazon.co.uk>
AuthorDate: Sun, 21 Jun 2026 22:53:58 +01:00
Committer: Thomas Gleixner <tglx@kernel.org>
CommitterDate: Fri, 10 Jul 2026 09:20:54 +02:00
timekeeping: Drive time_adjust skew via per-tick ntp_error transfer
The legacy adjtime() slew (ADJ_OFFSET_SINGLESHOT) was the last user of
tick_length != tick_length_base: it slewed the clock by inflating
tick_length directly, which delivered the correction imprecisely (e.g.
delivering only 4997.5µs when asked for a 5ms skew).
Deliver it accurately through the same per-tick mechanism that is now
used for time_offset, allowing it to contribute to skew_delta and thus
drive the delivery through ntp_error and mult selection.
To allow for accurate accounting, store the sub-microsecond part of
time_adjust is separately, while keeping time_adjust in microseconds
as that's the external API.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Assisted-by: Kiro:claude-opus-4.8
Link: https://patch.msgid.link/20260621220051.1030462-6-dwmw2@infradead.org
---
kernel/time/ntp.c | 136 ++++++++++++++++++++++++++++---------
kernel/time/timekeeping.c | 2 +-
2 files changed, 107 insertions(+), 31 deletions(-)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 064e68e..20d211b 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -41,6 +41,8 @@
* @time_freq: Frequency offset scaled nsecs/secs
* @time_reftime: Time at last adjustment in seconds
* @time_adjust: Adjustment value
+ * @time_adjust_frac: Sub-microsecond remainder of @time_adjust being
+ * delivered, in ns << NTP_SCALE_SHIFT (not divided by HZ).
* @ntp_tick_adj: Constant boot-param configurable NTP tick adjustment (upscaled)
* @cs_tick_adj: Fixed per-second adjustment compensating for the difference
* between the nominal NTP interval and the real time taken
@@ -77,6 +79,7 @@ struct ntp_data {
s64 time_freq;
time64_t time_reftime;
long time_adjust;
+ s64 time_adjust_frac;
s64 ntp_tick_adj;
s64 cs_tick_adj;
time64_t ntp_next_leap_sec;
@@ -110,6 +113,9 @@ static struct ntp_data tk_ntp_data[TIMEKEEPERS_MAX] = {
#define SECS_PER_DAY 86400
#define MAX_TICKADJ 500LL /* usecs */
+/* One microsecond of phase, in plain shifted-ns (ns << NTP_SCALE_SHIFT) */
+#define ONE_US_NS ((s64)NSEC_PER_USEC << NTP_SCALE_SHIFT)
+/* Per-tick MAX_TICKADJ slew, in plain shifted-ns */
#define MAX_TICKADJ_SCALED \
(((MAX_TICKADJ * NSEC_PER_USEC) << NTP_SCALE_SHIFT) / NTP_INTERVAL_FREQ)
#define MAX_TAI_OFFSET 100000
@@ -345,6 +351,7 @@ static void __ntp_clear(struct ntp_data *ntpdata)
{
/* Stop active adjtime() */
ntpdata->time_adjust = 0;
+ ntpdata->time_adjust_frac = 0;
ntpdata->time_status |= STA_UNSYNC;
ntpdata->time_maxerror = NTP_PHASE_LIMIT;
ntpdata->time_esterror = NTP_PHASE_LIMIT;
@@ -422,19 +429,87 @@ static s64 ntp_drain_time_offset(unsigned int tkid, s64 amount)
}
/*
+ * Drain the legacy adjtime() correction (time_adjust) as it is delivered.
+ *
+ * @amount is the total intentional per-tick skew for this accumulation
+ * (skew_delta << shift), in time_offset units (shifted_ns / HZ); it covers
+ * both the exponential time_offset slew and the linear adjtime slew. This
+ * function claims only the adjtime share — capped at the MAX_TICKADJ rate —
+ * and returns the remainder for ntp_drain_time_offset().
+ *
+ * time_adjust is in whole µs. The sub-µs remainder being delivered lives in
+ * time_adjust_frac (plain shifted-ns, i.e. ns << NTP_SCALE_SHIFT -- unlike
+ * time_offset these are NOT pre-divided by HZ); we top it up by borrowing
+ * whole microseconds from time_adjust as the drain consumes it.
+ */
+static s64 ntp_drain_time_adjust(unsigned int tkid, s64 amount, unsigned int shift)
+{
+ struct ntp_data *ntpdata = &tk_ntp_data[tkid];
+ /* Sign reference: time_adjust if any whole us remain, else the drawer */
+ s64 ref = ntpdata->time_adjust ? (s64)ntpdata->time_adjust
+ : ntpdata->time_adjust_frac;
+ s64 deliver, deficit, claimed;
+
+ if (!amount || !ref || signof(amount) != signof(ref))
+ return amount;
+
+ /*
+ * Phase to deliver this accumulation, in plain shifted-ns. The drain
+ * @amount is in ÷HZ units, so multiply by HZ first, then clamp to the
+ * MAX_TICKADJ rate (MAX_TICKADJ_SCALED is the per-tick slew in
+ * shifted-ns). Multiply-then-clamp avoids an s64 divide for the cap.
+ */
+ deliver = min(abs(amount) * NTP_INTERVAL_FREQ,
+ (s64)MAX_TICKADJ_SCALED << shift);
+
+ /* Top up the sub-µs drawer from whole-µs time_adjust as needed */
+ deficit = deliver - abs(ntpdata->time_adjust_frac);
+ if (deficit > 0 && ntpdata->time_adjust) {
+ long borrow = div64_u64(deficit + ONE_US_NS - 1, ONE_US_NS);
+
+ if (ntpdata->time_adjust > 0) {
+ borrow = min(borrow, ntpdata->time_adjust);
+ ntpdata->time_adjust -= borrow;
+ ntpdata->time_adjust_frac += (s64)borrow * ONE_US_NS;
+ } else {
+ /* Clamp without negating time_adjust (UB for LONG_MIN) */
+ if (ntpdata->time_adjust > -borrow)
+ borrow = -ntpdata->time_adjust;
+ ntpdata->time_adjust += borrow;
+ ntpdata->time_adjust_frac -= (s64)borrow * ONE_US_NS;
+ }
+ }
+
+ /* Never deliver more than the drawer holds */
+ deliver = min(deliver, abs(ntpdata->time_adjust_frac));
+ if (ntpdata->time_adjust_frac > 0)
+ ntpdata->time_adjust_frac -= deliver;
+ else
+ ntpdata->time_adjust_frac += deliver;
+
+ /* Return the unclaimed remainder in ÷HZ drain units for time_offset */
+ claimed = div_s64(deliver, NTP_INTERVAL_FREQ);
+ return amount - signof(amount) * claimed;
+}
+
+/*
* Drain one accumulation's worth of intentional skew as it is delivered.
*
* @amount is the total intentional per-tick skew for this accumulation
- * (skew_delta << shift), in time_offset units (shifted_ns / HZ). Returns
- * the amount actually claimed (same ÷HZ units).
+ * (skew_delta << shift), in time_offset units (shifted_ns / HZ). The
+ * adjtime() linear share is taken from time_adjust first (capped at the
+ * MAX_TICKADJ rate, hence @shift), then the exponential remainder from
+ * time_offset. Returns the amount actually claimed (same ÷HZ units).
*/
s64 ntp_drain_skew(unsigned int tkid, s64 amount, unsigned int shift)
{
- s64 unclaimed = ntp_drain_time_offset(tkid, amount);
+ s64 unclaimed = ntp_drain_time_adjust(tkid, amount, shift);
+
+ unclaimed = ntp_drain_time_offset(tkid, unclaimed);
/*
* Return the amount actually drained from the intentional
- * phase offset in time_offset.
+ * phase offset in time_offset and/or time_adjust.
*/
return amount - unclaimed;
}
@@ -544,11 +619,13 @@ int second_overflow(unsigned int tkid, time64_t secs)
* to zero, pass the bare minimum ±1 to ensure that it *does*
* actually drain completely to zero. It won't overshoot because
* logarithmic_accumulation() only drains what it can from
- * time_offset and the rest ends up in ntp_error which drives
- * the selection of 'mult' immediately each tick.
+ * time_offset or time_adjust, and the rest ends up in ntp_error
+ * which drives the selection of 'mult' immediately each tick.
*/
- if (ntpdata->time_offset) {
+ if (ntpdata->time_offset || ntpdata->time_adjust ||
+ ntpdata->time_adjust_frac) {
s64 off_chunk = ntp_offset_chunk(ntpdata, ntpdata->time_offset);
+ s64 adj_chunk = 0, net;
/*
* Once the exponential chunk rounds to zero, deliver the last
@@ -558,34 +635,31 @@ int second_overflow(unsigned int tkid, time64_t secs)
if (!off_chunk)
off_chunk = ntpdata->time_offset;
- /* Reduce to per-tick, then floor. */
- ntpdata->skew_delta = div_s64(off_chunk, NTP_INTERVAL_FREQ);
- if (!ntpdata->skew_delta)
- ntpdata->skew_delta = signof(off_chunk);
- } else {
- ntpdata->skew_delta = 0;
- }
+ if (ntpdata->time_adjust || ntpdata->time_adjust_frac) {
+ s64 adj;
- if (!ntpdata->time_adjust)
- goto out;
+ if (ntpdata->time_adjust >= MAX_TICKADJ)
+ adj = MAX_TICKADJ * ONE_US_NS;
+ else if (ntpdata->time_adjust <= -MAX_TICKADJ)
+ adj = -MAX_TICKADJ * ONE_US_NS;
+ else
+ adj = ntpdata->time_adjust * ONE_US_NS +
+ ntpdata->time_adjust_frac;
- if (ntpdata->time_adjust > MAX_TICKADJ) {
- ntpdata->time_adjust -= MAX_TICKADJ;
- ntpdata->tick_length += MAX_TICKADJ_SCALED;
- goto out;
- }
+ adj_chunk = div_s64(adj, NTP_INTERVAL_FREQ);
+ if (!adj_chunk)
+ adj_chunk = signof(ntpdata->time_adjust_frac);
+ }
- if (ntpdata->time_adjust < -MAX_TICKADJ) {
- ntpdata->time_adjust += MAX_TICKADJ;
- ntpdata->tick_length -= MAX_TICKADJ_SCALED;
- goto out;
+ /* Net is what the clock delivers; reduce to per-tick, then floor. */
+ net = off_chunk + adj_chunk;
+ ntpdata->skew_delta = div_s64(net, NTP_INTERVAL_FREQ);
+ if (!ntpdata->skew_delta && net)
+ ntpdata->skew_delta = signof(net);
+ } else {
+ ntpdata->skew_delta = 0;
}
- ntpdata->tick_length += (s64)(ntpdata->time_adjust * NSEC_PER_USEC / NTP_INTERVAL_FREQ)
- << NTP_SCALE_SHIFT;
- ntpdata->time_adjust = 0;
-
-out:
return leap;
}
@@ -878,6 +952,7 @@ int ntp_adjtimex(unsigned int tkid, struct __kernel_timex *txc, const struct tim
if (!(txc->modes & ADJ_OFFSET_READONLY)) {
/* adjtime() is independent from ntp_adjtime() */
ntpdata->time_adjust = txc->offset;
+ ntpdata->time_adjust_frac = 0;
ntp_update_frequency(ntpdata);
audit_ntp_set_old(ad, AUDIT_NTP_ADJUST, save_adjust);
@@ -1119,6 +1194,7 @@ static void hardpps_update_phase(struct ntp_data *ntpdata, long error)
NTP_INTERVAL_FREQ);
/* Cancel running adjtime() */
ntpdata->time_adjust = 0;
+ ntpdata->time_adjust_frac = 0;
}
/* Update jitter */
ntpdata->pps_jitter += (jitter - ntpdata->pps_jitter) >> PPS_INTMIN;
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 0e646e9..97db2e9 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -2611,7 +2611,7 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
/*
* When skewing, do so by adjusting ntp_error to impart an extra
* target delta into ntp_error per tick, limited to what can be
- * drained from time_offset to avoid overshoot.
+ * drained from time_offset / time_adjust to avoid overshoot.
*
* The base 'mult' value was calculated with the skew taken into
* account, such that the per-tick choice of 'mult' vs. 'mult+1'
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v7 6/7] timekeeping: Settle competing time_offset and time_adjust skew
2026-06-21 21:53 [PATCH v7 0/7] timekeeping/ntp: Fix drift tracking precision David Woodhouse
` (4 preceding siblings ...)
2026-06-21 21:53 ` [PATCH v7 5/7] timekeeping: Drive time_adjust " David Woodhouse
@ 2026-06-21 21:53 ` David Woodhouse
2026-07-07 22:22 ` [tip: timers/core] " tip-bot2 for David Woodhouse
2026-07-10 7:25 ` tip-bot2 for David Woodhouse
2026-06-21 21:54 ` [PATCH v7 7/7] ntp: Remove tick_length_base, use tick_length directly David Woodhouse
6 siblings, 2 replies; 30+ messages in thread
From: David Woodhouse @ 2026-06-21 21:53 UTC (permalink / raw)
To: Richard Cochran, Wen Gu, David Woodhouse, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John Stultz, Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Peter Zijlstra,
Thomas Weißschuh, Arnd Bergmann, Miroslav Lichvar,
Julien Ridoux, Ryan Luu, linux-kernel
From: David Woodhouse <dwmw@amazon.co.uk>
time_offset (the exponential PLL phase slew) and time_adjust (the
linear adjtime() slew) can be asked to move the clock in opposite
directions. second_overflow() folds only their *net* into the per-tick
skew_delta, so the cancelling overlap would never be drained from
either tracker by the per-tick code — and if they cancel exactly,
skew_delta is zero and neither converges at all.
Arguably we could just let one of them entirely cancel out the other
immediately, but that would be a change in userspace-visible behaviour.
Instead, preserve the existing behaviour by calculating the "conflict"
portion between the opposing skew each second, and transferring that
amount directly from one tracker to the other.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.8
---
kernel/time/ntp.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 20d211b27908..73a47dd775a0 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -514,6 +514,69 @@ s64 ntp_drain_skew(unsigned int tkid, s64 amount, unsigned int shift)
return amount - unclaimed;
}
+/*
+ * time_offset (drained exponentially) and time_adjust (drained linearly at the
+ * MAX_TICKADJ rate) can be asked to slew the clock in opposite directions.
+ * second_overflow() only folds their *net* into skew_delta, so the cancelling
+ * part would never be drained from either tracker via the per-tick code -- and
+ * if they cancel exactly, skew_delta is zero and neither converges at all.
+ *
+ * Settle that cancelling phase directly between the two here. No clock motion
+ * results (the opposing slews annihilate), but both move toward zero so neither
+ * stalls. @amount is the phase to take off time_offset, in its (÷HZ) units and
+ * with its sign; the same real magnitude comes off time_adjust in the opposite
+ * direction. Clamped so neither tracker is driven past zero.
+ */
+static void ntp_transfer_offset_adjust(struct ntp_data *ntpdata, s64 amount)
+{
+ s64 frac_delta, carry;
+
+ /*
+ * Don't drain time_offset past zero. @amount shares its sign and is
+ * normally bounded below it by ntp_offset_chunk(), but the ±1 skew_delta
+ * floor for a tiny time_offset can exceed it, so clamp.
+ */
+ if (abs(amount) > abs(ntpdata->time_offset))
+ amount = ntpdata->time_offset;
+ if (!amount)
+ return;
+
+ /*
+ * Remove the matching phase from time_adjust, in plain shifted-ns. No
+ * clamp against time_adjust's zero is needed: @amount is bounded by the
+ * adjtime chunk, which second_overflow() never lets exceed time_adjust's
+ * own pending phase, so this cannot overshoot.
+ */
+ frac_delta = amount * NTP_INTERVAL_FREQ;
+
+ ntpdata->time_offset -= amount;
+
+ /* Add the matching phase to time_adjust, carrying whole µs (O(1)). */
+ ntpdata->time_adjust_frac += frac_delta;
+ if (ntpdata->time_adjust_frac >= ONE_US_NS ||
+ ntpdata->time_adjust_frac <= -ONE_US_NS) {
+ carry = div64_s64(ntpdata->time_adjust_frac, ONE_US_NS);
+ ntpdata->time_adjust += carry;
+ ntpdata->time_adjust_frac -= carry * ONE_US_NS;
+ }
+
+ /*
+ * Keep time_adjust and its sub-µs remainder the same sign. The
+ * truncating carry above can leave them opposed (e.g. +4 µs paired
+ * with -250 ns), and ntp_drain_time_adjust() treats abs(time_adjust_frac)
+ * as same-direction drawer capacity -- an opposing remainder there makes
+ * it over-deliver phase that was never removed from the pile. Borrow or
+ * repay a single whole µs to realign; the total phase is unchanged.
+ */
+ if (ntpdata->time_adjust > 0 && ntpdata->time_adjust_frac < 0) {
+ ntpdata->time_adjust--;
+ ntpdata->time_adjust_frac += ONE_US_NS;
+ } else if (ntpdata->time_adjust < 0 && ntpdata->time_adjust_frac > 0) {
+ ntpdata->time_adjust++;
+ ntpdata->time_adjust_frac -= ONE_US_NS;
+ }
+}
+
/**
* ntp_get_next_leap - Returns the next leapsecond in CLOCK_REALTIME ktime_t
* @tkid: Timekeeper ID
@@ -651,6 +714,18 @@ int second_overflow(unsigned int tkid, time64_t secs)
adj_chunk = signof(ntpdata->time_adjust_frac);
}
+ /*
+ * If the two slews oppose, only their net would drive the
+ * per-tick drain, so the cancelling part would never drain from
+ * either tracker and an exact cancellation would stall both.
+ * Settle that overlap directly between them (no clock motion).
+ */
+ if (off_chunk && adj_chunk && signof(off_chunk) != signof(adj_chunk)) {
+ s64 conflict = min(abs(off_chunk), abs(adj_chunk));
+
+ ntp_transfer_offset_adjust(ntpdata, signof(off_chunk) * conflict);
+ }
+
/* Net is what the clock delivers; reduce to per-tick, then floor. */
net = off_chunk + adj_chunk;
ntpdata->skew_delta = div_s64(net, NTP_INTERVAL_FREQ);
--
2.54.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [tip: timers/core] timekeeping: Settle competing time_offset and time_adjust skew
2026-06-21 21:53 ` [PATCH v7 6/7] timekeeping: Settle competing time_offset and time_adjust skew David Woodhouse
@ 2026-07-07 22:22 ` tip-bot2 for David Woodhouse
2026-07-10 7:25 ` tip-bot2 for David Woodhouse
1 sibling, 0 replies; 30+ messages in thread
From: tip-bot2 for David Woodhouse @ 2026-07-07 22:22 UTC (permalink / raw)
To: linux-tip-commits; +Cc: David Woodhouse, Thomas Gleixner, x86, linux-kernel
The following commit has been merged into the timers/core branch of tip:
Commit-ID: c10c8c1ad90cb26d33688986c4f1b0b1081fa10c
Gitweb: https://git.kernel.org/tip/c10c8c1ad90cb26d33688986c4f1b0b1081fa10c
Author: David Woodhouse <dwmw@amazon.co.uk>
AuthorDate: Sun, 21 Jun 2026 22:53:59 +01:00
Committer: Thomas Gleixner <tglx@kernel.org>
CommitterDate: Wed, 08 Jul 2026 00:17:27 +02:00
timekeeping: Settle competing time_offset and time_adjust skew
time_offset (the exponential PLL phase slew) and time_adjust (the
linear adjtime() slew) can be asked to move the clock in opposite
directions. second_overflow() folds only their *net* into the per-tick
skew_delta, so the cancelling overlap would never be drained from
either tracker by the per-tick code — and if they cancel exactly,
skew_delta is zero and neither converges at all.
Arguably we could just let one of them entirely cancel out the other
immediately, but that would be a change in userspace-visible behaviour.
Instead, preserve the existing behaviour by calculating the "conflict"
portion between the opposing skew each second, and transferring that
amount directly from one tracker to the other.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Assisted-by: Kiro:claude-opus-4.8
Link: https://patch.msgid.link/20260621220051.1030462-7-dwmw2@infradead.org
---
kernel/time/ntp.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 75 insertions(+)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 20d211b..73a47dd 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -514,6 +514,69 @@ s64 ntp_drain_skew(unsigned int tkid, s64 amount, unsigned int shift)
return amount - unclaimed;
}
+/*
+ * time_offset (drained exponentially) and time_adjust (drained linearly at the
+ * MAX_TICKADJ rate) can be asked to slew the clock in opposite directions.
+ * second_overflow() only folds their *net* into skew_delta, so the cancelling
+ * part would never be drained from either tracker via the per-tick code -- and
+ * if they cancel exactly, skew_delta is zero and neither converges at all.
+ *
+ * Settle that cancelling phase directly between the two here. No clock motion
+ * results (the opposing slews annihilate), but both move toward zero so neither
+ * stalls. @amount is the phase to take off time_offset, in its (÷HZ) units and
+ * with its sign; the same real magnitude comes off time_adjust in the opposite
+ * direction. Clamped so neither tracker is driven past zero.
+ */
+static void ntp_transfer_offset_adjust(struct ntp_data *ntpdata, s64 amount)
+{
+ s64 frac_delta, carry;
+
+ /*
+ * Don't drain time_offset past zero. @amount shares its sign and is
+ * normally bounded below it by ntp_offset_chunk(), but the ±1 skew_delta
+ * floor for a tiny time_offset can exceed it, so clamp.
+ */
+ if (abs(amount) > abs(ntpdata->time_offset))
+ amount = ntpdata->time_offset;
+ if (!amount)
+ return;
+
+ /*
+ * Remove the matching phase from time_adjust, in plain shifted-ns. No
+ * clamp against time_adjust's zero is needed: @amount is bounded by the
+ * adjtime chunk, which second_overflow() never lets exceed time_adjust's
+ * own pending phase, so this cannot overshoot.
+ */
+ frac_delta = amount * NTP_INTERVAL_FREQ;
+
+ ntpdata->time_offset -= amount;
+
+ /* Add the matching phase to time_adjust, carrying whole µs (O(1)). */
+ ntpdata->time_adjust_frac += frac_delta;
+ if (ntpdata->time_adjust_frac >= ONE_US_NS ||
+ ntpdata->time_adjust_frac <= -ONE_US_NS) {
+ carry = div64_s64(ntpdata->time_adjust_frac, ONE_US_NS);
+ ntpdata->time_adjust += carry;
+ ntpdata->time_adjust_frac -= carry * ONE_US_NS;
+ }
+
+ /*
+ * Keep time_adjust and its sub-µs remainder the same sign. The
+ * truncating carry above can leave them opposed (e.g. +4 µs paired
+ * with -250 ns), and ntp_drain_time_adjust() treats abs(time_adjust_frac)
+ * as same-direction drawer capacity -- an opposing remainder there makes
+ * it over-deliver phase that was never removed from the pile. Borrow or
+ * repay a single whole µs to realign; the total phase is unchanged.
+ */
+ if (ntpdata->time_adjust > 0 && ntpdata->time_adjust_frac < 0) {
+ ntpdata->time_adjust--;
+ ntpdata->time_adjust_frac += ONE_US_NS;
+ } else if (ntpdata->time_adjust < 0 && ntpdata->time_adjust_frac > 0) {
+ ntpdata->time_adjust++;
+ ntpdata->time_adjust_frac -= ONE_US_NS;
+ }
+}
+
/**
* ntp_get_next_leap - Returns the next leapsecond in CLOCK_REALTIME ktime_t
* @tkid: Timekeeper ID
@@ -651,6 +714,18 @@ int second_overflow(unsigned int tkid, time64_t secs)
adj_chunk = signof(ntpdata->time_adjust_frac);
}
+ /*
+ * If the two slews oppose, only their net would drive the
+ * per-tick drain, so the cancelling part would never drain from
+ * either tracker and an exact cancellation would stall both.
+ * Settle that overlap directly between them (no clock motion).
+ */
+ if (off_chunk && adj_chunk && signof(off_chunk) != signof(adj_chunk)) {
+ s64 conflict = min(abs(off_chunk), abs(adj_chunk));
+
+ ntp_transfer_offset_adjust(ntpdata, signof(off_chunk) * conflict);
+ }
+
/* Net is what the clock delivers; reduce to per-tick, then floor. */
net = off_chunk + adj_chunk;
ntpdata->skew_delta = div_s64(net, NTP_INTERVAL_FREQ);
^ permalink raw reply [flat|nested] 30+ messages in thread* [tip: timers/core] timekeeping: Settle competing time_offset and time_adjust skew
2026-06-21 21:53 ` [PATCH v7 6/7] timekeeping: Settle competing time_offset and time_adjust skew David Woodhouse
2026-07-07 22:22 ` [tip: timers/core] " tip-bot2 for David Woodhouse
@ 2026-07-10 7:25 ` tip-bot2 for David Woodhouse
1 sibling, 0 replies; 30+ messages in thread
From: tip-bot2 for David Woodhouse @ 2026-07-10 7:25 UTC (permalink / raw)
To: linux-tip-commits; +Cc: David Woodhouse, Thomas Gleixner, x86, linux-kernel
The following commit has been merged into the timers/core branch of tip:
Commit-ID: 34ce97c33dca9595455a63d7801e360d19fd97ed
Gitweb: https://git.kernel.org/tip/34ce97c33dca9595455a63d7801e360d19fd97ed
Author: David Woodhouse <dwmw@amazon.co.uk>
AuthorDate: Sun, 21 Jun 2026 22:53:59 +01:00
Committer: Thomas Gleixner <tglx@kernel.org>
CommitterDate: Fri, 10 Jul 2026 09:20:55 +02:00
timekeeping: Settle competing time_offset and time_adjust skew
time_offset (the exponential PLL phase slew) and time_adjust (the
linear adjtime() slew) can be asked to move the clock in opposite
directions. second_overflow() folds only their *net* into the per-tick
skew_delta, so the cancelling overlap would never be drained from
either tracker by the per-tick code — and if they cancel exactly,
skew_delta is zero and neither converges at all.
Arguably we could just let one of them entirely cancel out the other
immediately, but that would be a change in userspace-visible behaviour.
Instead, preserve the existing behaviour by calculating the "conflict"
portion between the opposing skew each second, and transferring that
amount directly from one tracker to the other.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Assisted-by: Kiro:claude-opus-4.8
Link: https://patch.msgid.link/20260621220051.1030462-7-dwmw2@infradead.org
---
kernel/time/ntp.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 75 insertions(+)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 20d211b..73a47dd 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -514,6 +514,69 @@ s64 ntp_drain_skew(unsigned int tkid, s64 amount, unsigned int shift)
return amount - unclaimed;
}
+/*
+ * time_offset (drained exponentially) and time_adjust (drained linearly at the
+ * MAX_TICKADJ rate) can be asked to slew the clock in opposite directions.
+ * second_overflow() only folds their *net* into skew_delta, so the cancelling
+ * part would never be drained from either tracker via the per-tick code -- and
+ * if they cancel exactly, skew_delta is zero and neither converges at all.
+ *
+ * Settle that cancelling phase directly between the two here. No clock motion
+ * results (the opposing slews annihilate), but both move toward zero so neither
+ * stalls. @amount is the phase to take off time_offset, in its (÷HZ) units and
+ * with its sign; the same real magnitude comes off time_adjust in the opposite
+ * direction. Clamped so neither tracker is driven past zero.
+ */
+static void ntp_transfer_offset_adjust(struct ntp_data *ntpdata, s64 amount)
+{
+ s64 frac_delta, carry;
+
+ /*
+ * Don't drain time_offset past zero. @amount shares its sign and is
+ * normally bounded below it by ntp_offset_chunk(), but the ±1 skew_delta
+ * floor for a tiny time_offset can exceed it, so clamp.
+ */
+ if (abs(amount) > abs(ntpdata->time_offset))
+ amount = ntpdata->time_offset;
+ if (!amount)
+ return;
+
+ /*
+ * Remove the matching phase from time_adjust, in plain shifted-ns. No
+ * clamp against time_adjust's zero is needed: @amount is bounded by the
+ * adjtime chunk, which second_overflow() never lets exceed time_adjust's
+ * own pending phase, so this cannot overshoot.
+ */
+ frac_delta = amount * NTP_INTERVAL_FREQ;
+
+ ntpdata->time_offset -= amount;
+
+ /* Add the matching phase to time_adjust, carrying whole µs (O(1)). */
+ ntpdata->time_adjust_frac += frac_delta;
+ if (ntpdata->time_adjust_frac >= ONE_US_NS ||
+ ntpdata->time_adjust_frac <= -ONE_US_NS) {
+ carry = div64_s64(ntpdata->time_adjust_frac, ONE_US_NS);
+ ntpdata->time_adjust += carry;
+ ntpdata->time_adjust_frac -= carry * ONE_US_NS;
+ }
+
+ /*
+ * Keep time_adjust and its sub-µs remainder the same sign. The
+ * truncating carry above can leave them opposed (e.g. +4 µs paired
+ * with -250 ns), and ntp_drain_time_adjust() treats abs(time_adjust_frac)
+ * as same-direction drawer capacity -- an opposing remainder there makes
+ * it over-deliver phase that was never removed from the pile. Borrow or
+ * repay a single whole µs to realign; the total phase is unchanged.
+ */
+ if (ntpdata->time_adjust > 0 && ntpdata->time_adjust_frac < 0) {
+ ntpdata->time_adjust--;
+ ntpdata->time_adjust_frac += ONE_US_NS;
+ } else if (ntpdata->time_adjust < 0 && ntpdata->time_adjust_frac > 0) {
+ ntpdata->time_adjust++;
+ ntpdata->time_adjust_frac -= ONE_US_NS;
+ }
+}
+
/**
* ntp_get_next_leap - Returns the next leapsecond in CLOCK_REALTIME ktime_t
* @tkid: Timekeeper ID
@@ -651,6 +714,18 @@ int second_overflow(unsigned int tkid, time64_t secs)
adj_chunk = signof(ntpdata->time_adjust_frac);
}
+ /*
+ * If the two slews oppose, only their net would drive the
+ * per-tick drain, so the cancelling part would never drain from
+ * either tracker and an exact cancellation would stall both.
+ * Settle that overlap directly between them (no clock motion).
+ */
+ if (off_chunk && adj_chunk && signof(off_chunk) != signof(adj_chunk)) {
+ s64 conflict = min(abs(off_chunk), abs(adj_chunk));
+
+ ntp_transfer_offset_adjust(ntpdata, signof(off_chunk) * conflict);
+ }
+
/* Net is what the clock delivers; reduce to per-tick, then floor. */
net = off_chunk + adj_chunk;
ntpdata->skew_delta = div_s64(net, NTP_INTERVAL_FREQ);
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v7 7/7] ntp: Remove tick_length_base, use tick_length directly
2026-06-21 21:53 [PATCH v7 0/7] timekeeping/ntp: Fix drift tracking precision David Woodhouse
` (5 preceding siblings ...)
2026-06-21 21:53 ` [PATCH v7 6/7] timekeeping: Settle competing time_offset and time_adjust skew David Woodhouse
@ 2026-06-21 21:54 ` David Woodhouse
2026-07-07 22:22 ` [tip: timers/core] " tip-bot2 for David Woodhouse
2026-07-10 7:25 ` tip-bot2 for David Woodhouse
6 siblings, 2 replies; 30+ messages in thread
From: David Woodhouse @ 2026-06-21 21:54 UTC (permalink / raw)
To: Richard Cochran, Wen Gu, David Woodhouse, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John Stultz, Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Peter Zijlstra,
Thomas Weißschuh, Arnd Bergmann, Miroslav Lichvar,
Julien Ridoux, Ryan Luu, linux-kernel
From: David Woodhouse <dwmw@amazon.co.uk>
Now that nothing inflates tick_length beyond tick_length_base (the
adjtime path was converted to use time_offset in the previous commit),
the two fields are always equal.
Remove tick_length_base and keep tick_length as the single field.
Remove the per-second reset and the delta update in
ntp_update_frequency() since there is no separate base to track.
No functional change intended.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.6-1m
---
kernel/time/ntp.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 73a47dd775a0..d22b532ec536 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -26,8 +26,7 @@
/**
* struct ntp_data - Structure holding all NTP related state
* @tick_usec: USER_HZ period in microseconds
- * @tick_length: Adjusted tick length
- * @tick_length_base: Base value for @tick_length
+ * @tick_length: Tick length in ns << NTP_SCALE_SHIFT
* @time_state: State of the clock synchronization
* @time_status: Clock status bits
* @time_offset: Time adjustment in nanoseconds
@@ -68,7 +67,6 @@
struct ntp_data {
unsigned long tick_usec;
u64 tick_length;
- u64 tick_length_base;
int time_state;
int time_status;
s64 time_offset;
@@ -260,8 +258,7 @@ static inline void pps_fill_timex(struct ntp_data *ntpdata, struct __kernel_time
#endif /* CONFIG_NTP_PPS */
/*
- * Update tick_length and tick_length_base, based on tick_usec, ntp_tick_adj and
- * time_freq:
+ * Update tick_length based on tick_usec, ntp_tick_adj and time_freq:
*/
static void ntp_update_frequency(struct ntp_data *ntpdata)
{
@@ -279,8 +276,7 @@ static void ntp_update_frequency(struct ntp_data *ntpdata)
* Don't wait for the next second_overflow, apply the change to the
* tick length immediately:
*/
- ntpdata->tick_length += new_base - ntpdata->tick_length_base;
- ntpdata->tick_length_base = new_base;
+ ntpdata->tick_length = new_base;
}
static inline s64 ntp_update_offset_fll(struct ntp_data *ntpdata, s64 offset64, long secs)
@@ -358,7 +354,6 @@ static void __ntp_clear(struct ntp_data *ntpdata)
ntp_update_frequency(ntpdata);
- ntpdata->tick_length = ntpdata->tick_length_base;
ntpdata->time_offset = 0;
ntpdata->skew_delta = 0;
@@ -670,7 +665,6 @@ int second_overflow(unsigned int tkid, time64_t secs)
}
/* Compute the phase adjustment for the next second */
- ntpdata->tick_length = ntpdata->tick_length_base;
/* Check PPS signal */
pps_dec_valid(ntpdata);
--
2.54.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [tip: timers/core] ntp: Remove tick_length_base, use tick_length directly
2026-06-21 21:54 ` [PATCH v7 7/7] ntp: Remove tick_length_base, use tick_length directly David Woodhouse
@ 2026-07-07 22:22 ` tip-bot2 for David Woodhouse
2026-07-10 7:25 ` tip-bot2 for David Woodhouse
1 sibling, 0 replies; 30+ messages in thread
From: tip-bot2 for David Woodhouse @ 2026-07-07 22:22 UTC (permalink / raw)
To: linux-tip-commits; +Cc: David Woodhouse, Thomas Gleixner, x86, linux-kernel
The following commit has been merged into the timers/core branch of tip:
Commit-ID: f5abf0e07894551c50767bd9bc3a73e3fa7e4664
Gitweb: https://git.kernel.org/tip/f5abf0e07894551c50767bd9bc3a73e3fa7e4664
Author: David Woodhouse <dwmw@amazon.co.uk>
AuthorDate: Sun, 21 Jun 2026 22:54:00 +01:00
Committer: Thomas Gleixner <tglx@kernel.org>
CommitterDate: Wed, 08 Jul 2026 00:17:27 +02:00
ntp: Remove tick_length_base, use tick_length directly
Now that nothing inflates tick_length beyond tick_length_base (the
adjtime path was converted to use time_offset in the previous commit),
the two fields are always equal.
Remove tick_length_base and keep tick_length as the single field.
Remove the per-second reset and the delta update in
ntp_update_frequency() since there is no separate base to track.
No functional change intended.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Assisted-by: Kiro:claude-opus-4.6-1m
Link: https://patch.msgid.link/20260621220051.1030462-8-dwmw2@infradead.org
---
kernel/time/ntp.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 73a47dd..d22b532 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -26,8 +26,7 @@
/**
* struct ntp_data - Structure holding all NTP related state
* @tick_usec: USER_HZ period in microseconds
- * @tick_length: Adjusted tick length
- * @tick_length_base: Base value for @tick_length
+ * @tick_length: Tick length in ns << NTP_SCALE_SHIFT
* @time_state: State of the clock synchronization
* @time_status: Clock status bits
* @time_offset: Time adjustment in nanoseconds
@@ -68,7 +67,6 @@
struct ntp_data {
unsigned long tick_usec;
u64 tick_length;
- u64 tick_length_base;
int time_state;
int time_status;
s64 time_offset;
@@ -260,8 +258,7 @@ static inline void pps_fill_timex(struct ntp_data *ntpdata, struct __kernel_time
#endif /* CONFIG_NTP_PPS */
/*
- * Update tick_length and tick_length_base, based on tick_usec, ntp_tick_adj and
- * time_freq:
+ * Update tick_length based on tick_usec, ntp_tick_adj and time_freq:
*/
static void ntp_update_frequency(struct ntp_data *ntpdata)
{
@@ -279,8 +276,7 @@ static void ntp_update_frequency(struct ntp_data *ntpdata)
* Don't wait for the next second_overflow, apply the change to the
* tick length immediately:
*/
- ntpdata->tick_length += new_base - ntpdata->tick_length_base;
- ntpdata->tick_length_base = new_base;
+ ntpdata->tick_length = new_base;
}
static inline s64 ntp_update_offset_fll(struct ntp_data *ntpdata, s64 offset64, long secs)
@@ -358,7 +354,6 @@ static void __ntp_clear(struct ntp_data *ntpdata)
ntp_update_frequency(ntpdata);
- ntpdata->tick_length = ntpdata->tick_length_base;
ntpdata->time_offset = 0;
ntpdata->skew_delta = 0;
@@ -670,7 +665,6 @@ int second_overflow(unsigned int tkid, time64_t secs)
}
/* Compute the phase adjustment for the next second */
- ntpdata->tick_length = ntpdata->tick_length_base;
/* Check PPS signal */
pps_dec_valid(ntpdata);
^ permalink raw reply [flat|nested] 30+ messages in thread* [tip: timers/core] ntp: Remove tick_length_base, use tick_length directly
2026-06-21 21:54 ` [PATCH v7 7/7] ntp: Remove tick_length_base, use tick_length directly David Woodhouse
2026-07-07 22:22 ` [tip: timers/core] " tip-bot2 for David Woodhouse
@ 2026-07-10 7:25 ` tip-bot2 for David Woodhouse
1 sibling, 0 replies; 30+ messages in thread
From: tip-bot2 for David Woodhouse @ 2026-07-10 7:25 UTC (permalink / raw)
To: linux-tip-commits; +Cc: David Woodhouse, Thomas Gleixner, x86, linux-kernel
The following commit has been merged into the timers/core branch of tip:
Commit-ID: 794ddd6e15cfd133c4a5c01dac37a5da04cbe186
Gitweb: https://git.kernel.org/tip/794ddd6e15cfd133c4a5c01dac37a5da04cbe186
Author: David Woodhouse <dwmw@amazon.co.uk>
AuthorDate: Sun, 21 Jun 2026 22:54:00 +01:00
Committer: Thomas Gleixner <tglx@kernel.org>
CommitterDate: Fri, 10 Jul 2026 09:20:55 +02:00
ntp: Remove tick_length_base, use tick_length directly
Now that nothing inflates tick_length beyond tick_length_base (the
adjtime path was converted to use time_offset in the previous commit),
the two fields are always equal.
Remove tick_length_base and keep tick_length as the single field.
Remove the per-second reset and the delta update in
ntp_update_frequency() since there is no separate base to track.
No functional change intended.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Assisted-by: Kiro:claude-opus-4.6-1m
Link: https://patch.msgid.link/20260621220051.1030462-8-dwmw2@infradead.org
---
kernel/time/ntp.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 73a47dd..d22b532 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -26,8 +26,7 @@
/**
* struct ntp_data - Structure holding all NTP related state
* @tick_usec: USER_HZ period in microseconds
- * @tick_length: Adjusted tick length
- * @tick_length_base: Base value for @tick_length
+ * @tick_length: Tick length in ns << NTP_SCALE_SHIFT
* @time_state: State of the clock synchronization
* @time_status: Clock status bits
* @time_offset: Time adjustment in nanoseconds
@@ -68,7 +67,6 @@
struct ntp_data {
unsigned long tick_usec;
u64 tick_length;
- u64 tick_length_base;
int time_state;
int time_status;
s64 time_offset;
@@ -260,8 +258,7 @@ static inline void pps_fill_timex(struct ntp_data *ntpdata, struct __kernel_time
#endif /* CONFIG_NTP_PPS */
/*
- * Update tick_length and tick_length_base, based on tick_usec, ntp_tick_adj and
- * time_freq:
+ * Update tick_length based on tick_usec, ntp_tick_adj and time_freq:
*/
static void ntp_update_frequency(struct ntp_data *ntpdata)
{
@@ -279,8 +276,7 @@ static void ntp_update_frequency(struct ntp_data *ntpdata)
* Don't wait for the next second_overflow, apply the change to the
* tick length immediately:
*/
- ntpdata->tick_length += new_base - ntpdata->tick_length_base;
- ntpdata->tick_length_base = new_base;
+ ntpdata->tick_length = new_base;
}
static inline s64 ntp_update_offset_fll(struct ntp_data *ntpdata, s64 offset64, long secs)
@@ -358,7 +354,6 @@ static void __ntp_clear(struct ntp_data *ntpdata)
ntp_update_frequency(ntpdata);
- ntpdata->tick_length = ntpdata->tick_length_base;
ntpdata->time_offset = 0;
ntpdata->skew_delta = 0;
@@ -670,7 +665,6 @@ int second_overflow(unsigned int tkid, time64_t secs)
}
/* Compute the phase adjustment for the next second */
- ntpdata->tick_length = ntpdata->tick_length_base;
/* Check PPS signal */
pps_dec_valid(ntpdata);
^ permalink raw reply [flat|nested] 30+ messages in thread