Hello,
Roman had asked me to look at his recent set of time patches. So I did have a
quick look.
Generally I think removing code that proved to work for about 10 years or more
without having a replacement is to be avoided. What Roman did implement is a move
towards the new NTP kernel algorithms, but it's "neither fish nor meat": In part
it changes the semantics of syscalls, and it changes constants in an incompatible
way.
Specifically when considering that there exists a proven kernel code replacement
for some years now (which I'm at integrating to 2.6.15), I wonder whether it's a
wise decision to start hacking the code in such a way.
Roman may know more than I about how to make efficient 64-bit math on 32-bit
archs, but AFAIK he broke a few things:
15_time_offset and 18_time_freq change some well-known constants (like MAXPHASE)
by three orders of magnitude.
the new adjtime() (16_time_adjust, 12_time_adj) changes the semantics: Since about
Linux 0.99, adjtime() had the adjtime_is_accurate property, i.e. on the long term
it behaved like an addition.
Currently, unless I'm wrong, adjtime() for 1?s with HZ == 1024 would correct the
clock by nothing, while an adjtime() for 2?s would correct the clock by 1024ns on
the same system. For HZ == 100 things seem OK however.
My philosophy is "do it correct first, then optimize". Some test with the code
from last weekend (measure the time of an external "Pulse per Second" (plus the
first two derivations of it)) gave results like this (virtual nanosecond
granularity, i.e. get_offset() still has microsecond resolution, ntpd trying to
calibrate the clock):
assert 1120 time 1139767167.000402394 delta 1.000000030 jitter 1535064674
assert 1121 time 1139767168.000402457 delta 1.000000063 jitter 33
assert 1122 time 1139767169.000402521 delta 1.000000064 jitter 1
assert 1123 time 1139767170.000402586 delta 1.000000065 jitter 1
assert 1124 time 1139767171.000402635 delta 1.000000049 jitter -16
assert 1125 time 1139767172.000402667 delta 1.000000032 jitter -17
assert 1126 time 1139767173.000402684 delta 1.000000017 jitter -15
assert 1127 time 1139767174.000402688 delta 1.000000004 jitter -13
assert 1128 time 1139767175.000402684 delta 0.999999996 jitter -8
assert 1129 time 1139767176.000402676 delta 0.999999992 jitter -4
assert 1130 time 1139767177.000402668 delta 0.999999992 jitter 0
assert 1131 time 1139767178.000402660 delta 0.999999992 jitter 0
assert 1132 time 1139767179.000402654 delta 0.999999994 jitter 2
assert 1133 time 1139767180.000402650 delta 0.999999996 jitter 2
assert 1134 time 1139767181.000402648 delta 0.999999998 jitter 2
assert 1135 time 1139767182.000402648 delta 1.000000000 jitter 2
assert 1136 time 1139767183.000402648 delta 1.000000000 jitter 0
(The last released patch snapshot is named "PPSkit-light-alpha-2955m-
2.6.15.1.diff.gz" and can be found in the directory /pub/linux/daemons/ntp/PPS of
your favourite Linux mirror. At the moment there exist two uncommitted change sets
that have to be tested first)
So personally I'd suggest to consider that code base. Options are eiher
1) to optimize/streamline what you think is too ugly
2) make the whole NTP lcok calibration optional if you think it's computationally
too heavy (however, both GNOME and KDE hit the CPU much more than any of these
changes)
Regards,
Ulrich
P.S. I'll attach a longer sample (12kB) of my timing test.
Hi,
On Tue, 14 Feb 2006, Ulrich Windl wrote:
> 15_time_offset and 18_time_freq change some well-known constants (like MAXPHASE)
> by three orders of magnitude.
>
> the new adjtime() (16_time_adjust, 12_time_adj) changes the semantics: Since about
> Linux 0.99, adjtime() had the adjtime_is_accurate property, i.e. on the long term
> it behaved like an addition.
I disagree, could you please explain how you come to this conclusion?
The patches don't change the behaviour beyond that they increase
resolution and precision. Only the final patch changes the ntp code to
match the behaviour of ntp reference code without including all its mess.
bye, Roman
On 14 Feb 2006 at 14:21, Roman Zippel wrote:
> Hi,
>
> On Tue, 14 Feb 2006, Ulrich Windl wrote:
>
> > 15_time_offset and 18_time_freq change some well-known constants (like MAXPHASE)
> > by three orders of magnitude.
--- linux-2.6-mm.orig/include/linux/timex.h 2005-12-21 12:12:00.000000000 +0100
+++ linux-2.6-mm/include/linux/timex.h 2005-12-21 12:12:08.000000000 +0100
@@ -95,11 +95,11 @@
#define SHIFT_USEC 16 /* frequency offset scale (shift) */
#define FINENSEC (1L << SHIFT_SCALE) /* ~1 ns in phase units */
-#define MAXPHASE 512000L /* max phase error (us) */
+#define MAXPHASE 500000000L /* max phase error (ns) */
#define MAXFREQ (512L << SHIFT_USEC) /* max frequency error (ppm) */
#define MINSEC 16L /* min interval between updates (s) */
#define MAXSEC 1200L /* max interval between updates (s) */
-#define NTP_PHASE_LIMIT (MAXPHASE << 5) /* beyond max. dispersion */
+#define NTP_PHASE_LIMIT ((MAXPHASE / 1000) << 5) /* beyond max. dispersion */
> >
> > the new adjtime() (16_time_adjust, 12_time_adj) changes the semantics: Since about
> > Linux 0.99, adjtime() had the adjtime_is_accurate property, i.e. on the long term
> > it behaved like an addition.
>
> I disagree, could you please explain how you come to this conclusion?
+ tick_nsec_curr += time_adjust * 1000 / HZ;
Assuming 1024Hz interrupt frequency:
(1?s * 1000) / 1024 == 0ns; 0 * 1024 == 0?s, not 1?s
(2?s * 1000) / 1024 == 1ns; 1 * 1024 == 1.024?s, not 2?s
> The patches don't change the behaviour beyond that they increase
> resolution and precision. Only the final patch changes the ntp code to
> match the behaviour of ntp reference code without including all its mess.
It's quite hard to tell: The code is very different what I've ever seen.
Regards,
Ulrich
Hi,
On Tue, 14 Feb 2006, Ulrich Windl wrote:
> > > 15_time_offset and 18_time_freq change some well-known constants (like MAXPHASE)
> > > by three orders of magnitude.
>
> --- linux-2.6-mm.orig/include/linux/timex.h 2005-12-21 12:12:00.000000000 +0100
> +++ linux-2.6-mm/include/linux/timex.h 2005-12-21 12:12:08.000000000 +0100
> @@ -95,11 +95,11 @@
> #define SHIFT_USEC 16 /* frequency offset scale (shift) */
> #define FINENSEC (1L << SHIFT_SCALE) /* ~1 ns in phase units */
>
> -#define MAXPHASE 512000L /* max phase error (us) */
> +#define MAXPHASE 500000000L /* max phase error (ns) */
> #define MAXFREQ (512L << SHIFT_USEC) /* max frequency error (ppm) */
> #define MINSEC 16L /* min interval between updates (s) */
> #define MAXSEC 1200L /* max interval between updates (s) */
> -#define NTP_PHASE_LIMIT (MAXPHASE << 5) /* beyond max. dispersion */
> +#define NTP_PHASE_LIMIT ((MAXPHASE / 1000) << 5) /* beyond max. dispersion */
The reference timex.h has the same change for MAXPHASE and NTP_PHASE_LIMIT
is Linux specific. Where is the problem?
> > > the new adjtime() (16_time_adjust, 12_time_adj) changes the semantics: Since about
> > > Linux 0.99, adjtime() had the adjtime_is_accurate property, i.e. on the long term
> > > it behaved like an addition.
> >
> > I disagree, could you please explain how you come to this conclusion?
>
> + tick_nsec_curr += time_adjust * 1000 / HZ;
>
> Assuming 1024Hz interrupt frequency:
> (1?s * 1000) / 1024 == 0ns; 0 * 1024 == 0?s, not 1?s
> (2?s * 1000) / 1024 == 1ns; 1 * 1024 == 1.024?s, not 2?s
Ok, I didn't put much effort into optimizing it for uncommon HZ values.
Why is it so important? It's currently unused on any Linux machine
synchronized via NTP.
> > The patches don't change the behaviour beyond that they increase
> > resolution and precision. Only the final patch changes the ntp code to
> > match the behaviour of ntp reference code without including all its mess.
>
> It's quite hard to tell: The code is very different what I've ever seen.
Actually it's not that hard, under http://www.xs4all.nl/~zippel/ntp/ you
can also find the user space test code I used to verify it.
kernel.tar.Z is the old reference code, which the current Linux code is
based on, under patches-kernel you can find a number of patches to convert
it to the new model and which match the new kernel implementation.
I updated the kern.dat and added a nano.sh script with matching test
parameters for nanokernel, so you can compare the output of both test
programms.
bye, Roman
On 14 Feb 2006 at 15:33, Roman Zippel wrote:
[...]
> > Assuming 1024Hz interrupt frequency:
> > (1?s * 1000) / 1024 == 0ns; 0 * 1024 == 0?s, not 1?s
> > (2?s * 1000) / 1024 == 1ns; 1 * 1024 == 1.024?s, not 2?s
>
> Ok, I didn't put much effort into optimizing it for uncommon HZ values.
> Why is it so important? It's currently unused on any Linux machine
> synchronized via NTP.
Roman,
how do you know? When using "disable kernel", NTP relies on adjtime() to adjust
the time. Some people even prefer that, because the algorithms do floating point
math in user space instead of fixed-point maths in kernel space.
[...]
Regards,
Ulrich
Hi,
On Wed, 15 Feb 2006, Ulrich Windl wrote:
> > > Assuming 1024Hz interrupt frequency:
> > > (1?s * 1000) / 1024 == 0ns; 0 * 1024 == 0?s, not 1?s
> > > (2?s * 1000) / 1024 == 1ns; 1 * 1024 == 1.024?s, not 2?s
> >
> > Ok, I didn't put much effort into optimizing it for uncommon HZ values.
> > Why is it so important? It's currently unused on any Linux machine
> > synchronized via NTP.
>
> Roman,
>
> how do you know? When using "disable kernel", NTP relies on adjtime() to adjust
> the time. Some people even prefer that, because the algorithms do floating point
> math in user space instead of fixed-point maths in kernel space.
This still requires they choose an uncommon HZ value, which is not really
likely. Anyway, it's not really difficult to add the remainder to
time_adj_curr. Since the adjtime() has only a usec resolution and this
rounding error is only 1 usec, I didn't consider it to be that important.
bye, Roman
Hi!
> So personally I'd suggest to consider that code base. Options are eiher
> 1) to optimize/streamline what you think is too ugly
> 2) make the whole NTP lcok calibration optional if you think it's computationally
> too heavy (however, both GNOME and KDE hit the CPU much more than any of these
> changes)
Well, being less resource-hungry is similar to be less lethal than loaded gun.
Kernel time keeping should better have <0.1% cpu overhead.
Pavel
--
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms