[PATCH]time run too fast after S3

[PATCH]time run too fast after S3

[Li Shaohua]

Hi,
after resume from S3, 'date' shows time run too fast. Here is a patch.

Thanks,
Shaohua

Signed-off-by: Li Shaohua <shaohua.li@intel.com>

diff -puN arch/i386/kernel/time.c~wall_jiffies arch/i386/kernel/time.c
--- 2.6/arch/i386/kernel/time.c~wall_jiffies	2004-11-22 17:04:42.720038352 +0800
+++ 2.6-root/arch/i386/kernel/time.c	2004-11-22 17:06:21.373040816 +0800
@@ -343,12 +343,13 @@ static int timer_resume(struct sys_devic
 		hpet_reenable();
 #endif
 	sec = get_cmos_time() + clock_cmos_diff;
-	sleep_length = get_cmos_time() - sleep_start;
+	sleep_length = (get_cmos_time() - sleep_start) * HZ;
 	write_seqlock_irqsave(&xtime_lock, flags);
 	xtime.tv_sec = sec;
 	xtime.tv_nsec = 0;
 	write_sequnlock_irqrestore(&xtime_lock, flags);
-	jiffies += sleep_length * HZ;
+	jiffies += sleep_length;
+	wall_jiffies += sleep_length;
 	return 0;
 }
 

Re: [PATCH]time run too fast after S3

[john stultz]

On Mon, 2004-11-22 at 01:15, Li Shaohua wrote:
> after resume from S3, 'date' shows time run too fast. Here is a patch.
[snip]
> diff -puN arch/i386/kernel/time.c~wall_jiffies arch/i386/kernel/time.c
> --- 2.6/arch/i386/kernel/time.c~wall_jiffies	2004-11-22 17:04:42.720038352 +0800
> +++ 2.6-root/arch/i386/kernel/time.c	2004-11-22 17:06:21.373040816 +0800
> @@ -343,12 +343,13 @@ static int timer_resume(struct sys_devic
>  		hpet_reenable();
>  #endif
>  	sec = get_cmos_time() + clock_cmos_diff;
> -	sleep_length = get_cmos_time() - sleep_start;
> +	sleep_length = (get_cmos_time() - sleep_start) * HZ;
>  	write_seqlock_irqsave(&xtime_lock, flags);
>  	xtime.tv_sec = sec;
>  	xtime.tv_nsec = 0;
>  	write_sequnlock_irqrestore(&xtime_lock, flags);
> -	jiffies += sleep_length * HZ;
> +	jiffies += sleep_length;
> +	wall_jiffies += sleep_length;
>  	return 0;
>  }

I'm not all that familiar w/ the suspend code, but yea, this looks like
an improvement.  The previous code was wrong because they are setting
xtime themselves, and then updating only jiffies. At the next timer
interrupt, the difference between jiffies and wall_jiffies would then be
added to xtime again. 

Why they don't just use do_settimeofday() for all of this is a mystery
to me. Are we wanting to pretend timer ticks arrived while we were
suspended?

thanks
-john

Re: [PATCH]time run too fast after S3

[George Anzinger]

john stultz wrote:
> On Mon, 2004-11-22 at 01:15, Li Shaohua wrote:
> 
>>after resume from S3, 'date' shows time run too fast. Here is a patch.
> 
> [snip]
> 
>>diff -puN arch/i386/kernel/time.c~wall_jiffies arch/i386/kernel/time.c
>>--- 2.6/arch/i386/kernel/time.c~wall_jiffies	2004-11-22 17:04:42.720038352 +0800
>>+++ 2.6-root/arch/i386/kernel/time.c	2004-11-22 17:06:21.373040816 +0800
>>@@ -343,12 +343,13 @@ static int timer_resume(struct sys_devic
>> 		hpet_reenable();
>> #endif
>> 	sec = get_cmos_time() + clock_cmos_diff;
>>-	sleep_length = get_cmos_time() - sleep_start;
>>+	sleep_length = (get_cmos_time() - sleep_start) * HZ;
>> 	write_seqlock_irqsave(&xtime_lock, flags);
>> 	xtime.tv_sec = sec;
>> 	xtime.tv_nsec = 0;
>> 	write_sequnlock_irqrestore(&xtime_lock, flags);
>>-	jiffies += sleep_length * HZ;
>>+	jiffies += sleep_length;
>>+	wall_jiffies += sleep_length;
>> 	return 0;
>> }
> 
> 
> I'm not all that familiar w/ the suspend code, but yea, this looks like
> an improvement.  The previous code was wrong because they are setting
> xtime themselves, and then updating only jiffies. At the next timer
> interrupt, the difference between jiffies and wall_jiffies would then be
> added to xtime again. 
> 
> Why they don't just use do_settimeofday() for all of this is a mystery
> to me. Are we wanting to pretend timer ticks arrived while we were
> suspended?

I think that this way the uptime and start times of init and friends will be 
much more correct.  settimeofday() would move those around.  So, the short 
answer is, yes.

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/

Re: [PATCH]time run too fast after S3

[Nigel Cunningham]

Hi.

On Tue, 2004-11-23 at 05:33, john stultz wrote:
> I'm not all that familiar w/ the suspend code, but yea, this looks like
> an improvement.  The previous code was wrong because they are setting
> xtime themselves, and then updating only jiffies. At the next timer
> interrupt, the difference between jiffies and wall_jiffies would then be
> added to xtime again. 

That makes a lot of sense to me :> 

It also happens with suspend to disk now that Pavel and I have added
sysdev support to our implementations.

I was doing some looking in this area last week, but ran out of time. I
was seeing the clock being out - sometimes - by 1 hour+.

That section of code could also be improved by reusing the value of the
first call to get_cmos_time(). The way that function works, two
consecutive calls will cause a one second delay for the second call. A
__get_cmos_time function that doesn't wait for the start of a second was
suggested for where we only care about consistency and not about getting
the start of the second. I'll send a patch shortly.

> Why they don't just use do_settimeofday() for all of this is a mystery
> to me. Are we wanting to pretend timer ticks arrived while we were
> suspended?

I think that was Pavel's idea; something about getting process times
right. Speaking for myself, I might be being short sighted, but I just
want to save the user having to run /sbin/hwclock --hctosys afterwards.

Regards,

Nigel
-- 
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

You see, at just the right time, when we were still powerless, Christ
died for the ungodly.		-- Romans 5:6