[Regression] Negative time on Acer Ferrari One with current -git

[Regression] Negative time on Acer Ferrari One with current -git

[Rafael J. Wysocki]

Hi,

Trying to boot current -git on Acer Ferrari One (64-bit), I get mount errors
for all filesystems telling me that "now = (null) is in the future" and when
run 'date' it says: "date: time <long negative number> is out of range".

Any ideas anyone?

Rafael

Re: [Regression] Negative time on Acer Ferrari One with current -git

[Linus Torvalds]

On Fri, 21 May 2010, Rafael J. Wysocki wrote:
> 
> Trying to boot current -git on Acer Ferrari One (64-bit), I get mount errors
> for all filesystems telling me that "now = (null) is in the future" and when
> run 'date' it says: "date: time <long negative number> is out of range".
> 
> Any ideas anyone?

No ideas, but apart from the obvious "maybe you can pin it down a bit with 
a few bisection compiles", it's almost certainly through the timer tree 
merge from Thomas, which included various wall-clock-time changes from 
John Stultz & co.

I don't see anything else that would likely affect any wall-clock time, 
but who knows..

John, Thomas? Ring any bells?

		Linus

Re: [Regression] Negative time on Acer Ferrari One with current -git

[john stultz]

On Fri, 2010-05-21 at 15:19 -0700, Linus Torvalds wrote:
> 
> On Fri, 21 May 2010, Rafael J. Wysocki wrote:
> > 
> > Trying to boot current -git on Acer Ferrari One (64-bit), I get mount errors
> > for all filesystems telling me that "now = (null) is in the future" and when
> > run 'date' it says: "date: time <long negative number> is out of range".
> > 
> > Any ideas anyone?
> 
> No ideas, but apart from the obvious "maybe you can pin it down a bit with 
> a few bisection compiles", it's almost certainly through the timer tree 
> merge from Thomas, which included various wall-clock-time changes from 
> John Stultz & co.
> 
> I don't see anything else that would likely affect any wall-clock time, 
> but who knows..
> 
> John, Thomas? Ring any bells?

Hrm..  From my queue, I'd probably try reverting the following first:

6a867a3|| time: Remove xtime_cache

Rafael: I assume 2.6.34 is fine?

thanks
-john

Re: [Regression] Negative time on Acer Ferrari One with current -git

[Rafael J. Wysocki]

On Saturday 22 May 2010, john stultz wrote:
> On Fri, 2010-05-21 at 15:19 -0700, Linus Torvalds wrote:
> > 
> > On Fri, 21 May 2010, Rafael J. Wysocki wrote:
> > > 
> > > Trying to boot current -git on Acer Ferrari One (64-bit), I get mount errors
> > > for all filesystems telling me that "now = (null) is in the future" and when
> > > run 'date' it says: "date: time <long negative number> is out of range".
> > > 
> > > Any ideas anyone?
> > 
> > No ideas, but apart from the obvious "maybe you can pin it down a bit with 
> > a few bisection compiles", it's almost certainly through the timer tree 
> > merge from Thomas, which included various wall-clock-time changes from 
> > John Stultz & co.
> > 
> > I don't see anything else that would likely affect any wall-clock time, 
> > but who knows..
> > 
> > John, Thomas? Ring any bells?
> 
> Hrm..  From my queue, I'd probably try reverting the following first:
> 
> 6a867a3|| time: Remove xtime_cache

That doesn't help.
 
> Rafael: I assume 2.6.34 is fine?

Yes, it is.

Thanks,
Rafael

Re: [Regression] Negative time on Acer Ferrari One with current -git

[Thomas Gleixner]

On Fri, 21 May 2010, Linus Torvalds wrote:
> On Fri, 21 May 2010, Rafael J. Wysocki wrote:
> > 
> > Trying to boot current -git on Acer Ferrari One (64-bit), I get mount errors
> > for all filesystems telling me that "now = (null) is in the future" and when
> > run 'date' it says: "date: time <long negative number> is out of range".
> > 
> > Any ideas anyone?
> 
> No ideas, but apart from the obvious "maybe you can pin it down a bit with 
> a few bisection compiles", it's almost certainly through the timer tree 
> merge from Thomas, which included various wall-clock-time changes from 
> John Stultz & co.
> 
> I don't see anything else that would likely affect any wall-clock time, 
> but who knows..
> 
> John, Thomas? Ring any bells?

