Resume does not work after timekeeping change

Resume does not work after timekeeping change

[Jiri Slaby]

Hi,

my machine does not wake from suspend to RAM on my box running the -next
kernel. The last thing I see is "Disabling non-boot CPUs ...". I
bisected it to this commit:

commit 7ec98e15aa049b7a2ca73485f31cf4f90c34e2dd
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Thu Feb 21 22:51:39 2013 +0000

    timekeeping: Delay update of clock->cycle_last


Reverting that one on the top of -next-20130419 makes it work again.


I also tried it inside a VM using suspend to disk. There, it behaves
like it takes a minute or so to wake up. So I tried to wait on the real
HW too, but it never resumes there.

regards,
-- 
js
suse labs

Re: Resume does not work after timekeeping change

[Borislav Petkov]

On Sat, Apr 20, 2013 at 05:46:52PM +0200, Jiri Slaby wrote:
> Hi,
> 
> my machine does not wake from suspend to RAM on my box running the -next
> kernel. The last thing I see is "Disabling non-boot CPUs ...". I
> bisected it to this commit:
> 
> commit 7ec98e15aa049b7a2ca73485f31cf4f90c34e2dd
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Thu Feb 21 22:51:39 2013 +0000
> 
>     timekeeping: Delay update of clock->cycle_last
> 
> 
> Reverting that one on the top of -next-20130419 makes it work again.
> 
> 
> I also tried it inside a VM using suspend to disk. There, it behaves
> like it takes a minute or so to wake up. So I tried to wait on the real
> HW too, but it never resumes there.

Right, and I'm seeing it here too but it sometimes does resume after
about a minute or so and sometimes it doesn't.

Reverting the patch by hand ontop of tip/master fixes the issue here
too.

Btw, Ingo, remember how I was telling you that I'm seeing
temporary lockups with tip/master and resume issues
with the last line being "Disabling non-boot CPUs ..."
http://marc.info/?l=linux-kernel&m=136610574502492 - this commit was the
culprit.

Good job Jiri.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: Resume does not work after timekeeping change

[Thomas Gleixner]

On Sat, 20 Apr 2013, Jiri Slaby wrote:
> Hi,
> 
> my machine does not wake from suspend to RAM on my box running the -next
> kernel. The last thing I see is "Disabling non-boot CPUs ...". I
> bisected it to this commit:
> 
> commit 7ec98e15aa049b7a2ca73485f31cf4f90c34e2dd
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Thu Feb 21 22:51:39 2013 +0000
> 
>     timekeeping: Delay update of clock->cycle_last
> 
> 
> Reverting that one on the top of -next-20130419 makes it work again.

Does the patch below fix the issue?

Thanks,

	tglx
---
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 675f720..98cd470 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -951,7 +951,7 @@ static void timekeeping_resume(void)
 		__timekeeping_inject_sleeptime(tk, &ts_delta);
 
 	/* Re-base the last cycle value */
-	clock->cycle_last = cycle_now;
+	tk->cycle_last = clock->cycle_last = cycle_now;
 	tk->ntp_error = 0;
 	timekeeping_suspended = 0;
 	timekeeping_update(tk, false, true);

Re: Resume does not work after timekeeping change

[Borislav Petkov]

On Sun, Apr 21, 2013 at 04:48:45PM +0200, Thomas Gleixner wrote:
> Does the patch below fix the issue?

It does here.

Applied it ontop of tip/master of today and did three suspend/resume
cycles, back-to-back - looks good and no hickups.

Tested-by: Borislav Petkov <bp@suse.de>

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: Resume does not work after timekeeping change

[John Stultz]

On 04/20/2013 08:46 AM, Jiri Slaby wrote:
> Hi,
>
> my machine does not wake from suspend to RAM on my box running the -next
> kernel. The last thing I see is "Disabling non-boot CPUs ...". I
> bisected it to this commit:
>
> commit 7ec98e15aa049b7a2ca73485f31cf4f90c34e2dd
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Thu Feb 21 22:51:39 2013 +0000
>
>      timekeeping: Delay update of clock->cycle_last
>
>
> Reverting that one on the top of -next-20130419 makes it work again.
>
>
> I also tried it inside a VM using suspend to disk. There, it behaves
> like it takes a minute or so to wake up. So I tried to wait on the real
> HW too, but it never resumes there.

Thanks for chasing this down! Sorry for the trouble, does the following fix resolve this?

