Detect clock skew during suspend
cover Detect clock skew during suspend
Hi,
If the CMOS timer is changed when the system is suspended to disk in such a way that the time during the resume turns out to be earlier than the time before the suspend, the resume often fails and the system hangs (spins forever in the idle thread) due to driver problems.
For this reason it seems reasonable to make the timer .resume() routines detect such situations and prevent them from happening, which is done in the following two patches for i386 and x86_64.
Greetings,
Rafael
Signature · 2 lines
You never change things by fighting the existing reality. R. Buckminster Fuller
RFC PATCH 1/2 i386: Detect clock skew during suspend
Detect the situations in which the time after a resume from disk would be earlier than the time before the suspend and prevent them from happening on i386.
--- arch/i386/kernel/time.c | 24 ++++++++++++++++++------ 1 files changed, 18 insertions(+), 6 deletions(-)
arch/i386/kernel/time.c +18 -6
@@ -285,16 +285,19 @@ void notify_arch_cmos_timer(void)mod_timer (&sync_cmos_timer, jiffies +1 ); } -static long clock_cmos_diff, sleep_start; +static long clock_cmos_diff; +static unsigned long sleep_start;static int timer_suspend (struct sys_device *dev,pm_message_t state) {/* * Estimate time zone so that set_time can update the clock */ - clock_cmos_diff = -get_cmos_time (); +unsigned long ctime =get_cmos_time (); + + clock_cmos_diff = -ctime; clock_cmos_diff +=get_seconds (); - sleep_start =get_cmos_time (); + sleep_start = ctime;return 0 ; }
@@ -302,16 +305,25 @@ static int timer_resume(struct sys_devic {unsigned long flags;unsigned long sec; -unsigned long sleep_length; +unsigned long ctime =get_cmos_time (); +long sleep_length = (ctime - sleep_start) *HZ ;struct timespec ts; + +if (sleep_length <0 ) { +printk (KERN_WARNING "Time skew detected in timer resume!\n" ); +/* The time after the resume must not be earlier than the time +* before the suspend or some nasty things will happen +*/ + sleep_length =0 ; + ctime = sleep_start; + }#ifdef CONFIG_HPET_TIMER if (is_hpet_enabled ())hpet_reenable ();#endif setup_pit_timer (); - sec =get_cmos_time () + clock_cmos_diff; - sleep_length = (get_cmos_time () - sleep_start) *HZ ; + sec = ctime + clock_cmos_diff; ts.tv_sec = sec; ts.tv_nsec =0 ;do_settimeofday (&ts);
Re: [RFC][PATCH -mm 1/2] i386: Detect clock skew during suspend
Quoted 5 lines · Rafael J. Wysocki
Detect the situations in which the time after a resume from disk would be earlier than the time before the suspend and prevent them from happening on i386.
Signed-off-by Rafael J. Wysocki <[email protected]>
One minor comment, but otherwise looks good.
arch/i386/kernel/time.c static int timer_resume(struct sys_devic · line 313
Quoted 11 lines
arch/i386/kernel/time.c +5
-1@@ -302,16 +305,25 @@ static int timer_resume(struct sys_devic {unsigned long flags;unsigned long sec; -unsigned long sleep_length; +unsigned long ctime =get_cmos_time (); +long sleep_length = (ctime - sleep_start) *HZ ;struct timespec ts; + +if (sleep_length <0 ) { +printk (KERN_WARNING "Time skew detected in timer resume!\n" );
Please make sure the warning describes the CMOS clock going backwards, rather then just the vague "time skew detected" comment.
arch/i386/kernel/time.c static int timer_resume(struct sys_devic · line 314
Quoted 1 line
arch/i386/kernel/time.c +1
-0+/* The time after the resume must not be earlier than the time
s/time/CMOS clock/
thanks
-john
Re: [RFC][PATCH -mm 1/2] i386: Detect clock skew during suspend
Quoted 24 lines · john stultz
Quoted 5 lines · Rafael J. Wysocki
Detect the situations in which the time after a resume from disk would be earlier than the time before the suspend and prevent them from happening on i386.
Signed-off-by Rafael J. Wysocki <[email protected]>One minor comment, but otherwise looks good.
Review static int timer_resume(struct sys_devic · line 313
Quoted 11 lines
(quoted diff) +5
-1@@ -302,16 +305,25 @@ static int timer_resume(struct sys_devic { unsigned long flags; unsigned long sec; - unsigned long sleep_length; + unsigned long ctime = get_cmos_time(); + long sleep_length = (ctime - sleep_start) * HZ; struct timespec ts; + + if (sleep_length < 0) { + printk(KERN_WARNING "Time skew detected in timer resume!\n");Please make sure the warning describes the CMOS clock going backwards, rather then just the vague "time skew detected" comment.
Quoted 7 lines
Review static int timer_resume(struct sys_devic · line 314
Quoted 1 line
(quoted diff) +1
-0+ /* The time after the resume must not be earlier than the times/time/CMOS clock/
Acked-by John Stultz <[email protected]>
Thanks for the comment and ACK. :-)
Second revision of the patch follows.
Greetings,
Rafael
---
Detect the situations in which the time after a resume from disk would be earlier than the time before the suspend and prevent them from happening on i386.
--- arch/i386/kernel/time.c | 24 ++++++++++++++++++------ 1 files changed, 18 insertions(+), 6 deletions(-)
arch/i386/kernel/time.c +18 -6
@@ -285,16 +285,19 @@ void notify_arch_cmos_timer(void)mod_timer (&sync_cmos_timer, jiffies +1 ); } -static long clock_cmos_diff, sleep_start; +static long clock_cmos_diff; +static unsigned long sleep_start;static int timer_suspend (struct sys_device *dev,pm_message_t state) {/* * Estimate time zone so that set_time can update the clock */ - clock_cmos_diff = -get_cmos_time (); +unsigned long ctime =get_cmos_time (); + + clock_cmos_diff = -ctime; clock_cmos_diff +=get_seconds (); - sleep_start =get_cmos_time (); + sleep_start = ctime;return 0 ; }
@@ -302,16 +305,25 @@ static int timer_resume(struct sys_devic {unsigned long flags;unsigned long sec; -unsigned long sleep_length; +unsigned long ctime =get_cmos_time (); +long sleep_length = (ctime - sleep_start) *HZ ;struct timespec ts; + +if (sleep_length <0 ) { +printk (KERN_WARNING "CMOS clock skew detected in timer resume!\n" ); +/* The time after the resume must not be earlier than the time +* before the suspend or some nasty things will happen +*/ + sleep_length =0 ; + ctime = sleep_start; + }#ifdef CONFIG_HPET_TIMER if (is_hpet_enabled ())hpet_reenable ();#endif setup_pit_timer (); - sec =get_cmos_time () + clock_cmos_diff; - sleep_length = (get_cmos_time () - sleep_start) *HZ ; + sec = ctime + clock_cmos_diff; ts.tv_sec = sec; ts.tv_nsec =0 ;do_settimeofday (&ts);
RFC PATCH 2/2 x86_64: Detect clock skew during suspend
In reply to Rafael J. WysockiDetect the situations in which the time after a resume from disk would be earlier than the time before the suspend and prevent them from happening on x86_64.
--- arch/x86_64/kernel/time.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletion(-)
arch/x86_64/kernel/time.c +9 -1
@@ -1039,8 +1039,16 @@ static int timer_resume(struct sys_devicunsigned long flags;unsigned long sec;unsigned long ctime =get_cmos_time (); -unsigned long sleep_length = (ctime - sleep_start) *HZ ; +long sleep_length = (ctime - sleep_start) *HZ ; +if (sleep_length <0 ) { +printk (KERN_WARNING "Time skew detected in timer resume!\n" ); +/* The time after the resume must not be earlier than the time +* before the suspend or some nasty things will happen +*/ + sleep_length =0 ; + ctime = sleep_start; + }if (vxtime.hpet_address)hpet_reenable ();else
Re: [RFC][PATCH -mm 2/2] x86_64: Detect clock skew during suspend
Quoted 3 lines · Rafael J. Wysocki
Detect the situations in which the time after a resume from disk would be earlier than the time before the suspend and prevent them from happening on
x86_64.
Merged thanks
-Andi
Hi!
Quoted 8 lines
If the CMOS timer is changed when the system is suspended to disk in such a way that the time during the resume turns out to be earlier than the time before the suspend, the resume often fails and the system hangs (spins forever in the idle thread) due to driver problems.
For this reason it seems reasonable to make the timer .resume() routines detect such situations and prevent them from happening, which is done in the following two patches for i386 and
x86_64.
"Clock skew detected"... Maybe "time going backwards detected" ?
Anyway, patches look good to me, ACK.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html