lkml.org 
[lkml]   [2009]   [Sep]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] update clocksource raw_time in timekeeping_suspend
    From
    Date
    On Wed, 2009-09-09 at 15:35 +0800, ye janboe wrote:
    > after resume from suspend, raw_time is not updated in
    > timekeeping_suspend. CLOCK_MONOTONIC_RAW could not get the real hw
    > time.
    > This patch fix this issue.

    Hmm.. I'll admit suspend probably was less considered with
    CLOCK_MONOTONIC_RAW, so the semantics aren't well established.

    However, I do think we want CLOCK_MONOTONIC_RAW to at-least closely map
    to CLOCK_MONOTONIC (but *not* be NTP adjusted). I think that is what
    folks would most likely expect.

    However, that isn't what this patch seems to do.

    Over suspend, I believe all hardware counters reset, so this patch would
    seem to try to subtract the value back.

    This sort of makes sense for something like the TSC, which never wraps,
    so the raw_time would be set back to a tranlation of the actual TSC
    counter, but for other clocksources like the ACPI PM, it would only
    subtract at most 5 seconds. So this leaks hardware specific detail in an
    ugly way.

    Instead I suspect the most intuitive change would be to add in the
    sleep_length to the raw time. This keeps CLOCK_MONOTONIC_RAW behaving
    similarly to CLOCK_MONOTONIC, which I believe makes it more useful for
    folks using CLOCK_MONOTONIC_RAW for things like tuning time
    synchronization.

    But let me know more why you chose this implementation and maybe that
    will show some better insight in to how you expect it to behave.

    thanks
    -john



    > Signed-off-by: janboe <janboe.ye@gmail.com>
    > ---
    > kernel/time/timekeeping.c | 6 ++++++
    > 1 files changed, 6 insertions(+), 0 deletions(-)
    >
    > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
    > index e8c77d9..8420b85 100644
    > --- a/kernel/time/timekeeping.c
    > +++ b/kernel/time/timekeeping.c
    > @@ -331,6 +331,8 @@ static unsigned long timekeeping_suspend_time;
    > static int timekeeping_resume(struct sys_device *dev)
    > {
    > unsigned long flags;
    > + s64 nsec;
    > + cycle_t last_cycle, cycle_delta;
    > unsigned long now = read_persistent_clock();
    >
    > clocksource_resume();
    > @@ -346,8 +348,12 @@ static int timekeeping_resume(struct sys_device *dev)
    > }
    > update_xtime_cache(0);
    > /* re-base the last cycle value */
    > + last_cycle = clock->cycle_last;
    > clock->cycle_last = 0;
    > clock->cycle_last = clocksource_read(clock);
    > + cycle_delta = clock->cycle_last - last_cycle;
    > + nsec = cyc2ns(clock, cycle_delta);
    > + timespec_add_ns(&clock->raw_time, nsec);
    > clock->error = 0;
    > timekeeping_suspended = 0;
    > write_sequnlock_irqrestore(&xtime_lock, flags);



    \
     
     \ /
      Last update: 2009-09-10 01:11    [W:3.916 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site