Skip to contentSkip to conversation

2.6.21-rc5: swsusp: Not enough free memory

DISCUSSION Jiri Slaby 6 participants 36 messages Mar 29, 2007 – Apr 14, 2007

Subsystems (2)
lore
Share & tools
lore

Jiri Slaby

author
#0 lore raw

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,

Signature · 6 lines
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 <[email protected]> is an alias for /dev/null
#1 lore raw
Quoted 14 lines · 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 ...

Quoted 7 lines
# 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?

Quoted 7 lines

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

Quoted 5 lines
# 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

author
#2 lore raw

Rafael J. Wysocki napsal(a):

Quoted 21 lines · Jiri Slaby
Quoted 20 lines

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
Quoted 6 lines
Quoted 1 line

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.

Quoted 7 lines
Quoted 7 lines

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

Quoted 7 lines
Quoted 5 lines
# 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,

Signature · 6 lines
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 <[email protected]> is an alias for /dev/null
author
#3 lore raw
In reply to Rafael J. Wysocki

Rafael J. Wysocki napsal(a):

Quoted 21 lines · Jiri Slaby
Quoted 20 lines

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
Quoted 6 lines
Quoted 1 line

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,

Signature · 6 lines
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 <[email protected]> is an alias for /dev/null
#4 lore raw
Quoted 38 lines · Jiri Slaby

Rafael J. Wysocki napsal(a):

Quoted 21 lines · Jiri Slaby
Quoted 20 lines

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
Quoted 6 lines
Quoted 1 line

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...

Quoted 5 lines

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

author
#5 lore raw

Rafael J. Wysocki napsal(a):

Quoted 10 lines
Quoted 6 lines
Quoted 6 lines · Jiri Slaby
Quoted 5 lines

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,

Signature · 6 lines
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 <[email protected]> is an alias for /dev/null
#6 lore raw
Quoted 8 lines · Jiri Slaby

Rafael J. Wysocki napsal(a):

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines · Jiri Slaby
Quoted 5 lines

swsusp: critical section:
swsusp: Need to copy 131380 pages
swsusp: Not enough free memory
Error -12 suspending
Enabling non-boot CPUs ...

Quoted 5 lines
Quoted 3 lines

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).

Quoted 13 lines
# 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)
Quoted 2 lines

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(-)
kernel/power/snapshot.c +1 -1
Index: 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;
kernel/power/swsusp.c +7 -3
Index: 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);
author
#7 lore raw

Rafael J. Wysocki napsal(a):

Quoted 8 lines · Jiri Slaby
Quoted 7 lines

Rafael J. Wysocki napsal(a):

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines · Jiri Slaby
Quoted 5 lines

swsusp: critical section:
swsusp: Need to copy 131380 pages
swsusp: Not enough free memory
Error -12 suspending
Enabling non-boot CPUs ...

Quoted 7 lines
Quoted 4 lines
Quoted 3 lines

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,

Signature · 6 lines
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 <[email protected]> is an alias for /dev/null
#8 lore raw
Quoted 12 lines · Jiri Slaby

Rafael J. Wysocki napsal(a):

Quoted 11 lines · Jiri Slaby
Quoted 10 lines

Rafael J. Wysocki napsal(a):

Quoted 9 lines
Quoted 6 lines
Quoted 6 lines · Jiri Slaby
Quoted 5 lines

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. :-)

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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?

Quoted 15 lines

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

Quoted 1 line

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

Yup.

Quoted 5 lines

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(-)
kernel/power/swsusp.c +1 -1
Index: 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);
 				}
 			}
 
author
#9 lore raw

Rafael J. Wysocki napsal(a):

Quoted 1 line

Okay, so I think the appended patch is needed. Could you please revert the

[...]

Quoted 6 lines

Index: linux-2.6.21-rc5/kernel/power/swsusp.c
===================================================================

kernel/power/swsusp.c +0 -0
@@ -229,13 +229,13 @@ int swsusp_shrink_memory(void)
 		size += highmem_size;

kernel/power/swsusp.c int swsusp_shrink_memory(void) · line 238

Quoted 10 lines
kernel/power/swsusp.c +1 -1
 		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,

Signature · 6 lines
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 <[email protected]> is an alias for /dev/null
author
#10 lore raw
In reply to Rafael J. Wysocki

Rafael J. Wysocki napsal(a):

Quoted 1 line

Thanks for testing.

[...]

Quoted 4 lines
kernel/power/swsusp.c +0 -0
@@ -229,13 +229,13 @@ int swsusp_shrink_memory(void)
 		size += highmem_size;

kernel/power/swsusp.c int swsusp_shrink_memory(void) · line 238

Quoted 10 lines
kernel/power/swsusp.c +1 -1
 		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,

Signature · 6 lines
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 <[email protected]> is an alias for /dev/null
#11 lore raw
Quoted 12 lines · Jiri Slaby

Rafael J. Wysocki napsal(a):

Quoted 1 line

Thanks for testing.

[...]

Quoted 9 lines
kernel/power/swsusp.c +1 -0
@@ -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 -=
Quoted 8 lines