Went through the pile and nothing stands out.

The system has an AMD X2 with the jinxed C1E, so I'd rather suspect
that we managed to break C1E again.

Just tried to boot -git on my X2 machine and it does not come
up. Investigating.

Thanks,

	tglx

Re: [Regression] Negative time on Acer Ferrari One with current -git

[Thomas Gleixner]

On Sat, 22 May 2010, Thomas Gleixner wrote:
> On Fri, 21 May 2010, Linus Torvalds wrote:
> > On Fri, 21 May 2010, Rafael J. Wysocki wrote:
> > > 
> > > Trying to boot current -git on Acer Ferrari One (64-bit), I get mount errors
> > > for all filesystems telling me that "now = (null) is in the future" and when
> > > run 'date' it says: "date: time <long negative number> is out of range".
> > > 
> > > Any ideas anyone?
> > 
> > No ideas, but apart from the obvious "maybe you can pin it down a bit with 
> > a few bisection compiles", it's almost certainly through the timer tree 
> > merge from Thomas, which included various wall-clock-time changes from 
> > John Stultz & co.
> > 
> > I don't see anything else that would likely affect any wall-clock time, 
> > but who knows..
> > 
> > John, Thomas? Ring any bells?
> 
> Went through the pile and nothing stands out.
> 
> The system has an AMD X2 with the jinxed C1E, so I'd rather suspect
> that we managed to break C1E again.
> 
> Just tried to boot -git on my X2 machine and it does not come
> up. Investigating.

