Re: [Bug 4298] swsusp fails to suspend if CONFIG_DEBUG_PAGEALLOC is also enabled

Re: [Bug 4298] swsusp fails to suspend if CONFIG_DEBUG_PAGEALLOC is also enabled

[Pavel Machek]

Hi!

> Problem Description:
> swsusp normally works, but if I enable CONFIG_DEBUG_PAGEALLOC, it breaks --
> after "PM: snapshotting memory", swsusp never gets to the "critical section" and
> it kind of bounces back from the suspend attempt, for lack of a better way for
> me to describe the effect.

Okay, that is because low-level assembly requires PSE (4mb pages for
kernel) and DEBUG_PAGEALLOC disables that capability.

If you feel like rewriting assembly code to turn off paging (and thus
working with PSE), go ahead, but I do not think it is worth the
trouble.

OTOH we should at least tell people what went wrong, some people seen
same problem on VIA cpus... Please apply,

Signed-off-by: Pavel Machek <pavel@suse.cz>

								Pavel

--- clean/include/asm-i386/suspend.h	2004-12-25 13:35:02.000000000 +0100
+++ linux/include/asm-i386/suspend.h	2005-03-02 01:05:33.000000000 +0100
@@ -10,10 +10,12 @@
 arch_prepare_suspend(void)
 {
 	/* If you want to make non-PSE machine work, turn off paging
-           in do_magic. swsusp_pg_dir should have identity mapping, so
+           in swsusp_arch_suspend. swsusp_pg_dir should have identity mapping, so
            it could work...  */
-	if (!cpu_has_pse)
+	if (!cpu_has_pse) {
+		printk(KERN_ERR "PSE is required for swsusp.\n");
 		return -EPERM;
+	}
 	return 0;
 }
 
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: [Bug 4298] swsusp fails to suspend if CONFIG_DEBUG_PAGEALLOC is also enabled

[Andrew Morton]

Pavel Machek <pavel@ucw.cz> wrote:
>
> Hi!
> 
> > Problem Description:
> > swsusp normally works, but if I enable CONFIG_DEBUG_PAGEALLOC, it breaks --
> > after "PM: snapshotting memory", swsusp never gets to the "critical section" and
> > it kind of bounces back from the suspend attempt, for lack of a better way for
> > me to describe the effect.
> 
> Okay, that is because low-level assembly requires PSE (4mb pages for
> kernel) and DEBUG_PAGEALLOC disables that capability.
> 
> If you feel like rewriting assembly code to turn off paging (and thus
> working with PSE), go ahead, but I do not think it is worth the
> trouble.
> 
> OTOH we should at least tell people what went wrong, some people seen
> same problem on VIA cpus... Please apply,
> 

Isn't some Kconfig solution appropriate here?

> 
> --- clean/include/asm-i386/suspend.h	2004-12-25 13:35:02.000000000 +0100
> +++ linux/include/asm-i386/suspend.h	2005-03-02 01:05:33.000000000 +0100
> @@ -10,10 +10,12 @@
>  arch_prepare_suspend(void)
>  {
>  	/* If you want to make non-PSE machine work, turn off paging
> -           in do_magic. swsusp_pg_dir should have identity mapping, so
> +           in swsusp_arch_suspend. swsusp_pg_dir should have identity mapping, so
>             it could work...  */
> -	if (!cpu_has_pse)
> +	if (!cpu_has_pse) {
> +		printk(KERN_ERR "PSE is required for swsusp.\n");
>  		return -EPERM;
> +	}
>  	return 0;
>  }
>  
> -- 
> People were complaining that M$ turns users into beta-testers...
> ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

[PATCH] kconfig: DEBUG_PAGEALLOC and SOFTWARE_SUSPEND are incompatible on i386

[Barry K. Nathan]

On i386, SOFTWARE_SUSPEND requires the CPU to have PSE support, but
DEBUG_PAGEALLOC disables PSE. Thus, allowing both options to be enabled
simultaneously makes no sense. This patch disables DEBUG_PAGEALLOC if
SOFTWARE_SUSPEND is enabled; it also displays a comment to briefly
explain why DEBUG_PAGEALLOC is missing in that case.

