PM: Compress hibernation image with LZO
Subsystems (2)
PATCH 1/2 PM: Compress hibernation image with LZO
Please queue for merge in 2.6.36.
---------------------
Compress hibernation image with LZO in order to save on I/O and therefore time to hibernate/thaw.
---
kernel/power/Kconfig | 2 + kernel/power/swap.c | 205 ++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 183 insertions(+), 24 deletions(-)
kernel/power/Kconfig +2 -0
@@ -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/swap.c +181 -24
270 lines not on this page · raw message
Signature · 1 line
Bojan
Quoted 1 line · Bojan Smojver
Please queue for merge in 2.6.36.
Obviously that's not going to happen.
Any feedback at all (apart from why, which I answered on lkml)?
Signature · 1 line
Bojan
Quoted 6 lines · Bojan Smojver
Quoted 1 line · Bojan Smojver
Please queue for merge in 2.6.36.
Obviously that's not going to happen.
Any feedback at all (apart from why, which I answered on lkml)?
Well, it was a bit too late for the merge window and was at LinuxCon in Boston last week. I'll review the patch and let you know if I have any comments.
Thanks,
Rafael
Quoted 4 lines
Well, it was a bit too late for the merge window and was at LinuxCon in Boston
last week. I'll review the patch and let you know if I have any comments.
OK, thanks.
--
Bojan
Quoted 18 lines · Bojan Smojver
Please queue for merge in 2.6.36.
---------------------Compress hibernation image with LZO in order to save on I/O and therefore time to hibernate/thaw.
Signed-off-by Bojan Smojver <[email protected]>---
kernel/power/Kconfig | 2 + kernel/power/swap.c | 205 ++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 183 insertions(+), 24 deletions(-)kernel/power/Kconfig +0
-0
kernel/power/Kconfig config SUSPEND_FREEZER · line 141
Quoted 5 lines
kernel/power/Kconfig +2
-0@@ -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
I'm not a big fan of select in Kconfig. It ususally causes one to be surprised with the final choice of options in .config.
Does that work at all if CRYPTO_ALGAPI is not set?
Quoted 16 lines
kernel/power/Kconfig +0
-0select SUSPEND_NVS if HAS_IOMEM ---help--- Enable the suspend to disk (STD) functionality, which is usuallykernel/power/swap.c +1
-0@@ -24,6 +24,7 @@#include <linux/swapops.h> #include <linux/pm.h> #include <linux/slab.h> +#include <linux/lzo.h> #include "power.h"
kernel/power/swap.c static int swap_writer_finish(struct swap_map_handle *handle, · line 366
Quoted 9 lines
kernel/power/swap.c +6
-0@@ -357,6 +358,18 @@ 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 one time. */ +#define LZO_UNC_PAGES 32 +#define LZO_UNC_SIZE (LZO_UNC_PAGES * PAGE_SIZE)
These numbers seem to be totally arbitrary. Any justification, please?
Quoted 34 lines
kernel/power/swap.c +27
-0+/* 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) +/** * save_image - save the suspend image data */ @@ -372,6 +385,38 @@ 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 *unc, *cmp, *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 ; + } + + unc =vmalloc (LZO_UNC_SIZE ); +if (!unc) { +printk (KERN_ERR "PM: Failed to allocate LZO uncompressed\n" ); +vfree (wrk); +free_page ((unsigned long )page); +return -ENOMEM ;
kernel/power/swap.c static int save_image(struct swap_map_handle *handle, · line 419
Quoted 10 lines
kernel/power/swap.c +10
-0+ } + + 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 ; + }
I'd rather wouldn't like the above to be unconditional. If not for anything else, I think it would be nice to be able to switch the compression off for debugging purposes (like when we're not sure why the image is corrupted or similar). A kernel command line switch would suffice for that IMO.
Quoted 15 lines
kernel/power/swap.c +7
-2printk (KERN_INFO "PM: Saving image data pages (%u pages) ... " , nr_to_write);@@ -382,16 +427,57 @@ 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 (i =0 ; i <LZO_UNC_SIZE ; i +=PAGE_SIZE ) { + ret =snapshot_read_next (snapshot); +if (ret <0 ) + goto out_finish; + +if (!ret) +break ;
kernel/power/swap.c static int save_image(struct swap_map_handle *handle, · line 446
Quoted 10 lines
kernel/power/swap.c +9
-0+ +memcpy (unc + i,data_of (*snapshot),PAGE_SIZE ); + +if (!(nr_pages % m)) +printk (KERN_CONT "\b\b\b\b%3d%%" , nr_pages / m); + nr_pages++; + } + +if (!i)break ;
This statement is profoundly hard to decode. Perhaps there's a better name for 'i'? Like 'size' or similar?
Also please make it possible to switch the compression off here too.
The comments above seem to apply to the remaining code too.
Thanks,
Rafael
Quoted 4 lines · Rafael J. Wysocki
I'm not a big fan of select in
Kconfig. It ususally causes one to be surprised with the final choice of options in .config.Does that work at all if
CRYPTO_ALGAPIis not set?
My understanding is that these (LZO_COMPRESS/LZO_DECOMPRESS) do not depend on anything. But, I can test, just to be sure.
Quoted 4 lines
I'd rather wouldn't like the above to be unconditional. If not for anything else, I think it would be nice to be able to switch the compression off for debugging purposes (like when we're not sure why the image is corrupted or similar). A kernel command line switch would suffice for that IMO.
OK. I think the cleanest way may be to have a set of two "real" save_image()/load_image() functions. One set with compression, the other without, which are then called by save_image()/load_image(). OK?
Quoted 5 lines
kernel/power/swap.c static int save_image(struct swap_map_handle *handle, · line 446
Quoted 2 lines
kernel/power/swap.c +1
-0+if (!i)break ;This statement is profoundly hard to decode. Perhaps there's a better name for 'i'? Like 'size' or similar?
Right. Maybe I'm a bit too economical with names. Usually things like 'i' stand for a generic counter, which this is. I'll use something else if you don't like it.
Quoted 3 lines
Also please make it possible to switch the compression off here too.
The comments above seem to apply to the remaining code too.
OK, I'll prepare a patch that has compression/decompression as optional, using kernel command line switch. Obviously, if one attempts to thaw from an image that is not in an expected state (compressed/not compressed), this will wreak havoc. But, I guess that's understandable.
Signature · 1 line
Bojan
Quoted 2 lines · Bojan Smojver
OK, I'll prepare a patch that has compression/decompression as optional, using kernel command line switch.
Not tested, but here it is, so that you can tell me if this is roughly what you had in mind. I still have to see about Kconfig selects. Will get back to you on this.
Documentation/power/swsusp.txt +2 -1
@@ -66,7 +66,8 @@ swsusp saves the state of the machine into active swaps and then reboots or powerdowns. You must explicitly specify the swap partition to resume from with ``resume='' kernel option. If signature is found it loads and restores saved state. If the option ``noresume'' is specified as a boot parameter, it skips -the resuming. +the resuming. If the option ``nocompress'' is specified as a boot parameter, it +saves hibernation image without compression. In the meantime while the system is suspended you should not add/remove any of the hardware, write to the filesystems, etc.
kernel/power/Kconfig +2 -0
@@ -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/hibernate.c +10 -0
@@ -29,6 +29,7 @@#include "power.h" +static int nocompress =0 ;static int noresume =0 ;static char resume_file[256 ] =CONFIG_PM_STD_PARTITION ;dev_t swsusp_resume_device;
@@ -630,6 +631,8 @@ int hibernate(void)if (hibernation_mode ==HIBERNATION_PLATFORM ) flags |=SF_PLATFORM_MODE ; +if (nocompress) + flags |=SF_NOCOMPRESS_MODE ;pr_debug ("PM: writing image.\n" ); error =swsusp_write (flags);swsusp_free ();
@@ -996,12 +999,19 @@ static int __init resume_offset_setup(char *str)return 1 ; } +static int __init nocompress_setup (char *str) +{ + nocompress =1 ; +return 1 ; +} +static int __init noresume_setup (char *str) { noresume =1 ;return 1 ; } +__setup ("nocompress" , nocompress_setup);__setup ("noresume" , noresume_setup);__setup ("resume_offset=" , resume_offset_setup);__setup ("resume=" , resume_setup);
kernel/power/power.h +1 -0
@@ -134,6 +134,7 @@ extern int swsusp_swap_in_use(void);* the image header. */ #define SF_PLATFORM_MODE 1 +#define SF_NOCOMPRESS_MODE 2/* kernel/power/hibernate.c */ extern int swsusp_check (void );
kernel/power/swap.c +274 -5
321 lines not on this page · raw message
Signature · 1 line
Bojan
Quoted 2 lines · Bojan Smojver
I still have to see about
Kconfigselects. Will
get back to you on this
For instance, we have this:
---------------------
config JFFS2_LZO
bool "JFFS2 LZO compression support" if JFFS2_COMPRESSION_OPTIONS select LZO_COMPRESS
select LZO_DECOMPRESS
depends on JFFS2_FS
default n
help
minilzo-based compression. Generally works better than Zlib.
This feature was added in July, 2007. Say 'N' if you need compatibility with older bootloaders or kernels.
---------------------
And this:
---------------------
config ZRAM
tristate "Compressed RAM block device support"
depends on BLOCK
select LZO_COMPRESS
select LZO_DECOMPRESS
default n
help
Creates virtual block devices called /dev/zramX (X = 0, 1, ...). Pages written to these disks are compressed and stored in memory itself. These disks allow very fast I/O and compression provides good amounts of memory savings.
It has several use cases, for example: /tmp storage, use as swap disks and maybe many more.
See zram.txt for more information. Project home: http://compcache.googlecode.com/
---------------------
And this:
---------------------
config CRYPTO_LZO
tristate "LZO compression algorithm"
select CRYPTO_ALGAPI
select LZO_COMPRESS
select LZO_DECOMPRESS
help
This is the LZO algorithm.
---------------------
And finally, we have this:
---------------------
#
# compression support is select'ed if needed
#
config ZLIB_INFLATE
tristateconfig ZLIB_DEFLATE
tristate
config LZO_COMPRESS
tristate
config LZO_DECOMPRESS
tristate
---------------------
Which would suggest to me that selecting it is the right thing to do.
Signature · 1 line
Bojan
Quoted 15 lines · Bojan Smojver
Quoted 2 lines · Bojan Smojver
I still have to see about
Kconfigselects. Will
get back to you on thisFor instance, we have this:
---------------------
configJFFS2_LZO
bool "JFFS2 LZO compression support" ifJFFS2_COMPRESSION_OPTIONSselectLZO_COMPRESS
selectLZO_DECOMPRESS
depends onJFFS2_FS
default n
help
minilzo-based compression. Generally works better than Zlib.
Quoted 3 lines
This feature was added in July, 2007. Say 'N' if you need compatibility with older bootloaders or kernels.
---------------------
Actually I think it's preferable to select CRYPTO_LZO that will then select everything as necessary.
Thanks,
Rafael
Quoted 1 line · Bojan Smojver
Not tested, but here it is
This one is tested with both nocompress option passed to the kernel and without. Only one minor change compared to the previous patch - calculation in enough_swap() also takes into account the compress v. nocompress.
Documentation/power/swsusp.txt | 3 +- kernel/power/Kconfig | 2 + kernel/power/hibernate.c | 10 ++ kernel/power/power.h | 1 + kernel/power/swap.c | 288 +++++++++++++++++++++++++++++++++++++++- 5 files changed, 296 insertions(+), 8 deletions(-)
Documentation/power/swsusp.txt +2 -1
@@ -66,7 +66,8 @@ swsusp saves the state of the machine into active swaps and then reboots or powerdowns. You must explicitly specify the swap partition to resume from with ``resume='' kernel option. If signature is found it loads and restores saved state. If the option ``noresume'' is specified as a boot parameter, it skips -the resuming. +the resuming. If the option ``nocompress'' is specified as a boot parameter, it +saves hibernation image without compression. In the meantime while the system is suspended you should not add/remove any of the hardware, write to the filesystems, etc.
kernel/power/Kconfig +2 -0
@@ -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/hibernate.c +10 -0
@@ -29,6 +29,7 @@#include "power.h" +static int nocompress =0 ;static int noresume =0 ;static char resume_file[256 ] =CONFIG_PM_STD_PARTITION ;dev_t swsusp_resume_device;
@@ -630,6 +631,8 @@ int hibernate(void)if (hibernation_mode ==HIBERNATION_PLATFORM ) flags |=SF_PLATFORM_MODE ; +if (nocompress) + flags |=SF_NOCOMPRESS_MODE ;pr_debug ("PM: writing image.\n" ); error =swsusp_write (flags);swsusp_free ();
@@ -996,12 +999,19 @@ static int __init resume_offset_setup(char *str)return 1 ; } +static int __init nocompress_setup (char *str) +{ + nocompress =1 ; +return 1 ; +} +static int __init noresume_setup (char *str) { noresume =1 ;return 1 ; } +__setup ("nocompress" , nocompress_setup);__setup ("noresume" , noresume_setup);__setup ("resume_offset=" , resume_offset_setup);__setup ("resume=" , resume_setup);
kernel/power/power.h +1 -0
@@ -134,6 +134,7 @@ extern int swsusp_swap_in_use(void);* the image header. */ #define SF_PLATFORM_MODE 1 +#define SF_NOCOMPRESS_MODE 2/* kernel/power/hibernate.c */ extern int swsusp_check (void );
kernel/power/swap.c +281 -7
340 lines not on this page · raw message
Signature · 1 line
Bojan
Quoted 7 lines · Bojan Smojver
Quoted 1 line · Bojan Smojver
Not tested, but here it is
This one is tested with both nocompress option passed to the kernel and without. Only one minor change compared to the previous patch - calculation in
enough_swap()also takes into account the compress v. nocompress.
OK, it generally looks good.
Quoted 26 lines
Documentation/power/swsusp.txt | 3 +- kernel/power/Kconfig | 2 + kernel/power/hibernate.c | 10 ++ kernel/power/power.h | 1 + kernel/power/swap.c | 288 +++++++++++++++++++++++++++++++++++++++- 5 files changed, 296 insertions(+), 8 deletions(-)Documentation/power/swsusp.txt +2
-1@@ -66,7 +66,8 @@ swsusp saves the state of the machine into active swaps and then reboots or powerdowns. You must explicitly specify the swap partition to resume from with ``resume='' kernel option. If signature is found it loads and restores saved state. If the option ``noresume'' is specified as a boot parameter, it skips -the resuming. +the resuming. If the option ``nocompress'' is specified as a boot parameter, it +saves hibernation image without compression. In the meantime while the system is suspended you should not add/remove any of the hardware, write to the filesystems, etc.kernel/power/Kconfig +0
-0
kernel/power/Kconfig config SUSPEND_FREEZER · line 141
Quoted 5 lines
kernel/power/Kconfig +2
-0@@ -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
As I wrote, I think it's better to select CRYPTO_LZO here.
Thanks,
Rafael
Quoted 1 line · Rafael J. Wysocki
As I wrote, I think it's better to select
CRYPTO_LZOhere.
This does a different thing, as I understand it. It brings in a crypto API, which we neither need nor use (we just use compress/decompress from LZO library).
All the other systems I quoted in the other e-mail select this directly, because they don't need the crypto API either. So, I reckon we should do the same - it is simpler and brings in less stuff, which is always better.
But, if you say this is how it has to be, then we'll do that.
Signature · 1 line
Bojan
Quoted 7 lines · Bojan Smojver
Quoted 1 line · Rafael J. Wysocki
As I wrote, I think it's better to select
CRYPTO_LZOhere.This does a different thing, as I understand it. It brings in a crypto API, which we neither need nor use (we just use compress/decompress from LZO library).
Quoted 6 lines
All the other systems I quoted in the other e-mail select this directly, because they don't need the crypto API either. So, I reckon we should do the same - it is simpler and brings in less stuff, which is always better.
But, if you say this is how it has to be, then we'll do that.
Well, I looked at the code in the meantime and it apparently is sufficient to select LZO_COMPRESS and LZO_DECOMPRESS.
Thanks,
Rafael
Quoted 10 lines · Rafael J. Wysocki
Quoted 7 lines · Bojan Smojver
Quoted 1 line · Bojan Smojver
Not tested, but here it is
This one is tested with both nocompress option passed to the kernel and without. Only one minor change compared to the previous patch - calculation in
enough_swap()also takes into account the compress v. nocompress.OK, it generally looks good.
One more remark, though.
Since LZO compression is hardcoded into your code, perhaps you can rename save_image_cmp() to save_image_lzo() (and analogously in the other places)?
Thanks,
Rafael
Quoted 3 lines · Rafael J. Wysocki
Since LZO compression is hardcoded into your code, perhaps you can rename
save_image_cmp()tosave_image_lzo()(and analogously in the other places)?
Good point. Will change.
Signature · 1 line
Bojan
Quoted 1 line · Bojan Smojver
Good point. Will change.
Here you go:
Documentation/power/swsusp.txt | 3 +- kernel/power/Kconfig | 2 + kernel/power/hibernate.c | 10 ++ kernel/power/power.h | 1 + kernel/power/swap.c | 289 +++++++++++++++++++++++++++++++++++++++- 5 files changed, 297 insertions(+), 8 deletions(-)
Documentation/power/swsusp.txt +2 -1
@@ -66,7 +66,8 @@ swsusp saves the state of the machine into active swaps and then reboots or powerdowns. You must explicitly specify the swap partition to resume from with ``resume='' kernel option. If signature is found it loads and restores saved state. If the option ``noresume'' is specified as a boot parameter, it skips -the resuming. +the resuming. If the option ``nocompress'' is specified as a boot parameter, it +saves hibernation image without compression. In the meantime while the system is suspended you should not add/remove any of the hardware, write to the filesystems, etc.
kernel/power/Kconfig +2 -0
@@ -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/hibernate.c +10 -0
@@ -29,6 +29,7 @@#include "power.h" +static int nocompress =0 ;static int noresume =0 ;static char resume_file[256 ] =CONFIG_PM_STD_PARTITION ;dev_t swsusp_resume_device;
@@ -630,6 +631,8 @@ int hibernate(void)if (hibernation_mode ==HIBERNATION_PLATFORM ) flags |=SF_PLATFORM_MODE ; +if (nocompress) + flags |=SF_NOCOMPRESS_MODE ;pr_debug ("PM: writing image.\n" ); error =swsusp_write (flags);swsusp_free ();
@@ -996,12 +999,19 @@ static int __init resume_offset_setup(char *str)return 1 ; } +static int __init nocompress_setup (char *str) +{ + nocompress =1 ; +return 1 ; +} +static int __init noresume_setup (char *str) { noresume =1 ;return 1 ; } +__setup ("nocompress" , nocompress_setup);__setup ("noresume" , noresume_setup);__setup ("resume_offset=" , resume_offset_setup);__setup ("resume=" , resume_setup);
kernel/power/power.h +1 -0
@@ -134,6 +134,7 @@ extern int swsusp_swap_in_use(void);* the image header. */ #define SF_PLATFORM_MODE 1 +#define SF_NOCOMPRESS_MODE 2/* kernel/power/hibernate.c */ extern int swsusp_check (void );
kernel/power/swap.c +282 -7
341 lines not on this page · raw message
Signature · 1 line
Bojan
Quoted 4 lines · Bojan Smojver
Quoted 1 line · Bojan Smojver
Good point. Will change.
Here you go:
OK, so changelog please?
Or should I use the original one?
Rafael
Quoted 442 lines
Documentation/power/swsusp.txt | 3 +- kernel/power/Kconfig | 2 + kernel/power/hibernate.c | 10 ++ kernel/power/power.h | 1 + kernel/power/swap.c | 289 +++++++++++++++++++++++++++++++++++++++- 5 files changed, 297 insertions(+), 8 deletions(-)Documentation/power/swsusp.txt +2
-1@@ -66,7 +66,8 @@ swsusp saves the state of the machine into active swaps and then reboots or powerdowns. You must explicitly specify the swap partition to resume from with ``resume='' kernel option. If signature is found it loads and restores saved state. If the option ``noresume'' is specified as a boot parameter, it skips -the resuming. +the resuming. If the option ``nocompress'' is specified as a boot parameter, it +saves hibernation image without compression. In the meantime while the system is suspended you should not add/remove any of the hardware, write to the filesystems, etc.kernel/power/Kconfig +2
-0@@ -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 usuallykernel/power/hibernate.c +10
-0@@ -29,6 +29,7 @@#include "power.h" +static int nocompress =0 ;static int noresume =0 ;static char resume_file[256 ] =CONFIG_PM_STD_PARTITION ;dev_t swsusp_resume_device;@@ -630,6 +631,8 @@ int hibernate(void)if (hibernation_mode ==HIBERNATION_PLATFORM ) flags |=SF_PLATFORM_MODE ; +if (nocompress) + flags |=SF_NOCOMPRESS_MODE ;pr_debug ("PM: writing image.\n" ); error =swsusp_write (flags);swsusp_free ();@@ -996,12 +999,19 @@ static int __init resume_offset_setup(char *str)return 1 ; } +static int __init nocompress_setup (char *str) +{ + nocompress =1 ; +return 1 ; +} +static int __init noresume_setup (char *str) { noresume =1 ;return 1 ; } +__setup ("nocompress" , nocompress_setup);__setup ("noresume" , noresume_setup);__setup ("resume_offset=" , resume_offset_setup);__setup ("resume=" , resume_setup);kernel/power/power.h +1
-0@@ -134,6 +134,7 @@ extern int swsusp_swap_in_use(void);* the image header. */ #define SF_PLATFORM_MODE 1 +#define SF_NOCOMPRESS_MODE 2/* kernel/power/hibernate.c */ extern int swsusp_check (void );kernel/power/swap.c +282
-7@@ -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; } +/* 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 one time. */ +#define LZO_UNC_PAGES 32 +#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) +/** * save_image - save the suspend image data */ @@ -404,6 +417,133 @@ static int save_image(struct swap_map_handle *handle,return ret; } + +/** +* save_image_lzo - save the suspend image data, LZO compress +*/ + +static int save_image_lzo (struct swap_map_handle *handle, +struct snapshot_handle *snapshot, +unsigned int nr_to_write) +{ +unsigned int m; +int ret; +int nr_pages; +int err2; +struct bio *bio; +struct timeval start; +struct timeval stop; +size_t off, unc_len, cmp_len; +unsigned char *unc, *cmp, *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 ; + } + + unc =vmalloc (LZO_UNC_SIZE ); +if (!unc) { +printk (KERN_ERR "PM: Failed to allocate LZO uncompressed\n" ); +vfree (wrk); +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); +vfree (wrk); +free_page ((unsigned long )page); +return -ENOMEM ; + } + +printk (KERN_INFO "PM: Saving image data pages (%u pages) ... " , + nr_to_write); + m = nr_to_write /100 ; +if (!m) + m =1 ; + nr_pages =0 ; + bio =NULL ; +do_gettimeofday (&start); +while (1 ) { +for (off =0 ; off <LZO_UNC_SIZE ; off +=PAGE_SIZE ) { + ret =snapshot_read_next (snapshot); +if (ret <0 ) + goto out_finish; + +if (!ret) +break ; + +memcpy (unc + off,data_of (*snapshot),PAGE_SIZE ); + +if (!(nr_pages % m)) +printk (KERN_CONT "\b\b\b\b%3d%%" , nr_pages / m); + nr_pages++; + } + +if (!off) +break ; + + unc_len = off; + ret =lzo1x_1_compress (unc, unc_len, + cmp +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 compressed length\n" ); + ret =-1 ; +break ; + } + + *(size_t *)cmp = 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 (off =0 ; off <LZO_HEADER + cmp_len; off +=PAGE_SIZE ) { +memcpy (page, cmp + off,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) + ret = err2; +if (!ret) +printk (KERN_CONT "\b\b\b\bdone\n" ); +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; +} +/** * enough_swap - Make sure we have enough swap to save the image. * @@ -411,12 +551,18 @@ static int save_image(struct swap_map_handle *handle,* space avaiable from the resume partition. */ -static int enough_swap (unsigned int nr_pages) +static int enough_swap (unsigned int nr_pages,unsigned int flags) {unsigned int free_swap =count_swap_pages (root_swap,1 ); +unsigned int required =PAGES_FOR_IO ; + +if (flags &SF_NOCOMPRESS_MODE ) + required += nr_pages; +else + required += (nr_pages *LZO_CMP_PAGES ) /LZO_UNC_PAGES +1 ;pr_debug ("PM: Free swap pages: %u\n" , free_swap); -return free_swap > nr_pages +PAGES_FOR_IO ; +return free_swap > required; }/** @@ -443,7 +589,7 @@ int swsusp_write(unsigned int flags)printk (KERN_ERR "PM: Cannot get swap writer\n" );return error; } -if (!enough_swap (pages)) { +if (!enough_swap (pages, flags)) {printk (KERN_ERR "PM: Not enough free swap\n" ); error = -ENOSPC ; goto out_finish;@@ -458,8 +604,12 @@ int swsusp_write(unsigned int flags) } header = (struct swsusp_info *)data_of (snapshot); error =swap_write_page (&handle, header,NULL ); -if (!error) - error =save_image (&handle, &snapshot, pages -1 ); +if (!error) { +if (flags &SF_NOCOMPRESS_MODE ) + error =save_image (&handle, &snapshot, pages -1 ); +else + error =save_image_lzo (&handle, &snapshot, pages -1 ); + } out_finish: error =swap_writer_finish (&handle, flags, error);return error;@@ -590,6 +740,127 @@ static int load_image(struct swap_map_handle *handle, }/** +* load_image_lzo - load the image using the swap map handle, LZO +* decompress +* @handle and the snapshot handle @snapshot +* (assume there are @nr_pages pages to load) +*/ + +static int load_image_lzo (struct swap_map_handle *handle, +struct snapshot_handle *snapshot, +unsigned int nr_to_read) +{ +unsigned int m; +int error =0 ; +struct timeval start; +struct timeval stop; +unsigned nr_pages; +size_t off, unc_len, cmp_len; +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); + m = nr_to_read /100 ; +if (!m) + m =1 ; + nr_pages =0 ; +do_gettimeofday (&start); + + error =snapshot_write_next (snapshot); +if (error <=0 ) + goto out_finish; + +for ( ; ; ) { + error =swap_read_page (handle, page,NULL );/* sync */ +if (error) +break ; + + cmp_len = *(size_t *)page; +if (unlikely (!cmp_len || + cmp_len >lzo1x_worst_compress (LZO_UNC_SIZE ))) { +printk (KERN_ERR "PM: Invalid LZO compressed length\n" ); + error =-1 ; +break ; + } + +memcpy (cmp, page,PAGE_SIZE ); +for (off =PAGE_SIZE ; off <LZO_HEADER + cmp_len; off +=PAGE_SIZE ) { + error =swap_read_page (handle, page,NULL );/* sync */ +if (error) + goto out_finish; + +memcpy (cmp + off, page,PAGE_SIZE ); + } + + unc_len =LZO_UNC_SIZE ; + error =lzo1x_decompress_safe (cmp +LZO_HEADER , cmp_len, + unc, &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 -1 ))) { +printk (KERN_ERR "PM: Invalid LZO uncompressed length\n" ); + error =-1 ; +break ; + } + +for (off =0 ; off < unc_len; off +=PAGE_SIZE ) { +memcpy (data_of (*snapshot), unc + off,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; + } + } + +out_finish: +do_gettimeofday (&stop); +if (!error) { +printk ("\b\b\b\bdone\n" ); +snapshot_write_finalize (snapshot); +if (!snapshot_image_loaded (snapshot)) + error = -ENODATA ; + }else +printk ("\n" ); +swsusp_show_speed (&start, &stop, nr_to_read,"Read" ); + +vfree (cmp); +vfree (unc); +free_page ((unsigned long )page); + +return error; +} + +/** * swsusp_read - read the hibernation image. * @flags_p: flags passed by the "frozen" kernel in the image header should * be written into this memeory location @@ -612,8 +883,12 @@ int swsusp_read(unsigned int *flags_p) goto end;if (!error) error =swap_read_page (&handle, header,NULL ); -if (!error) - error =load_image (&handle, &snapshot, header->pages -1 ); +if (!error) { +if (*flags_p &SF_NOCOMPRESS_MODE ) + error =load_image (&handle, &snapshot, header->pages -1 ); +else + error =load_image_lzo (&handle, &snapshot, header->pages -1 ); + }swap_reader_finish (&handle); end:if (!error)
Quoted 3 lines · Rafael J. Wysocki
OK, so changelog please?
Or should I use the original one?
Yeah, the old one should be good enough, I guess. Here it is once again:
Compress hibernation image with LZO in order to save on I/O and therefore time to hibernate/thaw.
Signature · 1 line
Bojan
Quoted 11 lines · Bojan Smojver
Quoted 1 line · Bojan Smojver
Good point. Will change.
Here you go:
Documentation/power/swsusp.txt | 3 +- kernel/power/Kconfig | 2 + kernel/power/hibernate.c | 10 ++ kernel/power/power.h | 1 + kernel/power/swap.c | 289 +++++++++++++++++++++++++++++++++++++++- 5 files changed, 297 insertions(+), 8 deletions(-)
OK
I applied this patch to suspend-2.6/linux-next
(http://git.kernel.org/?p=linux/kernel/git/rafael/suspend-2.6.git;a=shortlog;h=refs/heads/linux-next)
but with some modifications, so please double check.
Thanks,
Rafael
Quoted 3 lines · Rafael J. Wysocki
I applied this patch to suspend-2.6/linux-next
(http://git.kernel.org/?p=linux/kernel/git/rafael/suspend-2.6.git;a=shortlog;h=refs/heads/linux-next)
but with some modifications, so please double check.
Thanks.
I may just pull the tree and give a try. Visually, it looked OK.
Signature · 1 line
Bojan
Hi.
Sorry for being slow to reply, but to my mind, having two copies of the whole save and load routines is just plain ugly and makes maintainability a nightmare.
Every time I've thought there's been a problem with compression or decompression, it has turned out to be a problem in my code instead. Let's not waste time and effort duplicating code unnecessarily.
Nigel's 2c.
Quoted 5 lines · Nigel Cunningham
Hi.
Sorry for being slow to reply, but to my mind, having two copies of the whole save and load routines is just plain ugly and makes maintainability a nightmare.
Well, how much code is actually duplicated by having separate routines for with and without compression? I don't think it's too much.
Quoted 3 lines
Every time I've thought there's been a problem with compression or decompression, it has turned out to be a problem in my code instead. Let's not waste time and effort duplicating code unnecessarily.
Having two separate functions for image saving/restoring, depending on cartain condition, is not wrong in principle as long as they are sufficiently different. So going back to the previous question, aren't they?
An alternative is to put several conditionals into a sensitive piece of code, which I'd rather prefer to avoid.
Thanks,
Rafael
Hi.
Quoted 14 lines · Rafael J. Wysocki
Quoted 5 lines · Nigel Cunningham
Hi.
Sorry for being slow to reply, but to my mind, having two copies of the whole save and load routines is just plain ugly and makes maintainability a nightmare.
Well, how much code is actually duplicated by having separate routines for with and without compression? I don't think it's too much.
Quoted 3 lines
Every time I've thought there's been a problem with compression or decompression, it has turned out to be a problem in my code instead. Let's not waste time and effort duplicating code unnecessarily.
Quoted 6 lines
Having two separate functions for image saving/restoring, depending on cartain condition, is not wrong in principle as long as they are sufficiently different. So going back to the previous question, aren't they?
An alternative is to put several conditionals into a sensitive piece of code, which I'd rather prefer to avoid.
I understand that it's better to have them separate if you have them both. What I'm arguing is that it's pointless to have the two versions in the first place. We don't normally add options to disable improvements, and there's no need to do so this time.
Regards,
Nigel
Quoted 10 lines · Bojan Smojver
Quoted 4 lines · Rafael J. Wysocki
I'm not a big fan of select in
Kconfig. It ususally causes one to be surprised with the final choice of options in .config.Does that work at all if
CRYPTO_ALGAPIis not set?My understanding is that these (LZO_COMPRESS/LZO_DECOMPRESS) do not depend on anything. But, I can test, just to be sure.
Quoted 8 lines
Quoted 4 lines
I'd rather wouldn't like the above to be unconditional. If not for anything else, I think it would be nice to be able to switch the compression off for debugging purposes (like when we're not sure why the image is corrupted or similar). A kernel command line switch would suffice for that IMO.
OK. I think the cleanest way may be to have a set of two "real" save_image()/load_image() functions. One set with compression, the other without, which are then called by save_image()/load_image(). OK?
Sounds reasonable.
Quoted 10 lines
Quoted 5 lines
kernel/power/swap.c static int save_image(struct swap_map_handle *handle, · line 446
Quoted 2 lines
kernel/power/swap.c +1
-0+if (!i)break ;This statement is profoundly hard to decode. Perhaps there's a better name for 'i'? Like 'size' or similar?
Right. Maybe I'm a bit too economical with names. Usually things like 'i' stand for a generic counter, which this is. I'll use something else if you don't like it.
Quoted 8 lines
Quoted 3 lines
Also please make it possible to switch the compression off here too.
The comments above seem to apply to the remaining code too.
OK, I'll prepare a patch that has compression/decompression as optional, using kernel command line switch. Obviously, if one attempts to thaw from an image that is not in an expected state (compressed/not compressed), this will wreak havoc. But, I guess that's understandable.
Yes, it is.
Thanks,
Rafael