From: Nigel Cunningham on
Hi.

On 04/08/10 12:02, KAMEZAWA Hiroyuki wrote:
> On Wed, 04 Aug 2010 11:50:41 +1000
> Nigel Cunningham<nigel(a)tuxonice.net> wrote:
>
>> Hi.
>>
>> Sorry for the delay in replying.
>>
>> On 03/08/10 11:59, Bojan Smojver wrote:
>>>> From all this, I only got "In swsusp_free()" printed on resume. So, it
>>> seems that save_image() does indeed free those vmalloc()-ed buffers and
>>> they are not saved in the image.
>>>
>>> I even put this in hibernate.c:
>>> ---------------------
>>> /* Restore control flow magically appears here */
>>> restore_processor_state();
>>> if (!in_suspend)
>>> platform_leave(platform_mode);
>>>
>>> printk(KERN_ERR "Resumed, checking swsusp_lzo_buffers.\n");
>>> if (swsusp_lzo_buffers) {
>>> printk (KERN_ERR "Setting vmalloc() buffers.\n");
>>> memset(swsusp_lzo_buffers, 0, 80 * PAGE_SIZE);
>>> }
>>> ---------------------
>>>
>>> This printed just "Resumed, checking swsusp_lzo_buffers.", meaning it
>>> was already set to NULL.
>>>
>>> Any further comments on this? Nigel, what do you reckon?
>>
>> I don't see what all the fuss was about. save_image is called after the
>> snapshot is made (hibernate called hibernation_snapshot to create the
>> image, then swsusp_write which in turn calls save_image), so there's no
>> possibility of the memory allocated by it being included in the image or
>> of a memory leak ocuring as a result.
>>
>
> I'm sorry if I'm wrong. Here is logic in my mind. (I'm reading hibernate(void).
> not sure about user.c )
> (linenumber is from 2.6.34 global, plz ignore)

Yeah, I'm ignoring uswsusp too. That shouldn't matter.

> 624 error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
> 625 if (error)
> 626 goto Thaw;
> 627
> 628 if (in_suspend) {
> 629 unsigned int flags = 0;
> 630
> 631 if (hibernation_mode == HIBERNATION_PLATFORM)
> 632 flags |= SF_PLATFORM_MODE;
> 633 pr_debug("PM: writing image.\n");
> 634 error = swsusp_write(flags);
>
> Then, swsusp_write() actually writes image to the disk. Right ?

Yes

> It finally calls save_image(). save_image() executes following loop.
>
> 433 while (1) {
> 434 ret = snapshot_read_next(snapshot, PAGE_SIZE);
> 435 if (ret<= 0)
> 436 break;
> 437 ret = swap_write_page(handle, data_of(*snapshot),&bio);
> 438 if (ret)
> 439 break;
> 440 if (!(nr_pages % m))
> 441 printk(KERN_CONT "\b\b\b\b%3d%%", nr_pages / m);
> 442 nr_pages++;
> 443 }
>
> Ok, here, the image written to disk is got by snapshot_read_next().
> Then, what it does ?
>
> ==
> 1650 page = pfn_to_page(memory_bm_next_pfn(&copy_bm));
> 1651 if (PageHighMem(page)) {
> 1652 /* Highmem pages are copied to the buffer,
> 1653 * because we can't return with a kmapped
> 1654 * highmem page (we may not be called again).
> 1655 */
> 1656 void *kaddr;
> 1657
> 1658 kaddr = kmap_atomic(page, KM_USER0);
> 1659 memcpy(buffer, kaddr, PAGE_SIZE);
> 1660 kunmap_atomic(kaddr, KM_USER0);
> 1661 handle->buffer = buffer;
> 1662 } else {
> 1663 handle->buffer = page_address(page);
> 1664 }
> ==
>
> Ok, what actually written to disk is an image of memory at save_image() is
> called.
>
> What happens at vmalloc ?
> 1. page tabels for vmalloc() is set up.
> 2. vm_struct object is allocated.
> 3. vmap is allcoated.
>
> Above "3" states are all saved into the disk image, as "used".
>
> Then, after resume, all vmalloc() area is resumed as "allocated".
>
> Wrong ?

Yes, because what's being written is the snapshot that was created in
hibernation_snapshot. Any memory you allocate afterwards is irrelevant
because it's not part of that snapshot that was made earlier and is now
being written to disk. Think of the point where hibernation_snapshot is
called as being like taking a photo, and this later part as like
printing the photo. Once you've taken the photo, people in the photo can
move around without making any difference to the photo you've taken. So
here. Our vmallocs and so on after the snapshot don't affect it.

Regards,

Nigel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Nigel Cunningham on
Hi.

On 04/08/10 12:24, KAMEZAWA Hiroyuki wrote:
> On Wed, 04 Aug 2010 12:24:57 +1000
> Nigel Cunningham<nigel(a)tuxonice.net> wrote:
>
>> Yes, because what's being written is the snapshot that was created in
>> hibernation_snapshot. Any memory you allocate afterwards is irrelevant
>> because it's not part of that snapshot that was made earlier and is now
>> being written to disk. Think of the point where hibernation_snapshot is
>> called as being like taking a photo, and this later part as like
>> printing the photo. Once you've taken the photo, people in the photo can
>> move around without making any difference to the photo you've taken. So
>> here. Our vmallocs and so on after the snapshot don't affect it.
>>
>
> I see. I misunderstood swsusp_save().
> Sorry for tons of noises.

No problem at all! You learn how it works and I get to make sure I do
understand how it works! :)

