LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Jörg-Volker Peetz" <jvpeetz@web.de>
To: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] correct inconsistent ntp interval/tick_length usage
Date: Fri, 29 Feb 2008 19:54:45 +0100	[thread overview]
Message-ID: <fq9kdl$j3$1@ger.gmane.org> (raw)
In-Reply-To: <1203647951.6150.80.camel@localhost.localdomain>

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.


      parent reply	other threads:[~2008-02-29 19:00 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-24  2:38 john stultz
2008-01-24  9:14 ` Valdis.Kletnieks
2008-01-25 14:07 ` Roman Zippel
2008-01-29  2:28   ` john stultz
2008-01-29  4:02     ` Roman Zippel
2008-01-30  2:14       ` john stultz
2008-01-31  1:55         ` Roman Zippel
2008-01-31  2:16           ` john stultz
2008-01-31  5:02             ` Roman Zippel
2008-02-02  1:02               ` John Stultz
2008-02-08 17:33                 ` Roman Zippel
2008-02-09  2:17                   ` john stultz
2008-02-09  4:47                     ` Roman Zippel
2008-02-09  5:04                       ` Andrew Morton
2008-02-09 14:13                         ` Roman Zippel
2008-02-10 18:45                     ` Roman Zippel
2008-02-12  0:09                       ` john stultz
2008-02-12 14:36                         ` Roman Zippel
2008-02-14  4:36                           ` john stultz
2008-02-16  4:24                             ` Roman Zippel
2008-02-19  1:02                               ` john stultz
2008-02-19  4:04                                 ` Roman Zippel
2008-02-20  1:50                                   ` john stultz
2008-02-20 17:08                                     ` Roman Zippel
2008-02-22  2:39                                       ` john stultz
2008-02-25 14:44                                         ` Roman Zippel
2008-02-29  4:49                                         ` [PATCH] Remove obsolete CLOCK_TICK_ADJUST Roman Zippel
2008-02-29  6:25                                           ` Ray Lee
2008-02-29 13:31                                             ` Roman Zippel
2008-02-29 22:08                                           ` Andrew Morton
2008-02-29 22:27                                             ` Roman Zippel
2008-02-29 22:38                                               ` Andrew Morton
2008-02-29 23:11                                               ` john stultz
2008-02-29 23:11                                           ` john stultz
2008-03-02  4:03                                             ` Roman Zippel
2008-02-29 18:54                                         ` Jörg-Volker Peetz [this message]

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='fq9kdl$j3$1@ger.gmane.org' \
    --to=jvpeetz@web.de \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH] correct inconsistent ntp interval/tick_length usage' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).