Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1643492 > unrolled thread

[PATCH RFC 0/3] Improve stability of system clock

Back to article view | Back to linux.kernel

Contents

  [PATCH RFC 0/3] Improve stability of system clock Miroslav Lichvar <mlichvar@redhat.com> - 2017-05-17 18:20 +0200
    [PATCH RFC 1/3] timekeeping: Remove support for old vsyscalls Miroslav Lichvar <mlichvar@redhat.com> - 2017-05-17 18:20 +0200
    [PATCH RFC 2/3] timekeeping: Don't align frequency adjustments to ticks Miroslav Lichvar <mlichvar@redhat.com> - 2017-05-17 18:20 +0200
    Re: [PATCH RFC 0/3] Improve stability of system clock John Stultz <john.stultz@linaro.org> - 2017-05-17 18:40 +0200
      Re: [PATCH RFC 0/3] Improve stability of system clock John Stultz <john.stultz@linaro.org> - 2017-05-17 19:10 +0200
        Re: [PATCH RFC 0/3] Improve stability of system clock Miroslav Lichvar <mlichvar@redhat.com> - 2017-05-17 19:30 +0200
          Re: [PATCH RFC 0/3] Improve stability of system clock John Stultz <john.stultz@linaro.org> - 2017-05-18 01:10 +0200
            Re: [PATCH RFC 0/3] Improve stability of system clock Richard Cochran <richardcochran@gmail.com> - 2017-05-18 07:00 +0200
              Re: [PATCH RFC 0/3] Improve stability of system clock John Stultz <john.stultz@linaro.org> - 2017-05-20 02:40 +0200
                Re: [PATCH RFC 0/3] Improve stability of system clock Rusty Russell <rusty@rustcorp.com.au> - 2017-05-22 02:40 +0200
      Re: [PATCH RFC 0/3] Improve stability of system clock Miroslav Lichvar <mlichvar@redhat.com> - 2017-05-17 19:10 +0200

#1643492 — [PATCH RFC 0/3] Improve stability of system clock

This is an attempt to improve stability and accuracy of the system clock
with very accurate time sources like the new PTP KVM clock or NTP/PTP
using hardware timestamping. It affects mainly kernels running with
NOHZ. It requires updating of the old ia64 and powerpc vsyscalls.

The main problem is that the error accumulated in the ntp_error register
takes too long to correct and this cannot be easily fixed. There are
four sources of the error:
- rounding of time for old vsyscalls
- alignment of frequency adjustments to ticks
- iterative correction of the multiplier
- limited resolution of the multipler

Instead of trying to correct the error faster, the patches remove the
first three sources. With the only remaining source the correction logic
can be simplified and the frequency of the clock is much more stable and
accurate.

Simulations of a frequency step in linux-tktest (values are in ppm and
nanoseconds):

Before:

nohz on             [1, samples/2]           [samples/2 + 1, samples]
samples         freq       dev       max      freq       dev       max
10           1.47222    1341.3    2217.8   0.06322       0.2       0.5
30           0.20799     849.5    2448.7   0.06311       0.2       0.6
100          0.04101     492.1    2895.2   0.06311       0.2       0.5
300          0.05660     295.5    3026.1   0.02064      28.3     108.9
1000         0.01994     409.8    2732.1   0.00355      13.7      52.2
3000         0.00477     469.1    3238.9   0.00070      11.0      40.9
10000        0.00081     377.3    3791.6   0.00013       9.4      36.2
30000        0.00016     259.9    4055.7   0.00004       8.9      34.1
100000       0.00003     159.0    4177.2   0.00000      13.7      58.4

nohz off            [1, samples/2]           [samples/2 + 1, samples]
samples         freq       dev       max      freq       dev       max
10           3.55062       6.2      10.8   0.05730       0.0       0.0
30           0.44672       4.5      14.1   0.05724       0.2       0.5
100          0.03649       2.7      17.4   0.05711       0.2       0.5
300          0.05815       1.7      18.7   0.06313       0.2       0.5
1000         0.06270       1.0      19.1   0.06315       0.2       0.5
3000         0.05720       1.9      19.9   0.02065       1.1       4.1
10000        0.01947      13.5      41.0   0.00339       0.5       1.7
30000        0.00448      17.5      75.9   0.00065       0.3       1.0
100000       0.00078      14.2     101.7   0.00012       0.2       0.7

After:

nohz on             [1, samples/2]           [samples/2 + 1, samples]
samples         freq       dev       max      freq       dev       max
10           0.01584       9.0      14.2   0.02937       2.7       7.2
30           0.00426      10.9      22.4   0.00481       6.5      19.2
100          0.00077      11.6      26.3   0.00074       9.0      26.9
300          0.00013      12.4      29.9   0.00018       8.7      29.3
1000         0.00003      12.6      31.8   0.00003       8.7      32.1
3000         0.00001      12.6      33.3   0.00001       9.1      33.4
10000        0.00000      12.9      34.0   0.00000       9.0      34.1
30000        0.00000      12.8      34.5   0.00000       9.0      34.5
100000       0.00000      16.5      51.2   0.00000      13.7      58.5

nohz off            [1, samples/2]           [samples/2 + 1, samples]
samples         freq       dev       max      freq       dev       max
10           0.10309       0.1       0.1   0.12717       0.0       0.1
30           0.04269       0.1       0.3   0.02592       0.1       0.4
100          0.00629       0.3       0.5   0.00521       0.2       0.5
300          0.00109       0.3       0.6   0.00099       0.2       0.5
1000         0.00019       0.3       0.6   0.00022       0.2       0.6
3000         0.00002       0.3       0.6   0.00002       0.2       0.6
10000        0.00000       0.3       0.6   0.00000       0.2       0.6
30000        0.00000       0.3       0.6   0.00000       0.2       0.6
100000       0.00000       0.3       0.6   0.00000       0.2       0.6

Miroslav Lichvar (3):
  timekeeping: Remove support for old vsyscalls
  timekeeping: Don't align frequency adjustments to ticks
  timekeeping: Determine multiplier directly from NTP tick length

 include/linux/timekeeper_internal.h |   9 +-
 kernel/time/Kconfig                 |   4 -
 kernel/time/timekeeping.c           | 184 +++++++++---------------------------
 3 files changed, 48 insertions(+), 149 deletions(-)

-- 
2.9.3

[toc] | [next] | [standalone]

#1643493 — [PATCH RFC 1/3] timekeeping: Remove support for old vsyscalls

As the last users of CONFIG_GENERIC_TIME_VSYSCALL_OLD have been updated,
the support can be removed from the timekeeping code.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
---
 include/linux/timekeeper_internal.h |  7 ------
 kernel/time/Kconfig                 |  4 ----
 kernel/time/timekeeping.c           | 44 -------------------------------------
 3 files changed, 55 deletions(-)

diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index 110f453..b7ae5b0 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -132,13 +132,6 @@ struct timekeeper {
 extern void update_vsyscall(struct timekeeper *tk);
 extern void update_vsyscall_tz(void);
 
-#elif defined(CONFIG_GENERIC_TIME_VSYSCALL_OLD)
-
-extern void update_vsyscall_old(struct timespec *ts, struct timespec *wtm,
-				struct clocksource *c, u32 mult,
-				u64 cycle_last);
-extern void update_vsyscall_tz(void);
-
 #else
 
 static inline void update_vsyscall(struct timekeeper *tk)
diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index 4008d9f..55d61a3 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -21,10 +21,6 @@ config CLOCKSOURCE_VALIDATE_LAST_CYCLE
 config GENERIC_TIME_VSYSCALL
 	bool
 
-# Timekeeping vsyscall support
-config GENERIC_TIME_VSYSCALL_OLD
-	bool
-
 # Old style timekeeping
 config ARCH_USES_GETTIMEOFFSET
 	bool
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 8fd77c6..ff542dd 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -487,44 +487,6 @@ static void halt_fast_timekeeper(struct timekeeper *tk)
 	update_fast_timekeeper(&tkr_dummy, &tk_fast_raw);
 }
 
-#ifdef CONFIG_GENERIC_TIME_VSYSCALL_OLD
-
-static inline void update_vsyscall(struct timekeeper *tk)
-{
-	struct timespec xt, wm;
-
-	xt = timespec64_to_timespec(tk_xtime(tk));
-	wm = timespec64_to_timespec(tk->wall_to_monotonic);
-	update_vsyscall_old(&xt, &wm, tk->tkr_mono.clock, tk->tkr_mono.mult,
-			    tk->tkr_mono.cycle_last);
-}
-
-static inline void old_vsyscall_fixup(struct timekeeper *tk)
-{
-	s64 remainder;
-
-	/*
-	* Store only full nanoseconds into xtime_nsec after rounding
-	* it up and add the remainder to the error difference.
-	* XXX - This is necessary to avoid small 1ns inconsistnecies caused
-	* by truncating the remainder in vsyscalls. However, it causes
-	* additional work to be done in timekeeping_adjust(). Once
-	* the vsyscall implementations are converted to use xtime_nsec
-	* (shifted nanoseconds), and CONFIG_GENERIC_TIME_VSYSCALL_OLD
-	* users are removed, this can be killed.
-	*/
-	remainder = tk->tkr_mono.xtime_nsec & ((1ULL << tk->tkr_mono.shift) - 1);
-	if (remainder != 0) {
-		tk->tkr_mono.xtime_nsec -= remainder;
-		tk->tkr_mono.xtime_nsec += 1ULL << tk->tkr_mono.shift;
-		tk->ntp_error += remainder << tk->ntp_error_shift;
-		tk->ntp_error -= (1ULL << tk->tkr_mono.shift) << tk->ntp_error_shift;
-	}
-}
-#else
-#define old_vsyscall_fixup(tk)
-#endif
-
 static RAW_NOTIFIER_HEAD(pvclock_gtod_chain);
 
 static void update_pvclock_gtod(struct timekeeper *tk, bool was_set)
@@ -2065,12 +2027,6 @@ void update_wall_time(void)
 	timekeeping_adjust(tk, offset);
 
 	/*
-	 * XXX This can be killed once everyone converts
-	 * to the new update_vsyscall.
-	 */
-	old_vsyscall_fixup(tk);
-
-	/*
 	 * Finally, make sure that after the rounding
 	 * xtime_nsec isn't larger than NSEC_PER_SEC
 	 */
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1643494 — [PATCH RFC 2/3] timekeeping: Don't align frequency adjustments to ticks

When the timekeeping multiplier is adjusted, the NTP error is adjusted
to correct the clock for the misalignment of the update to the start of
the tick. This error is corrected in later updates and the clock appears
as if the frequency was changed exactly on the tick.

Remove this correction to keep the point where the frequency is
effectively changed at the time of the update. This removes a major
source of the NTP error.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
---
 kernel/time/timekeeping.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index ff542dd..5ae6f27 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1760,8 +1760,6 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk,
 	 *	xtime_nsec_2 = xtime_nsec_1 - offset
 	 * Which simplfies to:
 	 *	xtime_nsec -= offset
