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

[David Woodhouse]

From: David Woodhouse <dwmw@xxxxxxxxxxxx>

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

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

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

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

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

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