: Hibernation: fix the number of pages used for hibernate/thaw buffering

PATCH Applied Bojan Smojver 3 participants 34 messages Apr 12, 2012 – Apr 30, 2012

Subsystems (2)

Suspend to RAM Hibernation

lore

Share & tools

lore

Bojan Smojver

author

2012-04-12 21:59 UTC #0 lore raw

PATCH v11 1/1 : Hibernation: fix the number of pages used for hibernate/thaw buffering

Hi Rafael,

Yes, I know - it is getting a bit ridiculous. Such a simple problem, so many patches. Anyhow...

Here we also fix the calculation of pages for read buffering. Not really very significant, but may become important in some really tight memory scenarios. And after thinking more about low_free_pages() inline function, Per was right - it is better to call an already existing function, rather than roll our own. Keeps kernel smaller and there is no significant performance gain otherwise.

Per,

Feel free to reply with your signed-off.

---------------------------------------
Hibernation/thaw fixes/improvements:

1. Calculate the number of required free pages based on non-high memory pages only, because that is where the buffers will come from.

2. Do not allocate memory for buffers from emergency pools, unless absolutely required. Do not warn about and do not retry non-essential failed allocations.

3. Do not check the amount of free pages left on every single page write, but wait until one map is completely populated and then check.

4. Set maximum number of pages for read buffering consistently, instead of inadvertently depending on the size of the sector type.

5. Fix copyright line, which I missed when I submitted the hibernation threading patch.

6. Dispense with bit shifting arithmetic to improve readability.

Signed-off-by Bojan Smojver <[email protected]>

---
 kernel/power/swap.c |   84 +++++++++++++++++++++++++++++++++++----------------
 1 files changed, 58 insertions(+), 26 deletions(-)

kernel/power/swap.c +58 ~~-26~~

index 8742fd0..11e22c0 100644
@@ -6,7 +6,7 @@
  *
  * Copyright (C) 1998,2001-2005 Pavel Machek <[email protected]>
  * Copyright (C) 2006 Rafael J. Wysocki <[email protected]>
- * Copyright (C) 2010 Bojan Smojver <[email protected]>
+ * Copyright (C) 2010-2012 Bojan Smojver <[email protected]>
  *
  * This file is released under the GPLv2.
  *

@@ -51,6 +51,23 @@
 
 #define MAP_PAGE_ENTRIES	(PAGE_SIZE / sizeof(sector_t) - 1)
 
