| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <BANLkTinARof1CUhvRoYC4jQqwAsWOX5PVw@mail.gmail.com>
Date: Wed, 1 Jun 2011 17:54:46 -0700
From: Arve Hjønnevåg <arve@...roid.com>
To: John Stultz <john.stultz@...aro.org>
Cc: linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 2/2] rtc: Avoid accumulating time drift in suspend/resume
On Tue, May 31, 2011 at 11:07 PM, John Stultz <john.stultz@...aro.org> wrote:
> Because the RTC interface is only a second granular interface,
> each time we read from the RTC for suspend/resume, we introduce a
> half second (on average) of error.
>
> In order to avoid this error accumulating as the system is suspended
> over and over, this patch measures the time delta between the RTC
> and the system CLOCK_REALTIME.
>
> If the delta is less then 2 seconds from the last suspend, we compensate
> by using the previous time delta (keeping it close). If it is larger
> then 2 seconds, we assume the clock was set or has been changed, so we
> do no correction and update the delta.
>
> Note: If NTP is running, ths could seem to "fight" with the NTP corrected
> time, where as if the system time was off by 1 second, and NTP slewed the
> value in, a suspend/resume cycle could undo this correction, by trying to
> restore the previous offset from the RTC. However, without this patch,
> since each read could cause almost a full second worth of error, its
> possible to get almost 2 seconds of error just from the suspend/resume
> cycle alone, so this about equal to any offset added by the compensation.
>
> Further on systems that suspend/resume frequently, this should keep time
> closer then NTP could compensate for if the errors were allowed to
> accumulate.
>
> Credits to Arve Hjønnevåg for suggesting this solution.
>
> This patch also improves some of the variable names and adds more clear
> comments.
>
> CC: Arve Hjønnevåg <arve@...roid.com>
> CC: Thomas Gleixner <tglx@...utronix.de>
> Signed-off-by: John Stultz <john.stultz@...aro.org>
> ---
> drivers/rtc/class.c | 65 +++++++++++++++++++++++++++++++++++++-------------
> 1 files changed, 48 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
> index 4194e59..a619228 100644
> --- a/drivers/rtc/class.c
> +++ b/drivers/rtc/class.c
> @@ -41,20 +41,41 @@ static void rtc_device_release(struct device *dev)
> * system's wall clock; restore it on resume().
> */
>
> -static time_t oldtime;
> -static struct timespec oldts;
> +static struct timespec old_rtc, old_system, old_delta;
> +
>
> static int rtc_suspend(struct device *dev, pm_message_t mesg)
> {
> struct rtc_device *rtc = to_rtc_device(dev);
> struct rtc_time tm;
> -
> + struct timespec delta, delta_delta;
> if (strcmp(dev_name(&rtc->dev), CONFIG_RTC_HCTOSYS_DEVICE) != 0)
> return 0;
>
> + /* snapshot the current RTC and system time at suspend*/
> rtc_read_time(rtc, &tm);
> - ktime_get_ts(&oldts);
> - rtc_tm_to_time(&tm, &oldtime);
> + getnstimeofday(&old_system);
> + rtc_tm_to_time(&tm, &old_rtc.tv_sec);
> +
> +
> + /*
> + * To avoid drift caused by repeated suspend/resumes,
> + * which each can add ~1 second drift error,
> + * try to compensate so the difference in system time
> + * and rtc time stays close to constant.
> + */
> + delta = timespec_sub(old_system, old_rtc);
> + delta_delta = timespec_sub(delta, old_delta);
> + if (abs(delta_delta.tv_sec) >= 2) {
> + /*
> + * if delta_delta is too large, assume time correction
> + * has occured and set old_delta to the current delta.
> + */
> + old_delta = delta;
> + } else {
> + /* Otherwise try to adjust old_system to compensate */
> + old_system = timespec_sub(old_system, delta_delta);
> + }
>
> return 0;
> }
> @@ -63,32 +84,42 @@ static int rtc_resume(struct device *dev)
> {
> struct rtc_device *rtc = to_rtc_device(dev);
> struct rtc_time tm;
> - time_t newtime;
> - struct timespec time;
> - struct timespec newts;
> + struct timespec new_system, new_rtc;
> + struct timespec sleep_time;
>
> if (strcmp(dev_name(&rtc->dev), CONFIG_RTC_HCTOSYS_DEVICE) != 0)
> return 0;
>
> - ktime_get_ts(&newts);
> + /* snapshot the current rtc and system time at resume */
> + getnstimeofday(&new_system);
> rtc_read_time(rtc, &tm);
> if (rtc_valid_tm(&tm) != 0) {
> pr_debug("%s: bogus resume time\n", dev_name(&rtc->dev));
> return 0;
> }
> - rtc_tm_to_time(&tm, &newtime);
> - if (newtime <= oldtime) {
> - if (newtime < oldtime)
> + rtc_tm_to_time(&tm, &new_rtc.tv_sec);
> + new_rtc.tv_nsec = 0;
> +
> + if (new_rtc.tv_sec <= old_rtc.tv_sec) {
> + if (new_rtc.tv_sec < old_rtc.tv_sec)
> pr_debug("%s: time travel!\n", dev_name(&rtc->dev));
> return 0;
> }
> - /* calculate the RTC time delta */
> - set_normalized_timespec(&time, newtime - oldtime, 0);
>
> - /* subtract kernel time between rtc_suspend to rtc_resume */
> - time = timespec_sub(time, timespec_sub(newts, oldts));
> + /* calculate the RTC time delta (sleep time)*/
> + sleep_time = timespec_sub(new_rtc, old_rtc);
> +
> + /*
> + * Since these RTC suspend/resume handlers are not called
> + * at the very end of suspend or the start of resume,
> + * some run-time may pass on either sides of the sleep time
> + * so subtract kernel run-time between rtc_suspend to rtc_resume
> + * to keep things accurate.
> + */
> + sleep_time = timespec_sub(sleep_time,
> + timespec_sub(new_system, old_system));
What happens if sleep_time is negative? I think this need to be
clamped to 0 to avoid backwards jumps when you wake up more than once
without the rtc advancing.
>
> - timekeeping_inject_sleeptime(&time);
> + timekeeping_inject_sleeptime(&sleep_time);
> return 0;
> }
>
> --
> 1.7.3.2.146.gca209
>
>
--
Arve Hjønnevåg
--
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