* [PATCH v7]: Hibernation: lower/better control the amount of pages used for buffering
@ 2012-03-27 23:41 Bojan Smojver
2012-03-28 21:29 ` Rafael J. Wysocki
0 siblings, 1 reply; 6+ messages in thread
From: Bojan Smojver @ 2012-03-27 23:41 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-kernel
Hi Rafael,
And finally with an inline function.
---------------------------------------
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.
Signed-off-by: Bojan Smojver <bojan@rexursive.com>
---
kernel/power/swap.c | 35 ++++++++++++++++++++++++-----------
1 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 8742fd0..0e94fc6 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -6,7 +6,7 @@
*
* Copyright (C) 1998,2001-2005 Pavel Machek <pavel@ucw.cz>
* Copyright (C) 2006 Rafael J. Wysocki <rjw@sisk.pl>
- * Copyright (C) 2010 Bojan Smojver <bojan@rexursive.com>
+ * Copyright (C) 2010-2012 Bojan Smojver <bojan@rexursive.com>
*
* This file is released under the GPLv2.
*
@@ -51,6 +51,15 @@
#define MAP_PAGE_ENTRIES (PAGE_SIZE / sizeof(sector_t) - 1)
+/*
+ * Number of pages required to be kept free while writing the image. Always
+ * three quarters of all available pages before the writing starts.
+ */
+static inline unsigned long reqd_free_pages(void)
+{
+ return (nr_free_pages() / 4) * 3;
+}
+
struct swap_map_page {
sector_t entries[MAP_PAGE_ENTRIES];
sector_t next_swap;
@@ -72,7 +81,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 +325,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 +360,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 = reqd_free_pages();
}
out:
return error;
@@ -404,7 +416,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 +627,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 = reqd_free_pages();
/*
* Start the CRC32 thread.
@@ -1129,8 +1141,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()) / 4;
read_pages = clamp_val(read_pages, LZO_CMP_PAGES, LZO_READ_PAGES);
for (i = 0; i < read_pages; i++) {
---------------------------------------
--
Bojan
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v7]: Hibernation: lower/better control the amount of pages used for buffering
2012-03-27 23:41 [PATCH v7]: Hibernation: lower/better control the amount of pages used for buffering Bojan Smojver
@ 2012-03-28 21:29 ` Rafael J. Wysocki
2012-03-29 3:38 ` Bojan Smojver
2012-03-30 1:43 ` Bojan Smojver
0 siblings, 2 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2012-03-28 21:29 UTC (permalink / raw)
To: Bojan Smojver; +Cc: linux-kernel, Linux PM mailing list
On Wednesday, March 28, 2012, Bojan Smojver wrote:
> Hi Rafael,
>
> And finally with an inline function.
OK, I'll take this one for v3.5. Thanks a lot for being persistent!
Rafael
> ---------------------------------------
> 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.
>
> Signed-off-by: Bojan Smojver <bojan@rexursive.com>
> ---
> kernel/power/swap.c | 35 ++++++++++++++++++++++++-----------
> 1 files changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index 8742fd0..0e94fc6 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -6,7 +6,7 @@
> *
> * Copyright (C) 1998,2001-2005 Pavel Machek <pavel@ucw.cz>
> * Copyright (C) 2006 Rafael J. Wysocki <rjw@sisk.pl>
> - * Copyright (C) 2010 Bojan Smojver <bojan@rexursive.com>
> + * Copyright (C) 2010-2012 Bojan Smojver <bojan@rexursive.com>
> *
> * This file is released under the GPLv2.
> *
> @@ -51,6 +51,15 @@
>
> #define MAP_PAGE_ENTRIES (PAGE_SIZE / sizeof(sector_t) - 1)
>
> +/*
> + * Number of pages required to be kept free while writing the image. Always
> + * three quarters of all available pages before the writing starts.
> + */
> +static inline unsigned long reqd_free_pages(void)
> +{
> + return (nr_free_pages() / 4) * 3;
> +}
> +
> struct swap_map_page {
> sector_t entries[MAP_PAGE_ENTRIES];
> sector_t next_swap;
> @@ -72,7 +81,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 +325,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 +360,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 = reqd_free_pages();
> }
> out:
> return error;
> @@ -404,7 +416,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 +627,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 = reqd_free_pages();
>
> /*
> * Start the CRC32 thread.
> @@ -1129,8 +1141,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()) / 4;
> read_pages = clamp_val(read_pages, LZO_CMP_PAGES, LZO_READ_PAGES);
>
> for (i = 0; i < read_pages; i++) {
> ---------------------------------------
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v7]: Hibernation: lower/better control the amount of pages used for buffering
2012-03-28 21:29 ` Rafael J. Wysocki
@ 2012-03-29 3:38 ` Bojan Smojver
2012-03-30 1:43 ` Bojan Smojver
1 sibling, 0 replies; 6+ messages in thread
From: Bojan Smojver @ 2012-03-29 3:38 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-kernel, Linux PM mailing list
On Wed, 2012-03-28 at 23:29 +0200, Rafael J. Wysocki wrote:
> OK, I'll take this one for v3.5.
Thanks.
Although, we may need this queued for stable as well. Folks that have
hit page allocation problems during hibernation will test and I will
then let you know.
> Thanks a lot for being persistent!
No trouble at all.
--
Bojan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7]: Hibernation: lower/better control the amount of pages used for buffering
2012-03-28 21:29 ` Rafael J. Wysocki
2012-03-29 3:38 ` Bojan Smojver
@ 2012-03-30 1:43 ` Bojan Smojver
2012-03-30 19:37 ` Rafael J. Wysocki
1 sibling, 1 reply; 6+ messages in thread
From: Bojan Smojver @ 2012-03-30 1:43 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-kernel, Linux PM mailing list
On Wed, 2012-03-28 at 23:29 +0200, Rafael J. Wysocki wrote:
> OK, I'll take this one for v3.5.
BTW, I thought that 3.4 merge window was still open. No?
--
Bojan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7]: Hibernation: lower/better control the amount of pages used for buffering
2012-03-30 1:43 ` Bojan Smojver
@ 2012-03-30 19:37 ` Rafael J. Wysocki
2012-03-30 20:45 ` Bojan Smojver
0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2012-03-30 19:37 UTC (permalink / raw)
To: Bojan Smojver; +Cc: linux-kernel, Linux PM mailing list
On Friday, March 30, 2012, Bojan Smojver wrote:
> On Wed, 2012-03-28 at 23:29 +0200, Rafael J. Wysocki wrote:
> > OK, I'll take this one for v3.5.
>
> BTW, I thought that 3.4 merge window was still open. No?
It is, but I have prepared my material for merging already and the patch
was too late. Sorry about that.
Still, if it is a regression fix, I can push it even after -rc1 is out,
so no worries in that case.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7]: Hibernation: lower/better control the amount of pages used for buffering
2012-03-30 19:37 ` Rafael J. Wysocki
@ 2012-03-30 20:45 ` Bojan Smojver
0 siblings, 0 replies; 6+ messages in thread
From: Bojan Smojver @ 2012-03-30 20:45 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-kernel, Linux PM mailing list
On Fri, 2012-03-30 at 21:37 +0200, Rafael J. Wysocki wrote:
> Still, if it is a regression fix
That is my suspicion, yes. I am waiting on feedback about the patch,
related to a bug report where hibernation hangs with 3.2 (but not
earlier). The cause of this bug seems to be kernel running out of pages,
because hibernation code took a smidgen too much.
--
Bojan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-03-30 20:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-27 23:41 [PATCH v7]: Hibernation: lower/better control the amount of pages used for buffering Bojan Smojver
2012-03-28 21:29 ` Rafael J. Wysocki
2012-03-29 3:38 ` Bojan Smojver
2012-03-30 1:43 ` Bojan Smojver
2012-03-30 19:37 ` Rafael J. Wysocki
2012-03-30 20:45 ` Bojan Smojver
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox