Bug 9541 - System reboots after restoring hibernation image (s2disk)
System reboots after restoring hibernation image (s2disk)
Status: CLOSED CODE_FIX
Product: Power Management
Classification: Unclassified
Component: Hibernation/Suspend
All Linux
: P1 normal
Assigned To: Rafael J. Wysocki
:
Depends on:
Blocks: 7216 9243
  Show dependency treegraph
 
Reported: 2007-12-11 02:11 UTC by Adam Jones
Modified: 2007-12-20 16:04 UTC (History)
2 users (show)

See Also:
Kernel Version: 2.6.24-rc4
Tree: Mainline
Regression: Yes


Attachments
Kernel .config (48.01 KB, text/plain)
2007-12-11 02:13 UTC, Adam Jones
Details

Description Adam Jones 2007-12-11 02:11:46 UTC
Most recent kernel where this bug did not occur: 2.6.20.1

Distribution: Gentoo

Hardware Environment: Dell XPS M1210 (Intel T7200; 1GB RAM)

Software Environment: x86_64 SMP kernel; no driver modules loaded
init=/bin/bash ; no daemons running
s2disk (suspend) 0.9_pre20071208

Problem Description:

Hibernation with s2disk appears to work correctly - image is saved, and the system powers down.  When resuming, the image is loaded (via the resume utility from an initrd), at which point the screen goes blank and the machine reboots after a brief delay.  The image creation step appears to work correctly even from within X, but resume always fails.

Steps to reproduce:

Boot with init=/bin/bash (and resume utility run from /linuxrc in initrd)
s2disk
Power on and resume with same kernel.
Comment 1 Adam Jones 2007-12-11 02:13:14 UTC
Created attachment 13969 [details]
Kernel .config
Comment 2 Anonymous Emailer 2007-12-11 02:21:02 UTC
Reply-To: akpm@linux-foundation.org

On Tue, 11 Dec 2007 02:11:48 -0800 (PST) bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=9541
> 
>            Summary: System reboots after restoring hibernation image
>                     (s2disk)
>            Product: Power Management
>            Version: 2.5
>      KernelVersion: 2.6.24-rc4
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Hibernation/Suspend
>         AssignedTo: power-management_other@kernel-bugs.osdl.org
>         ReportedBy: adam@yggdrasl.demon.co.uk
> 
> 
> Most recent kernel where this bug did not occur: 2.6.20.1
> 
> Distribution: Gentoo
> 
> Hardware Environment: Dell XPS M1210 (Intel T7200; 1GB RAM)
> 
> Software Environment: x86_64 SMP kernel; no driver modules loaded
> init=/bin/bash ; no daemons running
> s2disk (suspend) 0.9_pre20071208
> 
> Problem Description:
> 
> Hibernation with s2disk appears to work correctly - image is saved, and the
> system powers down.  When resuming, the image is loaded (via the resume utility
> from an initrd), at which point the screen goes blank and the machine reboots
> after a brief delay.  The image creation step appears to work correctly even
> from within X, but resume always fails.
> 
> Steps to reproduce:
> 
> Boot with init=/bin/bash (and resume utility run from /linuxrc in initrd)
> s2disk
> Power on and resume with same kernel.

Yeah, I just hunted this down.  Please test this:

From: Andrew Morton <akpm@linux-foundation.org>

Revert

    Commit:     efa4d2fb047b25a6be67fe92178a2a78da6b3f6a

        Hibernation: Use temporary page tables for kernel text mapping on x86_64

Because it causes my t61p to reboot right at the end of resume-from-disk.

Cc: Pavel Machek <pavel@ucw.cz>
Cc: Andi Kleen <ak@suse.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 arch/x86/kernel/suspend_64.c |   39 +++++----------------------------
 1 file changed, 6 insertions(+), 33 deletions(-)

diff -puN arch/x86/kernel/suspend_64.c~revert-hibernation-use-temporary-page-tables-for-kernel-text-mapping-on-x86_64 arch/x86/kernel/suspend_64.c
--- a/arch/x86/kernel/suspend_64.c~revert-hibernation-use-temporary-page-tables-for-kernel-text-mapping-on-x86_64
+++ a/arch/x86/kernel/suspend_64.c
@@ -192,42 +192,25 @@ static int res_phys_pud_init(pud_t *pud,
 	return 0;
 }
 
