2.6.21-rc5: swsusp: Not enough free memory

2.6.21-rc5: swsusp: Not enough free memory

[Jiri Slaby]

Hi,

I'm getting this while trying to swsups the machine in -rc5, -rc4 is fine:

Disabling non-boot CPUs
CPU 1 is now offline
SMP alternatives: switching to UP code
CPU1 is down
swsusp: critical section:
swsusp: Need to copy 131380 pages
swsusp: Not enough free memory
Error -12 suspending
Enabling non-boot CPUs ...

# cat /sys/power/resume
8:6
# cat /proc/swaps
Filename                                Type            Size    Used    Priority
/dev/sda6                               partition       1004020 0       -1

Any other info needed?

BTW. is this OK on resume?
PM: Writing back config space on device 0000:02:00.0 at offset 1 (was 100006, w
iting 100002)
pnp: Failed to activate device 00:04.
pnp: Failed to activate device 00:05.
 usbdev5.3_ep00: PM: resume from 0, parent 5-4 still 2

# ll /sys/bus/pnp/devices/00\:0{4,5}/driver
lrwxrwxrwx 1 root root 0 2007-03-29 09:16 /sys/bus/pnp/devices/00:04/driver
-> ../../../bus/pnp/drivers/i8042 kbd
lrwxrwxrwx 1 root root 0 2007-03-29 09:16 /sys/bus/pnp/devices/00:05/driver
-> ../../../bus/pnp/drivers/i8042 aux

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Hnus <hnus@fi.muni.cz> is an alias for /dev/null

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Rafael J. Wysocki]

On Thursday, 29 March 2007 09:44, Jiri Slaby wrote:
> Hi,
> 
> I'm getting this while trying to swsups the machine in -rc5, -rc4 is fine:
> 
> Disabling non-boot CPUs
> CPU 1 is now offline
> SMP alternatives: switching to UP code
> CPU1 is down
> swsusp: critical section:
> swsusp: Need to copy 131380 pages
> swsusp: Not enough free memory
> Error -12 suspending
> Enabling non-boot CPUs ...
> 
> # cat /sys/power/resume
> 8:6
> # cat /proc/swaps
> Filename                                Type            Size    Used    Priority
> /dev/sda6                               partition       1004020 0       -1
> 
> Any other info needed?

Beats me.  There were no changes that could result in such a thing between
-rc4 and -rc5, at least not in the swsusp department.

Could you please try to bisect?

> BTW. is this OK on resume?
> PM: Writing back config space on device 0000:02:00.0 at offset 1 (was 100006, w
> iting 100002)
> pnp: Failed to activate device 00:04.
> pnp: Failed to activate device 00:05.
>  usbdev5.3_ep00: PM: resume from 0, parent 5-4 still 2
> 
> # ll /sys/bus/pnp/devices/00\:0{4,5}/driver
> lrwxrwxrwx 1 root root 0 2007-03-29 09:16 /sys/bus/pnp/devices/00:04/driver
> -> ../../../bus/pnp/drivers/i8042 kbd
> lrwxrwxrwx 1 root root 0 2007-03-29 09:16 /sys/bus/pnp/devices/00:05/driver
> -> ../../../bus/pnp/drivers/i8042 aux

Well, I don't think so.

Greetings,
Rafael

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Jiri Slaby]

Rafael J. Wysocki napsal(a):
> On Thursday, 29 March 2007 09:44, Jiri Slaby wrote:
>> Hi,
>>
>> I'm getting this while trying to swsups the machine in -rc5, -rc4 is fine:
>>
>> Disabling non-boot CPUs
>> CPU 1 is now offline
>> SMP alternatives: switching to UP code
>> CPU1 is down
>> swsusp: critical section:
>> swsusp: Need to copy 131380 pages
>> swsusp: Not enough free memory
>> Error -12 suspending
>> Enabling non-boot CPUs ...
>>
>> # cat /sys/power/resume
>> 8:6
>> # cat /proc/swaps
>> Filename                                Type            Size    Used    Priority
>> /dev/sda6                               partition       1004020 0       -1
>>
>> Any other info needed?
> 
> Beats me.  There were no changes that could result in such a thing between
> -rc4 and -rc5, at least not in the swsusp department.
> 
> Could you please try to bisect?

Yes, I will.

>> BTW. is this OK on resume?
>> PM: Writing back config space on device 0000:02:00.0 at offset 1 (was 100006, w
>> iting 100002)
>> pnp: Failed to activate device 00:04.
>> pnp: Failed to activate device 00:05.
>>  usbdev5.3_ep00: PM: resume from 0, parent 5-4 still 2
>>
>> # ll /sys/bus/pnp/devices/00\:0{4,5}/driver
>> lrwxrwxrwx 1 root root 0 2007-03-29 09:16 /sys/bus/pnp/devices/00:04/driver
>> -> ../../../bus/pnp/drivers/i8042 kbd
>> lrwxrwxrwx 1 root root 0 2007-03-29 09:16 /sys/bus/pnp/devices/00:05/driver
>> -> ../../../bus/pnp/drivers/i8042 aux
> 
> Well, I don't think so.

(This is probably not a regression, it's in -rc4 too.)

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Hnus <hnus@fi.muni.cz> is an alias for /dev/null

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Jiri Slaby]

Rafael J. Wysocki napsal(a):
> On Thursday, 29 March 2007 09:44, Jiri Slaby wrote:
>> Hi,
>>
>> I'm getting this while trying to swsups the machine in -rc5, -rc4 is fine:
>>
>> Disabling non-boot CPUs
>> CPU 1 is now offline
>> SMP alternatives: switching to UP code
>> CPU1 is down
>> swsusp: critical section:
>> swsusp: Need to copy 131380 pages
>> swsusp: Not enough free memory
>> Error -12 suspending
>> Enabling non-boot CPUs ...
>>
>> # cat /sys/power/resume
>> 8:6
>> # cat /proc/swaps
>> Filename                                Type            Size    Used    Priority
>> /dev/sda6                               partition       1004020 0       -1
>>
>> Any other info needed?
> 
> Beats me.  There were no changes that could result in such a thing between
> -rc4 and -rc5, at least not in the swsusp department.
> 
> Could you please try to bisect?

Hm, there is some kind of magic.

First, I have fglrx, that taints kernel. If I use vesa drv with X, it
doesn't resume the card. If I try console, it doesn't resume it too.

fglrx + suspend.sf.net seems to work -- 3 unsuccesfull disk >
sys/power/state and after s2dsk with backspace during suspend, disk >
sys/power/state works. But this sceniario happened only once...

Next, it was so early to utter -rc4 is good, it happens there too, so it's
not a regression.

I have no idea what's wrong, is there any possibility to figure out, what
happens (esp. kick fglrx off)? Disable higmem? Try UP?

thanks,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Hnus <hnus@fi.muni.cz> is an alias for /dev/null

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Rafael J. Wysocki]

On Sunday, 1 April 2007 20:17, Jiri Slaby wrote:
> Rafael J. Wysocki napsal(a):
> > On Thursday, 29 March 2007 09:44, Jiri Slaby wrote:
> >> Hi,
> >>
> >> I'm getting this while trying to swsups the machine in -rc5, -rc4 is fine:
> >>
> >> Disabling non-boot CPUs
> >> CPU 1 is now offline
> >> SMP alternatives: switching to UP code
> >> CPU1 is down
> >> swsusp: critical section:
> >> swsusp: Need to copy 131380 pages
> >> swsusp: Not enough free memory
> >> Error -12 suspending
> >> Enabling non-boot CPUs ...
> >>
> >> # cat /sys/power/resume
> >> 8:6
> >> # cat /proc/swaps
> >> Filename                                Type            Size    Used    Priority
> >> /dev/sda6                               partition       1004020 0       -1
> >>
> >> Any other info needed?
> > 
> > Beats me.  There were no changes that could result in such a thing between
> > -rc4 and -rc5, at least not in the swsusp department.
> > 
> > Could you please try to bisect?
> 
> Hm, there is some kind of magic.
> 
> First, I have fglrx, that taints kernel. If I use vesa drv with X, it
> doesn't resume the card. If I try console, it doesn't resume it too.
> 
> fglrx + suspend.sf.net seems to work -- 3 unsuccesfull disk >
> sys/power/state and after s2dsk with backspace during suspend, disk >
> sys/power/state works. But this sceniario happened only once...
> 
> Next, it was so early to utter -rc4 is good, it happens there too, so it's
> not a regression.
> 
> I have no idea what's wrong, is there any possibility to figure out, what
> happens (esp. kick fglrx off)? Disable higmem? Try UP?

Well, I suspect this is somehow related to highmem, so you can try to check
if disabling highmem helps.

Still, I'd like to understand why it occurs (I can't reproduce it, so far) and
I have a theory.  Namely, I think that on your system the initial image size
is greater than 50% of RAM (you can check that by running
"cat /sys/power/image_size" before you suspend for the first time after a
fresh boot) and the memory shrinker fails to do its job in that case.

What s2disk does is to set image_size below 50% of the RAM size and that's why
the subsequent "echo disk > ..." suspend works too.

As a workaround, you can try to change the initial image size so that it's
smaller than a half of the RAM size.  If that works, I'd like to send you a
debug patch, if you don't mind. :-)

Greetings,
Rafael

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Jiri Slaby]

Rafael J. Wysocki napsal(a):
>>> On Thursday, 29 March 2007 09:44, Jiri Slaby wrote:
>>>> swsusp: critical section:
>>>> swsusp: Need to copy 131380 pages
>>>> swsusp: Not enough free memory
>>>> Error -12 suspending
>>>> Enabling non-boot CPUs ...

> As a workaround, you can try to change the initial image size so that it's
> smaller than a half of the RAM size.  If that works, I'd like to send you a
> debug patch, if you don't mind. :-)

Yes, post it.

# cat /sys/power/image_size
524288000
# echo disk >/sys/power/state
error
# echo disk >/sys/power/state
error
# echo $((400*1024*1024)) > /sys/power/image_size
# echo disk >/sys/power/state
ok
# dmesg|grep Memory:
Memory: 1027160k/1048256k available (2559k kernel code, 20348k reserved,
1114k data, 188k init)

I don't undestand it too much, but can't be shared video memory involved
somehow?

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Hnus <hnus@fi.muni.cz> is an alias for /dev/null

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Rafael J. Wysocki]

On Monday, 2 April 2007 10:24, Jiri Slaby wrote:
> Rafael J. Wysocki napsal(a):
> >>> On Thursday, 29 March 2007 09:44, Jiri Slaby wrote:
> >>>> swsusp: critical section:
> >>>> swsusp: Need to copy 131380 pages
> >>>> swsusp: Not enough free memory
> >>>> Error -12 suspending
> >>>> Enabling non-boot CPUs ...
> 
> > As a workaround, you can try to change the initial image size so that it's
> > smaller than a half of the RAM size.  If that works, I'd like to send you a
> > debug patch, if you don't mind. :-)
> 
> Yes, post it.

Appended.  Please send me the dmesg output from after a failing suspend
(or in case it doesn't fail, from after the first one).

> # cat /sys/power/image_size
> 524288000
> # echo disk >/sys/power/state
> error
> # echo disk >/sys/power/state
> error
> # echo $((400*1024*1024)) > /sys/power/image_size
> # echo disk >/sys/power/state
> ok
> # dmesg|grep Memory:
> Memory: 1027160k/1048256k available (2559k kernel code, 20348k reserved,
> 1114k data, 188k init)
> 
> I don't undestand it too much, but can't be shared video memory involved
> somehow?

No, I don't think so.  There probably is a bug in swsusp_shrink_memory().

Greetings,
Rafael

---
---
 kernel/power/snapshot.c |    2 +-
 kernel/power/swsusp.c   |   10 +++++++---
 2 files changed, 8 insertions(+), 4 deletions(-)

Index: linux-2.6.21-rc5/kernel/power/snapshot.c
===================================================================
--- linux-2.6.21-rc5.orig/kernel/power/snapshot.c
+++ linux-2.6.21-rc5/kernel/power/snapshot.c
@@ -1093,7 +1093,7 @@ static int enough_free_mem(unsigned int 
 	}
 
 	nr_pages += count_pages_for_highmem(nr_highmem);
-	pr_debug("swsusp: Normal pages needed: %u + %u + %u, available pages: %u\n",
+	printk("swsusp: Normal pages needed: %u + %u + %u, available pages: %u\n",
 		nr_pages, PAGES_FOR_IO, meta, free);
 
 	return free > nr_pages + PAGES_FOR_IO + meta;
Index: linux-2.6.21-rc5/kernel/power/swsusp.c
===================================================================
--- linux-2.6.21-rc5.orig/kernel/power/swsusp.c
+++ linux-2.6.21-rc5/kernel/power/swsusp.c
@@ -227,22 +227,26 @@ int swsusp_shrink_memory(void)
 		size = count_data_pages() + PAGES_FOR_IO;
 		tmp = size;
 		size += highmem_size;
-		for_each_zone (zone)
+		for_each_zone (zone) {
+			printk("Normal pages needed: %lu\n", tmp);
+			printk("Highmem pages needed: %lu\n", highmem_size);
 			if (populated_zone(zone)) {
+				tmp += snapshot_additional_pages(zone);
 				if (is_highmem(zone)) {
 					highmem_size -=
 					zone_page_state(zone, NR_FREE_PAGES);
 				} else {
 					tmp -= zone_page_state(zone, NR_FREE_PAGES);
 					tmp += zone->lowmem_reserve[ZONE_NORMAL];
-					tmp += snapshot_additional_pages(zone);
 				}
 			}
+		}
 
 		if (highmem_size < 0)
 			highmem_size = 0;
 
 		tmp += highmem_size;
+		printk("Pages needed (total): %lu\n", tmp);
 		if (tmp > 0) {
 			tmp = __shrink_memory(tmp);
 			if (!tmp)
@@ -252,7 +256,7 @@ int swsusp_shrink_memory(void)
 			tmp = __shrink_memory(size - (image_size / PAGE_SIZE));
 			pages += tmp;
 		}
-		printk("\b%c", p[i++%4]);
+		/*printk("\b%c", p[i++%4]);*/
 	} while (tmp > 0);
 	do_gettimeofday(&stop);
 	printk("\bdone (%lu pages freed)\n", pages);

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Jiri Slaby]

Rafael J. Wysocki napsal(a):
> On Monday, 2 April 2007 10:24, Jiri Slaby wrote:
>> Rafael J. Wysocki napsal(a):
>>>>> On Thursday, 29 March 2007 09:44, Jiri Slaby wrote:
>>>>>> swsusp: critical section:
>>>>>> swsusp: Need to copy 131380 pages
>>>>>> swsusp: Not enough free memory
>>>>>> Error -12 suspending
>>>>>> Enabling non-boot CPUs ...
>>> As a workaround, you can try to change the initial image size so that it's
>>> smaller than a half of the RAM size.  If that works, I'd like to send you a
>>> debug patch, if you don't mind. :-)
>> Yes, post it.
> 
> Appended.  Please send me the dmesg output from after a failing suspend
> (or in case it doesn't fail, from after the first one).

It doesn't fail, here is the output:

Shrinking memory...  Normal pages needed: 135122
Highmem pages needed: 0
Normal pages needed: 132256
Highmem pages needed: 0
Normal pages needed: 8271
Highmem pages needed: 0
Pages needed (total): 8271
Normal pages needed: 119657
Highmem pages needed: 0
Normal pages needed: 116791
Highmem pages needed: 0
Normal pages needed: 18446744073709528969
Highmem pages needed: 0
Pages needed (total): 18446744073709528969

Heh :), tmp is signed, this is -22647, I guess.

done (15505 pages freed)
Freed 62020 kbytes in 0.16 seconds (387.62 MB/s)

Complete dmesg may be obtained here:
http://www.fi.muni.cz/~xslaby/sklad/dmesg.swsusp

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Hnus <hnus@fi.muni.cz> is an alias for /dev/null

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Rafael J. Wysocki]

On Tuesday, 3 April 2007 09:37, Jiri Slaby wrote:
> Rafael J. Wysocki napsal(a):
> > On Monday, 2 April 2007 10:24, Jiri Slaby wrote:
> >> Rafael J. Wysocki napsal(a):
> >>>>> On Thursday, 29 March 2007 09:44, Jiri Slaby wrote:
> >>>>>> swsusp: critical section:
> >>>>>> swsusp: Need to copy 131380 pages
> >>>>>> swsusp: Not enough free memory
> >>>>>> Error -12 suspending
> >>>>>> Enabling non-boot CPUs ...
> >>> As a workaround, you can try to change the initial image size so that it's
> >>> smaller than a half of the RAM size.  If that works, I'd like to send you a
> >>> debug patch, if you don't mind. :-)
> >> Yes, post it.
> > 
> > Appended.  Please send me the dmesg output from after a failing suspend
> > (or in case it doesn't fail, from after the first one).
> 
> It doesn't fail, here is the output:

Okay, so I think the appended patch is needed.  Could you please revert the
debug one, apply the appended one instead and see if it helps?

> Shrinking memory...  Normal pages needed: 135122
> Highmem pages needed: 0
> Normal pages needed: 132256
> Highmem pages needed: 0
> Normal pages needed: 8271
> Highmem pages needed: 0
> Pages needed (total): 8271
> Normal pages needed: 119657
> Highmem pages needed: 0
> Normal pages needed: 116791
> Highmem pages needed: 0
> Normal pages needed: 18446744073709528969
> Highmem pages needed: 0
> Pages needed (total): 18446744073709528969
> 
> Heh :), tmp is signed, this is -22647, I guess.

Yup.

> done (15505 pages freed)
> Freed 62020 kbytes in 0.16 seconds (387.62 MB/s)
> 
> Complete dmesg may be obtained here:
> http://www.fi.muni.cz/~xslaby/sklad/dmesg.swsusp

Thanks for testing.

Greetings,
Rafael

---
 kernel/power/swsusp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.21-rc5/kernel/power/swsusp.c
===================================================================
--- linux-2.6.21-rc5.orig/kernel/power/swsusp.c
+++ linux-2.6.21-rc5/kernel/power/swsusp.c
@@ -229,13 +229,13 @@ int swsusp_shrink_memory(void)
 		size += highmem_size;
 		for_each_zone (zone)
 			if (populated_zone(zone)) {
+				tmp += snapshot_additional_pages(zone);
 				if (is_highmem(zone)) {
 					highmem_size -=
 					zone_page_state(zone, NR_FREE_PAGES);
 				} else {
 					tmp -= zone_page_state(zone, NR_FREE_PAGES);
 					tmp += zone->lowmem_reserve[ZONE_NORMAL];
-					tmp += snapshot_additional_pages(zone);
 				}
 			}
 

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Jiri Slaby]