thanks
-john

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 675f720..94041a9 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -952,6 +952,7 @@ static void timekeeping_resume(void)
  
  	/* Re-base the last cycle value */
  	clock->cycle_last = cycle_now;
+	tk->cycle_last = cycle_now;
  	tk->ntp_error = 0;
  	timekeeping_suspended = 0;
  	timekeeping_update(tk, false, true);

Re: Resume does not work after timekeeping change

[John Stultz]

On 04/22/2013 09:05 AM, John Stultz wrote:
> On 04/20/2013 08:46 AM, Jiri Slaby wrote:
>> Hi,
>>
>> my machine does not wake from suspend to RAM on my box running the -next
>> kernel. The last thing I see is "Disabling non-boot CPUs ...". I
>> bisected it to this commit:
>>
>> commit 7ec98e15aa049b7a2ca73485f31cf4f90c34e2dd
>> Author: Thomas Gleixner <tglx@linutronix.de>
>> Date:   Thu Feb 21 22:51:39 2013 +0000
>>
>>      timekeeping: Delay update of clock->cycle_last
>>
>>
>> Reverting that one on the top of -next-20130419 makes it work again.
>>
>>
>> I also tried it inside a VM using suspend to disk. There, it behaves
>> like it takes a minute or so to wake up. So I tried to wait on the real
>> HW too, but it never resumes there.
>
> Thanks for chasing this down! Sorry for the trouble, does the 
> following fix resolve this?
>
Nevermind, I see Thomas already provided the same fix yesterday.

thanks
-john

Re: Resume does not work after timekeeping change

[John Stultz]

On 04/21/2013 07:48 AM, Thomas Gleixner wrote:
> On Sat, 20 Apr 2013, Jiri Slaby wrote:
>> Hi,
>>
>> my machine does not wake from suspend to RAM on my box running the -next
>> kernel. The last thing I see is "Disabling non-boot CPUs ...". I
>> bisected it to this commit:
>>
>> commit 7ec98e15aa049b7a2ca73485f31cf4f90c34e2dd
>> Author: Thomas Gleixner <tglx@linutronix.de>
>> Date:   Thu Feb 21 22:51:39 2013 +0000
>>
>>      timekeeping: Delay update of clock->cycle_last
>>
>>
>> Reverting that one on the top of -next-20130419 makes it work again.
> Does the patch below fix the issue?
>
> Thanks,
>
> 	tglx
> ---
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 675f720..98cd470 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -951,7 +951,7 @@ static void timekeeping_resume(void)
>   		__timekeeping_inject_sleeptime(tk, &ts_delta);
>   
>   	/* Re-base the last cycle value */
> -	clock->cycle_last = cycle_now;
> +	tk->cycle_last = clock->cycle_last = cycle_now;

Looks good.

Acked-by: John Stultz <john.stultz@linaro.org>

[tip:timers/core] timekeeping: Update tk->cycle_last in resume

[tip-bot for Thomas Gleixner]

Commit-ID:  77c675ba18836802f6b73d2d773481d06ebc0f04
Gitweb:     http://git.kernel.org/tip/77c675ba18836802f6b73d2d773481d06ebc0f04
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Mon, 22 Apr 2013 09:37:04 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 22 Apr 2013 20:17:51 +0200

timekeeping: Update tk->cycle_last in resume

commit 7ec98e15aa (timekeeping: Delay update of clock->cycle_last)
forgot to update tk->cycle_last in the resume path. This results in a
stale value versus clock->cycle_last and prevents resume in the worst
case.

Reported-by: Jiri Slaby <jslaby@suse.cz>
Reported-and-tested-by: Borislav Petkov <bp@alien8.de>
Acked-by: John Stultz <john.stultz@linaro.org>
Cc: Linux-pm mailing list <linux-pm@lists.linux-foundation.org>
Link: http://lkml.kernel.org/r/alpine.LFD.2.02.1304211648150.21884@ionos
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/timekeeping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 675f720..98cd470 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -951,7 +951,7 @@ static void timekeeping_resume(void)
 		__timekeeping_inject_sleeptime(tk, &ts_delta);
 
 	/* Re-base the last cycle value */
-	clock->cycle_last = cycle_now;
+	tk->cycle_last = clock->cycle_last = cycle_now;
 	tk->ntp_error = 0;
 	timekeeping_suspended = 0;
 	timekeeping_update(tk, false, true);