Nigel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Nigel Cunningham on
Hi again.

On 03/08/10 12:30, Bojan Smojver wrote:
> On Tue, 2010-08-03 at 11:59 +1000, Bojan Smojver wrote:
>> PS. I also enhanced the patch to use overlapping compression in order
>> to save memory. Looks like that's causing it to be slower on
>> compression (we go down from 130 - 150 MB/s to around 105 - 110 MB/s),
>> but still over 3 times faster than regular swsusp code. Decompression
>> remains roughly the same around 100+ MB/s (this is double the speed of
>> current swsusp code). I will post this a bit later on.
>
> As promised.

Round here, people generally like to see improvements split up into
small patches that change just one thing. I'd therefore suggest
splitting the removal of sync_read from the rest of the patch. Is it
standalone? I'm not seeing the relationship between the two parts at the
moment.

> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index ca6066a..cb57eb9 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -137,6 +137,8 @@ config SUSPEND_FREEZER
> config HIBERNATION
> bool "Hibernation (aka 'suspend to disk')"
> depends on PM&& SWAP&& ARCH_HIBERNATION_POSSIBLE
> + select LZO_COMPRESS
> + select LZO_DECOMPRESS
> select SUSPEND_NVS if HAS_IOMEM
> ---help---
> Enable the suspend to disk (STD) functionality, which is usually
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 006270f..a760cf8 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -103,10 +103,6 @@ struct snapshot_handle {
> void *buffer; /* address of the block to read from
> * or write to
> */
> - int sync_read; /* Set to one to notify the caller of
> - * snapshot_write_next() that it may
> - * need to call wait_on_bio_chain()
> - */
> };
>
> /* This macro returns the address from/to which the caller of
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index 25ce010..f24ee24 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -2135,8 +2135,6 @@ int snapshot_write_next(struct snapshot_handle *handle)
> if (handle->cur> 1&& handle->cur> nr_meta_pages + nr_copy_pages)
> return 0;
>
> - handle->sync_read = 1;
> -
> if (!handle->cur) {
> if (!buffer)
> /* This makes the buffer be freed by swsusp_free() */
> @@ -2169,7 +2167,6 @@ int snapshot_write_next(struct snapshot_handle *handle)
> memory_bm_position_reset(&orig_bm);
> restore_pblist = NULL;
> handle->buffer = get_buffer(&orig_bm,&ca);
> - handle->sync_read = 0;
> if (IS_ERR(handle->buffer))
> return PTR_ERR(handle->buffer);
> }
> @@ -2178,8 +2175,6 @@ int snapshot_write_next(struct snapshot_handle *handle)
> handle->buffer = get_buffer(&orig_bm,&ca);
> if (IS_ERR(handle->buffer))
> return PTR_ERR(handle->buffer);
> - if (handle->buffer != buffer)
> - handle->sync_read = 0;
> }
> handle->cur++;
> return PAGE_SIZE;
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index b0bb217..5a83fd3 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -24,6 +24,7 @@
> #include<linux/swapops.h>
> #include<linux/pm.h>
> #include<linux/slab.h>
> +#include<linux/lzo.h>
>
> #include "power.h"
>
> @@ -357,6 +358,18 @@ static int swap_writer_finish(struct swap_map_handle *handle,
> return error;
> }
>
> +#define LZO_HEADER sizeof(size_t)
> +#define LZO_UNC_PAGES 64
> +#define LZO_UNC_SIZE (LZO_UNC_PAGES * PAGE_SIZE)
> +#define LZO_CMP_PAGES DIV_ROUND_UP(lzo1x_worst_compress(LZO_UNC_SIZE) + \
> + LZO_HEADER, PAGE_SIZE)
> +#define LZO_CMP_SIZE (LZO_CMP_PAGES * PAGE_SIZE)
> +#define LZO_OVH_PAGES DIV_ROUND_UP((LZO_UNC_SIZE > 0xBFFF ? \
> + 0xBFFF : LZO_UNC_SIZE) + \
> + LZO_UNC_SIZE / 16 + 64 + 3 + \
> + LZO_HEADER, PAGE_SIZE)
> +#define LZO_OVH_SIZE (LZO_OVH_PAGES + PAGE_SIZE)
> +