Rafael J. Wysocki napsal(a):
> Okay, so I think the appended patch is needed.  Could you please revert the
[...]
> Index: linux-2.6.21-rc5/kernel/power/swsusp.c
> ===================================================================
> --- linux-2.6.21-rc5.orig/kernel/power/swsusp.c
> +++ linux-2.6.21-rc5/kernel/power/swsusp.c
> @@ -229,13 +229,13 @@ int swsusp_shrink_memory(void)
>  		size += highmem_size;
>  		for_each_zone (zone)
>  			if (populated_zone(zone)) {
> +				tmp += snapshot_additional_pages(zone);
>  				if (is_highmem(zone)) {
>  					highmem_size -=
>  					zone_page_state(zone, NR_FREE_PAGES);
>  				} else {
>  					tmp -= zone_page_state(zone, NR_FREE_PAGES);
>  					tmp += zone->lowmem_reserve[ZONE_NORMAL];
> -					tmp += snapshot_additional_pages(zone);

This seems to work.

thanks,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Hnus <hnus@fi.muni.cz> is an alias for /dev/null

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Jiri Slaby]

Rafael J. Wysocki napsal(a):
> Thanks for testing.
[...]
> --- linux-2.6.21-rc5.orig/kernel/power/swsusp.c
> +++ linux-2.6.21-rc5/kernel/power/swsusp.c
> @@ -229,13 +229,13 @@ int swsusp_shrink_memory(void)
>  		size += highmem_size;
>  		for_each_zone (zone)
>  			if (populated_zone(zone)) {
> +				tmp += snapshot_additional_pages(zone);
>  				if (is_highmem(zone)) {
>  					highmem_size -=
>  					zone_page_state(zone, NR_FREE_PAGES);
>  				} else {
>  					tmp -= zone_page_state(zone, NR_FREE_PAGES);
>  					tmp += zone->lowmem_reserve[ZONE_NORMAL];
> -					tmp += snapshot_additional_pages(zone);

I have bad news for you :(. I thought I had unpatched kernel, but it happens
in -rc6 too.

Next, when this occurs, kernel CPU1 is not brought up and next suspend hangs
on "Suspending consoles".

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Hnus <hnus@fi.muni.cz> is an alias for /dev/null

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Rafael J. Wysocki]

On Monday, 9 April 2007 22:07, Jiri Slaby wrote:
> Rafael J. Wysocki napsal(a):
> > Thanks for testing.
> [...]
> > --- linux-2.6.21-rc5.orig/kernel/power/swsusp.c
> > +++ linux-2.6.21-rc5/kernel/power/swsusp.c
> > @@ -229,13 +229,13 @@ int swsusp_shrink_memory(void)
> >  		size += highmem_size;
> >  		for_each_zone (zone)
> >  			if (populated_zone(zone)) {
> > +				tmp += snapshot_additional_pages(zone);
> >  				if (is_highmem(zone)) {
> >  					highmem_size -=
> >  					zone_page_state(zone, NR_FREE_PAGES);
> >  				} else {
> >  					tmp -= zone_page_state(zone, NR_FREE_PAGES);
> >  					tmp += zone->lowmem_reserve[ZONE_NORMAL];
> > -					tmp += snapshot_additional_pages(zone);
> 
> I have bad news for you :(. I thought I had unpatched kernel, but it happens
> in -rc6 too.

I guess you mean you're still seeing the 'not enough memory to suspend'
problem?
 
> Next, when this occurs, kernel CPU1 is not brought up and next suspend hangs
> on "Suspending consoles".

Hm, there probably is a bug in an error path.  Do you use the built in swsusp
or s2disk?

Rafael

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Jiri Slaby]

Rafael J. Wysocki napsal(a):
> On Monday, 9 April 2007 22:07, Jiri Slaby wrote:
>> Rafael J. Wysocki napsal(a):
>>> Thanks for testing.
>> [...]
>>> --- linux-2.6.21-rc5.orig/kernel/power/swsusp.c
>>> +++ linux-2.6.21-rc5/kernel/power/swsusp.c
>>> @@ -229,13 +229,13 @@ int swsusp_shrink_memory(void)
>>>  		size += highmem_size;
>>>  		for_each_zone (zone)
>>>  			if (populated_zone(zone)) {
>>> +				tmp += snapshot_additional_pages(zone);
>>>  				if (is_highmem(zone)) {
>>>  					highmem_size -=
>>>  					zone_page_state(zone, NR_FREE_PAGES);
>>>  				} else {
>>>  					tmp -= zone_page_state(zone, NR_FREE_PAGES);
>>>  					tmp += zone->lowmem_reserve[ZONE_NORMAL];
>>> -					tmp += snapshot_additional_pages(zone);
>> I have bad news for you :(. I thought I had unpatched kernel, but it happens
>> in -rc6 too.
> 
> I guess you mean you're still seeing the 'not enough memory to suspend'
> problem?

Yes:
Disabling non-boot CPUs ...
kvm: disabling virtualization on CPU1
Breaking affinity for irq 9
CPU 1 is now offline
SMP alternatives: switching to UP code
CPU1 is down
swsusp: critical section:
swsusp: Need to copy 158309 pages
swsusp: Not enough free memory
Error -12 suspending
Enabling non-boot CPUs ...
SMP alternatives: switching to SMP code
Booting processor 1/2 APIC 0x1
Initializing CPU#1

>> Next, when this occurs, kernel CPU1 is not brought up and next suspend hangs
>> on "Suspending consoles".
> 
> Hm, there probably is a bug in an error path.  Do you use the built in swsusp
> or s2disk?

built-in. It happens only sometimes, here, as you can see, CPU1 was
successfully brought up.

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Hnus <hnus@fi.muni.cz> is an alias for /dev/null

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Rafael J. Wysocki]

On Wednesday, 11 April 2007 09:36, Jiri Slaby wrote:
> Rafael J. Wysocki napsal(a):
> > On Monday, 9 April 2007 22:07, Jiri Slaby wrote:
> >> Rafael J. Wysocki napsal(a):
> >>> Thanks for testing.
> >> [...]
> >>> --- linux-2.6.21-rc5.orig/kernel/power/swsusp.c
> >>> +++ linux-2.6.21-rc5/kernel/power/swsusp.c
> >>> @@ -229,13 +229,13 @@ int swsusp_shrink_memory(void)
> >>>  		size += highmem_size;
> >>>  		for_each_zone (zone)
> >>>  			if (populated_zone(zone)) {
> >>> +				tmp += snapshot_additional_pages(zone);
> >>>  				if (is_highmem(zone)) {
> >>>  					highmem_size -=
> >>>  					zone_page_state(zone, NR_FREE_PAGES);
> >>>  				} else {
> >>>  					tmp -= zone_page_state(zone, NR_FREE_PAGES);
> >>>  					tmp += zone->lowmem_reserve[ZONE_NORMAL];
> >>> -					tmp += snapshot_additional_pages(zone);
> >> I have bad news for you :(. I thought I had unpatched kernel, but it happens
> >> in -rc6 too.
> > 
> > I guess you mean you're still seeing the 'not enough memory to suspend'
> > problem?
> 
> Yes:
> Disabling non-boot CPUs ...
> kvm: disabling virtualization on CPU1
> Breaking affinity for irq 9
> CPU 1 is now offline
> SMP alternatives: switching to UP code
> CPU1 is down
> swsusp: critical section:
> swsusp: Need to copy 158309 pages
> swsusp: Not enough free memory
> Error -12 suspending
> Enabling non-boot CPUs ...
> SMP alternatives: switching to SMP code
> Booting processor 1/2 APIC 0x1
> Initializing CPU#1

How reproducible is it?  I'm going to try to reproduce it on one of my boxes.

> >> Next, when this occurs, kernel CPU1 is not brought up and next suspend hangs
> >> on "Suspending consoles".
> > 
> > Hm, there probably is a bug in an error path.  Do you use the built in swsusp
> > or s2disk?
> 
> built-in. It happens only sometimes, here, as you can see, CPU1 was
> successfully brought up.

Sigh.  Then probably there's a problem with the CPU hotplugging and it'll be
hard to find, I'm afraid.

Greetings,
Rafael

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Jiri Slaby]

Rafael J. Wysocki napsal(a):
> On Wednesday, 11 April 2007 09:36, Jiri Slaby wrote:
>> Rafael J. Wysocki napsal(a):
>>> On Monday, 9 April 2007 22:07, Jiri Slaby wrote:
>>>> I have bad news for you :(. I thought I had unpatched kernel, but it happens
>>>> in -rc6 too.
>>> I guess you mean you're still seeing the 'not enough memory to suspend'
>>> problem?
>> Yes:
>> Disabling non-boot CPUs ...
>> kvm: disabling virtualization on CPU1
>> Breaking affinity for irq 9
>> CPU 1 is now offline
>> SMP alternatives: switching to UP code
>> CPU1 is down
>> swsusp: critical section:
>> swsusp: Need to copy 158309 pages
>> swsusp: Not enough free memory
>> Error -12 suspending
>> Enabling non-boot CPUs ...
>> SMP alternatives: switching to SMP code
>> Booting processor 1/2 APIC 0x1
>> Initializing CPU#1
> 
> How reproducible is it?  I'm going to try to reproduce it on one of my boxes.

My tip is one of three cases: after some work on fresh boot -- some
consumers such as thunderbird, firefox, 10 or so terminals with
gnome-session. Single xterm + gnome-session semms not to be a problem.

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Rafael J. Wysocki]

On Wednesday, 11 April 2007 12:45, Jiri Slaby wrote:
> Rafael J. Wysocki napsal(a):
> > On Wednesday, 11 April 2007 09:36, Jiri Slaby wrote:
> >> Rafael J. Wysocki napsal(a):
> >>> On Monday, 9 April 2007 22:07, Jiri Slaby wrote:
> >>>> I have bad news for you :(. I thought I had unpatched kernel, but it happens
> >>>> in -rc6 too.
> >>> I guess you mean you're still seeing the 'not enough memory to suspend'
> >>> problem?
> >> Yes:
> >> Disabling non-boot CPUs ...
> >> kvm: disabling virtualization on CPU1
> >> Breaking affinity for irq 9
> >> CPU 1 is now offline
> >> SMP alternatives: switching to UP code
> >> CPU1 is down
> >> swsusp: critical section:
> >> swsusp: Need to copy 158309 pages
> >> swsusp: Not enough free memory
> >> Error -12 suspending
> >> Enabling non-boot CPUs ...
> >> SMP alternatives: switching to SMP code
> >> Booting processor 1/2 APIC 0x1
> >> Initializing CPU#1
> > 
> > How reproducible is it?  I'm going to try to reproduce it on one of my boxes.
> 
> My tip is one of three cases: after some work on fresh boot -- some
> consumers such as thunderbird, firefox, 10 or so terminals with
> gnome-session. Single xterm + gnome-session semms not to be a problem.

Does the workaround with setting the image size below 1/2 of RAM work for you?

Rafael

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Jiri Slaby]

Rafael J. Wysocki napsal(a):
> On Wednesday, 11 April 2007 12:45, Jiri Slaby wrote:
>> Rafael J. Wysocki napsal(a):
>>> On Wednesday, 11 April 2007 09:36, Jiri Slaby wrote:
>>>> Rafael J. Wysocki napsal(a):
>>>>> On Monday, 9 April 2007 22:07, Jiri Slaby wrote:
>>>>>> I have bad news for you :(. I thought I had unpatched kernel, but it happens
>>>>>> in -rc6 too.
>>>>> I guess you mean you're still seeing the 'not enough memory to suspend'
>>>>> problem?
>>>> Yes:
>>>> Disabling non-boot CPUs ...
>>>> kvm: disabling virtualization on CPU1
>>>> Breaking affinity for irq 9
>>>> CPU 1 is now offline
>>>> SMP alternatives: switching to UP code
>>>> CPU1 is down
>>>> swsusp: critical section:
>>>> swsusp: Need to copy 158309 pages
>>>> swsusp: Not enough free memory
>>>> Error -12 suspending
>>>> Enabling non-boot CPUs ...
>>>> SMP alternatives: switching to SMP code
>>>> Booting processor 1/2 APIC 0x1
>>>> Initializing CPU#1
>>> How reproducible is it?  I'm going to try to reproduce it on one of my boxes.
>> My tip is one of three cases: after some work on fresh boot -- some
>> consumers such as thunderbird, firefox, 10 or so terminals with
>> gnome-session. Single xterm + gnome-session semms not to be a problem.
> 
> Does the workaround with setting the image size below 1/2 of RAM work for you?

Yes. Yesterday I must set the value to 350M -- 400M was not enough.

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Rafael J. Wysocki]

On Wednesday, 11 April 2007 17:02, Jiri Slaby wrote:
> Rafael J. Wysocki napsal(a):
> > On Wednesday, 11 April 2007 12:45, Jiri Slaby wrote:
> >> Rafael J. Wysocki napsal(a):
> >>> On Wednesday, 11 April 2007 09:36, Jiri Slaby wrote:
> >>>> Rafael J. Wysocki napsal(a):
> >>>>> On Monday, 9 April 2007 22:07, Jiri Slaby wrote:
> >>>>>> I have bad news for you :(. I thought I had unpatched kernel, but it happens
> >>>>>> in -rc6 too.
> >>>>> I guess you mean you're still seeing the 'not enough memory to suspend'
> >>>>> problem?
> >>>> Yes:
> >>>> Disabling non-boot CPUs ...
> >>>> kvm: disabling virtualization on CPU1
> >>>> Breaking affinity for irq 9
> >>>> CPU 1 is now offline
> >>>> SMP alternatives: switching to UP code
> >>>> CPU1 is down
> >>>> swsusp: critical section:
> >>>> swsusp: Need to copy 158309 pages
> >>>> swsusp: Not enough free memory
> >>>> Error -12 suspending
> >>>> Enabling non-boot CPUs ...
> >>>> SMP alternatives: switching to SMP code
> >>>> Booting processor 1/2 APIC 0x1
> >>>> Initializing CPU#1
> >>> How reproducible is it?  I'm going to try to reproduce it on one of my boxes.
> >> My tip is one of three cases: after some work on fresh boot -- some
> >> consumers such as thunderbird, firefox, 10 or so terminals with
> >> gnome-session. Single xterm + gnome-session semms not to be a problem.
> > 
> > Does the workaround with setting the image size below 1/2 of RAM work for you?
> 
> Yes. Yesterday I must set the value to 350M -- 400M was not enough.

Well, I can't reproduce it.

Can you please try to reproduce it with the appended patch applied and send
the output of dmesg to me?

Greetings,
Rafael

---
 kernel/power/snapshot.c |    4 ++--
 kernel/power/swsusp.c   |   16 ++++++++++++----
 2 files changed, 14 insertions(+), 6 deletions(-)

Index: linux-2.6.21-rc6/kernel/power/snapshot.c
===================================================================
--- linux-2.6.21-rc6.orig/kernel/power/snapshot.c
+++ linux-2.6.21-rc6/kernel/power/snapshot.c
@@ -871,9 +871,9 @@ static int enough_free_mem(unsigned int 
 		if (!is_highmem(zone))
 			free += zone_page_state(zone, NR_FREE_PAGES);
 	}
-
+	printk("swsusp: Normal pages needed: %u\n", nr_pages);
 	nr_pages += count_pages_for_highmem(nr_highmem);
-	pr_debug("swsusp: Normal pages needed: %u + %u + %u, available pages: %u\n",
+	printk("swsusp: Normal pages needed: %u + %u + %u, available pages: %u\n",
 		nr_pages, PAGES_FOR_IO, meta, free);
 
 	return free > nr_pages + PAGES_FOR_IO + meta;
Index: linux-2.6.21-rc6/kernel/power/swsusp.c
===================================================================
--- linux-2.6.21-rc6.orig/kernel/power/swsusp.c
+++ linux-2.6.21-rc6/kernel/power/swsusp.c
@@ -214,8 +214,8 @@ int swsusp_shrink_memory(void)
 	long tmp;
 	struct zone *zone;
 	unsigned long pages = 0;
-	unsigned int i = 0;
-	char *p = "-\\|/";
+	/*unsigned int i = 0;
+	char *p = "-\\|/";*/
 	struct timeval start, stop;
 
 	printk("Shrinking memory...  ");
@@ -227,7 +227,9 @@ int swsusp_shrink_memory(void)
 		size = count_data_pages() + PAGES_FOR_IO;
 		tmp = size;
 		size += highmem_size;
-		for_each_zone (zone)
+		printk("Pages needed: %ld normal, %ld highmem\n",
+			tmp, highmem_size);
+		for_each_zone (zone) {
 			if (populated_zone(zone)) {
 				tmp += snapshot_additional_pages(zone);
 				if (is_highmem(zone)) {
@@ -238,11 +240,17 @@ int swsusp_shrink_memory(void)
 					tmp += zone->lowmem_reserve[ZONE_NORMAL];
 				}
 			}
+			printk("Pages needed: %ld normal, %ld highmem\n",
+				tmp, highmem_size);
+		}
 
 		if (highmem_size < 0)
 			highmem_size = 0;
 
+		printk("Pages needed: %ld normal, %ld highmem\n",
+			tmp, highmem_size);
 		tmp += highmem_size;
+		printk("Pages needed: %ld\n", tmp);
 		if (tmp > 0) {
 			tmp = __shrink_memory(tmp);
 			if (!tmp)
@@ -252,7 +260,7 @@ int swsusp_shrink_memory(void)
 			tmp = __shrink_memory(size - (image_size / PAGE_SIZE));
 			pages += tmp;
 		}
-		printk("\b%c", p[i++%4]);
+		/*printk("\b%c", p[i++%4]);*/
 	} while (tmp > 0);
 	do_gettimeofday(&stop);
 	printk("\bdone (%lu pages freed)\n", pages);

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Jiri Slaby]

On 4/12/07, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Wednesday, 11 April 2007 17:02, Jiri Slaby wrote:
> > Rafael J. Wysocki napsal(a):
> > > On Wednesday, 11 April 2007 12:45, Jiri Slaby wrote:
> > >> Rafael J. Wysocki napsal(a):
> > >>> On Wednesday, 11 April 2007 09:36, Jiri Slaby wrote:
> > >>>> Rafael J. Wysocki napsal(a):
> > >>>>> On Monday, 9 April 2007 22:07, Jiri Slaby wrote:
> > >>>>>> I have bad news for you :(. I thought I had unpatched kernel, but it happens
> > >>>>>> in -rc6 too.
> > >>>>> I guess you mean you're still seeing the 'not enough memory to suspend'
> > >>>>> problem?
> > >>>> Yes:
> > >>>> Disabling non-boot CPUs ...
> > >>>> kvm: disabling virtualization on CPU1
> > >>>> Breaking affinity for irq 9
> > >>>> CPU 1 is now offline
> > >>>> SMP alternatives: switching to UP code
> > >>>> CPU1 is down
> > >>>> swsusp: critical section:
> > >>>> swsusp: Need to copy 158309 pages
> > >>>> swsusp: Not enough free memory
> > >>>> Error -12 suspending
> > >>>> Enabling non-boot CPUs ...
> > >>>> SMP alternatives: switching to SMP code
> > >>>> Booting processor 1/2 APIC 0x1
> > >>>> Initializing CPU#1
> > >>> How reproducible is it?  I'm going to try to reproduce it on one of my boxes.
> > >> My tip is one of three cases: after some work on fresh boot -- some
> > >> consumers such as thunderbird, firefox, 10 or so terminals with
> > >> gnome-session. Single xterm + gnome-session semms not to be a problem.
> > >
> > > Does the workaround with setting the image size below 1/2 of RAM work for you?
> >
> > Yes. Yesterday I must set the value to 350M -- 400M was not enough.
>
> Well, I can't reproduce it.
>
> Can you please try to reproduce it with the appended patch applied and send
> the output of dmesg to me?
>
> Greetings,
> Rafael
>
> ---
>  kernel/power/snapshot.c |    4 ++--
>  kernel/power/swsusp.c   |   16 ++++++++++++----
>  2 files changed, 14 insertions(+), 6 deletions(-)

Shrinking memory...  Pages needed: 128103 normal, 0 highmem
Pages needed: 125226 normal, 0 highmem
Pages needed: -5757 normal, 0 highmem
Pages needed: -5757 normal, 0 highmem
Pages needed: -5757 normal, 0 highmem
Pages needed: -5757
Pages needed: 127953 normal, 0 highmem
Pages needed: 125076 normal, 0 highmem
Pages needed: -6043 normal, 0 highmem
Pages needed: -6043 normal, 0 highmem
Pages needed: -6043 normal, 0 highmem
Pages needed: -6043
done (200 pages freed)
Freed 800 kbytes in 0.16 seconds (5.00 MB/s)
Suspending console(s)
...
CPU1 is down
swsusp: critical section:
swsusp: Need to copy 131358 pages
swsusp: Normal pages needed: 131358
swsusp: Normal pages needed: 131358 + 1024 + 22, available pages: 130607
swsusp: Not enough free memory
Error -12 suspending
Enabling non-boot CPUs ...
SMP alternatives: switching to SMP code
Booting processor 1/2 APIC 0x1
Not responding.
Inquiring remote APIC #1...
... APIC #1 ID: failed
... APIC #1 VERSION: failed
... APIC #1 SPIV: failed
kvm: disabling virtualization on CPU1
Error taking CPU1 up: -5
PCI: Setting latency timer of device 0000:00:01.0 to 64

Please note the CPU#1 bring up problem too. Whole dmesg at:
http://www.fi.muni.cz/~xslaby/sklad/dmesg.nomem

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Rafael J. Wysocki]

On Friday, 13 April 2007 12:14, Jiri Slaby wrote:
> On 4/12/07, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Wednesday, 11 April 2007 17:02, Jiri Slaby wrote:
> > > Rafael J. Wysocki napsal(a):
> > > > On Wednesday, 11 April 2007 12:45, Jiri Slaby wrote:
> > > >> Rafael J. Wysocki napsal(a):
> > > >>> On Wednesday, 11 April 2007 09:36, Jiri Slaby wrote:
> > > >>>> Rafael J. Wysocki napsal(a):
> > > >>>>> On Monday, 9 April 2007 22:07, Jiri Slaby wrote:
> > > >>>>>> I have bad news for you :(. I thought I had unpatched kernel, but it happens
> > > >>>>>> in -rc6 too.
> > > >>>>> I guess you mean you're still seeing the 'not enough memory to suspend'
> > > >>>>> problem?
> > > >>>> Yes:
> > > >>>> Disabling non-boot CPUs ...
> > > >>>> kvm: disabling virtualization on CPU1
> > > >>>> Breaking affinity for irq 9
> > > >>>> CPU 1 is now offline
> > > >>>> SMP alternatives: switching to UP code
> > > >>>> CPU1 is down
> > > >>>> swsusp: critical section:
> > > >>>> swsusp: Need to copy 158309 pages
> > > >>>> swsusp: Not enough free memory
> > > >>>> Error -12 suspending
> > > >>>> Enabling non-boot CPUs ...
> > > >>>> SMP alternatives: switching to SMP code
> > > >>>> Booting processor 1/2 APIC 0x1
> > > >>>> Initializing CPU#1
> > > >>> How reproducible is it?  I'm going to try to reproduce it on one of my boxes.
> > > >> My tip is one of three cases: after some work on fresh boot -- some
> > > >> consumers such as thunderbird, firefox, 10 or so terminals with
> > > >> gnome-session. Single xterm + gnome-session semms not to be a problem.
> > > >
> > > > Does the workaround with setting the image size below 1/2 of RAM work for you?
> > >
> > > Yes. Yesterday I must set the value to 350M -- 400M was not enough.
> >
> > Well, I can't reproduce it.
> >
> > Can you please try to reproduce it with the appended patch applied and send
> > the output of dmesg to me?
> >
> > Greetings,
> > Rafael
> >
> > ---
> >  kernel/power/snapshot.c |    4 ++--
> >  kernel/power/swsusp.c   |   16 ++++++++++++----
> >  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> Shrinking memory...  Pages needed: 128103 normal, 0 highmem
> Pages needed: 125226 normal, 0 highmem
> Pages needed: -5757 normal, 0 highmem
> Pages needed: -5757 normal, 0 highmem
> Pages needed: -5757 normal, 0 highmem
> Pages needed: -5757
> Pages needed: 127953 normal, 0 highmem
> Pages needed: 125076 normal, 0 highmem
> Pages needed: -6043 normal, 0 highmem
> Pages needed: -6043 normal, 0 highmem
> Pages needed: -6043 normal, 0 highmem
> Pages needed: -6043
> done (200 pages freed)
> Freed 800 kbytes in 0.16 seconds (5.00 MB/s)
> Suspending console(s)
> ...
> CPU1 is down
> swsusp: critical section:
> swsusp: Need to copy 131358 pages
> swsusp: Normal pages needed: 131358
> swsusp: Normal pages needed: 131358 + 1024 + 22, available pages: 130607

Well, it looks like someone allocated about 6000 pages after we had freed
enough memory for suspending.

I suspect one of the device drivers plays some dirty tricks in its .suspend()
routine.  Now the question is which one.

I wonder if setting PAGES_FOR_IO in include/linux/suspend.h to eg. 8192 will
help.

> swsusp: Not enough free memory
> Error -12 suspending
> Enabling non-boot CPUs ...
> SMP alternatives: switching to SMP code
> Booting processor 1/2 APIC 0x1
> Not responding.
> Inquiring remote APIC #1...
> ... APIC #1 ID: failed
> ... APIC #1 VERSION: failed
> ... APIC #1 SPIV: failed
> kvm: disabling virtualization on CPU1
> Error taking CPU1 up: -5
> PCI: Setting latency timer of device 0000:00:01.0 to 64
> 
> Please note the CPU#1 bring up problem too.

Yes, and this seems to be related to the APIC.

Gautham, can you please tell me who's the right person to ask about it?

Rafael

Re: 2.6.21-rc5: swsusp: Not enough free memory

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]

Hi.

On Fri, 2007-04-13 at 14:00 +0200, Rafael J. Wysocki wrote:
> > 
> > Shrinking memory...  Pages needed: 128103 normal, 0 highmem
> > Pages needed: 125226 normal, 0 highmem
> > Pages needed: -5757 normal, 0 highmem
> > Pages needed: -5757 normal, 0 highmem
> > Pages needed: -5757 normal, 0 highmem
> > Pages needed: -5757
> > Pages needed: 127953 normal, 0 highmem
> > Pages needed: 125076 normal, 0 highmem
> > Pages needed: -6043 normal, 0 highmem
> > Pages needed: -6043 normal, 0 highmem
> > Pages needed: -6043 normal, 0 highmem
> > Pages needed: -6043
> > done (200 pages freed)
> > Freed 800 kbytes in 0.16 seconds (5.00 MB/s)
> > Suspending console(s)
> > ...
> > CPU1 is down
> > swsusp: critical section:
> > swsusp: Need to copy 131358 pages
> > swsusp: Normal pages needed: 131358
> > swsusp: Normal pages needed: 131358 + 1024 + 22, available pages: 130607
> 
> Well, it looks like someone allocated about 6000 pages after we had freed
> enough memory for suspending.

We have a tunable allowance in Suspend2 for this, because fglrx
allocates a lot of pages in its suspend routine if DRI is enabled. I
think some other drivers do too, but fglrx is the main one I know.

Nigel


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[RFD] swsusp problem: Drivers allocate much memory during suspend (was: Re: 2.6.21-rc5: swsusp: Not enough free memory)

[Rafael J. Wysocki]

On Friday, 13 April 2007 14:21, Nigel Cunningham wrote:
> Hi.
> 
> On Fri, 2007-04-13 at 14:00 +0200, Rafael J. Wysocki wrote:
> > > 
> > > Shrinking memory...  Pages needed: 128103 normal, 0 highmem
> > > Pages needed: 125226 normal, 0 highmem
> > > Pages needed: -5757 normal, 0 highmem
> > > Pages needed: -5757 normal, 0 highmem
> > > Pages needed: -5757 normal, 0 highmem
> > > Pages needed: -5757
> > > Pages needed: 127953 normal, 0 highmem
> > > Pages needed: 125076 normal, 0 highmem
> > > Pages needed: -6043 normal, 0 highmem
> > > Pages needed: -6043 normal, 0 highmem
> > > Pages needed: -6043 normal, 0 highmem
> > > Pages needed: -6043
> > > done (200 pages freed)
> > > Freed 800 kbytes in 0.16 seconds (5.00 MB/s)
> > > Suspending console(s)
> > > ...
> > > CPU1 is down
> > > swsusp: critical section:
> > > swsusp: Need to copy 131358 pages
> > > swsusp: Normal pages needed: 131358
> > > swsusp: Normal pages needed: 131358 + 1024 + 22, available pages: 130607
> > 
> > Well, it looks like someone allocated about 6000 pages after we had freed
> > enough memory for suspending.
> 
> We have a tunable allowance in Suspend2 for this, because fglrx
> allocates a lot of pages in its suspend routine if DRI is enabled. I
> think some other drivers do too, but fglrx is the main one I know.

I wasn't aware of that, thanks for the information.

I think this means we'll probably need to add a tunable, similar to image_size,
that will allow the users to specify how much spare memory they want to reserve
for suspending (instead of the constant PAGES_FOR_IO).  IMO we can call it
'spare_memory'.

Still, this doesn't look like a real solution, because it would require the
users affected by this problem to experiment with suspending in order to
figure out how much spare memory they will need.

IMO to really fix the problem, we should let the drivers that need much memory
for suspending allocate it _before_ the memory shrinker is called.  For this
purpose we can use notifiers that will be called before we start the shrinking
of memory.  Namely, if a driver needs to allocate substantial amount of memory
for suspending, it can register a notifier that will be called before we try to
shrink memory.  Then, the memory needed by the driver may be allocated in
this notifier (of course, in that case it will also have to be called if the
shrinking of memory fails, so that the memory allocated by the driver for
suspending can be freed) and used in the driver's .suspend() routine.

Comments welcome.

Greetings,
Rafael

Re: [RFD] swsusp problem: Drivers allocate much memory during suspend (was: Re: 2.6.21-rc5: swsusp: Not enough free memory)

[Nigel Cunningham]

Hi.

On Fri, 2007-04-13 at 22:41 +0200, Rafael J. Wysocki wrote:
> On Friday, 13 April 2007 14:21, Nigel Cunningham wrote:
> > Hi.
> > 
> > On Fri, 2007-04-13 at 14:00 +0200, Rafael J. Wysocki wrote:
> > > > 
> > > > Shrinking memory...  Pages needed: 128103 normal, 0 highmem
> > > > Pages needed: 125226 normal, 0 highmem
> > > > Pages needed: -5757 normal, 0 highmem
> > > > Pages needed: -5757 normal, 0 highmem
> > > > Pages needed: -5757 normal, 0 highmem
> > > > Pages needed: -5757
> > > > Pages needed: 127953 normal, 0 highmem
> > > > Pages needed: 125076 normal, 0 highmem
> > > > Pages needed: -6043 normal, 0 highmem
> > > > Pages needed: -6043 normal, 0 highmem
> > > > Pages needed: -6043 normal, 0 highmem
> > > > Pages needed: -6043
> > > > done (200 pages freed)
> > > > Freed 800 kbytes in 0.16 seconds (5.00 MB/s)
> > > > Suspending console(s)
> > > > ...
> > > > CPU1 is down
> > > > swsusp: critical section:
> > > > swsusp: Need to copy 131358 pages
> > > > swsusp: Normal pages needed: 131358
> > > > swsusp: Normal pages needed: 131358 + 1024 + 22, available pages: 130607
> > > 
> > > Well, it looks like someone allocated about 6000 pages after we had freed
> > > enough memory for suspending.
> > 
> > We have a tunable allowance in Suspend2 for this, because fglrx
> > allocates a lot of pages in its suspend routine if DRI is enabled. I
> > think some other drivers do too, but fglrx is the main one I know.
> 
> I wasn't aware of that, thanks for the information.
> 
> I think this means we'll probably need to add a tunable, similar to image_size,
> that will allow the users to specify how much spare memory they want to reserve
> for suspending (instead of the constant PAGES_FOR_IO).  IMO we can call it
> 'spare_memory'.
> 
> Still, this doesn't look like a real solution, because it would require the
> users affected by this problem to experiment with suspending in order to
> figure out how much spare memory they will need.
> 
> IMO to really fix the problem, we should let the drivers that need much memory
> for suspending allocate it _before_ the memory shrinker is called.  For this
> purpose we can use notifiers that will be called before we start the shrinking
> of memory.  Namely, if a driver needs to allocate substantial amount of memory
> for suspending, it can register a notifier that will be called before we try to
> shrink memory.  Then, the memory needed by the driver may be allocated in
> this notifier (of course, in that case it will also have to be called if the
> shrinking of memory fails, so that the memory allocated by the driver for
> suspending can be freed) and used in the driver's .suspend() routine.
> 
> Comments welcome.

Yeah. I've thought about it too. It could also be good for that acpi
routine that was allocating memory during in an atomic context with the
wrong flagas. Another idea that occurred to me would be to allow drivers
to have a routine saying how much memory they will need, which we could
call to calculate the allowance we need. Personally, I think the
notifier chain is simpler and preferable :)

Regards,

Nigel

Re: [RFD] swsusp problem: Drivers allocate much memory during suspend

[Chuck Ebbert]

Rafael J. Wysocki wrote:
> 
> IMO to really fix the problem, we should let the drivers that need much memory
> for suspending allocate it _before_ the memory shrinker is called.  For this
> purpose we can use notifiers that will be called before we start the shrinking
> of memory.  Namely, if a driver needs to allocate substantial amount of memory
> for suspending, it can register a notifier that will be called before we try to
> shrink memory.  Then, the memory needed by the driver may be allocated in
> this notifier (of course, in that case it will also have to be called if the
> shrinking of memory fails, so that the memory allocated by the driver for
> suspending can be freed) and used in the driver's .suspend() routine.
> 

Can't you just put a "prepare to suspend" function pointer in the
bus_type and device_driver structs?

Re: [RFD] swsusp problem: Drivers allocate much memory during suspend (was: Re: 2.6.21-rc5: swsusp: Not enough free memory)

[Pavel Machek]

Hi!

> > > Well, it looks like someone allocated about 6000 pages after we had freed
> > > enough memory for suspending.
> > 
> > We have a tunable allowance in Suspend2 for this, because fglrx
> > allocates a lot of pages in its suspend routine if DRI is enabled. I
> > think some other drivers do too, but fglrx is the main one I know.
> 
> I wasn't aware of that, thanks for the information.
> 
> I think this means we'll probably need to add a tunable, similar to image_size,
> that will allow the users to specify how much spare memory they want to reserve
> for suspending (instead of the constant PAGES_FOR_IO).  IMO we can call it
> 'spare_memory'.

Just increase PAGES_FOR_IO. This should not be tunable.

> Still, this doesn't look like a real solution, because it would require the
> users affected by this problem to experiment with suspending in order to
> figure out how much spare memory they will need.

Exactly.

> IMO to really fix the problem, we should let the drivers that need much memory
> for suspending allocate it _before_ the memory shrinker is called.  For this
> purpose we can use notifiers that will be called before we start the shrinking
> of memory.  Namely, if a driver needs to allocate substantial amount
> of memory

Yes please. Using that notifier without leaking the memory will be
"interesting" but if someone needs so much memory during suspend, let
them eat their own complexity.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] swsusp problem: Drivers allocate much memory during suspend (was: Re: 2.6.21-rc5: swsusp: Not enough free memory)

[Nigel Cunningham]

Hi.

On Sat, 2007-04-14 at 00:10 +0200, Pavel Machek wrote:
> Hi!
> 
> > > > Well, it looks like someone allocated about 6000 pages after we had freed
> > > > enough memory for suspending.
> > > 
> > > We have a tunable allowance in Suspend2 for this, because fglrx
> > > allocates a lot of pages in its suspend routine if DRI is enabled. I
> > > think some other drivers do too, but fglrx is the main one I know.
> > 
> > I wasn't aware of that, thanks for the information.
> > 
> > I think this means we'll probably need to add a tunable, similar to image_size,
> > that will allow the users to specify how much spare memory they want to reserve
> > for suspending (instead of the constant PAGES_FOR_IO).  IMO we can call it
> > 'spare_memory'.
> 
> Just increase PAGES_FOR_IO. This should not be tunable.

If we don't have a means for drivers to pre-allocate or say how much
memory they need, it should be tunable. Frankly, I'm startled that you
guys haven't heard of this issue before now. I can't believe everyone
who has ever wanted to hibernate with DRM enabled has been using
Suspend2. Maybe this is one of the sources of complaints that swsusp
isn't reliable?

> > IMO to really fix the problem, we should let the drivers that need much memory
> > for suspending allocate it _before_ the memory shrinker is called.  For this
> > purpose we can use notifiers that will be called before we start the shrinking
> > of memory.  Namely, if a driver needs to allocate substantial amount
> > of memory
> 
> Yes please. Using that notifier without leaking the memory will be
> "interesting" but if someone needs so much memory during suspend, let
> them eat their own complexity.

It doesn't need to be that complex. Add another (optional) function to
the driver model to let drivers say how much they want and it becomes
trivial. Maybe this idea should be preferred over the notifier chain.

Regards,

Nigel

Re: [linux-pm] [RFD] swsusp problem: Drivers allocate much memory during suspend (was: Re: 2.6.21-rc5: swsusp: Not enough free memory)

[Pavel Machek]

Hi!

> > > > > Well, it looks like someone allocated about 6000 pages after we had freed
> > > > > enough memory for suspending.
> > > > 
> > > > We have a tunable allowance in Suspend2 for this, because fglrx
> > > > allocates a lot of pages in its suspend routine if DRI is enabled. I
> > > > think some other drivers do too, but fglrx is the main one I know.
> > > 
> > > I wasn't aware of that, thanks for the information.
> > > 
> > > I think this means we'll probably need to add a tunable, similar to image_size,
> > > that will allow the users to specify how much spare memory they want to reserve
> > > for suspending (instead of the constant PAGES_FOR_IO).  IMO we can call it
> > > 'spare_memory'.
> > 
> > Just increase PAGES_FOR_IO. This should not be tunable.
> 
> If we don't have a means for drivers to pre-allocate or say how much
> memory they need, it should be tunable. Frankly, I'm startled that you
> guys haven't heard of this issue before now. I can't believe everyone
> who has ever wanted to hibernate with DRM enabled has been using
> Suspend2. Maybe this is one of the sources of complaints that swsusp
> isn't reliable?

We do not support closed-source drivers, and open-source drivers are
well behaved.

> > > IMO to really fix the problem, we should let the drivers that need much memory
> > > for suspending allocate it _before_ the memory shrinker is called.  For this
> > > purpose we can use notifiers that will be called before we start the shrinking
> > > of memory.  Namely, if a driver needs to allocate substantial amount
> > > of memory
> > 
> > Yes please. Using that notifier without leaking the memory will be
> > "interesting" but if someone needs so much memory during suspend, let
> > them eat their own complexity.
> 
> It doesn't need to be that complex. Add another (optional) function to
> the driver model to let drivers say how much they want and it becomes
> trivial. Maybe this idea should be preferred over the notifier chain.

Actually, it is trivial to prealocate during boot ;-). As the notifier
chain can be useful for other stuff, too, I'd go that way.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [linux-pm] [RFD] swsusp problem: Drivers allocate much memory during suspend (was: Re: 2.6.21-rc5: swsusp: Not enough free memory)

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 2796 bytes --]

Hi.

On Sat, 2007-04-14 at 00:38 +0200, Pavel Machek wrote:
> Hi!
> 
> > > > > > Well, it looks like someone allocated about 6000 pages after we had freed
> > > > > > enough memory for suspending.
> > > > > 
> > > > > We have a tunable allowance in Suspend2 for this, because fglrx
> > > > > allocates a lot of pages in its suspend routine if DRI is enabled. I
> > > > > think some other drivers do too, but fglrx is the main one I know.
> > > > 
> > > > I wasn't aware of that, thanks for the information.
> > > > 
> > > > I think this means we'll probably need to add a tunable, similar to image_size,
> > > > that will allow the users to specify how much spare memory they want to reserve
> > > > for suspending (instead of the constant PAGES_FOR_IO).  IMO we can call it
> > > > 'spare_memory'.
> > > 
> > > Just increase PAGES_FOR_IO. This should not be tunable.
> > 
> > If we don't have a means for drivers to pre-allocate or say how much
> > memory they need, it should be tunable. Frankly, I'm startled that you
> > guys haven't heard of this issue before now. I can't believe everyone
> > who has ever wanted to hibernate with DRM enabled has been using
> > Suspend2. Maybe this is one of the sources of complaints that swsusp
> > isn't reliable?
> 
> We do not support closed-source drivers, and open-source drivers are
> well behaved.

I didn't say fglrx was the only example. Any system using DRI (not DRM,
sorry), would, I think, be expected. I just mention fglrx because I have
a Radeon 200M that can only use fglrx for Beryl etc at the mo - it's the
one I'm familiar with.

> > > > IMO to really fix the problem, we should let the drivers that need much memory
> > > > for suspending allocate it _before_ the memory shrinker is called.  For this
> > > > purpose we can use notifiers that will be called before we start the shrinking
> > > > of memory.  Namely, if a driver needs to allocate substantial amount
> > > > of memory
> > > 
> > > Yes please. Using that notifier without leaking the memory will be
> > > "interesting" but if someone needs so much memory during suspend, let
> > > them eat their own complexity.
> > 
> > It doesn't need to be that complex. Add another (optional) function to
> > the driver model to let drivers say how much they want and it becomes
> > trivial. Maybe this idea should be preferred over the notifier chain.
> 
> Actually, it is trivial to prealocate during boot ;-). As the notifier
> chain can be useful for other stuff, too, I'd go that way.

Pavel! Talk sense! You're not seriously suggesting squirreling away 35
megabytes of a user's memory at boot just because they might want to
hibernate with DRI enabled later? Yes, 35 megabytes is a realistic
amount.

Regards,

Nigel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFD] swsusp problem: Drivers allocate much memory during suspend (was: Re: 2.6.21-rc5: swsusp: Not enough free memory)

[Rafael J. Wysocki]

On Saturday, 14 April 2007 00:10, Pavel Machek wrote:
> Hi!
> 
> > > > Well, it looks like someone allocated about 6000 pages after we had freed
> > > > enough memory for suspending.
> > > 
> > > We have a tunable allowance in Suspend2 for this, because fglrx
> > > allocates a lot of pages in its suspend routine if DRI is enabled. I
> > > think some other drivers do too, but fglrx is the main one I know.
> > 
> > I wasn't aware of that, thanks for the information.
> > 
> > I think this means we'll probably need to add a tunable, similar to image_size,
> > that will allow the users to specify how much spare memory they want to reserve
> > for suspending (instead of the constant PAGES_FOR_IO).  IMO we can call it
> > 'spare_memory'.
> 
> Just increase PAGES_FOR_IO. This should not be tunable.

Well, I'm not sure.  First, we don't really know what the value of it should be
and this alone is a good enough reason for making it tunable, IMHO.  Second, I
think different systems may need different PAGES_FOR_IO and taking just the
maximum (even if we learn how much that actually is) seems to be wasteful in
the vast majority of cases.  Finally, I think it may be possible to speed up
image saving by increasing PAGES_FOR_IO without playing with the
image size and we can let the user try it (think of distro kernels that are
compiled for many different users).

Still, I'm not going to fight for that.

> > Still, this doesn't look like a real solution, because it would require the
> > users affected by this problem to experiment with suspending in order to
> > figure out how much spare memory they will need.
> 
> Exactly.
> 
> > IMO to really fix the problem, we should let the drivers that need much memory
> > for suspending allocate it _before_ the memory shrinker is called.  For this
> > purpose we can use notifiers that will be called before we start the shrinking
> > of memory.  Namely, if a driver needs to allocate substantial amount
> > of memory
> 
> Yes please. Using that notifier without leaking the memory will be
> "interesting" but if someone needs so much memory during suspend, let
> them eat their own complexity.

Okay, I'm going to prepare a patch along these lines.

Greetings,
Rafael

Re: [RFD] swsusp problem: Drivers allocate much memory during suspend (was: Re: 2.6.21-rc5: swsusp: Not enough free memory)

[Nigel Cunningham]

Hi.

On Sat, 2007-04-14 at 00:35 +0200, Rafael J. Wysocki wrote:
> On Saturday, 14 April 2007 00:10, Pavel Machek wrote:
> > Hi!
> > 
> > > > > Well, it looks like someone allocated about 6000 pages after we had freed
> > > > > enough memory for suspending.
> > > > 
> > > > We have a tunable allowance in Suspend2 for this, because fglrx
> > > > allocates a lot of pages in its suspend routine if DRI is enabled. I
> > > > think some other drivers do too, but fglrx is the main one I know.
> > > 
> > > I wasn't aware of that, thanks for the information.
> > > 
> > > I think this means we'll probably need to add a tunable, similar to image_size,
> > > that will allow the users to specify how much spare memory they want to reserve
> > > for suspending (instead of the constant PAGES_FOR_IO).  IMO we can call it
> > > 'spare_memory'.
> > 
> > Just increase PAGES_FOR_IO. This should not be tunable.
> 
> Well, I'm not sure.  First, we don't really know what the value of it should be
> and this alone is a good enough reason for making it tunable, IMHO.  Second, I
> think different systems may need different PAGES_FOR_IO and taking just the
> maximum (even if we learn how much that actually is) seems to be wasteful in
> the vast majority of cases.  Finally, I think it may be possible to speed up
> image saving by increasing PAGES_FOR_IO without playing with the
> image size and we can let the user try it (think of distro kernels that are
> compiled for many different users).

It does vary according to the amount of video memory used for DRM, if I
understand correctly.

Nigel

Re: [RFD] swsusp problem: Drivers allocate much memory during suspend (was: Re: 2.6.21-rc5: swsusp: Not enough free memory)

[Pavel Machek]

Hi!

> > > > > Well, it looks like someone allocated about 6000 pages after we had freed
> > > > > enough memory for suspending.
> > > > 
> > > > We have a tunable allowance in Suspend2 for this, because fglrx
> > > > allocates a lot of pages in its suspend routine if DRI is enabled. I
> > > > think some other drivers do too, but fglrx is the main one I know.
> > > 
> > > I wasn't aware of that, thanks for the information.
> > > 
> > > I think this means we'll probably need to add a tunable, similar to image_size,
> > > that will allow the users to specify how much spare memory they want to reserve
> > > for suspending (instead of the constant PAGES_FOR_IO).  IMO we can call it
> > > 'spare_memory'.
> > 
> > Just increase PAGES_FOR_IO. This should not be tunable.
> 
> Well, I'm not sure.  First, we don't really know what the value of it should be
> and this alone is a good enough reason for making it tunable, IMHO.  Second, I
> think different systems may need different PAGES_FOR_IO and taking just the
> maximum (even if we learn how much that actually is) seems to be wasteful in

Well,  it is wasteful as in "we save slightly smaller image than we
could". That's okay with me.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] swsusp problem: Drivers allocate much memory during suspend (was: Re: 2.6.21-rc5: swsusp: Not enough free memory)

[Nigel Cunningham]

Hi.

On Sat, 2007-04-14 at 00:40 +0200, Pavel Machek wrote:
> Hi!
> 
> > > > > > Well, it looks like someone allocated about 6000 pages after we had freed
> > > > > > enough memory for suspending.
> > > > > 
> > > > > We have a tunable allowance in Suspend2 for this, because fglrx
> > > > > allocates a lot of pages in its suspend routine if DRI is enabled. I
> > > > > think some other drivers do too, but fglrx is the main one I know.
> > > > 
> > > > I wasn't aware of that, thanks for the information.
> > > > 
> > > > I think this means we'll probably need to add a tunable, similar to image_size,
> > > > that will allow the users to specify how much spare memory they want to reserve
> > > > for suspending (instead of the constant PAGES_FOR_IO).  IMO we can call it
> > > > 'spare_memory'.
> > > 
> > > Just increase PAGES_FOR_IO. This should not be tunable.
> > 
> > Well, I'm not sure.  First, we don't really know what the value of it should be
> > and this alone is a good enough reason for making it tunable, IMHO.  Second, I
> > think different systems may need different PAGES_FOR_IO and taking just the
> > maximum (even if we learn how much that actually is) seems to be wasteful in
> 
> Well,  it is wasteful as in "we save slightly smaller image than we
> could". That's okay with me.

No. If the driver can't allocate the memory, your call to device_suspend
will fail. This isn't about image size but about success or failure to
hibernate.

Nigel

Re: [RFD] swsusp problem: Drivers allocate much memory during suspend (was: Re: 2.6.21-rc5: swsusp: Not enough free memory)

[Rafael J. Wysocki]

On Saturday, 14 April 2007 00:45, Nigel Cunningham wrote:
> Hi.
> 
> On Sat, 2007-04-14 at 00:40 +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > > > > > Well, it looks like someone allocated about 6000 pages after we had freed
> > > > > > > enough memory for suspending.
> > > > > > 
> > > > > > We have a tunable allowance in Suspend2 for this, because fglrx
> > > > > > allocates a lot of pages in its suspend routine if DRI is enabled. I
> > > > > > think some other drivers do too, but fglrx is the main one I know.
> > > > > 
> > > > > I wasn't aware of that, thanks for the information.
> > > > > 
> > > > > I think this means we'll probably need to add a tunable, similar to image_size,
> > > > > that will allow the users to specify how much spare memory they want to reserve
> > > > > for suspending (instead of the constant PAGES_FOR_IO).  IMO we can call it
> > > > > 'spare_memory'.
> > > > 
> > > > Just increase PAGES_FOR_IO. This should not be tunable.
> > > 
> > > Well, I'm not sure.  First, we don't really know what the value of it should be
> > > and this alone is a good enough reason for making it tunable, IMHO.  Second, I
> > > think different systems may need different PAGES_FOR_IO and taking just the
> > > maximum (even if we learn how much that actually is) seems to be wasteful in
> > 
> > Well,  it is wasteful as in "we save slightly smaller image than we
> > could". That's okay with me.
> 
> No. If the driver can't allocate the memory, your call to device_suspend
> will fail. This isn't about image size but about success or failure to
> hibernate.

If we take PAGES_FOR_IO to be the maximum over all possible configurations
that can hibernate, the majority of systems will just create smaller images than
they could have created for smaller PAGES_FOR_IO, but all of them will be
able to hibernate. :-)

Greetings,
Rafael

Re: [RFD] swsusp problem: Drivers allocate much memory during suspend (was: Re: 2.6.21-rc5: swsusp: Not enough free memory)

[Nigel Cunningham]

Hi.

On Sat, 2007-04-14 at 00:57 +0200, Rafael J. Wysocki wrote:
> > > > Well, I'm not sure.  First, we don't really know what the value of it should be
> > > > and this alone is a good enough reason for making it tunable, IMHO.  Second, I
> > > > think different systems may need different PAGES_FOR_IO and taking just the
> > > > maximum (even if we learn how much that actually is) seems to be wasteful in
> > > 
> > > Well,  it is wasteful as in "we save slightly smaller image than we
> > > could". That's okay with me.
> > 
> > No. If the driver can't allocate the memory, your call to device_suspend
> > will fail. This isn't about image size but about success or failure to
> > hibernate.
> 
> If we take PAGES_FOR_IO to be the maximum over all possible configurations
> that can hibernate, the majority of systems will just create smaller images than
> they could have created for smaller PAGES_FOR_IO, but all of them will be
> able to hibernate. :-)