+/*
+ * Number of free pages that are not high.
+ */
+static inline unsigned long low_free_pages(void)
+{
+	return nr_free_pages() - nr_free_highpages();
+}
+
+/*
+ * Number of pages required to be kept free while writing the image. Always
+ * half of all available low pages before the writing starts.
+ */
+static inline unsigned long reqd_free_pages(void)
+{
+	return low_free_pages() / 2;
+}
+
 struct swap_map_page {
 	sector_t entries[MAP_PAGE_ENTRIES];
 	sector_t next_swap;

@@ -72,7 +89,7 @@ struct swap_map_handle {
 	sector_t cur_swap;
 	sector_t first_sector;
 	unsigned int k;
-	unsigned long nr_free_pages, written;
+	unsigned long reqd_free_pages;
 	u32 crc32;
 };

@@ -265,14 +282,17 @@ static int write_page(void *buf, sector_t offset, struct bio **bio_chain)
 		return -ENOSPC;
 
 	if (bio_chain) {
-		src = (void *)__get_free_page(__GFP_WAIT | __GFP_HIGH);
+		src = (void *)__get_free_page(__GFP_WAIT | __GFP_NOWARN |
+		                              __GFP_NORETRY);
 		if (src) {
 			copy_page(src, buf);
 		} else {
 			ret = hib_wait_on_bio_chain(bio_chain); /* Free pages */
 			if (ret)
 				return ret;
-			src = (void *)__get_free_page(__GFP_WAIT | __GFP_HIGH);
+			src = (void *)__get_free_page(__GFP_WAIT |
+			                              __GFP_NOWARN |
+			                              __GFP_NORETRY);
 			if (src) {
 				copy_page(src, buf);
 			} else {

@@ -316,8 +336,7 @@ static int get_swap_writer(struct swap_map_handle *handle)
 		goto err_rel;
 	}
 	handle->k = 0;
-	handle->nr_free_pages = nr_free_pages() >> 1;
-	handle->written = 0;
+	handle->reqd_free_pages = reqd_free_pages();
 	handle->first_sector = handle->cur_swap;
 	return 0;
 err_rel:

@@ -351,12 +370,17 @@ static int swap_write_page(struct swap_map_handle *handle, void *buf,
 		clear_page(handle->cur);
 		handle->cur_swap = offset;
 		handle->k = 0;
-	}
-	if (bio_chain && ++handle->written > handle->nr_free_pages) {
-		error = hib_wait_on_bio_chain(bio_chain);
-		if (error)
-			goto out;
-		handle->written = 0;
+
+		if (bio_chain && low_free_pages() <= handle->reqd_free_pages) {
+			error = hib_wait_on_bio_chain(bio_chain);
+			if (error)
+				goto out;
+			/*
+			 * Recalculate the number of required free pages, to
+			 * make sure we never take more than half.
+			 */
+			handle->reqd_free_pages = reqd_free_pages();
+		}
 	}
  out:
 	return error;

@@ -403,8 +427,9 @@ static int swap_writer_finish(struct swap_map_handle *handle,
 /* Maximum number of threads for compression/decompression. */
 #define LZO_THREADS	3
 
-/* Maximum number of pages for read buffering. */
-#define LZO_READ_PAGES	(MAP_PAGE_ENTRIES * 8)
+/* Minimum/maximum number of pages for read buffering. */
+#define LZO_MIN_RD_PAGES	1024
+#define LZO_MAX_RD_PAGES	8192
 
 
 /**

@@ -615,12 +640,6 @@ static int save_image_lzo(struct swap_map_handle *handle,
 	}
 
 	/*
-	 * Adjust number of free pages after all allocations have been done.
-	 * We don't want to run out of pages when writing.
-	 */
-	handle->nr_free_pages = nr_free_pages() >> 1;
-
-	/*
 	 * Start the CRC32 thread.
 	 */
 	init_waitqueue_head(&crc->go);

@@ -641,6 +660,12 @@ static int save_image_lzo(struct swap_map_handle *handle,
 		goto out_clean;
 	}
 
+	/*
+	 * Adjust the number of required free pages after all allocations have
+	 * been done. We don't want to run out of pages when writing.
+	 */
+	handle->reqd_free_pages = reqd_free_pages();
+
 	printk(KERN_INFO
 		"PM: Using %u thread(s) for compression.\n"
 		"PM: Compressing and saving image data (%u pages) ...     ",

@@ -1051,7 +1076,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
 	unsigned i, thr, run_threads, nr_threads;
 	unsigned ring = 0, pg = 0, ring_size = 0,
 	         have = 0, want, need, asked = 0;
-	unsigned long read_pages;
+	unsigned long read_pages = 0;
 	unsigned char **page = NULL;
 	struct dec_data *data = NULL;
 	struct crc_data *crc = NULL;

@@ -1063,7 +1088,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
 	nr_threads = num_online_cpus() - 1;
 	nr_threads = clamp_val(nr_threads, 1, LZO_THREADS);
 
-	page = vmalloc(sizeof(*page) * LZO_READ_PAGES);
+	page = vmalloc(sizeof(*page) * LZO_MAX_RD_PAGES);
 	if (!page) {
 		printk(KERN_ERR "PM: Failed to allocate LZO page\n");
 		ret = -ENOMEM;

@@ -1128,15 +1153,22 @@ static int load_image_lzo(struct swap_map_handle *handle,
 	}
 
 	/*
-	 * Adjust number of pages for read buffering, in case we are short.
+	 * Set the number of pages for read buffering.
+	 * This is complete guesswork, because we'll only know the real
+	 * picture once prepare_image() is called, which is much later on
+	 * during the image load phase. We'll assume the worst case and
+	 * say that none of the image pages are from high memory.
 	 */
-	read_pages = (nr_free_pages() - snapshot_get_image_size()) >> 1;
-	read_pages = clamp_val(read_pages, LZO_CMP_PAGES, LZO_READ_PAGES);
+	if (low_free_pages() > snapshot_get_image_size())
+		read_pages = (low_free_pages() - snapshot_get_image_size()) / 2;
+	read_pages = clamp_val(read_pages, LZO_MIN_RD_PAGES, LZO_MAX_RD_PAGES);
 
 	for (i = 0; i < read_pages; i++) {
 		page[i] = (void *)__get_free_page(i < LZO_CMP_PAGES ?
 		                                  __GFP_WAIT | __GFP_HIGH :
-		                                  __GFP_WAIT);
+		                                  __GFP_WAIT | __GFP_NOWARN |
+		                                  __GFP_NORETRY);
+
 		if (!page[i]) {
 			if (i < LZO_CMP_PAGES) {
 				ring_size = i;

---------------------------------------

Signature · 1 line

Bojan

Per Olofsson

2012-04-15 12:47 UTC #1 lore raw

2012-04-12 23:59, Bojan Smojver skrev:

Quoted 12 lines

Hi Rafael,
Yes, I know - it is getting a bit ridiculous. Such a simple problem, so many patches. Anyhow...
Here we also fix the calculation of pages for read buffering. Not really very significant, but may become important in some really tight memory scenarios. And after thinking more about low_free_pages() inline function, Per was right - it is better to call an already existing function, rather than roll our own. Keeps kernel smaller and there is no significant performance gain otherwise.

Quoted 3 lines

Per,
Feel free to reply with your signed-off.

Signed-off-by Per Olofsson <[email protected]>

Signature · 1 line

Pelle

Rafael J. Wysocki

2012-04-15 22:04 UTC #2 lore raw

Quoted 13 lines · Per Olofsson

2012-04-12 23:59, Bojan Smojver skrev:
Quoted 12 lines
Hi Rafael,
Yes, I know - it is getting a bit ridiculous. Such a simple problem, so many patches. Anyhow...
Here we also fix the calculation of pages for read buffering. Not really very significant, but may become important in some really tight memory scenarios. And after thinking more about low_free_pages() inline function, Per was right - it is better to call an already existing function, rather than roll our own. Keeps kernel smaller and there is no significant performance gain otherwise.

Quoted 5 lines

Quoted 3 lines
Per,
Feel free to reply with your signed-off.
Signed-off-by Per Olofsson <[email protected]>

Why signed-off?

Rafael

Bojan Smojver

author

2012-04-15 22:31 UTC #3 lore raw

Quoted 3 lines · Rafael J. Wysocki

Quoted 1 line
Signed-off-by Per Olofsson <[email protected]>
Why signed-off?

Per contributed significantly to the development of the patch. In fact, it was Per who noticed what the problem with number of free pages calculation during hibernation actually was.

Signature · 1 line

Bojan

Rafael J. Wysocki

2012-04-16 18:34 UTC #4 lore raw

Quoted 8 lines · Bojan Smojver

Quoted 3 lines · Rafael J. Wysocki
Quoted 1 line
Signed-off-by Per Olofsson <[email protected]>
Why signed-off?
Per contributed significantly to the development of the patch. In fact, it was Per who noticed what the problem with number of free pages calculation during hibernation actually was.

OK, but you wrote the patch in the end.

Reviewed-by from Per would be more appropriate.

Signed-off-by means as much as "I certify that am the author of this patch" or "This patch has gone through my hands and I know who the author is" (it implies "Acked-by" in the latter case). None of these is the case here.

Thanks,
Rafael

Per Olofsson

Reviewed-by

2012-04-16 19:47 UTC #5 lore raw

2012-04-16 20:38, Rafael J. Wysocki skrev:

Quoted 10 lines · Bojan Smojver

Quoted 8 lines · Rafael J. Wysocki
Quoted 3 lines
Quoted 1 line
Signed-off-by Per Olofsson <[email protected]>
Why signed-off?
Per contributed significantly to the development of the patch. In fact, it was Per who noticed what the problem with number of free pages calculation during hibernation actually was.

Quoted 7 lines

OK, but you wrote the patch in the end.
Reviewed-by from Per would be more appropriate.
Signed-off-by means as much as "I certify that am the author of this patch" or "This patch has gone through my hands and I know who the author is" (it implies "Acked-by" in the latter case). None of these is the case here.

OK, let's say Reviewed-by then.

Reviewed-by Per Olofsson <[email protected]>

Signature · 1 line

Pelle

Bojan Smojver

author

2012-04-18 21:17 UTC #6 lore raw

Quoted 6 lines · Per Olofsson

OK, let's say Reviewed-by then.
Reviewed-by Per Olofsson <[email protected]>
Signature · 1 line
Pelle

Rafael,

Anything you still need here?

Signature · 1 line

Bojan

Rafael J. Wysocki

2012-04-18 21:26 UTC #7 lore raw

Quoted 10 lines · Bojan Smojver

Quoted 4 lines · Per Olofsson
OK, let's say Reviewed-by then.
Reviewed-by Per Olofsson <[email protected]>
Rafael,
Anything you still need here?

No, thanks, I'm going to apply the patch.

Rafael

Bojan Smojver

author

2012-04-18 22:14 UTC #8 lore raw

Quoted 1 line · Rafael J. Wysocki

No, thanks, I'm going to apply the patch.

Thanks.

Signature · 1 line

Bojan

Bojan Smojver

author

2012-04-21 22:55 UTC #9 lore raw

In reply to Rafael J. Wysocki

Quoted 8 lines · Rafael J. Wysocki

Quoted 7 lines · Bojan Smojver
Quoted 4 lines · Per Olofsson
OK, let's say Reviewed-by then.
Reviewed-by Per Olofsson <[email protected]>

Quoted 7 lines

Quoted 3 lines
Rafael,
Anything you still need here?
No, thanks, I'm going to apply the patch.
Rafael

Any chance of it in 3.3.3? Looks like rc1 of it is already out there...

Signature · 1 line

Bojan

Rafael J. Wysocki

2012-04-22 12:16 UTC #10 lore raw

Quoted 12 lines · Bojan Smojver

Quoted 10 lines · Rafael J. Wysocki
Quoted 9 lines · Bojan Smojver
Quoted 4 lines · Per Olofsson
OK, let's say Reviewed-by then.
Reviewed-by Per Olofsson <[email protected]>
Rafael,

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
Anything you still need here?
No, thanks, I'm going to apply the patch.
Rafael
Any chance of it in 3.3.3?

Nope.

Quoted 1 line

Looks like rc1 of it is already out there...

I'm going to push it for v3.5, so it will only appear in 3.4.y I guess.

Thanks,
Rafael

Per Olofsson

2012-04-22 11:51 UTC #11 lore raw

2012-04-22 13:47, Rafael J. Wysocki skrev:

Quoted 5 lines

Quoted 1 line
Quoted 1 line
Any chance of it in 3.3.3?
Nope.
Quoted 1 line
Quoted 1 line
Looks like rc1 of it is already out there...
I'm going to push it for v3.5, so it will only appear in 3.4.y I guess.

What about 3.2.y?

Signature · 1 line

Pelle

Rafael J. Wysocki

2012-04-22 11:58 UTC #12 lore raw

Quoted 8 lines · Per Olofsson

2012-04-22 13:47, Rafael J. Wysocki skrev:
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Any chance of it in 3.3.3?
Nope.
Quoted 1 line
Quoted 1 line
Looks like rc1 of it is already out there...
I'm going to push it for v3.5, so it will only appear in 3.4.y I guess.
What about 3.2.y?

That depends on Greg. If 3.2.y is still being continued when 3.5 is out, I think it also will appear in there. I'll just add the "CC stable" tag to the commit.

Thanks,
Rafael

Bojan Smojver

author

2012-04-22 12:17 UTC #13 lore raw

In reply to Rafael J. Wysocki

Quoted 21 lines · Rafael J. Wysocki

Quoted 12 lines · Bojan Smojver
Quoted 10 lines · Rafael J. Wysocki
Quoted 9 lines · Bojan Smojver
Quoted 4 lines · Per Olofsson
OK, let's say Reviewed-by then.
Reviewed-by Per Olofsson <[email protected]>
Rafael,
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Anything you still need here?
No, thanks, I'm going to apply the patch.
Rafael
Any chance of it in 3.3.3?

Quoted 8 lines

Nope.
Quoted 1 line
Looks like rc1 of it is already out there...
I'm going to push it for v3.5, so it will only appear in 3.4.y I guess.
Thanks,
Rafael

Don't understand why not. This is a fix for a regression in 3.2 and above. Is there a new policy on regression fixes? I thought this should be queued for 3.4, 3.3 and 3.2. Otherwise, people will continue to see hangs in those versions.

Signature · 1 line

Bojan

Rafael J. Wysocki

2012-04-22 20:25 UTC #14 lore raw

Quoted 25 lines · Bojan Smojver

Quoted 23 lines · Rafael J. Wysocki
Quoted 12 lines · Bojan Smojver
Quoted 10 lines · Rafael J. Wysocki
Quoted 9 lines · Bojan Smojver
Quoted 4 lines · Per Olofsson
OK, let's say Reviewed-by then.
Reviewed-by Per Olofsson <[email protected]>
Rafael,
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Anything you still need here?
No, thanks, I'm going to apply the patch.
Rafael
Any chance of it in 3.3.3?
Nope.

Quoted 8 lines

Quoted 6 lines
Quoted 1 line
Looks like rc1 of it is already out there...
I'm going to push it for v3.5, so it will only appear in 3.4.y I guess.
Thanks,
Rafael
Don't understand why not. This is a fix for a regression in 3.2 and above.

Yes, in 3.2, so it is old enough. If this were a regression in 3.3, I'd push it for 3.4.

Quoted 3 lines

Is there a new policy on regression fixes? I thought this should be queued for 3.4, 3.3 and 3.2. Otherwise, people will continue to see hangs in those versions.

Well, it was late for the 3.4 merge window and it wasn't clear whether or not the patch was a regression fix at that time. It is not very straightforward and in my opinion it should be tested a bit wider before it goes into -stable.

So as I said, v3.5 is the target with whatever -stable trees are relevant at the time the patch is merged.

Thanks,
Rafael

Per Olofsson

2012-04-22 20:35 UTC #15 lore raw

2012-04-22 22:29, Rafael J. Wysocki skrev:

Quoted 4 lines

Well, it was late for the 3.4 merge window and it wasn't clear whether or not the patch was a regression fix at that time. It is not very straightforward and in my opinion it should be tested a bit wider before it goes into -stable.

Hopefully it will soon get applied in Debian unstable and then get much more testing.

It is also possible to create a much smaller patch which only subtracts the high pages and nothing else.

Signature · 1 line

Pelle

Rafael J. Wysocki

2012-04-22 20:45 UTC #16 lore raw

Quoted 11 lines · Per Olofsson

2012-04-22 22:29, Rafael J. Wysocki skrev:
Quoted 4 lines
Well, it was late for the 3.4 merge window and it wasn't clear whether or not the patch was a regression fix at that time. It is not very straightforward and in my opinion it should be tested a bit wider before it goes into -stable.
Hopefully it will soon get applied in Debian unstable and then get much more testing.
It is also possible to create a much smaller patch which only subtracts the high pages and nothing else.

I've just pushed it to my linux-next branch, so it should appear in linux-next tomorrow.

Still, we are a few weeks from the 3.5 merge window, so it's not that much time to wait anyway.

Thanks,
Rafael

Bojan Smojver

author

2012-04-22 22:29 UTC #17 lore raw

In reply to Per Olofsson

Quoted 2 lines · Per Olofsson

It is also possible to create a much smaller patch which only subtracts the high pages and nothing else.

For instance:
---------------------------------------
Hibernation regression fix, since 3.2:

Calculate the number of required free pages based on non-high memory pages only, because that is where the buffers will come from.

Signed-off-by Bojan Smojver <[email protected]>

---
 kernel/power/swap.c |   33 +++++++++++++++++++++++++--------
 1 files changed, 25 insertions(+), 8 deletions(-)

kernel/power/swap.c +22 -6

index 8742fd0..fdf834f 100644
@@ -51,6 +51,23 @@
 
 #define MAP_PAGE_ENTRIES	(PAGE_SIZE / sizeof(sector_t) - 1)
 
+/*
+ * Number of free pages that are not high.
+ */
+static inline unsigned long low_free_pages(void)
+{
+	return nr_free_pages() - nr_free_highpages();
+}
+
+/*
+ * Number of pages required to be kept free while writing the image. Always
+ * half of all available low pages before the writing starts.
+ */
+static inline unsigned long reqd_free_pages(void)
+{
+	return low_free_pages() / 2;
+}
+
 struct swap_map_page {
 	sector_t entries[MAP_PAGE_ENTRIES];
 	sector_t next_swap;

@@ -72,7 +89,7 @@ struct swap_map_handle {
 	sector_t cur_swap;
 	sector_t first_sector;
 	unsigned int k;
-	unsigned long nr_free_pages, written;
+	unsigned long reqd_free_pages;
 	u32 crc32;
 };

@@ -316,8 +333,7 @@ static int get_swap_writer(struct swap_map_handle *handle)
 		goto err_rel;
 	}
 	handle->k = 0;
-	handle->nr_free_pages = nr_free_pages() >> 1;
-	handle->written = 0;
+	handle->reqd_free_pages = reqd_free_pages();
 	handle->first_sector = handle->cur_swap;
 	return 0;
 err_rel:

@@ -352,11 +368,11 @@ static int swap_write_page(struct swap_map_handle *handle, void *buf,
 		handle->cur_swap = offset;
 		handle->k = 0;
 	}
-	if (bio_chain && ++handle->written > handle->nr_free_pages) {
+	if (bio_chain && low_free_pages() <= handle->reqd_free_pages) {
 		error = hib_wait_on_bio_chain(bio_chain);
 		if (error)
 			goto out;
-		handle->written = 0;
+		handle->reqd_free_pages = reqd_free_pages();
 	}
  out:
 	return error;

@@ -618,7 +634,7 @@ static int save_image_lzo(struct swap_map_handle *handle,
 	 * Adjust number of free pages after all allocations have been done.
 	 * We don't want to run out of pages when writing.
 	 */
-	handle->nr_free_pages = nr_free_pages() >> 1;
+	handle->reqd_free_pages = reqd_free_pages();
 
 	/*
 	 * Start the CRC32 thread.

---------------------------------------

Signature · 1 line

Bojan

Bojan Smojver

author

2012-04-23 21:31 UTC #18 lore raw

Quoted 1 line · Bojan Smojver

Hibernation regression fix, since 3.2

Rafael,

Any chance of queuing up this simpler patch for stable? We don't want people's computers to hang on hibernation when there is a fix, right?

Signature · 1 line

Bojan

Rafael J. Wysocki

2012-04-23 21:54 UTC #19 lore raw

Quoted 7 lines · Bojan Smojver

Quoted 1 line · Bojan Smojver
Hibernation regression fix, since 3.2
Rafael,
Any chance of queuing up this simpler patch for stable? We don't want people's computers to hang on hibernation when there is a fix, right?

Please resend it with a better changelog. I mean, please explain what the regression is and why it is being fixed this way.

Thanks,
Rafael

Rafael J. Wysocki

2012-04-23 21:59 UTC #20 lore raw

Quoted 6 lines · Rafael J. Wysocki

Quoted 5 lines · Bojan Smojver
Quoted 1 line · Bojan Smojver
Hibernation regression fix, since 3.2
Rafael,

Quoted 7 lines

Quoted 2 lines
Any chance of queuing up this simpler patch for stable? We don't want people's computers to hang on hibernation when there is a fix, right?
OK
Please resend it with a better changelog. I mean, please explain what the regression is and why it is being fixed this way.

Also please say which commit introduced the regression.

Thanks,
Rafael

Bojan Smojver

author

2012-04-23 22:40 UTC #21 lore raw

In reply to Rafael J. Wysocki

Quoted 2 lines · Rafael J. Wysocki

Please resend it with a better changelog. I mean, please explain what the regression is and why it is being fixed this way.

---------------------------------------
Hibernation regression fix, since 3.2.

Calculate the number of required free pages based on non-high memory pages only, because that is where the buffers will come from.

Commit 081a9d043c983f161b78fdc4671324d1342b86bc introduced a new buffer page allocation logic during hibernation, in order to improve the performance. The amount of pages allocated was calculated based on total amount of pages available, although only non-high memory pages are usable for this purpose. This caused hibernation code to attempt to over allocate pages on platforms that have high memory, which led to hangs.

A more elaborate patch, which also addressed other hibernation/thaw issues, has been merged into linux-next, commit e9cbc5a6270be7aa9c42d9b15293ba9ac7161262.

Signed-off-by Bojan Smojver <[email protected]>

---
 kernel/power/swap.c |   33 +++++++++++++++++++++++++--------
 1 files changed, 25 insertions(+), 8 deletions(-)

kernel/power/swap.c +22 -6

index 8742fd0..fdf834f 100644
@@ -51,6 +51,23 @@
 
 #define MAP_PAGE_ENTRIES	(PAGE_SIZE / sizeof(sector_t) - 1)
 
+/*
+ * Number of free pages that are not high.
+ */
+static inline unsigned long low_free_pages(void)
+{
+	return nr_free_pages() - nr_free_highpages();
+}
+
+/*
+ * Number of pages required to be kept free while writing the image. Always
+ * half of all available low pages before the writing starts.
+ */
+static inline unsigned long reqd_free_pages(void)
+{
+	return low_free_pages() / 2;
+}
+
 struct swap_map_page {
 	sector_t entries[MAP_PAGE_ENTRIES];
 	sector_t next_swap;

@@ -72,7 +89,7 @@ struct swap_map_handle {
 	sector_t cur_swap;
 	sector_t first_sector;
 	unsigned int k;
-	unsigned long nr_free_pages, written;
+	unsigned long reqd_free_pages;
 	u32 crc32;
 };

@@ -316,8 +333,7 @@ static int get_swap_writer(struct swap_map_handle *handle)
 		goto err_rel;
 	}
 	handle->k = 0;
-	handle->nr_free_pages = nr_free_pages() >> 1;
-	handle->written = 0;
+	handle->reqd_free_pages = reqd_free_pages();
 	handle->first_sector = handle->cur_swap;
 	return 0;
 err_rel:

@@ -352,11 +368,11 @@ static int swap_write_page(struct swap_map_handle *handle, void *buf,
 		handle->cur_swap = offset;
 		handle->k = 0;
 	}
-	if (bio_chain && ++handle->written > handle->nr_free_pages) {
+	if (bio_chain && low_free_pages() <= handle->reqd_free_pages) {
 		error = hib_wait_on_bio_chain(bio_chain);
 		if (error)
 			goto out;
-		handle->written = 0;
+		handle->reqd_free_pages = reqd_free_pages();
 	}
  out:
 	return error;

@@ -618,7 +634,7 @@ static int save_image_lzo(struct swap_map_handle *handle,
 	 * Adjust number of free pages after all allocations have been done.
 	 * We don't want to run out of pages when writing.
 	 */
-	handle->nr_free_pages = nr_free_pages() >> 1;
+	handle->reqd_free_pages = reqd_free_pages();
 
 	/*
 	 * Start the CRC32 thread.

---------------------------------------

Signature · 1 line

Bojan

Rafael J. Wysocki

Applied

2012-04-24 22:07 UTC #22 lore raw

Quoted 17 lines · Bojan Smojver

Quoted 2 lines · Rafael J. Wysocki
Please resend it with a better changelog. I mean, please explain what the regression is and why it is being fixed this way.
---------------------------------------
Hibernation regression fix, since 3.2.
Calculate the number of required free pages based on non-high memory pages only, because that is where the buffers will come from.
Commit 081a9d043c983f161b78fdc4671324d1342b86bc introduced a new buffer page allocation logic during hibernation, in order to improve the performance. The amount of pages allocated was calculated based on total amount of pages available, although only non-high memory pages are usable for this purpose. This caused hibernation code to attempt to over allocate pages on platforms that have high memory, which led to hangs.

Quoted 5 lines

A more elaborate patch, which also addressed other hibernation/thaw issues, has been merged into linux-next, commit e9cbc5a6270be7aa9c42d9b15293ba9ac7161262.
Signed-off-by Bojan Smojver <[email protected]>

Applied to linux-pm/linux-next. Will push to Linus later this week.

Thanks,
Rafael

Quoted 77 lines

---
 kernel/power/swap.c |   33 +++++++++++++++++++++++++--------
 1 files changed, 25 insertions(+), 8 deletions(-)

kernel/power/swap.c +22 -7

index 8742fd0..fdf834f 100644
@@ -51,6 +51,23 @@
 
 #define MAP_PAGE_ENTRIES	(PAGE_SIZE / sizeof(sector_t) - 1)
 
+/*
+ * Number of free pages that are not high.
+ */
+static inline unsigned long low_free_pages(void)
+{
+	return nr_free_pages() - nr_free_highpages();
+}
+
+/*
+ * Number of pages required to be kept free while writing the image. Always
+ * half of all available low pages before the writing starts.
+ */
+static inline unsigned long reqd_free_pages(void)
+{
+	return low_free_pages() / 2;
+}
+
 struct swap_map_page {
 	sector_t entries[MAP_PAGE_ENTRIES];
 	sector_t next_swap;

@@ -72,7 +89,7 @@ struct swap_map_handle {
 	sector_t cur_swap;
 	sector_t first_sector;
 	unsigned int k;
-	unsigned long nr_free_pages, written;
+	unsigned long reqd_free_pages;
 	u32 crc32;
 };

@@ -316,8 +333,7 @@ static int get_swap_writer(struct swap_map_handle *handle)
 		goto err_rel;
 	}
 	handle->k = 0;
-	handle->nr_free_pages = nr_free_pages() >> 1;
-	handle->written = 0;
+	handle->reqd_free_pages = reqd_free_pages();
 	handle->first_sector = handle->cur_swap;
 	return 0;
 err_rel:

@@ -352,11 +368,11 @@ static int swap_write_page(struct swap_map_handle *handle, void *buf,
 		handle->cur_swap = offset;
 		handle->k = 0;
 	}
-	if (bio_chain && ++handle->written > handle->nr_free_pages) {
+	if (bio_chain && low_free_pages() <= handle->reqd_free_pages) {
 		error = hib_wait_on_bio_chain(bio_chain);
 		if (error)
 			goto out;
-		handle->written = 0;
+		handle->reqd_free_pages = reqd_free_pages();
 	}
  out:
 	return error;

@@ -618,7 +634,7 @@ static int save_image_lzo(struct swap_map_handle *handle,
 	 * Adjust number of free pages after all allocations have been done.
 	 * We don't want to run out of pages when writing.
 	 */
-	handle->nr_free_pages = nr_free_pages() >> 1;
+	handle->reqd_free_pages = reqd_free_pages();
 
 	/*
 	 * Start the CRC32 thread.
---------------------------------------

Bojan Smojver

author

2012-04-25 00:55 UTC #23 lore raw

Quoted 1 line · Rafael J. Wysocki

Applied to linux-pm/linux-next. Will push to Linus later this week.

Thank you.

Signature · 1 line

Bojan

Bojan Smojver

author

2012-04-26 03:28 UTC #24 lore raw

In reply to Rafael J. Wysocki

Quoted 1 line · Rafael J. Wysocki

Applied to linux-pm/linux-next.

Yeah, that was the wrong thing to do. The simpler patch I sent was for stable only, as you requested.

This patch should go to linux-next:

http://marc.info/?l=linux-kernel&m=133426808115262&w=2

Signature · 1 line

Bojan

Rafael J. Wysocki

2012-04-26 19:55 UTC #25 lore raw

Quoted 5 lines · Bojan Smojver

Quoted 1 line · Rafael J. Wysocki
Applied to linux-pm/linux-next.
Yeah, that was the wrong thing to do. The simpler patch I sent was for stable only, as you requested.

No, I didn't requested that. There's nothing like stable-only patches, we only backport mainline commits to -stable.

Quoted 3 lines

This patch should go to linux-next:
http://marc.info/?l=linux-kernel&m=133426808115262&w=2

So I'll include it into my v3.5 push and I don't want to hear about how urgent it is any more. Is that clear enough?

Rafael

Bojan Smojver

author

2012-04-26 21:13 UTC #26 lore raw

Quoted 2 lines · Rafael J. Wysocki

No, I didn't requested that. There's nothing like stable-only patches, we only backport mainline commits to -stable.

Yeah, you told me that my patch was not straightforward, so I broke off part of it for -stable that would fix just the most critical problem.

Quoted 8 lines

Quoted 3 lines
This patch should go to linux-next:
http://marc.info/?l=linux-kernel&m=133426808115262&w=2
OK
So I'll include it into my v3.5 push and I don't want to hear about how urgent it is any more. Is that clear enough?

Look, I created a problem on some people's computer in 3.2. I feel responsible for fixing it. I do not want their computers to hang because of something I wrote, especially if there is a fix.

So, if I will say one more time. This patch is a fix for a regression, caused by me, since 3.2. Please ask -stable maintainers to include it in version in 3.2 and above.

If you want me to create a set of patches instead and then you backport just one, let me know. I will do whatever is necessary to fix this regression.

Signature · 1 line

Bojan

Rafael J. Wysocki

2012-04-26 22:06 UTC #27 lore raw

Quoted 7 lines · Bojan Smojver

Quoted 2 lines · Rafael J. Wysocki
No, I didn't requested that. There's nothing like stable-only patches, we only backport mainline commits to -stable.
Yeah, you told me that my patch was not straightforward, so I broke off part of it for -stable that would fix just the most critical problem.

Which is entirely OK, but see below.

Quoted 9 lines

Quoted 8 lines
Quoted 3 lines
This patch should go to linux-next:
http://marc.info/?l=linux-kernel&m=133426808115262&w=2
OK
So I'll include it into my v3.5 push and I don't want to hear about how urgent it is any more. Is that clear enough?

Quoted 7 lines

Look, I created a problem on some people's computer in 3.2. I feel responsible for fixing it. I do not want their computers to hang because of something I wrote, especially if there is a fix.
So, if I will say one more time. This patch is a fix for a regression, caused by me, since 3.2. Please ask -stable maintainers to include it in version in 3.2 and above.

The -stable rules are such that it only is permitted to put a commit into -stable if the analogous commit is already in the mainline. That also covers reverts and such.

Quoted 3 lines

If you want me to create a set of patches instead and then you backport just one, let me know. I will do whatever is necessary to fix this regression.

Can you just create a second patch on top of what's in linux-next now?

Rafael

Bojan Smojver

author

2012-04-26 22:22 UTC #28 lore raw

Quoted 1 line · Rafael J. Wysocki

Can you just create a second patch on top of what's in linux-next now?

Shall do.

Signature · 1 line

Bojan

Bojan Smojver

author

2012-04-27 00:16 UTC #29 lore raw

In reply to Rafael J. Wysocki

Quoted 1 line · Rafael J. Wysocki

Can you just create a second patch on top of what's in linux-next now?

Here it is.

---------------------------------------
Hibernation/thaw fixes/improvements:

1. Do not allocate memory for buffers from emergency pools, unless absolutely required. Do not warn about and do not retry non-essential failed allocations.

2. Do not check the amount of free pages left on every single page write, but wait until one map is completely populated and then check.

3. Set maximum number of pages for read buffering consistently, instead of inadvertently depending on the size of the sector type.

4. Fix copyright line, which I missed when I submitted the hibernation threading patch.

5. Dispense with bit shifting arithmetic to improve readability.

6. Really recalculate the number of pages required to be free after all allocations have been done.

7. Fix calculation of pages required for read buffering. Only count in pages that do not belong to high memory.

Signed-off-by Bojan Smojver <[email protected]>

---
 kernel/power/swap.c |   62 ++++++++++++++++++++++++++++++++-------------------
 1 files changed, 39 insertions(+), 23 deletions(-)

kernel/power/swap.c +39 ~~-23~~

index eef311a..11e22c0 100644
@@ -6,7 +6,7 @@
  *
  * Copyright (C) 1998,2001-2005 Pavel Machek <[email protected]>
  * Copyright (C) 2006 Rafael J. Wysocki <[email protected]>
- * Copyright (C) 2010 Bojan Smojver <[email protected]>
+ * Copyright (C) 2010-2012 Bojan Smojver <[email protected]>
  *
  * This file is released under the GPLv2.
  *

@@ -282,14 +282,17 @@ static int write_page(void *buf, sector_t offset, struct bio **bio_chain)
 		return -ENOSPC;
 
 	if (bio_chain) {
-		src = (void *)__get_free_page(__GFP_WAIT | __GFP_HIGH);
+		src = (void *)__get_free_page(__GFP_WAIT | __GFP_NOWARN |
+		                              __GFP_NORETRY);
 		if (src) {
 			copy_page(src, buf);
 		} else {
 			ret = hib_wait_on_bio_chain(bio_chain); /* Free pages */
 			if (ret)
 				return ret;
-			src = (void *)__get_free_page(__GFP_WAIT | __GFP_HIGH);
+			src = (void *)__get_free_page(__GFP_WAIT |
+			                              __GFP_NOWARN |
+			                              __GFP_NORETRY);
 			if (src) {
 				copy_page(src, buf);
 			} else {

@@ -367,12 +370,17 @@ static int swap_write_page(struct swap_map_handle *handle, void *buf,
 		clear_page(handle->cur);
 		handle->cur_swap = offset;
 		handle->k = 0;
-	}
-	if (bio_chain && low_free_pages() <= handle->reqd_free_pages) {
-		error = hib_wait_on_bio_chain(bio_chain);
-		if (error)
-			goto out;
-		handle->reqd_free_pages = reqd_free_pages();
+
+		if (bio_chain && low_free_pages() <= handle->reqd_free_pages) {
+			error = hib_wait_on_bio_chain(bio_chain);
+			if (error)
+				goto out;
+			/*
+			 * Recalculate the number of required free pages, to
+			 * make sure we never take more than half.
+			 */
+			handle->reqd_free_pages = reqd_free_pages();
+		}
 	}
  out:
 	return error;

@@ -419,8 +427,9 @@ static int swap_writer_finish(struct swap_map_handle *handle,
 /* Maximum number of threads for compression/decompression. */
 #define LZO_THREADS	3
 
-/* Maximum number of pages for read buffering. */
-#define LZO_READ_PAGES	(MAP_PAGE_ENTRIES * 8)
+/* Minimum/maximum number of pages for read buffering. */
+#define LZO_MIN_RD_PAGES	1024
+#define LZO_MAX_RD_PAGES	8192
 
 
 /**

@@ -631,12 +640,6 @@ static int save_image_lzo(struct swap_map_handle *handle,
 	}
 
 	/*
-	 * Adjust number of free pages after all allocations have been done.
-	 * We don't want to run out of pages when writing.
-	 */
-	handle->reqd_free_pages = reqd_free_pages();
-
-	/*
 	 * Start the CRC32 thread.
 	 */
 	init_waitqueue_head(&crc->go);

@@ -657,6 +660,12 @@ static int save_image_lzo(struct swap_map_handle *handle,
 		goto out_clean;
 	}
 
+	/*
+	 * Adjust the number of required free pages after all allocations have
+	 * been done. We don't want to run out of pages when writing.
+	 */
+	handle->reqd_free_pages = reqd_free_pages();
+
 	printk(KERN_INFO
 		"PM: Using %u thread(s) for compression.\n"
 		"PM: Compressing and saving image data (%u pages) ...     ",

@@ -1067,7 +1076,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
 	unsigned i, thr, run_threads, nr_threads;
 	unsigned ring = 0, pg = 0, ring_size = 0,
 	         have = 0, want, need, asked = 0;
-	unsigned long read_pages;
+	unsigned long read_pages = 0;
 	unsigned char **page = NULL;
 	struct dec_data *data = NULL;
 	struct crc_data *crc = NULL;

@@ -1079,7 +1088,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
 	nr_threads = num_online_cpus() - 1;
 	nr_threads = clamp_val(nr_threads, 1, LZO_THREADS);
 
-	page = vmalloc(sizeof(*page) * LZO_READ_PAGES);
+	page = vmalloc(sizeof(*page) * LZO_MAX_RD_PAGES);
 	if (!page) {
 		printk(KERN_ERR "PM: Failed to allocate LZO page\n");
 		ret = -ENOMEM;

@@ -1144,15 +1153,22 @@ static int load_image_lzo(struct swap_map_handle *handle,
 	}
 
 	/*
-	 * Adjust number of pages for read buffering, in case we are short.
+	 * Set the number of pages for read buffering.
+	 * This is complete guesswork, because we'll only know the real
+	 * picture once prepare_image() is called, which is much later on
+	 * during the image load phase. We'll assume the worst case and
+	 * say that none of the image pages are from high memory.
 	 */
-	read_pages = (nr_free_pages() - snapshot_get_image_size()) >> 1;
-	read_pages = clamp_val(read_pages, LZO_CMP_PAGES, LZO_READ_PAGES);
+	if (low_free_pages() > snapshot_get_image_size())
+		read_pages = (low_free_pages() - snapshot_get_image_size()) / 2;
+	read_pages = clamp_val(read_pages, LZO_MIN_RD_PAGES, LZO_MAX_RD_PAGES);
 
 	for (i = 0; i < read_pages; i++) {
 		page[i] = (void *)__get_free_page(i < LZO_CMP_PAGES ?
 		                                  __GFP_WAIT | __GFP_HIGH :
-		                                  __GFP_WAIT);
+		                                  __GFP_WAIT | __GFP_NOWARN |
+		                                  __GFP_NORETRY);
+
 		if (!page[i]) {
 			if (i < LZO_CMP_PAGES) {
 				ring_size = i;

---------------------------------------

Signature · 1 line

Bojan

Bojan Smojver

author

2012-04-27 00:26 UTC #30 lore raw

Quoted 1 line · Bojan Smojver

Here it is.

So, a quick summary:

These patches should go to linux-pm/linux-next (in that order):

http://marc.info/?l=linux-kernel&m=133522096505280&w=2
http://marc.info/?l=linux-kernel&m=133548592706940&w=2

Only this patch should to to -stable:

http://marc.info/?l=linux-kernel&m=133522096505280&w=2

Signature · 1 line

Bojan

Rafael J. Wysocki

2012-04-29 21:04 UTC #31 lore raw

Quoted 9 lines · Bojan Smojver

Quoted 1 line · Bojan Smojver
Here it is.
So, a quick summary:
These patches should go to linux-pm/linux-next (in that order):
http://marc.info/?l=linux-kernel&m=133522096505280&w=2
http://marc.info/?l=linux-kernel&m=133548592706940&w=2

Both are in my linux-next branch.

Quoted 3 lines

Only this patch should to to -stable:
http://marc.info/?l=linux-kernel&m=133522096505280&w=2

And this one is in my fixes branch in addition and will be pushed to Linus shortly.

Thanks,
Rafael

Bojan Smojver

author

2012-04-30 01:43 UTC #32 lore raw

Quoted 5 lines · Rafael J. Wysocki

Quoted 4 lines
These patches should go to linux-pm/linux-next (in that order):
http://marc.info/?l=linux-kernel&m=133522096505280&w=2
http://marc.info/?l=linux-kernel&m=133548592706940&w=2

Quoted 8 lines

Both are in my linux-next branch.
Quoted 3 lines
Only this patch should to to -stable:
http://marc.info/?l=linux-kernel&m=133522096505280&w=2
And this one is in my fixes branch in addition and will be pushed to Linus shortly.

Excellent.

Signature · 1 line

Bojan

Rafael J. Wysocki

Applied

2012-04-29 21:03 UTC #33 lore raw

In reply to Bojan Smojver

Quoted 23 lines · Bojan Smojver

Quoted 1 line · Rafael J. Wysocki
Can you just create a second patch on top of what's in linux-next now?
Here it is.
---------------------------------------
Hibernation/thaw fixes/improvements:
1. Do not allocate memory for buffers from emergency pools, unless absolutely required. Do not warn about and do not retry non-essential failed allocations.
2. Do not check the amount of free pages left on every single page write, but wait until one map is completely populated and then check.
3. Set maximum number of pages for read buffering consistently, instead of inadvertently depending on the size of the sector type.
4. Fix copyright line, which I missed when I submitted the hibernation threading patch.
5. Dispense with bit shifting arithmetic to improve readability.

Quoted 7 lines

6. Really recalculate the number of pages required to be free after all allocations have been done.
7. Fix calculation of pages required for read buffering. Only count in pages that do not belong to high memory.
Signed-off-by Bojan Smojver <[email protected]>

Applied to linux-pm/linux-next.

Thanks,
Rafael

Quoted 147 lines

---
 kernel/power/swap.c |   62 ++++++++++++++++++++++++++++++++-------------------
 1 files changed, 39 insertions(+), 23 deletions(-)

kernel/power/swap.c +39 ~~-24~~

index eef311a..11e22c0 100644
@@ -6,7 +6,7 @@
  *
  * Copyright (C) 1998,2001-2005 Pavel Machek <[email protected]>
  * Copyright (C) 2006 Rafael J. Wysocki <[email protected]>
- * Copyright (C) 2010 Bojan Smojver <[email protected]>
+ * Copyright (C) 2010-2012 Bojan Smojver <[email protected]>
  *
  * This file is released under the GPLv2.
  *

@@ -282,14 +282,17 @@ static int write_page(void *buf, sector_t offset, struct bio **bio_chain)
 		return -ENOSPC;
 
 	if (bio_chain) {
-		src = (void *)__get_free_page(__GFP_WAIT | __GFP_HIGH);
+		src = (void *)__get_free_page(__GFP_WAIT | __GFP_NOWARN |
+		                              __GFP_NORETRY);
 		if (src) {
 			copy_page(src, buf);
 		} else {
 			ret = hib_wait_on_bio_chain(bio_chain); /* Free pages */
 			if (ret)
 				return ret;
-			src = (void *)__get_free_page(__GFP_WAIT | __GFP_HIGH);
+			src = (void *)__get_free_page(__GFP_WAIT |
+			                              __GFP_NOWARN |
+			                              __GFP_NORETRY);
 			if (src) {
 				copy_page(src, buf);
 			} else {

@@ -367,12 +370,17 @@ static int swap_write_page(struct swap_map_handle *handle, void *buf,
 		clear_page(handle->cur);
 		handle->cur_swap = offset;
 		handle->k = 0;
-	}
-	if (bio_chain && low_free_pages() <= handle->reqd_free_pages) {
-		error = hib_wait_on_bio_chain(bio_chain);
-		if (error)
-			goto out;
-		handle->reqd_free_pages = reqd_free_pages();
+
+		if (bio_chain && low_free_pages() <= handle->reqd_free_pages) {
+			error = hib_wait_on_bio_chain(bio_chain);
+			if (error)
+				goto out;
+			/*
+			 * Recalculate the number of required free pages, to
+			 * make sure we never take more than half.
+			 */
+			handle->reqd_free_pages = reqd_free_pages();
+		}
 	}
  out:
 	return error;

@@ -419,8 +427,9 @@ static int swap_writer_finish(struct swap_map_handle *handle,
 /* Maximum number of threads for compression/decompression. */
 #define LZO_THREADS	3
 
-/* Maximum number of pages for read buffering. */
-#define LZO_READ_PAGES	(MAP_PAGE_ENTRIES * 8)
+/* Minimum/maximum number of pages for read buffering. */
+#define LZO_MIN_RD_PAGES	1024
+#define LZO_MAX_RD_PAGES	8192
 
 
 /**

@@ -631,12 +640,6 @@ static int save_image_lzo(struct swap_map_handle *handle,
 	}
 
 	/*
-	 * Adjust number of free pages after all allocations have been done.
-	 * We don't want to run out of pages when writing.
-	 */
-	handle->reqd_free_pages = reqd_free_pages();
-
-	/*
 	 * Start the CRC32 thread.
 	 */
 	init_waitqueue_head(&crc->go);

@@ -657,6 +660,12 @@ static int save_image_lzo(struct swap_map_handle *handle,
 		goto out_clean;
 	}
 
+	/*
+	 * Adjust the number of required free pages after all allocations have
+	 * been done. We don't want to run out of pages when writing.
+	 */
+	handle->reqd_free_pages = reqd_free_pages();
+
 	printk(KERN_INFO
 		"PM: Using %u thread(s) for compression.\n"
 		"PM: Compressing and saving image data (%u pages) ...     ",

@@ -1067,7 +1076,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
 	unsigned i, thr, run_threads, nr_threads;
 	unsigned ring = 0, pg = 0, ring_size = 0,
 	         have = 0, want, need, asked = 0;
-	unsigned long read_pages;
+	unsigned long read_pages = 0;
 	unsigned char **page = NULL;
 	struct dec_data *data = NULL;
 	struct crc_data *crc = NULL;

@@ -1079,7 +1088,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
 	nr_threads = num_online_cpus() - 1;
 	nr_threads = clamp_val(nr_threads, 1, LZO_THREADS);
 
-	page = vmalloc(sizeof(*page) * LZO_READ_PAGES);
+	page = vmalloc(sizeof(*page) * LZO_MAX_RD_PAGES);
 	if (!page) {
 		printk(KERN_ERR "PM: Failed to allocate LZO page\n");
 		ret = -ENOMEM;

@@ -1144,15 +1153,22 @@ static int load_image_lzo(struct swap_map_handle *handle,
 	}
 
 	/*
-	 * Adjust number of pages for read buffering, in case we are short.
+	 * Set the number of pages for read buffering.
+	 * This is complete guesswork, because we'll only know the real
+	 * picture once prepare_image() is called, which is much later on
+	 * during the image load phase. We'll assume the worst case and
+	 * say that none of the image pages are from high memory.
 	 */
-	read_pages = (nr_free_pages() - snapshot_get_image_size()) >> 1;
-	read_pages = clamp_val(read_pages, LZO_CMP_PAGES, LZO_READ_PAGES);
+	if (low_free_pages() > snapshot_get_image_size())
+		read_pages = (low_free_pages() - snapshot_get_image_size()) / 2;
+	read_pages = clamp_val(read_pages, LZO_MIN_RD_PAGES, LZO_MAX_RD_PAGES);
 
 	for (i = 0; i < read_pages; i++) {
 		page[i] = (void *)__get_free_page(i < LZO_CMP_PAGES ?
 		                                  __GFP_WAIT | __GFP_HIGH :
-		                                  __GFP_WAIT);
+		                                  __GFP_WAIT | __GFP_NOWARN |
+		                                  __GFP_NORETRY);
+
 		if (!page[i]) {
 			if (i < LZO_CMP_PAGES) {
 				ring_size = i;
---------------------------------------