Re: [GIT pull] ntp updates for 2.6.31

[Ingo Molnar <mingo@elte.hu>]

* John Stultz <johnstul@us.ibm.com> wrote:

> On Tue, 2009-06-16 at 14:52 +0200, Miroslav Lichvar wrote:
> > On Tue, Jun 16, 2009 at 11:06:47AM +0200, Ingo Molnar wrote:
> > > 
> > > * john stultz <johnstul@us.ibm.com> wrote:
> > > 
> > > > Linus,
> > > >     You probably didn't see this before merging.  Could you yank the
> > > > above two patches?  Miroslav (RH package maintainer for ntpd), has
> > > > voiced concerns that the SHIFT_PLL patch breaks the NTP design and is
> > > > worried it may negatively effect NTP networks of systems running with
> > > > different SHIFT_PLL values.
> > > > 
> > > > While the patch does greatly improve NTP convergence times, and so 
> > > > far no negative results have been seen in tests, its out of an 
> > > > abundance of caution and a desire to keep the adjtimex behavior 
> > > > stable that I requested Thomas and Ingo to hold off on merging 
> > > > this patch, while I work with Miroslav to see if we cannot get the 
> > > > same benefit by adjusting the userspace NTPd.
> > 
> > [..]
> > 
> > > Each OS should converge back to the correct time _as fast as 
> > > physically possible_. If this is a problem and if someone wants 
> > > crappy time and longer periods of convergence for some odd reason 
> > > then that header file change can be edited by hand even. It's not 
> > > like it's that hard to change, if there's genuine interest.
> > > 
> > > So i'm against any revert on this basis. If another basis comes up 
> > > we can reconsider of course. What do you think?
> > 
> > I think the most important one is following the NTP specification.
> > 
> > If Linux really needs to have the fastest PLL, could it be done by
> > modifying the time constant passed in adjtimex structure instead of
> > changing SHIFT_PLL? The PLL response will be exactly the same, but it
> > will allow the applications (and admins) to detect that it is
> > different than expected.
> > 
> > Something like:
> > 
> > --- a/kernel/time/ntp.c
> > +++ b/kernel/time/ntp.c
> > @@ -425,6 +425,8 @@
> >  		time_constant = txc->constant;
> >  		if (!(time_status & STA_NANO))
> >  			time_constant += 4;
> > +		/* We want faster PLL */ 
> > +		time_constant -= 2;
> >  		time_constant = min(time_constant, (long)MAXTC);
> >  		time_constant = max(time_constant, 0l);
> >  	}
> 
> 
> It looks mathematically equivalent, although I've not had time to 
> test it yet. Probably needs a bigger comment :)
> 
> The nice thing with this version is that we're able to expose that 
> the behavior would be different then other systems, but the other 
> side of that coin might be that when the user specifies a 
> time_constant value, the interface will show a different one being 
> used. This might cause some bug reports saying the interface isn't 
> responding properly, or something. Although this is already the 
> case for !STA_NANO, and so far few have noticed.

Sounds good to me. It feels a bit quirky that we 'correct' the 
user-space provided parameter by 2 ... Definitely needs a big 
comment.

	Ingo