You also use PAGES_FOR_IO in enough_free_mem. Say you set it to the 9000
pages I mentioned before (35M). On a machine with 64 megabytes of
memory, you'll never be able to suspend because you'll never satisfy

free > nr_pages + PAGES_FOR_IO + meta

I'll freely admit that 64 megabytes is tiny nowadays, but it's not
completely unknown. The point is really that you're effectively making
swsusp unusable for machines with RAM < (PAGES_FOR_IO * (say) 3). But
what do you set PAGES_FOR_IO to? There'll always be someone with
$WHIZ_BANG_CONFIG who is pushing to have the value increased, and every
increase knocks out more of your lowend users.

Regards,

Nigel

Re: [RFD] swsusp problem: Drivers allocate much memory during suspend (was: Re: 2.6.21-rc5: swsusp: Not enough free memory)

[Rafael J. Wysocki]

On Saturday, 14 April 2007 01:03, Nigel Cunningham wrote:
> Hi.
> 
> On Sat, 2007-04-14 at 00:57 +0200, Rafael J. Wysocki wrote:
> > > > > Well, I'm not sure.  First, we don't really know what the value of it should be
> > > > > and this alone is a good enough reason for making it tunable, IMHO.  Second, I
> > > > > think different systems may need different PAGES_FOR_IO and taking just the
> > > > > maximum (even if we learn how much that actually is) seems to be wasteful in
> > > > 
> > > > Well,  it is wasteful as in "we save slightly smaller image than we
> > > > could". That's okay with me.
> > > 
> > > No. If the driver can't allocate the memory, your call to device_suspend
> > > will fail. This isn't about image size but about success or failure to
> > > hibernate.
> > 
> > If we take PAGES_FOR_IO to be the maximum over all possible configurations
> > that can hibernate, the majority of systems will just create smaller images than
> > they could have created for smaller PAGES_FOR_IO, but all of them will be
> > able to hibernate. :-)
> 
> You also use PAGES_FOR_IO in enough_free_mem. Say you set it to the 9000
> pages I mentioned before (35M). On a machine with 64 megabytes of
> memory, you'll never be able to suspend because you'll never satisfy
> 
> free > nr_pages + PAGES_FOR_IO + meta

