On Fri, 29 Sep 2006 23:58:21 -0000
Thomas Gleixner <[email protected]> wrote:
> From: John Stultz <[email protected]>
>
> persistent clock support: do proper timekeeping across suspend/resume.
How?
> +/* Weak dummy function for arches that do not yet support it.
> + * XXX - Do be sure to remove it once all arches implement it.
> + */
> +unsigned long __attribute__((weak)) read_persistent_clock(void)
> +{
> + return 0;
> +}
Seconds? microseconds? jiffies? walltime? uptime?
Needs some comments.
> void __init timekeeping_init(void)
> {
> - unsigned long flags;
> + unsigned long flags, sec = read_persistent_clock();
So it apparently returns seconds-since-epoch?
If so, why?
> write_seqlock_irqsave(&xtime_lock, flags);
>
> @@ -758,11 +769,18 @@ void __init timekeeping_init(void)
> clocksource_calculate_interval(clock, tick_nsec);
> clock->cycle_last = clocksource_read(clock);
>
> + xtime.tv_sec = sec;
> + xtime.tv_nsec = (jiffies % HZ) * (NSEC_PER_SEC / HZ);
Why is it valid to take the second from the persistent clock and the
fraction-of-a-second from jiffies? Some comments describing the
implementation would improve its understandability and maintainability.
This statement can set xtime.tv_nsec to a value >= NSEC_PER_SEC. Should it
not be normalised?
> + set_normalized_timespec(&wall_to_monotonic,
> + -xtime.tv_sec, -xtime.tv_nsec);
> +
> write_sequnlock_irqrestore(&xtime_lock, flags);
> }
>
>
> static int timekeeping_suspended;
> +static unsigned long timekeeping_suspend_time;
In what units?
> +
> /**
> * timekeeping_resume - Resumes the generic timekeeping subsystem.
> * @dev: unused
> @@ -773,14 +791,23 @@ static int timekeeping_suspended;
> */
> static int timekeeping_resume(struct sys_device *dev)
> {
> - unsigned long flags;
> + unsigned long flags, now = read_persistent_clock();
Would whoever keeps doing that please stop it? This:
unsigned long flags;
unsigned long now = read_persistent_clock();
is more readable and makes for more readable patches in the future.
> write_seqlock_irqsave(&xtime_lock, flags);
> - /* restart the last cycle value */
> +
> + if (now && (now > timekeeping_suspend_time)) {
> + unsigned long sleep_length = now - timekeeping_suspend_time;
> + xtime.tv_sec += sleep_length;
> + jiffies_64 += sleep_length * HZ;
sleep_length will overflow if we slept for more than 49 days, and HZ=1000.
> + }
> + /* re-base the last cycle value */
> clock->cycle_last = clocksource_read(clock);
> clock->error = 0;
> timekeeping_suspended = 0;
> write_sequnlock_irqrestore(&xtime_lock, flags);
> +
> + hrtimer_notify_resume();
> +
> return 0;
> }
>
> @@ -790,6 +817,7 @@ static int timekeeping_suspend(struct sy
>
> write_seqlock_irqsave(&xtime_lock, flags);
> timekeeping_suspended = 1;
> + timekeeping_suspend_time = read_persistent_clock();
> write_sequnlock_irqrestore(&xtime_lock, flags);
> return 0;
> }
>
> --
-
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]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
