Re: [PATCH] correct inconsistent ntp interval/tick_length usage
From: Jörg-Volker Peetz
Date: Fri Feb 29 2008 - 14:00:24 EST
john stultz wrote:
<snip>
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index c88b591..fe25c94 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -43,19 +43,32 @@ long time_freq; /* frequency offset (scaled ppm)*/
static long time_reftime; /* time at last adjustment (s) */
long time_adjust;
+static s64 granularity_error_adjust;
+
+void ntp_set_granularity_error(s64 len)
+{
+ granularity_error_adjust = len * NTP_INTERVAL_FREQ;
+}
+
static void ntp_update_frequency(void)
{
u64 second_length = (u64)(tick_usec * NSEC_PER_USEC * USER_HZ)
<< TICK_LENGTH_SHIFT;
- second_length += (s64)CLOCK_TICK_ADJUST << TICK_LENGTH_SHIFT;
- second_length += (s64)time_freq << (TICK_LENGTH_SHIFT - SHIFT_NSEC);
+ s64 adj;
+
+ /* Compensate for clocksource granularity error */
+ second_length += granularity_error_adjust;
+
+ /* Scale the base second length by the frequency adjustment */
+ adj = second_length * time_freq;
+ do_div(adj, 1000000);
+ second_length += adj>>SHIFT_NSEC;
tick_length_base = second_length;
+ do_div(tick_length_base, NTP_INTERVAL_FREQ);
do_div(second_length, HZ);
tick_nsec = second_length >> TICK_LENGTH_SHIFT;
-
- do_div(tick_length_base, NTP_INTERVAL_FREQ);
}
/**
<snip>
Hi John,
out of curiosity and inspired by the patch you suggested, I did a test
with the following ntp_update_frequency function in kernel/time/ntp.c
of kernel 2.6.24.3 using NO_HZ and the hpet timer:
static void ntp_update_frequency(void)
{
s64 adj;
u64 second_length = (u64)(tick_usec * NSEC_PER_USEC * USER_HZ)
<< TICK_LENGTH_SHIFT;
printk(KERN_NOTICE "*ntp* timefreq = %lld\n", (s64)time_freq);
printk(KERN_NOTICE "*ntp* s len = %lld\n", second_length);
printk(KERN_NOTICE "*ntp* corr 1 = %lld\n",
(s64)time_freq << (TICK_LENGTH_SHIFT - SHIFT_NSEC));
/* Scale the base second length by the frequency adjustment */
adj = second_length * time_freq;
do_div(adj, 1000000);
printk(KERN_NOTICE "*ntp* corr 2 = %lld\n", adj>>SHIFT_NSEC);
second_length += (s64)time_freq << (TICK_LENGTH_SHIFT - SHIFT_NSEC);
tick_length_base = second_length;
do_div(tick_length_base, NTP_INTERVAL_FREQ);
do_div(second_length, HZ);
tick_nsec = second_length >> TICK_LENGTH_SHIFT;
}
Running this kernel and ntpd I get numbers like the following in my
syslog file:
Feb 29 12:28:16 skadi kernel: *ntp* timefreq = 305345
Feb 29 12:28:16 skadi kernel: *ntp* s len = 4294967296000000000
Feb 29 12:28:16 skadi kernel: *ntp* corr 1 = 320177438720
Feb 29 12:28:16 skadi kernel: *ntp* corr 2 = 3030411349
Feb 29 12:36:53 skadi kernel: *ntp* timefreq = 730456
Feb 29 12:36:53 skadi kernel: *ntp* s len = 4294967296000000000
Feb 29 12:36:53 skadi kernel: *ntp* corr 1 = 765938630656
Feb 29 12:36:53 skadi kernel: *ntp* corr 2 = 2434829845
Feb 29 12:39:03 skadi kernel: *ntp* timefreq = 868771
Feb 29 12:39:03 skadi kernel: *ntp* s len = 4294967296000000000
Feb 29 12:39:03 skadi kernel: *ntp* corr 1 = 910972420096
Feb 29 12:39:03 skadi kernel: *ntp* corr 2 = 2301870005
So the original correction and the correction suggested by you
differ significantly by a factor of approximately 1000.
Incidentally, both corrections are nearly neglectable compared to
second_length.
But I think the correction suggested by you is calculated wrong due to
an overflow of the multiplication
second_length * time_freq
Calculating the correction as
(second_length / 1000000) * time_freq
the correction would be 1311446788997120000 which is much bigger as
the original correction 320177438720 (by a factor of 10^7).
Is it normal to have a second_length of approx. 4 * 10^18 ?
In what units?
--
Regards,
Jörg-Volker.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/