From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754218Ab2D2VDS (ORCPT ); Sun, 29 Apr 2012 17:03:18 -0400 Received: from ogre.sisk.pl ([193.178.161.156]:44515 "EHLO ogre.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752784Ab2D2VDQ (ORCPT ); Sun, 29 Apr 2012 17:03:16 -0400 From: "Rafael J. Wysocki" To: Bojan Smojver Subject: Re: [PATCH v11]: Hibernation: fix the number of pages used for hibernate/thaw buffering Date: Sun, 29 Apr 2012 23:07:51 +0200 User-Agent: KMail/1.13.6 (Linux/3.4.0-rc4+; KDE/4.6.0; x86_64; ; ) Cc: Per Olofsson , linux-kernel@vger.kernel.org, Linux PM list References: <1334267969.2573.14.camel@shrek.rexursive.com> <201204270011.20430.rjw@sisk.pl> <1335485772.2136.21.camel@shrek.rexursive.com> In-Reply-To: <1335485772.2136.21.camel@shrek.rexursive.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201204292307.52040.rjw@sisk.pl> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Friday, April 27, 2012, Bojan Smojver wrote: > On Fri, 2012-04-27 at 00:11 +0200, Rafael J. Wysocki wrote: > > 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 Applied to linux-pm/linux-next. Thanks, Rafael > --- > kernel/power/swap.c | 62 ++++++++++++++++++++++++++++++++------------------- > 1 files changed, 39 insertions(+), 23 deletions(-) > > diff --git a/kernel/power/swap.c b/kernel/power/swap.c > index eef311a..11e22c0 100644 > --- a/kernel/power/swap.c > +++ b/kernel/power/swap.c > @@ -6,7 +6,7 @@ > * > * Copyright (C) 1998,2001-2005 Pavel Machek > * Copyright (C) 2006 Rafael J. Wysocki > - * Copyright (C) 2010 Bojan Smojver > + * Copyright (C) 2010-2012 Bojan Smojver > * > * 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; > --------------------------------------- > >