mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Woodhouse <dwmw2@infradead.org>
To: "Richard Cochran" <richardcochran@gmail.com>,
	"Wen Gu" <guwen@linux.alibaba.com>,
	"David Woodhouse" <dwmw2@infradead.org>,
	"Andrew Lunn" <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"John Stultz" <jstultz@google.com>,
	"Thomas Gleixner" <tglx@kernel.org>,
	"Stephen Boyd" <sboyd@kernel.org>,
	"Anna-Maria Behnsen" <anna-maria@linutronix.de>,
	"Frederic Weisbecker" <frederic@kernel.org>,
	"Shuah Khan" <shuah@kernel.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Thomas Weißschuh" <thomas.weissschuh@linutronix.de>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Miroslav Lichvar" <mlichvar@redhat.com>,
	"Julien Ridoux" <ridouxj@amazon.com>,
	"Ryan Luu" <rluu@amazon.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v4 2/7] timekeeping: Remove xtime_remainder from ntp_error accumulation
Date: Mon, 25 May 2026 14:54:34 +0100	[thread overview]
Message-ID: <20260525135904.126282-3-dwmw2@infradead.org> (raw)
In-Reply-To: <20260525135904.126282-1-dwmw2@infradead.org>

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

The ntp_error accumulator tracks the difference between the time actually
reported to consumers in xtime, and the *intended* time. The former is
subject to a sawtooth effect due to the quantisation of 'mult', which
means that it actually advances by 'xtime_interval' each tick, while
the intended clock advances by 'ntp_tick'.

By dithering between adjacent integer values of 'mult' which result in
an 'xtime_interval' slightly higher/lower than the intended tick length,
the advancement of xtime is kept on average to the intended rate.

The accounting should therefore adjust ntp_error by adding ntp_tick and
subtracting xtime_interval on each tick.

Since commit a386b5af8edd ("time: Compensate for rounding on
odd-frequency clocksources") the value subtracted has been
(xtime_interval + xtime_remainder), which is wrong. The effect is a
systematic drift whose magnitude depends on the value of xtime_remainder
and the NTP frequency correction. NTP masks this by continuously
adjusting the frequency to compensate, but with a fixed frequency (or an
external reference clock like vmclock), the drift is exposed.

The value of xtime_remainder actually does represent the difference
between the tick period and xtime_interval, so simply adding it instead
of (+ tick length - xtime_remainder) might have made sense... except
that it's only calculated once at boot time, so it's inaccurate anyway.
So just kill it with fire.

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

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

diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index e36d11e33e0c..da6cf383bedc 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
@@ -178,7 +176,6 @@ struct timekeeper {
 
 	u64			cycle_interval;
 	u64			xtime_interval;
-	s64			xtime_remainder;
 	u64			raw_interval;
 
 	ktime_t			next_leap_ktime;
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index b53225b91b4b..9b20f2101c45 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -365,7 +365,6 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
 
 	/* Go back from cycles -> shifted ns */
 	tk->xtime_interval = interval * clock->mult;
-	tk->xtime_remainder = ntpinterval - tk->xtime_interval;
 	tk->raw_interval = interval * clock->mult;
 
 	 /* if changing clocks, convert xtime_nsec shift units */
@@ -2400,8 +2399,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);
 	}
 
 	/*
@@ -2526,8 +2525,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


  parent reply	other threads:[~2026-05-25 13:59 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-25 13:54 [PATCH v4 0/7] timekeeping/ntp: Fix drift tracking precision David Woodhouse
2026-05-25 13:54 ` [PATCH v4 1/7] MAINTAINERS: Add Miroslav as timekeeping reviewer David Woodhouse
2026-05-25 13:54 ` David Woodhouse [this message]
2026-05-29 14:41   ` [PATCH v4 2/7] timekeeping: Remove xtime_remainder from ntp_error accumulation David Woodhouse
2026-05-25 13:54 ` [PATCH v4 3/7] timekeeping: Account for monotonicity adjustment in ntp_error David Woodhouse
2026-05-25 13:54 ` [PATCH v4 4/7] timekeeping: Guard against divide-by-zero in timekeeping_adjust David Woodhouse
2026-05-25 13:54 ` [PATCH v4 5/7] timekeeping: Drive time_offset skew via per-tick ntp_error transfer David Woodhouse
2026-05-27 18:02   ` David Woodhouse
2026-05-25 13:54 ` [PATCH v4 6/7] ntp: Convert adjtime() to use time_offset instead of tick_length inflation David Woodhouse
2026-05-25 13:54 ` [PATCH v4 7/7] ntp: Remove tick_length_base, use tick_length directly David Woodhouse

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260525135904.126282-3-dwmw2@infradead.org \
    --to=dwmw2@infradead.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=anna-maria@linutronix.de \
    --cc=arnd@arndb.de \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=frederic@kernel.org \
    --cc=guwen@linux.alibaba.com \
    --cc=jstultz@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mlichvar@redhat.com \
    --cc=pabeni@redhat.com \
    --cc=peterz@infradead.org \
    --cc=richardcochran@gmail.com \
    --cc=ridouxj@amazon.com \
    --cc=rluu@amazon.com \
    --cc=sboyd@kernel.org \
    --cc=shuah@kernel.org \
    --cc=tglx@kernel.org \
    --cc=thomas.weissschuh@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®