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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |