lkml.org 
[lkml]   [2005]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC][PATCH 2/2] Reduced NTP rework (part 2)
From
Date
On Thu, 2005-09-29 at 20:43 +0200, Roman Zippel wrote:
> On Tue, 27 Sep 2005, john stultz wrote:
> > The idea being:
> >
> > update_wall_clock():
> > ticks = jiffies - wall_jiffies
> > while (ticks):
> > ticks--
> > xtime += tick_nsec + ntp_adjustment
> >
> >
> > isn't that different from:
> >
> > timekeeping_periodic_hook():
> > now = timesource_read(ts)
> > delta_cycle = now - last
> > while (delta_cycle > interval_cycle):
> > delta_cycle -= interval_cycle
> > system_time += interval_nsec
>
> BTW that's not what you do in the first part of the patch:
>
> +static void ntp_advance(unsigned long interval_ns)

Well, I was trying to describe what I am going to follow the NTP patches
with.

So yes, the reduced NTP rework patches are not discussed in the above
(but ntp_advance() does have a place in the above, I just left it out to
shorten the comparison), but they allow the two examples above to look
similar.

For clarity here's the ntp details included

update_wall_clock():
ticks = jiffies - wall_jiffies
while (ticks):
ticks--
xtime += tick_nsec + ntp_adjustment
ntp_advance(tick_nsec)


timekeeping_periodic_hook():
now = timesource_read(ts)
delta_cycle = now - last
while (delta_cycle > interval_cycle):
delta_cycle -= interval_cycle
system_time += interval_nsec
ntp_advance(interval_nsec)


> I'm quite sure that the interval_ns is wrong, it's important to advance
> the ntp state in constant intervals (i.e. interval_cycle). Your patch
> already includes time adjustments and e.g. the "while (interval_ns >=
> tick_nsec)" loop is not executed anymore, once time_adjust_step becomes
> negative.

Commenting the specific code would help clarify this. If I'm
understanding you, you're talking about the following logic:

static void ntp_advance(unsigned long interval_ns):
static unsigned long interval_sum;

/* increment the interval sum */
interval_sum += interval_ns

/* calculate the per tick singleshot adjtime adjustment step */
while (interval_ns >= tick_nsec):
time_adjust_step = time_adjust
if (time_adjust_step):
time_adjust_step = min(time_adjust_step, tickadj)
time_adjust_step = max(time_adjust_step, -tickadj)
time_adjust -= time_adjust_step
interval_ns -= tick_nsec

I'm not sure I understand the problem if time_adjust_step becomes
negative.

> In general I would prefer it if we could finalize the basic design first,
> before doing such changes, otherwise I'm afraid we need a cleanup of the
> cleanup.

Well, that's evolution. :) But really, the reduced ntp rework stuff
isn't a cleanup. Its just the bare minimum changes to NTP that I'm
needing for my generic timeofday code. Hopefully the reduced changes
will clarify what exactly I need (or think I need ;) from the NTP
subsystem to get my timekeeping code to function properly. Then maybe
you (or anyone else - don't let Roman have all the fun) can point to a
better way.


> > The only difference between continuous and tick based systems would then
> > be in gettimeofday() (which really could be the same with a simple
> > #define)
> >
> > continuous_gettimeofday():
> > now = timesource_read(ts)
> > delta_cycle = now - last
> > delta_nsec = cyc2ns(timesource, delta_cycle)
> > return system_time + delta_nsec
> >
> > tick_gettime():
> > now = timesource_read(jiffes_timesource)
> > delta_cycle = now - last
> > delta_nsec = cyc2ns(timesource, delta_cycle)
> > delta_nsec += arch_get_offset()
> > return system_time + delta_nsec
>
> The basic idea of gettimeofday is of course always the same: "base +
> get_offset() * mult". I can understand the temptation to unify the
> implementation, but please accept the current reality that we have
> different gettimeofday implementations (for whatever reasons), so unifying
> them would be a premature change. If the situation changes later we can
> still do that unification.

I'm sorta going at it from the other way (call me optimistic :), where
I'm trying to unify what I can until I hit the exception. Then I'll
happily break out an arch specific gettimeofday implementation.

Once again, I do appreciate your feedback. Hopefully I'll have patches
out later today for you to look at.

thanks
-john



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-09-29 21:37    [W:1.399 / U:1.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site