Second boot worked, I love heisenbugs. :(

Rafael, can you check whether the C1E idle is selected. Also which
clocksource is used ?

Thanks,

	tglx

Re: [Regression] Negative time on Acer Ferrari One with current -git

[Rafael J. Wysocki]

On Saturday 22 May 2010, Thomas Gleixner wrote:
> On Sat, 22 May 2010, Thomas Gleixner wrote:
> > On Fri, 21 May 2010, Linus Torvalds wrote:
> > > On Fri, 21 May 2010, Rafael J. Wysocki wrote:
> > > > 
> > > > Trying to boot current -git on Acer Ferrari One (64-bit), I get mount errors
> > > > for all filesystems telling me that "now = (null) is in the future" and when
> > > > run 'date' it says: "date: time <long negative number> is out of range".
> > > > 
> > > > Any ideas anyone?
> > > 
> > > No ideas, but apart from the obvious "maybe you can pin it down a bit with 
> > > a few bisection compiles", it's almost certainly through the timer tree 
> > > merge from Thomas, which included various wall-clock-time changes from 
> > > John Stultz & co.
> > > 
> > > I don't see anything else that would likely affect any wall-clock time, 
> > > but who knows..
> > > 
> > > John, Thomas? Ring any bells?
> > 
> > Went through the pile and nothing stands out.
> > 
> > The system has an AMD X2 with the jinxed C1E, so I'd rather suspect
> > that we managed to break C1E again.
> > 
> > Just tried to boot -git on my X2 machine and it does not come
> > up. Investigating.
> 
> Second boot worked, I love heisenbugs. :(
> 
> Rafael, can you check whether the C1E idle is selected.

Can you please remind me how I can check that?

> Also which clocksource is used ?

hpet

Thanks,
Rafael

Re: [Regression] Negative time on Acer Ferrari One with current -git

[Borislav Petkov]

From: "Rafael J. Wysocki" <rjw@sisk.pl>
Date: Sat, May 22, 2010 at 07:34:09PM +0200

> > Rafael, can you check whether the C1E idle is selected.
> 
> Can you please remind me how I can check that?

look for

"using C1E aware idle routine"

in dmesg.

-- 
Regards/Gruss,
    Boris.

Re: [Regression] Negative time on Acer Ferrari One with current -git

[Rafael J. Wysocki]

On Saturday 22 May 2010, Borislav Petkov wrote:
> From: "Rafael J. Wysocki" <rjw@sisk.pl>
> Date: Sat, May 22, 2010 at 07:34:09PM +0200
> 
> > > Rafael, can you check whether the C1E idle is selected.
> > 
> > Can you please remind me how I can check that?
> 
> look for
> 
> "using C1E aware idle routine"

Yes, it prints that.

Rafael

Re: [Regression] Negative time on Acer Ferrari One with current -git

[Thomas Gleixner]

On Sat, 22 May 2010, Rafael J. Wysocki wrote:
> On Saturday 22 May 2010, Thomas Gleixner wrote:
> 
> > Also which clocksource is used ?
> 
> hpet

Hmm, there is only one hpet related commit sin .34:  30a564be9d
and I can hardly see how this should be related.

Thanks,

	tglx

Re: [Regression] Negative time on Acer Ferrari One with current -git

[Thomas Gleixner]

On Sat, 22 May 2010, Thomas Gleixner wrote:

> On Sat, 22 May 2010, Rafael J. Wysocki wrote:
> > On Saturday 22 May 2010, Thomas Gleixner wrote:
> > 
> > > Also which clocksource is used ?
> > 
> > hpet
> 
> Hmm, there is only one hpet related commit sin .34:  30a564be9d
> and I can hardly see how this should be related.

Does the problem persist if you disable HPET on the kernel command line ?
 
Thanks,

	tglx

Re: [Regression] Negative time on Acer Ferrari One with current -git

[Rafael J. Wysocki]

On Saturday 22 May 2010, Thomas Gleixner wrote:
> On Sat, 22 May 2010, Thomas Gleixner wrote:
> 
> > On Sat, 22 May 2010, Rafael J. Wysocki wrote:
> > > On Saturday 22 May 2010, Thomas Gleixner wrote:
> > > 
> > > > Also which clocksource is used ?
> > > 
> > > hpet
> > 
> > Hmm, there is only one hpet related commit sin .34:  30a564be9d
> > and I can hardly see how this should be related.
> 
> Does the problem persist if you disable HPET on the kernel command line ?

I didn't try that, but I bisected it in the meantime which lead to:

commit 64ce4c2f5252f25798117fa80a027993163d6d84
Author: John Stultz <johnstul@us.ibm.com>
Date:   Thu Mar 11 14:04:47 2010 -0800

    time: Clean up warp_clock()

    warp_clock() currently accesses timekeeping internal state directly, which
    is unnecessary.  Convert it to use the proper timekeeping interfaces.

    Signed-off-by: John Stultz <johnstul@us.ibm.com>
    Cc: Ingo Molnar <mingo@elte.hu>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

So I don't think it's an hpet issue. :-)

I'm now going to revert that commit and see what happens.

Thanks,
Rafael

Re: [Regression] Negative time on Acer Ferrari One with current -git

[Rafael J. Wysocki]

On Saturday 22 May 2010, Rafael J. Wysocki wrote:
> On Saturday 22 May 2010, Thomas Gleixner wrote:
> > On Sat, 22 May 2010, Thomas Gleixner wrote:
> > 
> > > On Sat, 22 May 2010, Rafael J. Wysocki wrote:
> > > > On Saturday 22 May 2010, Thomas Gleixner wrote:
> > > > 
> > > > > Also which clocksource is used ?
> > > > 
> > > > hpet
> > > 
> > > Hmm, there is only one hpet related commit sin .34:  30a564be9d
> > > and I can hardly see how this should be related.
> > 
> > Does the problem persist if you disable HPET on the kernel command line ?
> 
> I didn't try that, but I bisected it in the meantime which lead to:
> 
> commit 64ce4c2f5252f25798117fa80a027993163d6d84
> Author: John Stultz <johnstul@us.ibm.com>
> Date:   Thu Mar 11 14:04:47 2010 -0800
> 
>     time: Clean up warp_clock()
> 
>     warp_clock() currently accesses timekeeping internal state directly, which
>     is unnecessary.  Convert it to use the proper timekeeping interfaces.
> 
>     Signed-off-by: John Stultz <johnstul@us.ibm.com>
>     Cc: Ingo Molnar <mingo@elte.hu>
>     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>     Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> 
> So I don't think it's an hpet issue. :-)
> 
> I'm now going to revert that commit and see what happens.

After reverting commit 64ce4c2f above things work again.

