: Compress hibernation image with LZO (in-kernel)

This patch speeds up hibernate/thaw operations

Quoted 1 line · Bojan Smojver

This patch speeds up hibernate/thaw operations

This is probably a bit simpler.

In general, it looks good. Is an order 6 allocation really necessary, though?

Is an order 6 allocation really necessary,
though?

Quoted 2 lines · Nigel Cunningham

Is an order 6 allocation really necessary,
though?

I tried with 5 and 4 and got slightly lower throughput numbers. With 6, I was getting 145 to 150 MB/s, with 4 I'm getting around 130 MB/s (this is all on hibernate).

So, here is one with 4.

PS. I guess with this, read_sync can simply disappear as well.

How about vmallocing the cmp as well? That would greatly reduce the potential for page allocation failures while still letting you use an order 6 area.

Quoted 1 line

PS. I guess with this, read_sync can simply disappear as well.

I haven't looked at the code for a while, but it might still be needed for the header? I know that in TuxOnIce, I need to read the first page synchronously when bootstrapping reading the image (can't read the next page until you know where it is, and its location is on the first page). Since swsusp uses those index pages, I think it would have the same issue - they would need to be read before it could read the following pages. Of course I'm going off memory :)

Quoted 3 lines · Nigel Cunningham

How about vmallocing the cmp as well? That would greatly reduce the potential for page allocation failures while still letting you use an order 6 area.

In save_image(), that worked. In load_image() it would cause a crash (something about kernel not being able to satisfy paging request). So, I just made it __get_free_pages() instead. But, yeah good point.

Keep in mind that I have absolutely no idea how kernel memory allocation works. I'm kinda coping and pasting code and hoping it doesn't crash :-)

Quoted 9 lines

Quoted 1 line

PS. I guess with this, read_sync can simply disappear as well.

I haven't looked at the code for a while, but it might still be needed for the header? I know that in TuxOnIce, I need to read the first page synchronously when bootstrapping reading the image (can't read the next page until you know where it is, and its location is on the first page). Since swsusp uses those index pages, I think it would have the same issue - they would need to be read before it could read the following pages. Of course I'm going off memory :)

I think it can go, because the header is already read/written with&bio set to NULL (sync read). See patch to remove read_sync.

It should be possible to do the allocation at that point. I might see if I can take a look later on.

I can go back to that easily.

I did 3 hibernate/thaw cycles with it. Images varied from about 850 MB, 1.1 GB to 750 MB. I was getting 156/141 MB/s, 121/118 MBs and 141/130 MBs speeds. Obviously, these things depend on compression ratios achieved etc.

Quoted 1 line · Bojan Smojver

I can go back to that easily.

So, here is that whole enchilada one more time (it includes sync_read removal patch as well).

I did 3 hibernate/thaw cycles with it. Images varied from about 850 MB, 1.1 GB to 750 MB. I was getting 156/141 MB/s, 121/118 MBs and 141/130 MBs speeds. Obviously, these things depend on compression ratios achieved etc.

I guess the number of pages (i.e. LZO_UNC_PAGES) could be made configurable as well.

PS. Inline, as requested.

kernel/power/swap.c +4 -0

