Hi,
after resume from S3, 'date' shows time run too fast. Here is a patch.
Thanks,
Shaohua
Signed-off-by: Li Shaohua <[email protected]>
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;
}
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
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
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 [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/