I have tested this patch against oldconfig and menuconfig on 2.6.11-bk2.

Signed-off-by: Barry K. Nathan <barryn@pobox.com>

--- linux-2.6.11-bk2/arch/i386/Kconfig.debug	2004-12-24 13:34:45.000000000 -0800
+++ linux-2.6.11-bk2-bkn2/arch/i386/Kconfig.debug	2005-03-06 20:59:07.000000000 -0800
@@ -38,9 +38,12 @@
 
 	  This option will slow down process creation somewhat.
 
+comment "Page alloc debug is incompatible with Software Suspend on i386"
+	depends on DEBUG_KERNEL && SOFTWARE_SUSPEND
+
 config DEBUG_PAGEALLOC
 	bool "Page alloc debugging"
-	depends on DEBUG_KERNEL
+	depends on DEBUG_KERNEL && !SOFTWARE_SUSPEND
 	help
 	  Unmap pages from the kernel linear mapping after free_pages().
 	  This results in a large slowdown, but helps to find certain types

Re: [PATCH] kconfig: DEBUG_PAGEALLOC and SOFTWARE_SUSPEND are incompatible on i386

[Pavel Machek]

On Ne 06-03-05 21:12:41, Barry K. Nathan wrote:
> On i386, SOFTWARE_SUSPEND requires the CPU to have PSE support, but
> DEBUG_PAGEALLOC disables PSE. Thus, allowing both options to be enabled
> simultaneously makes no sense. This patch disables DEBUG_PAGEALLOC if
> SOFTWARE_SUSPEND is enabled; it also displays a comment to briefly
> explain why DEBUG_PAGEALLOC is missing in that case.

ACK.
									Pavel

> --- linux-2.6.11-bk2/arch/i386/Kconfig.debug	2004-12-24 13:34:45.000000000 -0800
> +++ linux-2.6.11-bk2-bkn2/arch/i386/Kconfig.debug	2005-03-06 20:59:07.000000000 -0800
> @@ -38,9 +38,12 @@
>  
>  	  This option will slow down process creation somewhat.
>  
> +comment "Page alloc debug is incompatible with Software Suspend on i386"
> +	depends on DEBUG_KERNEL && SOFTWARE_SUSPEND
> +
>  config DEBUG_PAGEALLOC
>  	bool "Page alloc debugging"
> -	depends on DEBUG_KERNEL
> +	depends on DEBUG_KERNEL && !SOFTWARE_SUSPEND
>  	help
>  	  Unmap pages from the kernel linear mapping after free_pages().
>  	  This results in a large slowdown, but helps to find certain types

-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: [Bug 4298] swsusp fails to suspend if CONFIG_DEBUG_PAGEALLOC is also enabled

[Stefan Seyfried]

Andrew Morton wrote:
> Pavel Machek <pavel@ucw.cz> wrote:

>> Okay, that is because low-level assembly requires PSE (4mb pages for
>> kernel) and DEBUG_PAGEALLOC disables that capability.
>> 
>> If you feel like rewriting assembly code to turn off paging (and thus
>> working with PSE), go ahead, but I do not think it is worth the
>> trouble.
>> 
>> OTOH we should at least tell people what went wrong, some people seen
>> same problem on VIA cpus... Please apply,
>> 
> 
> Isn't some Kconfig solution appropriate here?

Yes, but only for the CONFIG_DEBUG_PAGEALLOC case, it does not solve the
"cpu has no PSE" case for VIA CPUs. So the Kconfig solution is an extra
bonus.

    Stefan

Re: [Bug 4298] swsusp fails to suspend if CONFIG_DEBUG_PAGEALLOC is also enabled

[Nigel Cunningham]

Hi,