Some commenting for these #defines would be good, especially the 'magic'
numbers 0xBFF, 16, 64 and 3. It's all a bit unreadable, too :) Maybe
something like (untested...):

#define LZO_HEADER sizeof(size_t)
#define LZO_UNC_PAGES 64
#define LZO_UNC_SIZE (LZO_UNC_PAGES * PAGE_SIZE)
#define LZO_CMP_PAGES \
DIV_ROUND_UP(lzo1x_worst_compress(LZO_UNC_SIZE) + LZO_HEADER,
PAGE_SIZE)
#define LZO_CMP_SIZE (LZO_CMP_PAGES * PAGE_SIZE)
#define LZO_MAGIC_1 0xBFFF /* Rename to whatever this is */
#define LZO_MAGIC_2 16 /* Rename */
#define LZO_MAGIC_3 (64 + 3) /* Rename */
#define LZO_OVH_PAGES
DIV_ROUND_UP(min(LZO_UNC_SIZE, LZO_MAGIC_1) + \
LZO_UNC_SIZE / LZO_MAGIC2 + \
LZO_MAGIC_3 + \
LZO_HEADER,
PAGE_SIZE)
#define LZO_OVH_SIZE (LZO_OVH_PAGES + PAGE_SIZE)


> /**
> * save_image - save the suspend image data
> */
> @@ -372,6 +385,29 @@ static int save_image(struct swap_map_handle *handle,
> struct bio *bio;
> struct timeval start;
> struct timeval stop;
> + size_t ul, cl;

rename to bytes_in and bytes_out? I can guess they mean
uncompressed/compressed length, but it's a little cryptic.

> + unsigned char *buf, *wrk, *page;
> +
> + page = (void *)__get_free_page(__GFP_WAIT | __GFP_HIGH);
> + if (!page) {
> + printk(KERN_ERR "PM: Failed to allocate LZO page\n");
> + return -ENOMEM;
> + }
> +
> + wrk = vmalloc(LZO1X_1_MEM_COMPRESS);
> + if (!wrk) {
> + printk(KERN_ERR "PM: Failed to allocate LZO workspace\n");
> + free_page((unsigned long)page);
> + return -ENOMEM;
> + }
> +
> + buf = vmalloc(LZO_UNC_SIZE + LZO_OVH_SIZE);
> + if (!buf) {
> + printk(KERN_ERR "PM: Failed to allocate LZO buffer\n");
> + vfree(wrk);
> + free_page((unsigned long)page);
> + return -ENOMEM;
> + }
>
> printk(KERN_INFO "PM: Saving image data pages (%u pages) ... ",
> nr_to_write);
> @@ -382,16 +418,50 @@ static int save_image(struct swap_map_handle *handle,
> bio = NULL;
> do_gettimeofday(&start);
> while (1) {
> - ret = snapshot_read_next(snapshot);
> - if (ret<= 0)
> + for (ul = 0; ul< LZO_UNC_SIZE; ul += PAGE_SIZE) {
> + ret = snapshot_read_next(snapshot);
> + if (ret< 0)

Not sure why my email client (Thunderbird) is shifting your comparison
operators :(

> + goto out_finish;
> +
> + if (ret == 0)

if (!ret)

(Further down, too)

> + break;
> +
> + memcpy(buf + LZO_OVH_SIZE + ul,
> + data_of(*snapshot), PAGE_SIZE);
> +
> + if (!(nr_pages % m))
> + printk(KERN_CONT "\b\b\b\b%3d%%", nr_pages / m);
> + nr_pages++;
> + }
> +
> + if (ul == 0)
> + break;
> +
> + ret = lzo1x_1_compress(buf + LZO_OVH_SIZE, ul,
> + buf + LZO_HEADER,&cl, wrk);
> + if (ret< 0) {
> + printk(KERN_ERR "PM: LZO compression failed\n");
> break;
> - ret = swap_write_page(handle, data_of(*snapshot),&bio);
> - if (ret)
> + }
> +
> + if (unlikely(cl == 0 || LZO_HEADER + cl> LZO_CMP_SIZE)) {
> + printk(KERN_ERR "PM: Invalid LZO length\n");
> + ret = -1;
> break;
> - if (!(nr_pages % m))
> - printk(KERN_CONT "\b\b\b\b%3d%%", nr_pages / m);
> - nr_pages++;
> + }
> +
> + *(size_t *)buf = cl;
> +
> + for (ul = 0; ul< LZO_HEADER + cl; ul += PAGE_SIZE) {
> + memcpy(page, buf + ul, PAGE_SIZE);
> +
> + ret = swap_write_page(handle, page,&bio);
> + if (ret)
> + goto out_finish;
> + }
> }
> +
> +out_finish:
> err2 = hib_wait_on_bio_chain(&bio);
> do_gettimeofday(&stop);
> if (!ret)
> @@ -401,6 +471,11 @@ static int save_image(struct swap_map_handle *handle,
> else
> printk(KERN_CONT "\n");
> swsusp_show_speed(&start,&stop, nr_to_write, "Wrote");
> +
> + vfree(buf);
> + vfree(wrk);
> + free_page((unsigned long)page);
> +
> return ret;
> }
>
> @@ -416,7 +491,8 @@ static int enough_swap(unsigned int nr_pages)
> unsigned int free_swap = count_swap_pages(root_swap, 1);
>
> pr_debug("PM: Free swap pages: %u\n", free_swap);
> - return free_swap> nr_pages + PAGES_FOR_IO;
> + return free_swap>
> + (nr_pages * LZO_CMP_PAGES) / LZO_UNC_PAGES + 1 + PAGES_FOR_IO;

PAGES_FOR_IO doesn't belong here, but that's not your problem :)

> }
>
> /**
> @@ -547,9 +623,22 @@ static int load_image(struct swap_map_handle *handle,
> int error = 0;
> struct timeval start;
> struct timeval stop;
> - struct bio *bio;
> - int err2;
> unsigned nr_pages;
> + size_t ul, cl, off;

As per comments on ul and cl above.

[...]

Regards,

Nigel

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Bojan Smojver on
On Wed, 2010-08-04 at 12:42 +1000, Nigel Cunningham wrote:
> Is it standalone? I'm not seeing the relationship between the two
> parts at the moment.

Once the compression/decompression gets applied, then it can be a
standalone patch, because sync_read is no longer used (we need the data
_now_ in order to decompress it, so async read would not give us much).

I already have it split on my box, so once I finalize, that's how I'll
send.

Points taken for all the other comments. Thank you.

PS. Right now, I'm seeing segfaults on resume with my
overlapping/in-place approach. So, I'll have to find out what is going
on first.

--
Bojan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Bojan Smojver on
On Wed, 2010-08-04 at 12:47 +1000, Bojan Smojver wrote:
> Points taken for all the other comments. Thank you.

Style wise, does this look better?

PS. Don't worry about contents yet - still working out the kinks.

diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index ca6066a..cb57eb9 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -137,6 +137,8 @@ config SUSPEND_FREEZER
config HIBERNATION
bool "Hibernation (aka 'suspend to disk')"
depends on PM && SWAP && ARCH_HIBERNATION_POSSIBLE
+ select LZO_COMPRESS
+ select LZO_DECOMPRESS
select SUSPEND_NVS if HAS_IOMEM
---help---
Enable the suspend to disk (STD) functionality, which is usually
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index b0bb217..1515e2c 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -24,6 +24,7 @@
#include <linux/swapops.h>
#include <linux/pm.h>
#include <linux/slab.h>
+#include <linux/lzo.h>

#include "power.h"

@@ -357,6 +358,30 @@ static int swap_writer_finish(struct swap_map_handle *handle,
return error;
}

+/* We need to remember how much compressed data we need to read. */
+#define LZO_HEADER sizeof(size_t)
+
+/* Number of pages/bytes we'll compress at a time. */
+#define LZO_UNC_PAGES 64
+#define LZO_UNC_SIZE (LZO_UNC_PAGES * PAGE_SIZE)
+
+/* Number of pages/bytes we need for compressed data (worst case). */
+#define LZO_CMP_PAGES DIV_ROUND_UP(lzo1x_worst_compress(LZO_UNC_SIZE) + \
+ LZO_HEADER, PAGE_SIZE)
+#define LZO_CMP_SIZE (LZO_CMP_PAGES * PAGE_SIZE)
+
+/* As per examples/overlap.c in lzo distribution, we need to add more space
+ * to the end of the buffer in order to perform overlapping compression. We
+ * also add LZO_HEADER, because our compressed data starts at that offset.
+ * Then we round it off on PAGE_SIZE boundary, so that we can copy full pages
+ * safely. We call this overhead. Magic numbers and formula below.
+ */
+#define LZO_OVH_PAGES DIV_ROUND_UP((LZO_UNC_SIZE > 0xBFFF ? \
+ 0xBFFF : LZO_UNC_SIZE) + \
+ LZO_UNC_SIZE / 16 + 64 + 3 + \
+ LZO_HEADER, PAGE_SIZE)
+#define LZO_OVH_SIZE (LZO_OVH_PAGES + PAGE_SIZE)
+
/**
* save_image - save the suspend image data
*/
@@ -372,6 +397,29 @@ static int save_image(struct swap_map_handle *handle,
struct bio *bio;
struct timeval start;
struct timeval stop;
+ size_t i, unc_len, cmp_len;
+ unsigned char *buf, *wrk, *page;
+
+ page = (void *)__get_free_page(__GFP_WAIT | __GFP_HIGH);
+ if (!page) {
+ printk(KERN_ERR "PM: Failed to allocate LZO page\n");
+ return -ENOMEM;
+ }
+
+ wrk = vmalloc(LZO1X_1_MEM_COMPRESS);
+ if (!wrk) {
+ printk(KERN_ERR "PM: Failed to allocate LZO workspace\n");
+ free_page((unsigned long)page);
+ return -ENOMEM;
+ }
+
+ buf = vmalloc(LZO_UNC_SIZE + LZO_OVH_SIZE);
+ if (!buf) {
+ printk(KERN_ERR "PM: Failed to allocate LZO buffer\n");
+ vfree(wrk);
+ free_page((unsigned long)page);
+ return -ENOMEM;
+ }

printk(KERN_INFO "PM: Saving image data pages (%u pages) ... ",
nr_to_write);
@@ -382,16 +430,59 @@ static int save_image(struct swap_map_handle *handle,
bio = NULL;
do_gettimeofday(&start);
while (1) {
- ret = snapshot_read_next(snapshot);
- if (ret <= 0)
+ for (unc_len = 0;
+ unc_len < LZO_UNC_SIZE; unc_len += PAGE_SIZE) {
+ ret = snapshot_read_next(snapshot);
+ if (ret < 0)
+ goto out_finish;
+
+ if (!ret)
+ break;
+
+ /* Copy uncompressed data to the top of the buffer. */
+ memcpy(buf + LZO_OVH_SIZE + unc_len,
+ data_of(*snapshot), PAGE_SIZE);
+
+ if (!(nr_pages % m))
+ printk(KERN_CONT "\b\b\b\b%3d%%", nr_pages / m);
+ nr_pages++;
+ }
+
+ if (!unc_len)
break;
- ret = swap_write_page(handle, data_of(*snapshot), &bio);
- if (ret)
+
+ ret = lzo1x_1_compress(buf + LZO_OVH_SIZE, unc_len,
+ buf + LZO_HEADER, &cmp_len, wrk);
+ if (ret < 0) {
+ printk(KERN_ERR "PM: LZO compression failed\n");
+ break;
+ }
+
+ if (unlikely(!cmp_len ||
+ cmp_len > lzo1x_worst_compress(unc_len))) {
+ printk(KERN_ERR "PM: Invalid LZO length\n");
+ ret = -1;
break;
- if (!(nr_pages % m))
- printk(KERN_CONT "\b\b\b\b%3d%%", nr_pages / m);
- nr_pages++;
+ }
+
+ *(size_t *)buf = cmp_len;
+
+ /* Given we are writing one page at a time to disk, we copy
+ * that much from the buffer, although the last bit will likely
+ * be smaller than full page. This is OK - we saved the length
+ * of the compressed data, so any garbage at the end will be
+ * discarded when we read it.
+ */
+ for (i = 0; i < LZO_HEADER + cmp_len; i += PAGE_SIZE) {
+ memcpy(page, buf + i, PAGE_SIZE);
+
+ ret = swap_write_page(handle, page, &bio);
+ if (ret)
+ goto out_finish;
+ }
}
+
+out_finish:
err2 = hib_wait_on_bio_chain(&bio);
do_gettimeofday(&stop);
if (!ret)
@@ -401,6 +492,11 @@ static int save_image(struct swap_map_handle *handle,
else
printk(KERN_CONT "\n");
swsusp_show_speed(&start, &stop, nr_to_write, "Wrote");
+
+ vfree(buf);
+ vfree(wrk);
+ free_page((unsigned long)page);
+
return ret;
}

@@ -416,7 +512,8 @@ static int enough_swap(unsigned int nr_pages)
unsigned int free_swap = count_swap_pages(root_swap, 1);

pr_debug("PM: Free swap pages: %u\n", free_swap);
- return free_swap > nr_pages + PAGES_FOR_IO;
+ return free_swap >
+ (nr_pages * LZO_CMP_PAGES) / LZO_UNC_PAGES + 1 + PAGES_FOR_IO;
}

/**
@@ -547,9 +644,22 @@ static int load_image(struct swap_map_handle *handle,
int error = 0;
struct timeval start;
struct timeval stop;
- struct bio *bio;
- int err2;
unsigned nr_pages;
+ size_t i, off, unc_len, cmp_len;
+ unsigned char *buf, *page;
+
+ page = (void *)__get_free_page(__GFP_WAIT | __GFP_HIGH);
+ if (!page) {
+ printk(KERN_ERR "PM: Failed to allocate LZO page\n");
+ return -ENOMEM;
+ }
+
+ buf = vmalloc(LZO_CMP_SIZE);
+ if (!buf) {
+ printk(KERN_ERR "PM: Failed to allocate LZO buffer\n");
+ free_page((unsigned long)page);
+ return -ENOMEM;
+ }

printk(KERN_INFO "PM: Loading image data pages (%u pages) ... ",
nr_to_read);
@@ -557,27 +667,73 @@ static int load_image(struct swap_map_handle *handle,
if (!m)
m = 1;
nr_pages = 0;
- bio = NULL;
do_gettimeofday(&start);
+
+ error = snapshot_write_next(snapshot);
+ if (error <= 0)
+ goto out_finish;
+
for ( ; ; ) {
- error = snapshot_write_next(snapshot);
- if (error <= 0)
- break;
- error = swap_read_page(handle, data_of(*snapshot), &bio);
+ error = swap_read_page(handle, page, NULL); /* sync */
if (error)
break;
- if (snapshot->sync_read)
- error = hib_wait_on_bio_chain(&bio);
- if (error)
+
+ cmp_len = *(size_t *)page;
+ if (unlikely(!cmp_len ||
+ cmp_len > lzo1x_worst_compress(LZO_UNC_SIZE))) {
+ printk(KERN_ERR "PM: Invalid LZO length\n");
+ error = -1;
break;
- if (!(nr_pages % m))
- printk("\b\b\b\b%3d%%", nr_pages / m);
- nr_pages++;
+ }
+
+ /* As per examples/overlap.c in lzo distribution, we copy
+ * compressed data to the top of the in-place decompression
+ * buffer. */
+ off = LZO_CMP_SIZE - cmp_len;
+ memcpy(buf + off, page + LZO_HEADER,
+ (cmp_len < PAGE_SIZE - LZO_HEADER ?
+ cmp_len : PAGE_SIZE - LZO_HEADER));
+
+ for (i = PAGE_SIZE - LZO_HEADER; i < cmp_len; i += PAGE_SIZE) {
+ error = swap_read_page(handle, page, NULL); /* sync */
+ if (error)
+ goto out_finish;
+
+ memcpy(buf + off + i, page,
+ (cmp_len - unc_len < PAGE_SIZE ?
+ cmp_len - unc_len : PAGE_SIZE));
+ }
+
+ unc_len = LZO_UNC_SIZE;
+ error = lzo1x_decompress_safe(buf + off, cmp_len,
+ buf, &unc_len);
+ if (error < 0) {
+ printk(KERN_ERR "PM: LZO decompression failed\n");
+ break;
+ }
+
+ if (unlikely(!unc_len ||
+ unc_len > LZO_UNC_SIZE || unc_len % PAGE_SIZE)) {
+ printk(KERN_ERR "PM: Invalid LZO length\n");
+ error = -1;
+ break;
+ }
+
+ for (i = 0; i < unc_len; i += PAGE_SIZE) {
+ memcpy(data_of(*snapshot), buf + i, PAGE_SIZE);
+
+ if (!(nr_pages % m))
+ printk("\b\b\b\b%3d%%", nr_pages / m);
+ nr_pages++;
+
+ error = snapshot_write_next(snapshot);
+ if (error <= 0)
+ goto out_finish;
+ }
}
- err2 = hib_wait_on_bio_chain(&bio);
+
+out_finish:
do_gettimeofday(&stop);
- if (!error)
- error = err2;
if (!error) {
printk("\b\b\b\bdone\n");
snapshot_write_finalize(snapshot);
@@ -586,6 +742,10 @@ static int load_image(struct swap_map_handle *handle,
} else
printk("\n");
swsusp_show_speed(&start, &stop, nr_to_read, "Read");
+
+ vfree(buf);
+ free_page((unsigned long)page);
+
return error;
}


--
Bojan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/