+	wrk = vmalloc(LZO1X_1_MEM_COMPRESS);
+	if (!wrk) {
+		printk(KERN_ERR "PM: Failed to allocate LZO workspace\n");
+		free_page((unsigned long)page);

kernel/power/swap.c +131 -24

+
+	cmp = vmalloc(LZO_CMP_SIZE);
+	if (!cmp) {
+		printk(KERN_ERR "PM: Failed to allocate LZO compressed\n");
+		vfree(unc);
+		vfree(wrk);
+		free_page((unsigned long)page);
+		return -ENOMEM;
+	}
 
 	printk(KERN_INFO "PM: Saving image data pages (%u pages) ...     ",
 		nr_to_write);

@@ -382,16 +422,48 @@ 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)
+				goto out_finish;
+
+			if (ret == 0)
+				break;
+
+			memcpy(unc + 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(unc, ul, cmp + 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 *)cmp = cl;
+
+		for (ul = 0; ul < LZO_HEADER + cl; ul += PAGE_SIZE) {
+			memcpy(page, cmp + 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 +473,12 @@ static int save_image(struct swap_map_handle *handle,
 	else
 		printk(KERN_CONT "\n");
 	swsusp_show_speed(&start, &stop, nr_to_write, "Wrote");
+
+	vfree(cmp);
+	vfree(unc);
+	vfree(wrk);
+	free_page((unsigned long)page);
+
 	return ret;
 }
 

@@ -416,7 +494,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 + PAGES_FOR_IO;
 }
 
 /**

@@ -547,9 +626,30 @@ 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;
+	unsigned char *unc, *cmp, *page;
+
+	page = (void *)__get_free_page(__GFP_WAIT | __GFP_HIGH);
+	if (!page) {
+		printk(KERN_ERR "PM: Failed to allocate LZO page\n");
+		return -ENOMEM;
+	}
+
+	unc = vmalloc(LZO_UNC_SIZE);
+	if (!unc) {
+		printk(KERN_ERR "PM: Failed to allocate LZO uncompressed\n");
+		free_page((unsigned long)page);
+		return -ENOMEM;
+	}
+
+	cmp = vmalloc(LZO_CMP_SIZE);
+	if (!cmp) {
+		printk(KERN_ERR "PM: Failed to allocate LZO compressed\n");
+		vfree(unc);
+		free_page((unsigned long)page);
+		return -ENOMEM;
+	}
 
 	printk(KERN_INFO "PM: Loading image data pages (%u pages) ...     ",
 		nr_to_read);

@@ -557,27 +657,60 @@ 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)
+		memcpy(cmp, page, PAGE_SIZE);
+
+		cl = *(size_t *)cmp;
+		if (unlikely(cl == 0 || LZO_HEADER + cl > LZO_CMP_SIZE)) {
+			printk(KERN_ERR "PM: Invalid LZO length\n");
+			error = -1;
+			break;
+		}
+
+		for (ul = PAGE_SIZE; ul < LZO_HEADER + cl; ul += PAGE_SIZE) {
+			error = swap_read_page(handle, page, NULL); /* sync */
+			if (error)
+				goto out_finish;
+			memcpy(cmp + ul, page, PAGE_SIZE);
+		}
+
+		ul = LZO_UNC_SIZE;
+		error = lzo1x_decompress_safe(cmp + LZO_HEADER, cl, unc, &ul);
+		if (error < 0) {
+			printk(KERN_ERR "PM: LZO decompression failed\n");
 			break;
-		if (!(nr_pages % m))
-			printk("\b\b\b\b%3d%%", nr_pages / m);
-		nr_pages++;
+		}
+
+		if (unlikely(ul == 0 || ul > LZO_UNC_SIZE)) {
+			printk(KERN_ERR "PM: Invalid LZO length\n");
+			error = -1;
+			break;
+		}
+
+		for (cl = 0; cl < ul; cl += PAGE_SIZE) {
+			memcpy(data_of(*snapshot), unc + cl, 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 +719,11 @@ static int load_image(struct swap_map_handle *handle,
 	} else
 		printk("\n");
 	swsusp_show_speed(&start, &stop, nr_to_read, "Read");
+
+	vfree(cmp);
+	vfree(unc);
+	free_page((unsigned long)page);
+
 	return error;
 }
 

Signature · 8 lines

-- 
Bojan

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

Now, vmallc() is used here. Then, following will happen.

1. vmalloc()
-> vmalloc adds vmap objects and set page table entries.

2. saving image
-> At taking snapshot of memory to the disk, above vmalloc() area
is
saved to disk as it is.
...
3. At restore
Because you dont't remember which vmalloc() area was used for
creating
snapshot, you can't free it at swsusp_free().

memory leak ?

Quoted 15 lines · KAMEZAWA Hiroyuki

Now, vmallc() is used here. Then, following will happen.

1. vmalloc()
-> vmalloc adds vmap objects and set page table entries.

2. saving image
-> At taking snapshot of memory to the disk, above vmalloc() area
is
saved to disk as it is.
...
3. At restore
Because you dont't remember which vmalloc() area was used for
creating
snapshot, you can't free it at swsusp_free().

Quoted 1 line

memory leak ?

To be honest, I'm not sure.

However, I thought that by the time save_image() is called, snapshot has already been taken, no?
------------------

        error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
        if (error)
                goto Thaw;

        if (in_suspend) {
                unsigned int flags = 0;

                if (hibernation_mode == HIBERNATION_PLATFORM)
                        flags |= SF_PLATFORM_MODE;
                pr_debug("PM: writing image.\n");
                error = swsusp_write(flags);  <--- this calls save_image()

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

So, me thinks that these allocations will not be in the snapshot image.

Why swsusp can avoid memory leak is that it records which pages should be freed after resume in the bitmap, which will be saved to image header(?)

Quoted 3 lines · KAMEZAWA Hiroyuki

Why swsusp can avoid memory leak is that it records which pages should be freed after resume in the bitmap, which will be saved to image header(?)

Right. So, are you saying that all allocations in save_image() should be done using __get_free_page() or __get_free_pages() and not with vmalloc()?

Then, you can free it because you remember pointers.

OK, get it. Will rework.

From all this, I only got "In swsusp_free()" printed on resume. So, it

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.

Quoted 6 lines · Bojan Smojver

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.

kernel/power/Kconfig +2 -0

index ca6066a..cb57eb9 100644
@@ -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

kernel/power/power.h +0 -4

index 006270f..a760cf8 100644
@@ -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

kernel/power/snapshot.c +0 -5

index 25ce010..f24ee24 100644
@@ -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;

kernel/power/swap.c +3 -0

index b0bb217..5a83fd3 100644
@@ -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

kernel/power/swap.c +22 -0

+	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);

kernel/power/swap.c +40 -5

+				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;
  }
 

Is it standalone? I'm not seeing the relationship between the two parts at the moment.

Points taken for all the other comments. Thank you.

Quoted 1 line · Bojan Smojver

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.

kernel/power/Kconfig +0 -0

  	select SUSPEND_NVS if HAS_IOMEM
  	---help---
  	  Enable the suspend to disk (STD) functionality, which is usually

kernel/power/swap.c +9 -0

index b0bb217..1515e2c 100644
@@ -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). */