To be precise, I reverted both commit 64ce4c2f and commit 6a867a3 (time: 
Remove xtime_cache), but since the symptoms continued to apprear after
reverting the latter alone, it's quite clear that commit 64ce4c2f breaks things
on this box.

Thanks,
Rafael

Re: [Regression] Negative time on Acer Ferrari One with current -git

[Thomas Gleixner]

On Sat, 22 May 2010, Rafael J. Wysocki wrote:
> After reverting commit 64ce4c2f above things work again.
> 
> To be precise, I reverted both commit 64ce4c2f and commit 6a867a3 (time: 
> Remove xtime_cache), but since the symptoms continued to apprear after
> reverting the latter alone, it's quite clear that commit 64ce4c2f breaks things
> on this box.

I assume the cmos clock of this machine is not on UTC, right ? Does
the patch below fix the issue ?

@John: Can you please check the other users of timespec_add_safe() in
timekeeping as well ?

Thanks,

	tglx

---------->
Subject: timekeeping: Fix timezone update
From: Thomas Gleixner <tglx@linutronix.de>
Date: Sun, 23 May 2010 08:14:45 +0200

commit 64ce4c2f (time: Clean up warp_clock()) breaks the timezone
update in a very subtle way. To avoid the direct access to timekeeping
internals it adds the timezone delta to the current time with
timespec_add_safe(). This works nicely when the timezone delta is > 0.
If timezone delta is < 0 then the wrap check in timespec_add_safe()
triggers and timespec_add_safe() returns TIME_MAX and screws up
timekeeping completely. 

This is not surprising as the comment above timespec_add_safe() says:
    It's assumed that both values are valid (>= 0)

The function was created to avoid overflow issues when adding the
select() timeout to current time, where the above applies.

Add the timezone seconds adjustment directly.