Well, in fact our tunable would need to be independent of PAGES_FOR_IO so that
the memory shrinker could free some spare memory for the drivers.

> I'll freely admit that 64 megabytes is tiny nowadays, but it's not
> completely unknown. The point is really that you're effectively making
> swsusp unusable for machines with RAM < (PAGES_FOR_IO * (say) 3). But
> what do you set PAGES_FOR_IO to? There'll always be someone with
> $WHIZ_BANG_CONFIG who is pushing to have the value increased, and every
> increase knocks out more of your lowend users.

Yes, that's why I said I wasn't sure whether or not the additional tunable was
needed.

Still, with the notifiers we have a chance to handle the problem automatically
and the tunable would require the user to set the right value.

Greetings,
Rafael

Re: [linux-pm] [RFD] swsusp problem: Drivers allocate much memory during suspend (was: Re: 2.6.21-rc5: swsusp: Not enough free memory)

[Rafael J. Wysocki]

Hi,

On Saturday, 14 April 2007 00:35, Rafael J. Wysocki wrote:
> On Saturday, 14 April 2007 00:10, Pavel Machek wrote:
[--snip--] 
> > > IMO to really fix the problem, we should let the drivers that need much memory
> > > for suspending allocate it _before_ the memory shrinker is called.  For this
> > > purpose we can use notifiers that will be called before we start the shrinking
> > > of memory.  Namely, if a driver needs to allocate substantial amount
> > > of memory
> > 
> > Yes please. Using that notifier without leaking the memory will be
> > "interesting" but if someone needs so much memory during suspend, let
> > them eat their own complexity.
> 
> Okay, I'm going to prepare a patch along these lines.

