lkml.org 
[lkml]   [2005]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCH 2/2] Reduced NTP rework (part 2)
Hi,

On Tue, 27 Sep 2005, john stultz wrote:

> However, with you idea, on some arches we have to keep two timekeeping
> subsystems running at once, with code trying to smoothly synchronize
> them. I got a little frustrated trying to generate a clean
> implementation and decided to skip that idea for now. I'm not ruling it
> out, but I wanted to explore some other ideas I have.
>
> So I started playing with a few different approaches in the short term,
> trying to adapt some of your ideas (such as using fixed interval time
> accumulation to avoid the multiply at tick time) to my existing code.

I don't want to keep you from playing with different ideas, but I would
strongly suggest you do it first with an userspace simulator. If you
change too much, be prepared to do some sort of analysis of these changes.
A simulator would be one way, but plain math would be fine too.
It's really important that you get the math right first and then develop
the kernel model from that. It would also be ok if you do a first
prototype using nsec, but it's important to also look at the long term
stability of the clock and how well it works together with the NTP daemon.

> 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)

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.
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.
> 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.
> Logic seen in the m68k time.c

I know that code and I really want to replace it with something better. :)
Unfortunately I didn't catch the crap introduced by nsec conversion.

bye, Roman
-
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 20:46    [from the cache]
©2003-2011 Jasper Spaans