kernel/power/swsusp.c int swsusp_shrink_memory(void) · line 238

Quoted 5 lines
kernel/power/swsusp.c +0 -1
 					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?

Quoted 2 lines

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

author
#12 lore raw

Rafael J. Wysocki napsal(a):

Quoted 21 lines · Jiri Slaby
Quoted 19 lines

Rafael J. Wysocki napsal(a):

Quoted 1 line

Thanks for testing.

[...]

Quoted 4 lines
kernel/power/swsusp.c +0 -0
@@ -229,13 +229,13 @@ int swsusp_shrink_memory(void)
 		size += highmem_size;

kernel/power/swsusp.c int swsusp_shrink_memory(void) · line 238

Quoted 10 lines
kernel/power/swsusp.c +1 -1
 		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.

Quoted 2 lines

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

Quoted 5 lines
Quoted 2 lines

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,

Signature · 6 lines
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 <[email protected]> is an alias for /dev/null
#13 lore raw
Quoted 25 lines · Jiri Slaby

Rafael J. Wysocki napsal(a):

Quoted 21 lines · Jiri Slaby
Quoted 19 lines

Rafael J. Wysocki napsal(a):

Quoted 1 line

Thanks for testing.

[...]

Quoted 4 lines
kernel/power/swsusp.c +0 -0
@@ -229,13 +229,13 @@ int swsusp_shrink_memory(void)
 		size += highmem_size;

kernel/power/swsusp.c int swsusp_shrink_memory(void) · line 238

Quoted 10 lines
kernel/power/swsusp.c +1 -1
 		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.

Quoted 2 lines

I guess you mean you're still seeing the 'not enough memory to suspend' problem?

Quoted 15 lines

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.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

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

author
#14 lore raw

Rafael J. Wysocki napsal(a):

Quoted 23 lines · Jiri Slaby
Quoted 6 lines

Rafael J. Wysocki napsal(a):

Quoted 5 lines · Jiri Slaby
Quoted 2 lines

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?

Quoted 15 lines

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

Quoted 1 line

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,

Signature · 4 lines
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
#15 lore raw
Quoted 24 lines · Jiri Slaby

Rafael J. Wysocki napsal(a):

Quoted 23 lines · Jiri Slaby
Quoted 21 lines

Rafael J. Wysocki napsal(a):

Quoted 5 lines · Jiri Slaby
Quoted 2 lines

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

Quoted 5 lines
Quoted 1 line

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

author
#16 lore raw

Rafael J. Wysocki napsal(a):

Quoted 24 lines · Jiri Slaby
Quoted 23 lines

Rafael J. Wysocki napsal(a):

Quoted 22 lines · Jiri Slaby
Quoted 21 lines

Rafael J. Wysocki napsal(a):

Quoted 5 lines · Jiri Slaby
Quoted 2 lines

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

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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,

Signature · 4 lines
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
#17 lore raw
Quoted 25 lines · Jiri Slaby

Rafael J. Wysocki napsal(a):

Quoted 24 lines · Jiri Slaby
Quoted 23 lines

Rafael J. Wysocki napsal(a):

Quoted 22 lines · Jiri Slaby
Quoted 21 lines

Rafael J. Wysocki napsal(a):

Quoted 5 lines · Jiri Slaby
Quoted 2 lines

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

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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(-)
kernel/power/snapshot.c +2 -2
Index: 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;
kernel/power/swsusp.c +12 -4
Index: 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);
author
#18 lore raw
Quoted 40 lines · Rafael J. Wysocki
Quoted 25 lines · Jiri Slaby

Rafael J. Wysocki napsal(a):

Quoted 24 lines · Jiri Slaby
Quoted 23 lines

Rafael J. Wysocki napsal(a):

Quoted 22 lines · Jiri Slaby
Quoted 21 lines

Rafael J. Wysocki napsal(a):

Quoted 5 lines · Jiri Slaby
Quoted 2 lines

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

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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?

Quoted 7 lines

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,

Signature · 4 lines
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
#19 lore raw
Quoted 62 lines · Jiri Slaby
Quoted 40 lines · Rafael J. Wysocki
Quoted 25 lines · Jiri Slaby

Rafael J. Wysocki napsal(a):

Quoted 24 lines · Jiri Slaby
Quoted 23 lines

Rafael J. Wysocki napsal(a):

Quoted 22 lines · Jiri Slaby
Quoted 21 lines

Rafael J. Wysocki napsal(a):

Quoted 5 lines · Jiri Slaby
Quoted 2 lines

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

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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?

Quoted 7 lines

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)

Quoted 8 lines

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.

Quoted 14 lines

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

Quoted 1 line

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

#20 lore raw

Hi.

Quoted 23 lines · Rafael J. Wysocki
Quoted 14 lines

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)

Quoted 8 lines

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

Quoted 2 lines

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

#21 lore raw

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

Quoted 26 lines · Nigel Cunningham

Hi.

Quoted 23 lines · Rafael J. Wysocki
Quoted 22 lines

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

Quoted 6 lines
Quoted 2 lines

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

#22 lore raw

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

Hi.

Quoted 55 lines · Rafael J. Wysocki
Quoted 26 lines · Nigel Cunningham