The appended patch shows how I think this might look like.

There are a couple of things I'm not sure about in it:
1) Names (if you have better ideas, please tell me)
2) I don't know if SUSPEND_THAW_PREPARE is necessary, at least for now I don't
see any obvious case in which it may be useful, but I've added it for symmetry
3) Perhaps we can use the second argument of raw_notifier_call_chain() to pass
the information of what kind of suspend is going to happen (eg. STD vs STR), in
which case we'll need a second argument for suspend_notifier_call_chain()

Greetings,
Rafael

---
 include/linux/notifier.h |    6 +++++
 include/linux/suspend.h  |   27 ++++++++++++++++++++++---
 kernel/power/Makefile    |    2 -
 kernel/power/disk.c      |   21 ++++++++++++++++++++
 kernel/power/main.c      |   13 ++++++++++++
 kernel/power/notify.c    |   49 +++++++++++++++++++++++++++++++++++++++++++++++
 kernel/power/power.h     |    3 ++
 kernel/power/user.c      |   19 ++++++++++++++++++
 8 files changed, 135 insertions(+), 5 deletions(-)

Index: linux-2.6.21-rc6/kernel/power/notify.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.21-rc6/kernel/power/notify.c	2007-04-14 23:35:35.000000000 +0200
@@ -0,0 +1,49 @@
+/*
+ * linux/kernel/power/notify.c
+ *
+ * This file contains functions used for registering and calling suspend
+ * notifiers that can be used by subsystems for carrying out some special
+ * suspend-related operations.
+ *
+ * Copyright (C) 2007 Rafael J. Wysocki <rjw@sisk.pl>
+ *
+ * This file is released under the GPLv2.
+ *
+ */
+
+#include <linux/smp.h>
+#include <linux/notifier.h>
+#include <linux/module.h>
+#include <linux/suspend.h>
+
+static DEFINE_MUTEX(suspend_notifier_lock);
+
+static RAW_NOTIFIER_HEAD(suspend_chain);
+
+int register_suspend_notifier(struct notifier_block *nb)
+{
+	int ret;
+	mutex_lock(&suspend_notifier_lock);
+	ret = raw_notifier_chain_register(&suspend_chain, nb);
+	mutex_unlock(&suspend_notifier_lock);
+	return ret;
+}
+EXPORT_SYMBOL(register_suspend_notifier);
+
+void unregister_suspend_notifier(struct notifier_block *nb)
+{
+	mutex_lock(&suspend_notifier_lock);
+	raw_notifier_chain_unregister(&suspend_chain, nb);
+	mutex_unlock(&suspend_notifier_lock);
+}
+EXPORT_SYMBOL(unregister_suspend_notifier);
+
+int suspend_notifier_call_chain(unsigned long val)
+{
+	int error;
+
+	mutex_lock(&suspend_notifier_lock);
+	error = raw_notifier_call_chain(&suspend_chain, val, NULL);
+	mutex_unlock(&suspend_notifier_lock);
+	return error;
+}
Index: linux-2.6.21-rc6/include/linux/suspend.h
===================================================================
--- linux-2.6.21-rc6.orig/include/linux/suspend.h	2007-04-14 22:04:58.000000000 +0200
+++ linux-2.6.21-rc6/include/linux/suspend.h	2007-04-14 23:32:18.000000000 +0200
@@ -24,15 +24,16 @@ struct pbe {
 extern void drain_local_pages(void);
 extern void mark_free_pages(struct zone *zone);
 
-#if defined(CONFIG_PM) && defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
+#ifdef CONFIG_PM
+#if defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
 extern int pm_prepare_console(void);
 extern void pm_restore_console(void);
 #else
 static inline int pm_prepare_console(void) { return 0; }
 static inline void pm_restore_console(void) {}
-#endif
+#endif /* defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE) */
 
-#if defined(CONFIG_PM) && defined(CONFIG_SOFTWARE_SUSPEND)
+#ifdef CONFIG_SOFTWARE_SUSPEND
 /* kernel/power/swsusp.c */
 extern int software_suspend(void);
 /* kernel/power/snapshot.c */
@@ -52,7 +53,7 @@ static inline void register_nosave_regio
 static inline int swsusp_page_is_forbidden(struct page *p) { return 0; }
 static inline void swsusp_set_page_free(struct page *p) {}
 static inline void swsusp_unset_page_free(struct page *p) {}
-#endif /* defined(CONFIG_PM) && defined(CONFIG_SOFTWARE_SUSPEND) */
+#endif /* CONFIG_SOFTWARE_SUSPEND */
 
 void save_processor_state(void);
 void restore_processor_state(void);
@@ -60,4 +61,22 @@ struct saved_context;
 void __save_processor_state(struct saved_context *ctxt);
 void __restore_processor_state(struct saved_context *ctxt);
 
+int register_suspend_notifier(struct notifier_block *nb);
+void unregister_suspend_notifier(struct notifier_block *nb);
+
+#define suspend_notifier(fn, pri) {				\
+	static struct notifier_block fn##_nb =			\
+		{ .notifier_call = fn, .priority = pri };	\
+	register_suspend_notifier(&fn##_nb);			\
+}
+#else /* CONFIG_PM */
+static inline int register_suspend_notifier(struct notifier_block *nb) {
+	return 0;
+}
+static inline void unregister_suspend_notifier(struct notifier_block *nb) {
+}
+
+#define suspend_notifier(fn, pri)	do { (void)(fn); } while (0)
+#endif
+
 #endif /* _LINUX_SWSUSP_H */
Index: linux-2.6.21-rc6/kernel/power/Makefile
===================================================================
--- linux-2.6.21-rc6.orig/kernel/power/Makefile	2007-02-04 19:44:54.000000000 +0100
+++ linux-2.6.21-rc6/kernel/power/Makefile	2007-04-14 23:34:30.000000000 +0200
@@ -3,7 +3,7 @@ ifeq ($(CONFIG_PM_DEBUG),y)
 EXTRA_CFLAGS	+=	-DDEBUG
 endif
 
-obj-y				:= main.o process.o console.o
+obj-y				:= main.o process.o console.o notify.o
 obj-$(CONFIG_PM_LEGACY)		+= pm.o
 obj-$(CONFIG_SOFTWARE_SUSPEND)	+= swsusp.o disk.o snapshot.o swap.o user.o
 
Index: linux-2.6.21-rc6/kernel/power/power.h
===================================================================
--- linux-2.6.21-rc6.orig/kernel/power/power.h	2007-04-14 23:38:11.000000000 +0200
+++ linux-2.6.21-rc6/kernel/power/power.h	2007-04-14 23:40:07.000000000 +0200
@@ -171,3 +171,6 @@ extern int suspend_enter(suspend_state_t
 struct timeval;
 extern void swsusp_show_speed(struct timeval *, struct timeval *,
 				unsigned int, char *);
+
+/* kernel/power/notify.c */
+extern int suspend_notifier_call_chain(unsigned long val);
Index: linux-2.6.21-rc6/include/linux/notifier.h
===================================================================
--- linux-2.6.21-rc6.orig/include/linux/notifier.h	2007-02-04 19:44:54.000000000 +0100
+++ linux-2.6.21-rc6/include/linux/notifier.h	2007-04-15 00:31:04.000000000 +0200
@@ -187,5 +187,11 @@ extern int srcu_notifier_call_chain(stru
 #define CPU_DOWN_FAILED		0x0006 /* CPU (unsigned)v NOT going down */
 #define CPU_DEAD		0x0007 /* CPU (unsigned)v dead */
 
+#define SUSPEND_PREPARE		0x0002 /* Going to freeze tasks */
+#define SUSPEND_ENTER_PREPARE	0x0003 /* Tasks are frozen, we are suspending */
+#define SUSPEND_THAW_PREPARE	0x0004 /* Going to thaw frozen tasks */
+#define SUSPEND_FINISHED	0x0005 /* Tasks have been thawed */
+#define SUSPEND_RESTORE_PREPARE	0x0006 /* STD restore is going to happen */
+
 #endif /* __KERNEL__ */
 #endif /* _LINUX_NOTIFIER_H */
Index: linux-2.6.21-rc6/kernel/power/disk.c
===================================================================
--- linux-2.6.21-rc6.orig/kernel/power/disk.c	2007-04-14 21:49:23.000000000 +0200
+++ linux-2.6.21-rc6/kernel/power/disk.c	2007-04-15 00:31:44.000000000 +0200
@@ -100,6 +100,7 @@ static int prepare_processes(void)
 	pm_prepare_console();
 	if (freeze_processes()) {
 		error = -EBUSY;
+		suspend_notifier_call_chain(SUSPEND_THAW_PREPARE);
 		unprepare_processes();
 	}
 	return error;
@@ -127,6 +128,10 @@ int pm_suspend_disk(void)
 	if (error)
 		goto Exit;
 
+	error = suspend_notifier_call_chain(SUSPEND_PREPARE);
+	if (error)
+		goto Finish;
+
 	error = prepare_processes();
 	if (error)
 		goto Finish;
@@ -137,6 +142,10 @@ int pm_suspend_disk(void)
 		goto Thaw;
 	}
 
+	error = suspend_notifier_call_chain(SUSPEND_ENTER_PREPARE);
+	if (error)
+		goto Thaw;
+
 	/* Free memory before shutting down devices. */
 	error = swsusp_shrink_memory();
 	if (error)
@@ -193,8 +202,10 @@ int pm_suspend_disk(void)
 	device_resume();
 	resume_console();
  Thaw:
+	suspend_notifier_call_chain(SUSPEND_THAW_PREPARE);
 	unprepare_processes();
  Finish:
+	suspend_notifier_call_chain(SUSPEND_FINISHED);
 	free_basic_memory_bitmaps();
  Exit:
 	atomic_inc(&snapshot_device_available);
@@ -255,6 +266,10 @@ static int software_resume(void)
 	if (error)
 		goto Finish;
 
+	error = suspend_notifier_call_chain(SUSPEND_PREPARE);
+	if (error)
+		goto Done;
+
 	pr_debug("PM: Preparing processes for restore.\n");
 	error = prepare_processes();
 	if (error) {
@@ -271,6 +286,10 @@ static int software_resume(void)
 		goto Thaw;
 	}
 
+	error = suspend_notifier_call_chain(SUSPEND_RESTORE_PREPARE);
+	if (error)
+		goto Thaw;
+
 	pr_debug("PM: Preparing devices for restore.\n");
 
 	suspend_console();
@@ -289,8 +308,10 @@ static int software_resume(void)
 	resume_console();
  Thaw:
 	printk(KERN_ERR "PM: Restore failed, recovering.\n");
+	suspend_notifier_call_chain(SUSPEND_THAW_PREPARE);
 	unprepare_processes();
  Done:
+	suspend_notifier_call_chain(SUSPEND_FINISHED);
 	free_basic_memory_bitmaps();
  Finish:
 	atomic_inc(&snapshot_device_available);
Index: linux-2.6.21-rc6/kernel/power/main.c
===================================================================
--- linux-2.6.21-rc6.orig/kernel/power/main.c	2007-04-07 12:15:28.000000000 +0200
+++ linux-2.6.21-rc6/kernel/power/main.c	2007-04-15 00:27:52.000000000 +0200
@@ -69,11 +69,19 @@ static int suspend_prepare(suspend_state
 
 	pm_prepare_console();
 
+	error = suspend_notifier_call_chain(SUSPEND_PREPARE);
+	if (error)
+		goto Finish;
+
 	if (freeze_processes()) {
 		error = -EAGAIN;
 		goto Thaw;
 	}
 
+	error = suspend_notifier_call_chain(SUSPEND_ENTER_PREPARE);
+	if (error)
+		goto Thaw;
+
 	if ((free_pages = global_page_state(NR_FREE_PAGES))
 			< FREE_PAGE_NUMBER) {
 		pr_debug("PM: free some memory\n");
@@ -106,8 +114,11 @@ static int suspend_prepare(suspend_state
 	device_resume();
 	resume_console();
  Thaw:
+	suspend_notifier_call_chain(SUSPEND_THAW_PREPARE);
 	thaw_processes();
+ Finish:
 	pm_restore_console();
+	suspend_notifier_call_chain(SUSPEND_FINISHED);
 	return error;
 }
 
@@ -145,8 +156,10 @@ static void suspend_finish(suspend_state
 	pm_finish(state);
 	device_resume();
 	resume_console();
+	suspend_notifier_call_chain(SUSPEND_THAW_PREPARE);
 	thaw_processes();
 	pm_restore_console();
+	suspend_notifier_call_chain(SUSPEND_FINISHED);
 }
 
 
Index: linux-2.6.21-rc6/kernel/power/user.c
===================================================================
--- linux-2.6.21-rc6.orig/kernel/power/user.c	2007-04-14 21:49:23.000000000 +0200
+++ linux-2.6.21-rc6/kernel/power/user.c	2007-04-15 00:38:23.000000000 +0200
@@ -148,6 +148,10 @@ static inline int snapshot_suspend(int p
 	int error;
 
 	mutex_lock(&pm_mutex);
+	error = suspend_notifier_call_chain(SUSPEND_ENTER_PREPARE);
+	if (error)
+		goto Finish;
+
 	/* Free memory before shutting down devices. */
 	error = swsusp_shrink_memory();
 	if (error)
@@ -186,6 +190,10 @@ static inline int snapshot_restore(int p
 
 	mutex_lock(&pm_mutex);
 	pm_prepare_console();
+	error = suspend_notifier_call_chain(SUSPEND_RESTORE_PREPARE);
+	if (error)
+		goto Finish;
+
 	if (platform_suspend) {
 		error = platform_prepare();
 		if (error)
@@ -236,7 +244,12 @@ static int snapshot_ioctl(struct inode *
 		if (data->frozen)
 			break;
 		mutex_lock(&pm_mutex);
+		error = suspend_notifier_call_chain(SUSPEND_PREPARE);
+		if (error)
+			break;
+
 		if (freeze_processes()) {
+			suspend_notifier_call_chain(SUSPEND_THAW_PREPARE);
 			thaw_processes();
 			error = -EBUSY;
 		}
@@ -249,7 +262,9 @@ static int snapshot_ioctl(struct inode *
 		if (!data->frozen)
 			break;
 		mutex_lock(&pm_mutex);
+		suspend_notifier_call_chain(SUSPEND_THAW_PREPARE);
 		thaw_processes();
+		suspend_notifier_call_chain(SUSPEND_FINISHED);
 		mutex_unlock(&pm_mutex);
 		data->frozen = 0;
 		break;
@@ -350,6 +365,10 @@ static int snapshot_ioctl(struct inode *
 			break;
 		}
 
+		error = suspend_notifier_call_chain(SUSPEND_ENTER_PREPARE);
+		if (error)
+			break;
+
 		if (pm_ops->prepare) {
 			error = pm_ops->prepare(PM_SUSPEND_MEM);
 			if (error)