LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Roman Zippel <zippel@linux-m68k.org>
To: john stultz <johnstul@us.ibm.com>
Cc: lkml <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Ingo Molnar <mingo@elte.hu>, Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH] correct inconsistent ntp interval/tick_length usage
Date: Sun, 10 Feb 2008 19:45:32 +0100 (CET)	[thread overview]
Message-ID: <Pine.LNX.4.64.0802101917030.3378@scrub.home> (raw)
In-Reply-To: <1202523452.6174.45.camel@localhost.localdomain>

Hi,

On Fri, 8 Feb 2008, john stultz wrote:

> -ENOPATCH
> 
> We're taking weeks to critique fairly small bug fix. I'm sure we both
> have better things to do then continue to misunderstand each other. I'll
> do the testing and will happily ack it if it resolves the issue.

I don't want to just send a patch, I want you to understand why your 
approach is wrong.

> Now, If you're disputing that I'm correcting the wrong side of the
> equation, then we're getting somewhere. But its still not clear to me
> how you're suggesting the other side (which is calculated in
> ntp_update_frequency) be changed.
> [..]
> You keep on bringing up NO_HZ, and again, the bug I'm trying to fix
> occurs with or without NO_HZ. The fix I proposed resolves the issue with
> or without NO_HZ.

The correction is incorrect for NO_HZ.
Let's try it the other way around, as my explanation seem to lack 
something.
Please try to explain what this correction actually means and why it 
should be correct for NO_HZ as well.

> > The only other alternative would be to calculate this correction 
> > dynamically. For this you leave NTP_INTERVAL_LENGTH as is and when 
> > changing clocks you check whether "abs(((cs->xtime_interval * 
> > NTP_INTERVAL_FREQ) >> cs->shift) - NSEC_PER_SEC)" exceeds a certain limit 
> > (e.g. 200usec) and in this case you print a warning message, that the 
> > clock has large base drift value and is a bad ntp source and apply a 
> > correction value. This way the correction only hits the very few system 
> > which might need it and it would be the prefered solution, but it also 
> > requires a few more changes.
> 
> Uh, that seems to be just checking if the xtime_interval is off base, or
> if the ntp correction has gone too far. I just don't see how this
> connects to the issue at hand.

Above is the key to understanding the problem, if this difference is small 
enough there is no need to correct anything.

This is the original patch which introduced the correction:

http://git.kernel.org/?p=linux/kernel/git/torvalds/old-2.6-bkcvs.git;a=commitdiff;h=69c1cd9218e4cf3016b0f435d6ef3dffb5a53860

Keep in mind that at that time PIT was used as the base clock (even if the 
tsc was used, it was relative to PIT). So how much of those assumptions 
are still valid today (especially with NO_HZ enabled)?

bye, Roman

  parent reply	other threads:[~2008-02-10 18:52 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 [this message]
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                                         ` [PATCH] correct inconsistent ntp interval/tick_length usage Jörg-Volker Peetz

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=Pine.LNX.4.64.0802101917030.3378@scrub.home \
    --to=zippel@linux-m68k.org \
    --cc=akpm@linux-foundation.org \
    --cc=johnstul@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rostedt@goodmis.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).