Skip to contentSkip to conversation

: Hibernation: lower/better control the amount of pages used for buffering

PATCH Superseded Bojan Smojver 2 participants 2 messages Mar 23, 2012 – Mar 27, 2012

Subsystems (2)

v4 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

PATCH v4 1/1 : Hibernation: lower/better control the amount of pages used for buffering

Hi Rafael,

One more version. This time we require that 3/4 of pages be available when writing the image, otherwise we flush. Just erring on the side of caution. Otherwise the same as v3.

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

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

2. Use at most one quarter of free pages for read buffering.

3. Require that number of free pages when writing the image never falls below three quarters of total free pages available.

---
 kernel/power/swap.c |   26 +++++++++++++++-----------
 1 files changed, 15 insertions(+), 11 deletions(-)
kernel/power/swap.c +15 -11
index 8742fd0..b06fed3 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.
  *
@@ -72,7 +72,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 +316,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 = (nr_free_pages() >> 2) * 3;
 	handle->first_sector = handle->cur_swap;
 	return 0;
 err_rel:
@@ -352,11 +351,15 @@ 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 && nr_free_pages() < handle->reqd_free_pages) {
 		error = hib_wait_on_bio_chain(bio_chain);
 		if (error)
 			goto out;
-		handle->written = 0;
+		/*
+		 * Recalculate the number of required free pages, to make sure
+		 * we never take more than a quarter.
+		 */
+		handle->reqd_free_pages = (nr_free_pages() >> 2) * 3;
 	}
  out:
 	return error;
@@ -404,7 +407,7 @@ static int swap_writer_finish(struct swap_map_handle *handle,
 #define LZO_THREADS	3
 
 /* Maximum number of pages for read buffering. */
-#define LZO_READ_PAGES	(MAP_PAGE_ENTRIES * 8)
+#define LZO_READ_PAGES	8192
 
 
 /**
@@ -615,10 +618,10 @@ 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.
+	 * 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->nr_free_pages = nr_free_pages() >> 1;
+	handle->reqd_free_pages = (nr_free_pages() >> 2) * 3;
 
 	/*
 	 * Start the CRC32 thread.
@@ -1129,8 +1132,9 @@ static int load_image_lzo(struct swap_map_handle *handle,
 
 	/*
 	 * Adjust number of pages for read buffering, in case we are short.
+	 * Never take more than a quarter of all available pages.
 	 */
-	read_pages = (nr_free_pages() - snapshot_get_image_size()) >> 1;
+	read_pages = (nr_free_pages() - snapshot_get_image_size()) >> 2;
 	read_pages = clamp_val(read_pages, LZO_CMP_PAGES, LZO_READ_PAGES);
 
 	for (i = 0; i < read_pages; i++) {

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

Signature · 1 line
Bojan
#1 lore raw
Quoted 45 lines · Bojan Smojver

Hi Rafael,

One more version. This time we require that 3/4 of pages be available when writing the image, otherwise we flush. Just erring on the side of caution. Otherwise the same as v3.

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

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

2. Use at most one quarter of free pages for read buffering.

3. Require that number of free pages when writing the image never falls below three quarters of total free pages available.

---
 kernel/power/swap.c |   26 +++++++++++++++-----------
 1 files changed, 15 insertions(+), 11 deletions(-)
kernel/power/swap.c +2 -2
index 8742fd0..b06fed3 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.
  *
@@ -72,7 +72,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;
 };
 

kernel/power/swap.c static int get_swap_writer(struct swap_map_handle *handle) · line 319

Quoted 6 lines
kernel/power/swap.c +1 -2
@@ -316,8 +316,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 = (nr_free_pages() >> 2) * 3;

This computation is done in three different places in the same way. Perhaps create a macro or a static inline function for that?

Also, I'm not sure if the microoptimization here actually helps a lot. The compiler should be smart enough to optimize that properly. So, I'd just write (nr_free_pages() / 4) * 3 whose meaning is much more obvious. :-)

Apart from this, looks good.

Thanks,
Rafael

Quoted 58 lines
kernel/power/swap.c +12 -8
 	handle->first_sector = handle->cur_swap;
 	return 0;
 err_rel:
@@ -352,11 +351,15 @@ 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 && nr_free_pages() < handle->reqd_free_pages) {
 		error = hib_wait_on_bio_chain(bio_chain);
 		if (error)
 			goto out;
-		handle->written = 0;
+		/*
+		 * Recalculate the number of required free pages, to make sure
+		 * we never take more than a quarter.
+		 */
+		handle->reqd_free_pages = (nr_free_pages() >> 2) * 3;
 	}
  out:
 	return error;
@@ -404,7 +407,7 @@ static int swap_writer_finish(struct swap_map_handle *handle,
 #define LZO_THREADS	3
 
 /* Maximum number of pages for read buffering. */
-#define LZO_READ_PAGES	(MAP_PAGE_ENTRIES * 8)
+#define LZO_READ_PAGES	8192
 
 
 /**
@@ -615,10 +618,10 @@ 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.
+	 * 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->nr_free_pages = nr_free_pages() >> 1;
+	handle->reqd_free_pages = (nr_free_pages() >> 2) * 3;
 
 	/*
 	 * Start the CRC32 thread.
@@ -1129,8 +1132,9 @@ static int load_image_lzo(struct swap_map_handle *handle,
 
 	/*
 	 * Adjust number of pages for read buffering, in case we are short.
+	 * Never take more than a quarter of all available pages.
 	 */
-	read_pages = (nr_free_pages() - snapshot_get_image_size()) >> 1;
+	read_pages = (nr_free_pages() - snapshot_get_image_size()) >> 2;
 	read_pages = clamp_val(read_pages, LZO_CMP_PAGES, LZO_READ_PAGES);
 
 	for (i = 0; i < read_pages; i++) {
---------------------------------------