Hi.

Quoted 23 lines · Rafael J. Wysocki
Quoted 22 lines

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

Quoted 6 lines
Quoted 2 lines

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.

Quoted 1 line

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

#23 lore raw

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

In reply to Rafael J. Wysocki
Quoted 11 lines · Rafael J. Wysocki

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?

#24 lore raw

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

In reply to Rafael J. Wysocki

Hi!

Quoted 7 lines
Quoted 6 lines
Quoted 2 lines

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.

Quoted 6 lines

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.

Quoted 3 lines

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.

Quoted 5 lines

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#25 lore raw

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

Quoted 9 lines · Pavel Machek

Hi!

Quoted 7 lines
Quoted 6 lines
Quoted 2 lines

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.

Quoted 8 lines
Quoted 6 lines

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.

Quoted 12 lines
Quoted 3 lines

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.

Quoted 5 lines

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

Quoted 3 lines

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

#26 lore raw

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

Hi.

Quoted 19 lines · Rafael J. Wysocki
Quoted 9 lines · Pavel Machek

Hi!

Quoted 7 lines
Quoted 6 lines
Quoted 2 lines

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.

Quoted 8 lines
Quoted 6 lines

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.

Quoted 8 lines

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

#27 lore raw

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

In reply to Rafael J. Wysocki

Hi!

Quoted 14 lines
Quoted 14 lines
Quoted 13 lines
Quoted 6 lines
Quoted 2 lines

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'.

Quoted 6 lines
Quoted 1 line

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#28 lore raw

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

Hi.

Quoted 18 lines · Pavel Machek

Hi!

Quoted 16 lines
Quoted 15 lines
Quoted 7 lines
Quoted 6 lines
Quoted 2 lines

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.

Quoted 6 lines

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.

Quoted 7 lines
Quoted 4 lines

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

#29 lore raw

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

Quoted 26 lines · Nigel Cunningham

Hi.

Quoted 23 lines · Pavel Machek

Hi!

Quoted 20 lines
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
Quoted 2 lines

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.

Quoted 8 lines
Quoted 6 lines

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

Quoted 6 lines
Quoted 2 lines

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

#30 lore raw

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

Hi.

Quoted 8 lines · Rafael J. Wysocki
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines

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.

Quoted 8 lines
Quoted 3 lines

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

#31 lore raw

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

Quoted 20 lines · Nigel Cunningham

Hi.

Quoted 8 lines · Rafael J. Wysocki
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines

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.

Quoted 8 lines
Quoted 3 lines

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. :-)

Quoted 5 lines

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.

Quoted 6 lines

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

#32 lore raw

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

In reply to Rafael J. Wysocki

Hi,

Quoted 1 line · Rafael J. Wysocki

On Saturday, 14 April 2007 00:10, Pavel Machek wrote:

[--snip--]

Quoted 11 lines
Quoted 9 lines
Quoted 5 lines

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(-)
kernel/power/notify.c new file +49 -0
Index: linux-2.6.21-rc6/kernel/power/notify.c
===================================================================
@@ -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 <[email protected]>
+ *
+ * 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;
+}
include/linux/suspend.h +23 -4
Index: linux-2.6.21-rc6/include/linux/suspend.h
===================================================================
@@ -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 */
kernel/power/Makefile +1 -1
Index: linux-2.6.21-rc6/kernel/power/Makefile
===================================================================
@@ -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
 
kernel/power/power.h +3 -0
Index: linux-2.6.21-rc6/kernel/power/power.h
===================================================================
@@ -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);
include/linux/notifier.h +6 -0
Index: linux-2.6.21-rc6/include/linux/notifier.h
===================================================================
@@ -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 */
kernel/power/disk.c +21 -0
Index: linux-2.6.21-rc6/kernel/power/disk.c
===================================================================
@@ -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);
kernel/power/main.c +13 -0
Index: linux-2.6.21-rc6/kernel/power/main.c
===================================================================
@@ -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);
 }
 
 
kernel/power/user.c +19 -0
Index: linux-2.6.21-rc6/kernel/power/user.c
===================================================================
@@ -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)
#33 lore raw

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

In reply to Pavel Machek

Hi.

Quoted 9 lines · Pavel Machek

Hi!

Quoted 7 lines
Quoted 6 lines
Quoted 2 lines

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.

Quoted 8 lines
Quoted 6 lines

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?

Quoted 9 lines
Quoted 5 lines

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

#34 lore raw

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

Hi!

Quoted 14 lines
Quoted 14 lines
Quoted 13 lines
Quoted 6 lines
Quoted 2 lines

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'.

Quoted 8 lines
Quoted 1 line

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.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

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

Quoted 7 lines
Quoted 3 lines

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#35 lore raw

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

Hi.

Quoted 25 lines · Pavel Machek

Hi!

Quoted 14 lines
Quoted 14 lines
Quoted 13 lines
Quoted 6 lines
Quoted 2 lines

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'.

Quoted 8 lines
Quoted 1 line

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?

Quoted 2 lines

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.

Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
Quoted 5 lines

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.

Quoted 6 lines
Quoted 3 lines

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