-	 *
-	 * XXX - TODO: Doc ntp_error calculation.
 	 */
 	if ((mult_adj > 0) && (tk->tkr_mono.mult + mult_adj < mult_adj)) {
 		/* NTP adjustment caused clocksource mult overflow */
@@ -1772,7 +1770,6 @@ 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 -= (interval - offset) << tk->ntp_error_shift;
 }
 
 /*
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1643501

On Wed, May 17, 2017 at 9:13 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
> This is an attempt to improve stability and accuracy of the system clock
> with very accurate time sources like the new PTP KVM clock or NTP/PTP
> using hardware timestamping. It affects mainly kernels running with
> NOHZ. It requires updating of the old ia64 and powerpc vsyscalls.
>
> The main problem is that the error accumulated in the ntp_error register
> takes too long to correct and this cannot be easily fixed. There are
> four sources of the error:
> - rounding of time for old vsyscalls
> - alignment of frequency adjustments to ticks
> - iterative correction of the multiplier
> - limited resolution of the multipler
>
> Instead of trying to correct the error faster, the patches remove the
> first three sources. With the only remaining source the correction logic
> can be simplified and the frequency of the clock is much more stable and
> accurate.
>
> Simulations of a frequency step in linux-tktest (values are in ppm and
> nanoseconds):

So thanks for sending these out. I still need to look them over in
depth, but can I make another ask here?  :)

Could you submit your linux-tktest infrastructure to the kselftests dir?

It would be really nice for folks to be able to reproduce your results
be able to do similar testing for regressions.

thanks
-john

[toc] | [prev] | [next] | [standalone]

#1643518

On Wed, May 17, 2017 at 9:57 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
> On Wed, May 17, 2017 at 09:30:31AM -0700, John Stultz wrote:
>> So thanks for sending these out. I still need to look them over in
>> depth, but can I make another ask here?  :)
>>
>> Could you submit your linux-tktest infrastructure to the kselftests dir?
>
> I can, but it's a mess that breaks frequently as the timekeeping and
> other kernel code changes. Are you sure you want that in the kernel
> tree? :)

Being a mess is a slight concern, but as for breaking, if its
in-kernel, then folks can't make changes that break it, right?

thanks
-john

[toc] | [prev] | [next] | [standalone]

#1643537

On Wed, May 17, 2017 at 10:02:00AM -0700, John Stultz wrote:
> On Wed, May 17, 2017 at 9:57 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
> > On Wed, May 17, 2017 at 09:30:31AM -0700, John Stultz wrote:
> >> Could you submit your linux-tktest infrastructure to the kselftests dir?
> >
> > I can, but it's a mess that breaks frequently as the timekeeping and
> > other kernel code changes. Are you sure you want that in the kernel
> > tree? :)
> 
> Being a mess is a slight concern, but as for breaking, if its
> in-kernel, then folks can't make changes that break it, right?

It duplicates/stubs quite a few kernel functions that are needed to
compile and link the timekeeping.c file into an executable. See
linux-tktest/missing.c. If their signature changes, or new functions
are needed, it will break.

Is there a better way to run the timekeeping code in an userspace
application? I suspect it would need something like the Linux Kernel
Library project.

-- 
Miroslav Lichvar

[toc] | [prev] | [next] | [standalone]

#1643689

On Wed, May 17, 2017 at 10:22 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
> On Wed, May 17, 2017 at 10:02:00AM -0700, John Stultz wrote:
>> On Wed, May 17, 2017 at 9:57 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
>> > On Wed, May 17, 2017 at 09:30:31AM -0700, John Stultz wrote:
>> >> Could you submit your linux-tktest infrastructure to the kselftests dir?
>> >
>> > I can, but it's a mess that breaks frequently as the timekeeping and
>> > other kernel code changes. Are you sure you want that in the kernel
>> > tree? :)
>>
>> Being a mess is a slight concern, but as for breaking, if its
>> in-kernel, then folks can't make changes that break it, right?
>
> It duplicates/stubs quite a few kernel functions that are needed to
> compile and link the timekeeping.c file into an executable. See
> linux-tktest/missing.c. If their signature changes, or new functions
> are needed, it will break.

Hopefully we can fix it in the kernel as well then?

Or maybe it will help inform how we could refactor the time code to
better support the simulation code?

> Is there a better way to run the timekeeping code in an userspace
> application? I suspect it would need something like the Linux Kernel
> Library project.

I dunno. There's probably a cleaner way to go about it, but I also
feel like the benefit of just having the test in the kernel tree is
that it can be managed as a unified whole, rather then the test being
a separate thing and always playing catchup to kernel changes.

thanks
-john

[toc] | [prev] | [next] | [standalone]

#1643813

On Wed, May 17, 2017 at 04:06:07PM -0700, John Stultz wrote:
> On Wed, May 17, 2017 at 10:22 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
> > Is there a better way to run the timekeeping code in an userspace
> > application? I suspect it would need something like the Linux Kernel
> > Library project.
> 
> I dunno. There's probably a cleaner way to go about it, but I also
> feel like the benefit of just having the test in the kernel tree is
> that it can be managed as a unified whole, rather then the test being
> a separate thing and always playing catchup to kernel changes.

I vaguely recall a rant on the list years ago from a Linux bigwhig
saying how we don't support that kind of thing.  But maybe it is my
imagination.  In any case, IMHO running user space tests for chunks of
kernel code can be quite useful.

Thanks,
Richard

[toc] | [prev] | [next] | [standalone]

#1646036

On Wed, May 17, 2017 at 9:54 PM, Richard Cochran
<richardcochran@gmail.com> wrote:
> On Wed, May 17, 2017 at 04:06:07PM -0700, John Stultz wrote:
>> On Wed, May 17, 2017 at 10:22 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
>> > Is there a better way to run the timekeeping code in an userspace
>> > application? I suspect it would need something like the Linux Kernel
>> > Library project.
>>
>> I dunno. There's probably a cleaner way to go about it, but I also
>> feel like the benefit of just having the test in the kernel tree is
>> that it can be managed as a unified whole, rather then the test being
>> a separate thing and always playing catchup to kernel changes.
>
> I vaguely recall a rant on the list years ago from a Linux bigwhig
> saying how we don't support that kind of thing.  But maybe it is my
> imagination.  In any case, IMHO running user space tests for chunks of
> kernel code can be quite useful.

So a few years ago I mentioned this at a testing session at I think
Linux Plubmers' and Rusty (CC'ed) commented that he had some netfilter
(or iptables?) simulator code that never made it upstream. However,
now that kselftests are integrated with the kernel this could change.
At least that's my memory of the discussion.

Anyway, I still think its worth trying to submit. Worse case its a
huge pain and we pull it back out?

thanks
-john

[toc] | [prev] | [next] | [standalone]

#1646476

John Stultz <john.stultz@linaro.org> writes:
> On Wed, May 17, 2017 at 9:54 PM, Richard Cochran
> <richardcochran@gmail.com> wrote:
>> On Wed, May 17, 2017 at 04:06:07PM -0700, John Stultz wrote:
>>> On Wed, May 17, 2017 at 10:22 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
>>> > Is there a better way to run the timekeeping code in an userspace
>>> > application? I suspect it would need something like the Linux Kernel
>>> > Library project.
>>>
>>> I dunno. There's probably a cleaner way to go about it, but I also
>>> feel like the benefit of just having the test in the kernel tree is
>>> that it can be managed as a unified whole, rather then the test being
>>> a separate thing and always playing catchup to kernel changes.
>>
>> I vaguely recall a rant on the list years ago from a Linux bigwhig
>> saying how we don't support that kind of thing.  But maybe it is my
>> imagination.  In any case, IMHO running user space tests for chunks of
>> kernel code can be quite useful.
>
> So a few years ago I mentioned this at a testing session at I think
> Linux Plubmers' and Rusty (CC'ed) commented that he had some netfilter
> (or iptables?) simulator code that never made it upstream. However,
> now that kselftests are integrated with the kernel this could change.
> At least that's my memory of the discussion.

Yep, we did it with nfsim, but forward porting was a PITA.  Good luck!

Rusty.

[toc] | [prev] | [next] | [standalone]

#1643526

On Wed, May 17, 2017 at 09:30:31AM -0700, John Stultz wrote:
> So thanks for sending these out. I still need to look them over in
> depth, but can I make another ask here?  :)
> 
> Could you submit your linux-tktest infrastructure to the kselftests dir?

I can, but it's a mess that breaks frequently as the timekeeping and
other kernel code changes. Are you sure you want that in the kernel
tree? :)

-- 
Miroslav Lichvar

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web