Reported-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Index: linux-2.6/kernel/time.c
===================================================================
--- linux-2.6.orig/kernel/time.c
+++ linux-2.6/kernel/time.c
@@ -132,10 +132,10 @@ SYSCALL_DEFINE2(gettimeofday, struct tim
  */
 static inline void warp_clock(void)
 {
-	struct timespec delta, adjust;
-	delta.tv_sec = sys_tz.tz_minuteswest * 60;
-	delta.tv_nsec = 0;
-	adjust = timespec_add_safe(current_kernel_time(), delta);
+	struct timespec adjust;
+
+	adjust = current_kernel_time();
+	adjust.tv_sec += sys_tz.tz_minuteswest * 60;
 	do_settimeofday(&adjust);
 }
 

Re: [Regression] Negative time on Acer Ferrari One with current -git

[Rafael J. Wysocki]

On Sunday 23 May 2010, Thomas Gleixner wrote:
> On Sat, 22 May 2010, Rafael J. Wysocki wrote:
> > After reverting commit 64ce4c2f above things work again.
> > 
> > To be precise, I reverted both commit 64ce4c2f and commit 6a867a3 (time: 
> > Remove xtime_cache), but since the symptoms continued to apprear after
> > reverting the latter alone, it's quite clear that commit 64ce4c2f breaks things
> > on this box.
> 
> I assume the cmos clock of this machine is not on UTC, right ?

Yup.

> Does the patch below fix the issue ?

Yes, it does, thanks!

> @John: Can you please check the other users of timespec_add_safe() in
> timekeeping as well ?
> 
> Thanks,
> 
> 	tglx
> 
> ---------->
> Subject: timekeeping: Fix timezone update
> From: Thomas Gleixner <tglx@linutronix.de>
> Date: Sun, 23 May 2010 08:14:45 +0200
> 
> commit 64ce4c2f (time: Clean up warp_clock()) breaks the timezone
> update in a very subtle way. To avoid the direct access to timekeeping
> internals it adds the timezone delta to the current time with
> timespec_add_safe(). This works nicely when the timezone delta is > 0.
> If timezone delta is < 0 then the wrap check in timespec_add_safe()
> triggers and timespec_add_safe() returns TIME_MAX and screws up
> timekeeping completely. 
> 
> This is not surprising as the comment above timespec_add_safe() says:
>     It's assumed that both values are valid (>= 0)
> 
> The function was created to avoid overflow issues when adding the
> select() timeout to current time, where the above applies.
> 
> Add the timezone seconds adjustment directly.
> 
> Reported-by: Rafael J. Wysocki <rjw@sisk.pl>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  kernel/time.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> Index: linux-2.6/kernel/time.c
> ===================================================================
> --- linux-2.6.orig/kernel/time.c
> +++ linux-2.6/kernel/time.c
> @@ -132,10 +132,10 @@ SYSCALL_DEFINE2(gettimeofday, struct tim
>   */
>  static inline void warp_clock(void)
>  {
> -	struct timespec delta, adjust;
> -	delta.tv_sec = sys_tz.tz_minuteswest * 60;
> -	delta.tv_nsec = 0;
> -	adjust = timespec_add_safe(current_kernel_time(), delta);
> +	struct timespec adjust;
> +
> +	adjust = current_kernel_time();
> +	adjust.tv_sec += sys_tz.tz_minuteswest * 60;
>  	do_settimeofday(&adjust);
>  }

Rafael

Re: [Regression] Negative time on Acer Ferrari One with current -git

[john stultz]

On Sun, 2010-05-23 at 08:40 +0200, Thomas Gleixner wrote:
> On Sat, 22 May 2010, Rafael J. Wysocki wrote:
> > After reverting commit 64ce4c2f above things work again.
> > 
> > To be precise, I reverted both commit 64ce4c2f and commit 6a867a3 (time: 
> > Remove xtime_cache), but since the symptoms continued to apprear after
> > reverting the latter alone, it's quite clear that commit 64ce4c2f breaks things
> > on this box.
> 
> I assume the cmos clock of this machine is not on UTC, right ? Does
> the patch below fix the issue ?

Oof. Thanks for catching that. 


> @John: Can you please check the other users of timespec_add_safe() in
> timekeeping as well ?

So monotonic_to_bootbased() looks ok, as total_sleep_time should always
be positive. 

timekeeping_resume() also looks ok, since it we make sure the delta
between the resume time and the suspend time is positive.

That said, it may be worth open coding the:
	set_normalized_timespec(&ret, a.tv_sec + b.tv_sec, 
				a.tv_nsec + b.tv_nsec);

just so we don't have similar mix ups in the future, since
timespec_add_safe is just such a reassuring and comforting name :)



> Thanks,
> 
> 	tglx
> 
> ---------->
> Subject: timekeeping: Fix timezone update
> From: Thomas Gleixner <tglx@linutronix.de>
> Date: Sun, 23 May 2010 08:14:45 +0200
> 
> commit 64ce4c2f (time: Clean up warp_clock()) breaks the timezone
> update in a very subtle way. To avoid the direct access to timekeeping
> internals it adds the timezone delta to the current time with
> timespec_add_safe(). This works nicely when the timezone delta is > 0.
> If timezone delta is < 0 then the wrap check in timespec_add_safe()
> triggers and timespec_add_safe() returns TIME_MAX and screws up
> timekeeping completely. 
> 
> This is not surprising as the comment above timespec_add_safe() says:
>     It's assumed that both values are valid (>= 0)
> 
> The function was created to avoid overflow issues when adding the
> select() timeout to current time, where the above applies.
> 
> Add the timezone seconds adjustment directly.
> 
> Reported-by: Rafael J. Wysocki <rjw@sisk.pl>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>\

Acked-by: John Stultz <johnstul@us.ibm.com>

> ---
>  kernel/time.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> Index: linux-2.6/kernel/time.c
> ===================================================================
> --- linux-2.6.orig/kernel/time.c
> +++ linux-2.6/kernel/time.c
> @@ -132,10 +132,10 @@ SYSCALL_DEFINE2(gettimeofday, struct tim
>   */
>  static inline void warp_clock(void)
>  {
> -	struct timespec delta, adjust;
> -	delta.tv_sec = sys_tz.tz_minuteswest * 60;
> -	delta.tv_nsec = 0;
> -	adjust = timespec_add_safe(current_kernel_time(), delta);
> +	struct timespec adjust;
> +
> +	adjust = current_kernel_time();
> +	adjust.tv_sec += sys_tz.tz_minuteswest * 60;
>  	do_settimeofday(&adjust);
>  }
>