-static int res_kernel_text_pud_init(pud_t *pud, unsigned long start)
-{
-	pmd_t *pmd;
-	unsigned long paddr;
-
-	pmd = (pmd_t *)get_safe_page(GFP_ATOMIC);
-	if (!pmd)
-		return -ENOMEM;
-	set_pud(pud + pud_index(start), __pud(__pa(pmd) | _KERNPG_TABLE));
-	for (paddr = 0; paddr < KERNEL_TEXT_SIZE; pmd++, paddr += PMD_SIZE) {
-		unsigned long pe;
-
-		pe = __PAGE_KERNEL_LARGE_EXEC | _PAGE_GLOBAL | paddr;
-		pe &= __supported_pte_mask;
-		set_pmd(pmd, __pmd(pe));
-	}
-
-	return 0;
-}
-
 static int set_up_temporary_mappings(void)
 {
 	unsigned long start, end, next;
-	pud_t *pud;
 	int error;
 
 	temp_level4_pgt = (pgd_t *)get_safe_page(GFP_ATOMIC);
 	if (!temp_level4_pgt)
 		return -ENOMEM;
 
+	/* It is safe to reuse the original kernel mapping */
+	set_pgd(temp_level4_pgt + pgd_index(__START_KERNEL_map),
+		init_level4_pgt[pgd_index(__START_KERNEL_map)]);
+
 	/* Set up the direct mapping from scratch */
 	start = (unsigned long)pfn_to_kaddr(0);
 	end = (unsigned long)pfn_to_kaddr(end_pfn);
 
 	for (; start < end; start = next) {
-		pud = (pud_t *)get_safe_page(GFP_ATOMIC);
+		pud_t *pud = (pud_t *)get_safe_page(GFP_ATOMIC);
 		if (!pud)
 			return -ENOMEM;
 		next = start + PGDIR_SIZE;
@@ -238,17 +221,7 @@ static int set_up_temporary_mappings(voi
 		set_pgd(temp_level4_pgt + pgd_index(start),
 			mk_kernel_pgd(__pa(pud)));
 	}
-
-	/* Set up the kernel text mapping from scratch */
-	pud = (pud_t *)get_safe_page(GFP_ATOMIC);
-	if (!pud)
-		return -ENOMEM;
-	error = res_kernel_text_pud_init(pud, __START_KERNEL_map);
-	if (!error)
-		set_pgd(temp_level4_pgt + pgd_index(__START_KERNEL_map),
-			__pgd(__pa(pud) | _PAGE_TABLE));
-
-	return error;
+	return 0;
 }
 
 int swsusp_arch_resume(void)
_

Comment 3 Adam Jones 2007-12-11 03:10:09 UTC
Yep, that fixes it.  Hibernate and resume from/to X worked first time.  Thanks!

I guess this and the freezer-do_mounts_initrd-debug.patch need merging for 2.6.24.
Comment 4 Andrew Morton 2007-12-11 03:19:33 UTC
Well..  hopefully we can find the bug and fix it rather than a
plain revert.  Let's wait for Rafael to have think...

What the heck is freezer-do_mounts_initrd-debug.patch?
Comment 5 Adam Jones 2007-12-11 03:20:40 UTC
On Tue, 11 Dec 2007 02:20:31 -0800, Andrew Morton
<akpm@linux-foundation.org> wrote:

> Yeah, I just hunted this down.  Please test this:
> 
> From: Andrew Morton <akpm@linux-foundation.org>
> 
> Revert
> 
>     Commit:     efa4d2fb047b25a6be67fe92178a2a78da6b3f6a
> 
>         Hibernation: Use temporary page tables for kernel text
> mapping on x86_64

This (together with the freezer/linuxrc patch) fixes the problem for me.
Hibernation now works fine from within X, and everything comes back up
as expected.

Thanks for the (startlingly) quick response.
Comment 6 Adam Jones 2007-12-11 03:36:49 UTC
(In reply to comment #4)

> What the heck is freezer-do_mounts_initrd-debug.patch?

Rafael's patch from bug #9345, which I think may already have been applied to -git:
http://git.kernel.org/gitweb.cgi?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=8baabde66c60a84781c718c28fe283ed411a7bd0
Comment 7 Rafael J. Wysocki 2007-12-11 12:51:38 UTC
Can anyone who owns this bug reassign it to me, please?

Comment #6 is correct, BTW.
Comment 8 Rafael J. Wysocki 2007-12-11 13:04:55 UTC
(In reply to comment #7)
> Can anyone who owns this bug reassign it to me, please?

OK, I did that myself (using the other bugzilla account I have - looks like there's a bug in the user interface).
Comment 9 Rafael J. Wysocki 2007-12-11 14:42:32 UTC
(In reply to comment #4)
> Well..  hopefully we can find the bug and fix it rather than a
> plain revert.  Let's wait for Rafael to have think...

The failure is highly suspicious, as it doesn't appear on the AMD-based boxes I tested this patch on.  Well, perhaps my kernels are just configured differently than yours.

For now, I have no idea what's wrong, so I think the safest solution for 2.6.24 would be to revert commit efa4d2fb047b25a6be67fe92178a2a78da6b3f6a as suggested.
Comment 10 Rafael J. Wysocki 2007-12-12 15:45:20 UTC
The only reasonable explanation of the problem which comes to mind is that your kernels make the kernel code be located in a different physical memory region than  hibernation-use-temporary-page-tables-for-kernel-text-mapping-on-x86_64.patch assumes it to be in.

I see that the Adam's kernel is relacatable.  Andrew, is your (failing) kernel relocatable too?
Comment 11 Rafael J. Wysocki 2007-12-20 16:04:52 UTC
Fixed by:

commit 5867a78f41f84e5388448da62c183255dc22601f
Author: Andrew Morton <akpm@linux-foundation.org>
Date:   Mon Dec 17 16:19:45 2007 -0800

    revert "Hibernation: Use temporary page tables for kernel text mapping on x86_64"

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=5867a78f41f84e5388448da62c183255dc22601f

Note You need to log in before you can comment on or make changes to this bug.