Messages in this thread | | | Subject | Re: [RFC][PATCH 2/2] Reduced NTP rework (part 2) | From | john stultz <> | Date | Thu, 29 Sep 2005 12:33:42 -0700 |
| |
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/
| |