On Mon, 2005-03-07 at 19:01, Stefan Seyfried wrote:
> Andrew Morton wrote:
> > Pavel Machek <pavel@ucw.cz> wrote:
> 
> >> Okay, that is because low-level assembly requires PSE (4mb pages for
> >> kernel) and DEBUG_PAGEALLOC disables that capability.
> >> 
> >> If you feel like rewriting assembly code to turn off paging (and thus
> >> working with PSE), go ahead, but I do not think it is worth the
> >> trouble.
> >> 
> >> OTOH we should at least tell people what went wrong, some people seen
> >> same problem on VIA cpus... Please apply,
> >> 
> > 
> > Isn't some Kconfig solution appropriate here?
> 
> Yes, but only for the CONFIG_DEBUG_PAGEALLOC case, it does not solve the
> "cpu has no PSE" case for VIA CPUs. So the Kconfig solution is an extra
> bonus.

Yeah - separate issues. Suspend2 has worked with CONFIG_DEBUG_PAGEALLOC.
You just have to map in all the pages before doing the atomic copies.

Regards,

Nigel
-- 
Nigel Cunningham
Software Engineer, Canberra, Australia
http://www.cyclades.com
Bus: +61 (2) 6291 9554; Hme: +61 (2) 6292 8028;  Mob: +61 (417) 100 574

Maintainer of Suspend2 Kernel Patches http://softwaresuspend.berlios.de

Re: [Bug 4298] swsusp fails to suspend if CONFIG_DEBUG_PAGEALLOC is also enabled

[Barry K. Nathan]

On Mon, Mar 07, 2005 at 09:01:47AM +0100, Stefan Seyfried wrote:
> Andrew Morton wrote:
[snip]
> > Isn't some Kconfig solution appropriate here?
> 
> Yes, but only for the CONFIG_DEBUG_PAGEALLOC case, it does not solve the
> "cpu has no PSE" case for VIA CPUs. So the Kconfig solution is an extra
> bonus.

Note that I've posted a Kconfig solution here:
http://marc.theaimsgroup.com/?l=linux-kernel&m=111017249931972&w=2

Regarding Pavel's patch, it seems to me that it might be better to print
the message at boot time, instead of (or in addition to?) his patch.
Maybe we should be disabling swsusp altogether at boot in that case, if
that's not unreasonably hard to implement.

-Barry K. Nathan <barryn@pobox.com>

Re: [Bug 4298] swsusp fails to suspend if CONFIG_DEBUG_PAGEALLOC is also enabled

[Pavel Machek]

Hi!

> > > Isn't some Kconfig solution appropriate here?
> > 
> > Yes, but only for the CONFIG_DEBUG_PAGEALLOC case, it does not solve the
> > "cpu has no PSE" case for VIA CPUs. So the Kconfig solution is an extra
> > bonus.
> 
> Note that I've posted a Kconfig solution here:
> http://marc.theaimsgroup.com/?l=linux-kernel&m=111017249931972&w=2
> 
> Regarding Pavel's patch, it seems to me that it might be better to print
> the message at boot time, instead of (or in addition to?) his patch.
> Maybe we should be disabling swsusp altogether at boot in that case, if
> that's not unreasonably hard to implement.

Hmm, yes, that would certainly be better. It would need new
per-architecture hook... Feel free to implement it.
								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: [Bug 4298] swsusp fails to suspend if CONFIG_DEBUG_PAGEALLOC is also enabled

[Barry K. Nathan]

On Mon, Mar 07, 2005 at 10:35:12AM +0100, Pavel Machek wrote:
> > Regarding Pavel's patch, it seems to me that it might be better to print
> > the message at boot time, instead of (or in addition to?) his patch.
> > Maybe we should be disabling swsusp altogether at boot in that case, if
> > that's not unreasonably hard to implement.
> 
> Hmm, yes, that would certainly be better. It would need new
> per-architecture hook... Feel free to implement it.

It looks like it's going to be more difficult than I was hoping for.

I would recommend accepting Pavel's patch. It's a definite improvement
over the existing code, and it could be a long while until I have a
boot-time message patch that is both functional and clean enough to be
incorporated into mainline.

-Barry K. Nathan <barryn@pobox.com>