But, having said 'okay' above, I should point out that you might want to select CRYPTO too, since LZO depends on it. (And perhaps I should ask whether you should be using cryptoapi rather than LZO directly?)

OK, I will check. I kinda remember the docs just saying to select the above.

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.

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.

Quoted 1 line

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.

Hi.

Sorry for the delay in replying.

Quoted 21 lines · Bojan Smojver

Quoted 1 line

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.

Quoted 1 line

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.

Then, after resume, all vmalloc() area is resumed as "allocated".

Wrong ?

Quoted 3 lines · KAMEZAWA Hiroyuki

Then, after resume, all vmalloc() area is resumed as "allocated".

Wrong ?

I actually tried remembering vmalloc() returned pointers into a global variable as you suggested. On resume, they were always set to NULL, which would suggest that what has gotten into the image was the state before vmalloc() was called in save_image(). See:
http://lkml.org/lkml/2010/8/2/537.

Anyone else wants to comment here?

Quoted 11 lines · On Wed, 04 Aug 2010 12:14:19 +1000 Bojan Smojver<[email protected]> wrote:

Quoted 3 lines · KAMEZAWA Hiroyuki

Then, after resume, all vmalloc() area is resumed as "allocated".

Wrong ?

I actually tried remembering vmalloc() returned pointers into a global variable as you suggested. On resume, they were always set to NULL, which would suggest that what has gotten into the image was the state before vmalloc() was called in save_image(). See:
http://lkml.org/lkml/2010/8/2/537.

Quoted 2 lines

Anyone else wants to comment here?

Hmm, ok. let's see the result.

The reason I mention about the race is my patch corrupts saved image by changing swap_map[] status and swap-cache radix-tree during save_image().

Maybe I don't understand something important.

Quoted 26 lines · On Wed, 04 Aug 2010 11:50:41 +1000 Nigel Cunningham<[email protected]> wrote:

Hi.

Sorry for the delay in replying.

Quoted 21 lines · Bojan Smojver

Quoted 1 line

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.

Quoted 8 lines

Quoted 1 line

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)

  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 ?

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.

Quoted 9 lines · On Wed, 04 Aug 2010 12:24:57 +1000 Nigel Cunningham<[email protected]> 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.

This patch speeds up hibernate/thaw operations (in kernel ones) by compressing the pages using LZO. Tests on my ThinkPad T510 show that hibernation times can be cut by the factor of about 4 for an image of around 1 GB (from about a minute down to about 15 seconds).

Why do it in kernel? s2disk should already be faster...