[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1207152108.6211.12.camel@jstultz-laptop>
Date: Wed, 02 Apr 2008 09:01:48 -0700
From: John Stultz <johnstul@...ibm.com>
To: Roman Zippel <zippel@...ux-m68k.org>
Cc: lkml <linux-kernel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 1/2] Introduce clocksource_forward_now
On Wed, 2008-04-02 at 13:39 +0200, Roman Zippel wrote:
> Hi,
>
> On Tue, 18 Mar 2008, john stultz wrote:
>
> > My solution is to introduce CLOCK_MONOTONIC_RAW. This exposes a
> > nanosecond based time value, that increments starting at bootup and has
> > no frequency adjustments made to it what so ever.
>
> There is a problem with the time offset since the last update_wall_time()
> call, which isn't taken into account when switching clocks (possibly
> during suspend/resume too), so that the clock might jump back during a
> clock switch.
Yep, thanks for catching that!
> To avoid making the whole more complex it's better to do small cleanup
> first, so this patch introduces clocksource_forward_now() which takes care
> of this offset since the last update_wall_time() call and adds it to the
> clock, so there is no need anymore to deal with it explicitly.
> This is also gets rid of the timekeeping_suspend_nsecs hack, instead of
> waiting until resume, the value is accumulated during suspend. In the end
> there is only a single user of __get_nsec_offset() left, so I integrated
> it back to getnstimeofday().
>
> bye, Roman
>
> Signed-off-by: Roman Zippel <zippel@...ux-m68k.org>
Looks ok to me.
Acked-by: John Stultz <johnstul@...ibm.com>
> ---
> kernel/time/timekeeping.c | 67 ++++++++++++++++++++--------------------------
> 1 file changed, 30 insertions(+), 37 deletions(-)
>
> Index: linux-2.6/kernel/time/timekeeping.c
> ===================================================================
> --- linux-2.6.orig/kernel/time/timekeeping.c
> +++ linux-2.6/kernel/time/timekeeping.c
> @@ -58,27 +58,23 @@ struct clocksource *clock;
>
> #ifdef CONFIG_GENERIC_TIME
> /**
> - * __get_nsec_offset - Returns nanoseconds since last call to periodic_hook
> + * clocksource_forward_now - update clock to the current time
> *
> - * private function, must hold xtime_lock lock when being
> - * called. Returns the number of nanoseconds since the
> - * last call to update_wall_time() (adjusted by NTP scaling)
> + * Forward the current clock to update its state since the last call to
> + * update_wall_time(). This is useful before significant clock changes,
> + * as it avoids having to deal with this time offset explicitly.
> */
> -static inline s64 __get_nsec_offset(void)
> +static void clocksource_forward_now(void)
> {
> cycle_t cycle_now, cycle_delta;
> - s64 ns_offset;
> + s64 nsec;
>
> - /* read clocksource: */
> cycle_now = clocksource_read(clock);
> -
> - /* calculate the delta since the last update_wall_time: */
> cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
> + clock->cycle_last = cycle_now;
>
> - /* convert to nanoseconds: */
> - ns_offset = cyc2ns(clock, cycle_delta);
> -
> - return ns_offset;
> + nsec = cyc2ns(clock, cycle_delta);
> + timespec_add_ns(&xtime, nsec);
> }
>
> /**
> @@ -89,6 +85,7 @@ static inline s64 __get_nsec_offset(void
> */
> void getnstimeofday(struct timespec *ts)
> {
> + cycle_t cycle_now, cycle_delta;
> unsigned long seq;
> s64 nsecs;
>
> @@ -96,7 +93,15 @@ void getnstimeofday(struct timespec *ts)
> seq = read_seqbegin(&xtime_lock);
>
> *ts = xtime;
> - nsecs = __get_nsec_offset();
> +
> + /* read clocksource: */
> + cycle_now = clocksource_read(clock);
> +
> + /* calculate the delta since the last update_wall_time: */
> + cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
> +
> + /* convert to nanoseconds: */
> + nsecs = cyc2ns(clock, cycle_delta);
>
> } while (read_seqretry(&xtime_lock, seq));
>
> @@ -130,21 +135,19 @@ EXPORT_SYMBOL(do_gettimeofday);
> int do_settimeofday(struct timespec *tv)
> {
> unsigned long flags;
> - time_t wtm_sec, sec = tv->tv_sec;
> - long wtm_nsec, nsec = tv->tv_nsec;
>
> if ((unsigned long)tv->tv_nsec >= NSEC_PER_SEC)
> return -EINVAL;
>
> write_seqlock_irqsave(&xtime_lock, flags);
>
> - nsec -= __get_nsec_offset();
> + clocksource_forward_now();
>
> - wtm_sec = wall_to_monotonic.tv_sec + (xtime.tv_sec - sec);
> - wtm_nsec = wall_to_monotonic.tv_nsec + (xtime.tv_nsec - nsec);
> + wall_to_monotonic.tv_sec += xtime.tv_sec - tv->tv_sec;
> + timespec_add_ns(&wall_to_monotonic, xtime.tv_nsec - tv->tv_nsec);
> +
> + xtime = *tv;
>
> - set_normalized_timespec(&xtime, sec, nsec);
> - set_normalized_timespec(&wall_to_monotonic, wtm_sec, wtm_nsec);
> update_xtime_cache(0);
>
> clock->error = 0;
> @@ -170,21 +173,16 @@ EXPORT_SYMBOL(do_settimeofday);
> static void change_clocksource(void)
> {
> struct clocksource *new;
> - cycle_t now;
> - u64 nsec;
>
> new = clocksource_get_next();
>
> if (clock == new)
> return;
>
> - now = clocksource_read(new);
> - nsec = __get_nsec_offset();
> - timespec_add_ns(&xtime, nsec);
> + clocksource_forward_now();
>
> clock = new;
> - clock->cycle_last = now;
> -
> + clock->cycle_last = clocksource_read(new);
> clock->error = 0;
> clock->xtime_nsec = 0;
> clocksource_calculate_interval(clock, NTP_INTERVAL_LENGTH);
> @@ -199,8 +197,8 @@ static void change_clocksource(void)
> */
> }
> #else
> +static inline void clocksource_forward_now(void) { }
> static inline void change_clocksource(void) { }
> -static inline s64 __get_nsec_offset(void) { return 0; }
> #endif
>
> /**
> @@ -264,8 +262,6 @@ void __init timekeeping_init(void)
> static int timekeeping_suspended;
> /* time in seconds when suspend began */
> static unsigned long timekeeping_suspend_time;
> -/* xtime offset when we went into suspend */
> -static s64 timekeeping_suspend_nsecs;
>
> /**
> * timekeeping_resume - Resumes the generic timekeeping subsystem.
> @@ -291,8 +287,6 @@ static int timekeeping_resume(struct sys
> wall_to_monotonic.tv_sec -= sleep_length;
> total_sleep_time += sleep_length;
> }
> - /* Make sure that we have the correct xtime reference */
> - timespec_add_ns(&xtime, timekeeping_suspend_nsecs);
> update_xtime_cache(0);
> /* re-base the last cycle value */
> clock->cycle_last = clocksource_read(clock);
> @@ -317,8 +311,7 @@ static int timekeeping_suspend(struct sy
> timekeeping_suspend_time = read_persistent_clock();
>
> write_seqlock_irqsave(&xtime_lock, flags);
> - /* Get the current xtime offset */
> - timekeeping_suspend_nsecs = __get_nsec_offset();
> + clocksource_forward_now();
> timekeeping_suspended = 1;
> write_sequnlock_irqrestore(&xtime_lock, flags);
>
> @@ -459,10 +452,10 @@ void update_wall_time(void)
> */
> while (offset >= clock->cycle_interval) {
> /* accumulate one interval */
> - clock->xtime_nsec += clock->xtime_interval;
> - clock->cycle_last += clock->cycle_interval;
> offset -= clock->cycle_interval;
> + clock->cycle_last += clock->cycle_interval;
>
> + clock->xtime_nsec += clock->xtime_interval;
> if (clock->xtime_nsec >= (u64)NSEC_PER_SEC << clock->shift) {
> clock->xtime_nsec -= (u64)NSEC_PER_SEC << clock->shift;
> xtime.tv_sec++;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists