linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] PM: HIBERNATION: skip the swap size check if the snapshot image size is anticipative
@ 2011-11-25  8:07 Barry Song
  2011-11-25 10:21 ` Matthieu CASTET
  2011-12-15  8:50 ` Barry Song
  0 siblings, 2 replies; 9+ messages in thread
From: Barry Song @ 2011-11-25  8:07 UTC (permalink / raw)
  To: pavel, rjw, linux-pm
  Cc: linux-arm-kernel, linux-kernel, workgroup.linux, Barry Song,
	Barry Song, Xiangzhen Ye

From: Barry Song <baohua.song@csr.com>

Current swsusp requires swap partitions even larger than real saved pages
based on the worst compression ratio:
but for an embedded system, which has limited storage space, then it might
can't give the large partition to save snapshot.
In the another way, some embedded systems can definitely know the most size
needed for snapshot since they run some specific application lists.
So this patch provides the possibility for users to tell kernel even
the system has a little snapshot partition, but it is still enough.
For example, if the system need to save 120MB memory, origin swsusp will require
a 130MB partition to save snapshot. but if users know 30MB is enough for them(
compressed image will be less than 30MB), they just make a 30MB partition by
echo 0 > /sys/power/check_swap_size

Signed-off-by: Barry Song <Baohua.Song@csr.com>
Cc: Xiangzhen Ye <Xiangzhen.Ye@csr.com>
---
 -v2:drop swap_enough bootargs and use /sys/power/check_swap_size node

 Documentation/power/interface.txt |    5 +++++
 kernel/power/hibernate.c          |   22 ++++++++++++++++++++++
 kernel/power/power.h              |    2 ++
 kernel/power/swap.c               |    9 +++++++++
 4 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/Documentation/power/interface.txt b/Documentation/power/interface.txt
index c537834..5e205f0 100644
--- a/Documentation/power/interface.txt
+++ b/Documentation/power/interface.txt
@@ -47,6 +47,11 @@ Writing to this file will accept one of
        'testproc'
        'test'
 
+/sys/power/check_swap_size controls whether we can skip checking the swap
+partition size by worst compression ratio. If users know the swap partition
+is enough for compressed snapshot, write 0 to /sys/power/check_swap_size.
+It is useful for an embedded system with known running softwares.
+
 /sys/power/image_size controls the size of the image created by
 the suspend-to-disk mechanism.  It can be written a string
 representing a non-negative integer that will be used as an upper
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 1c53f7f..5552473 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -1024,11 +1024,33 @@ static ssize_t reserved_size_store(struct kobject *kobj,
 
 power_attr(reserved_size);
 
+static ssize_t check_swap_size_show(struct kobject *kobj, struct kobj_attribute *attr,
+			       char *buf)
+{
+	return sprintf(buf, "%d\n", check_swap_size);
+}
+
+static ssize_t check_swap_size_store(struct kobject *kobj, struct kobj_attribute *attr,
+				const char *buf, size_t n)
+{
+	int check_size;
+
+	if (sscanf(buf, "%d", &check_size) == 1) {
+		check_swap_size = check_size;
+		return n;
+	}
+
+	return -EINVAL;
+}
+
+power_attr(check_swap_size);
+
 static struct attribute * g[] = {
 	&disk_attr.attr,
 	&resume_attr.attr,
 	&image_size_attr.attr,
 	&reserved_size_attr.attr,
+	&check_swap_size_attr.attr,
 	NULL,
 };
 
diff --git a/kernel/power/power.h b/kernel/power/power.h
index 23a2db1..4f0fa78 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -74,6 +74,8 @@ static struct kobj_attribute _name##_attr = {	\
 
 /* Preferred image size in bytes (default 500 MB) */
 extern unsigned long image_size;
+/* If 0, skip checking whether the swap size is enough for compressed snapshot */
+extern int check_swap_size;
 /* Size of memory reserved for drivers (default SPARE_PAGES x PAGE_SIZE) */
 extern unsigned long reserved_size;
 extern int in_suspend;
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 11a594c..db90195 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -37,6 +37,12 @@
 #define HIBERNATE_SIG	"S1SUSPEND"
 
 /*
+ * if users know swap partitions are enough for compressed snapshots,
+ * echo 0 > /sys/power/check_swap_size
+ */
+int check_swap_size = 1;
+
+/*
  *	The swap map is a data structure used for keeping track of each page
  *	written to a swap partition.  It consists of many swap_map_page
  *	structures that contain each an array of MAP_PAGE_ENTRIES swap entries.
@@ -772,6 +778,9 @@ static int enough_swap(unsigned int nr_pages, unsigned int flags)
 	unsigned int free_swap = count_swap_pages(root_swap, 1);
 	unsigned int required;
 
+	if (!check_swap_size)
+		return 1;
+
 	pr_debug("PM: Free swap pages: %u\n", free_swap);
 
 	required = PAGES_FOR_IO + ((flags & SF_NOCOMPRESS_MODE) ?
-- 
1.7.1



Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] PM: HIBERNATION: skip the swap size check if the snapshot image size is anticipative
  2011-11-25  8:07 [PATCH v2] PM: HIBERNATION: skip the swap size check if the snapshot image size is anticipative Barry Song
@ 2011-11-25 10:21 ` Matthieu CASTET
  2011-11-25 13:42   ` Barry Song
  2011-12-15  8:50 ` Barry Song
  1 sibling, 1 reply; 9+ messages in thread
From: Matthieu CASTET @ 2011-11-25 10:21 UTC (permalink / raw)
  To: Barry Song
  Cc: pavel, rjw, linux-pm, workgroup.linux, Xiangzhen Ye,
	linux-kernel, linux-arm-kernel, Barry Song

Hi,

but did embedded system will really use swap interface ?

Often they don't have swap and can already save suspend image in disk/nand :
- SNAPSHOT_CREATE_IMAGE
- read it from /dev/snapshot and write it to a file.

The only problem with the current interface, is that you can't SNAPSHOT_UNFREEZE
before reading the image.
But this is need for some filesystem that have thread.


For example we have a demo where we save the suspend image in a ubi volume and
restore it from bootloader.


Matthieu

Barry Song a écrit :
> From: Barry Song <baohua.song@csr.com>
> 
> Current swsusp requires swap partitions even larger than real saved pages
> based on the worst compression ratio:
> but for an embedded system, which has limited storage space, then it might
> can't give the large partition to save snapshot.
> In the another way, some embedded systems can definitely know the most size
> needed for snapshot since they run some specific application lists.
> So this patch provides the possibility for users to tell kernel even
> the system has a little snapshot partition, but it is still enough.
> For example, if the system need to save 120MB memory, origin swsusp will require
> a 130MB partition to save snapshot. but if users know 30MB is enough for them(
> compressed image will be less than 30MB), they just make a 30MB partition by
> echo 0 > /sys/power/check_swap_size
> 
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> Cc: Xiangzhen Ye <Xiangzhen.Ye@csr.com>
> ---
>  -v2:drop swap_enough bootargs and use /sys/power/check_swap_size node
> 
>  Documentation/power/interface.txt |    5 +++++
>  kernel/power/hibernate.c          |   22 ++++++++++++++++++++++
>  kernel/power/power.h              |    2 ++
>  kernel/power/swap.c               |    9 +++++++++
>  4 files changed, 38 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/power/interface.txt b/Documentation/power/interface.txt
> index c537834..5e205f0 100644
> --- a/Documentation/power/interface.txt
> +++ b/Documentation/power/interface.txt
> @@ -47,6 +47,11 @@ Writing to this file will accept one of
>         'testproc'
>         'test'
>  
> +/sys/power/check_swap_size controls whether we can skip checking the swap
> +partition size by worst compression ratio. If users know the swap partition
> +is enough for compressed snapshot, write 0 to /sys/power/check_swap_size.
> +It is useful for an embedded system with known running softwares.
> +
>  /sys/power/image_size controls the size of the image created by
>  the suspend-to-disk mechanism.  It can be written a string
>  representing a non-negative integer that will be used as an upper
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 1c53f7f..5552473 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -1024,11 +1024,33 @@ static ssize_t reserved_size_store(struct kobject *kobj,
>  
>  power_attr(reserved_size);
>  
> +static ssize_t check_swap_size_show(struct kobject *kobj, struct kobj_attribute *attr,
> +			       char *buf)
> +{
> +	return sprintf(buf, "%d\n", check_swap_size);
> +}
> +
> +static ssize_t check_swap_size_store(struct kobject *kobj, struct kobj_attribute *attr,
> +				const char *buf, size_t n)
> +{
> +	int check_size;
> +
> +	if (sscanf(buf, "%d", &check_size) == 1) {
> +		check_swap_size = check_size;
> +		return n;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +power_attr(check_swap_size);
> +
>  static struct attribute * g[] = {
>  	&disk_attr.attr,
>  	&resume_attr.attr,
>  	&image_size_attr.attr,
>  	&reserved_size_attr.attr,
> +	&check_swap_size_attr.attr,
>  	NULL,
>  };
>  
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 23a2db1..4f0fa78 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -74,6 +74,8 @@ static struct kobj_attribute _name##_attr = {	\
>  
>  /* Preferred image size in bytes (default 500 MB) */
>  extern unsigned long image_size;
> +/* If 0, skip checking whether the swap size is enough for compressed snapshot */
> +extern int check_swap_size;
>  /* Size of memory reserved for drivers (default SPARE_PAGES x PAGE_SIZE) */
>  extern unsigned long reserved_size;
>  extern int in_suspend;
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index 11a594c..db90195 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -37,6 +37,12 @@
>  #define HIBERNATE_SIG	"S1SUSPEND"
>  
>  /*
> + * if users know swap partitions are enough for compressed snapshots,
> + * echo 0 > /sys/power/check_swap_size
> + */
> +int check_swap_size = 1;
> +
> +/*
>   *	The swap map is a data structure used for keeping track of each page
>   *	written to a swap partition.  It consists of many swap_map_page
>   *	structures that contain each an array of MAP_PAGE_ENTRIES swap entries.
> @@ -772,6 +778,9 @@ static int enough_swap(unsigned int nr_pages, unsigned int flags)
>  	unsigned int free_swap = count_swap_pages(root_swap, 1);
>  	unsigned int required;
>  
> +	if (!check_swap_size)
> +		return 1;
> +
>  	pr_debug("PM: Free swap pages: %u\n", free_swap);
>  
>  	required = PAGES_FOR_IO + ((flags & SF_NOCOMPRESS_MODE) ?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] PM: HIBERNATION: skip the swap size check if the snapshot image size is anticipative
  2011-11-25 10:21 ` Matthieu CASTET
@ 2011-11-25 13:42   ` Barry Song
  0 siblings, 0 replies; 9+ messages in thread
From: Barry Song @ 2011-11-25 13:42 UTC (permalink / raw)
  To: Matthieu CASTET
  Cc: Barry Song, Xiangzhen Ye, linux-kernel, workgroup.linux, rjw,
	pavel, Barry Song, linux-pm, linux-arm-kernel

Hi Matthieu,

2011/11/25 Matthieu CASTET <matthieu.castet@parrot.com>
>
> Hi,
>
> but did embedded system will really use swap interface ?

if you self-define the hibernation flow, you don't need swap. but for
embedded system, we actually can still use the standard flow, swapon
before hibernation, and swapoff after restore. so the swap partition
only works as a partition to save used pages based on swap format. it
is not a runtime swap partition at all.

>
> Often they don't have swap and can already save suspend image in disk/nand :
> - SNAPSHOT_CREATE_IMAGE
> - read it from /dev/snapshot and write it to a file.
>
> The only problem with the current interface, is that you can't SNAPSHOT_UNFREEZE
> before reading the image.
> But this is need for some filesystem that have thread.
>
>
> For example we have a demo where we save the suspend image in a ubi volume and
> restore it from bootloader.
yes. we can definitely resume snapshot from bootloader directly, that
depends on your self-defined hibernation flow. if that works well, it
will save about 0.5s kernel boot time for a typical cortex-a9 ARM 1Ghz
system.

but you still need to take care you only compress and save used pages
but not all pages. sync, drop page caches and kill non-system
processes will help much to decrease hibernation image size. and
standard hibernation flow will only save used pages, that is very
important for decreasing snapshot size and restore faster.

i am happy to hear you have a demo. do you have a website to show your
demo and techinical details?

>
>
> Matthieu
>
-barry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] PM: HIBERNATION: skip the swap size check if the snapshot image size is anticipative
  2011-11-25  8:07 [PATCH v2] PM: HIBERNATION: skip the swap size check if the snapshot image size is anticipative Barry Song
  2011-11-25 10:21 ` Matthieu CASTET
@ 2011-12-15  8:50 ` Barry Song
  2012-01-05 18:46   ` Pavel Machek
  1 sibling, 1 reply; 9+ messages in thread
From: Barry Song @ 2011-12-15  8:50 UTC (permalink / raw)
  To: Barry Song
  Cc: pavel, rjw, linux-pm, workgroup.linux, Xiangzhen Ye,
	linux-kernel, linux-arm-kernel, Barry Song

Pavel ,Rafael, any feedback?
-barry

2011/11/25 Barry Song <Barry.Song@csr.com>:
> From: Barry Song <baohua.song@csr.com>
>
> Current swsusp requires swap partitions even larger than real saved pages
> based on the worst compression ratio:
> but for an embedded system, which has limited storage space, then it might
> can't give the large partition to save snapshot.
> In the another way, some embedded systems can definitely know the most size
> needed for snapshot since they run some specific application lists.
> So this patch provides the possibility for users to tell kernel even
> the system has a little snapshot partition, but it is still enough.
> For example, if the system need to save 120MB memory, origin swsusp will require
> a 130MB partition to save snapshot. but if users know 30MB is enough for them(
> compressed image will be less than 30MB), they just make a 30MB partition by
> echo 0 > /sys/power/check_swap_size
>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> Cc: Xiangzhen Ye <Xiangzhen.Ye@csr.com>
> ---
>  -v2:drop swap_enough bootargs and use /sys/power/check_swap_size node
>
>  Documentation/power/interface.txt |    5 +++++
>  kernel/power/hibernate.c          |   22 ++++++++++++++++++++++
>  kernel/power/power.h              |    2 ++
>  kernel/power/swap.c               |    9 +++++++++
>  4 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/power/interface.txt b/Documentation/power/interface.txt
> index c537834..5e205f0 100644
> --- a/Documentation/power/interface.txt
> +++ b/Documentation/power/interface.txt
> @@ -47,6 +47,11 @@ Writing to this file will accept one of
>        'testproc'
>        'test'
>
> +/sys/power/check_swap_size controls whether we can skip checking the swap
> +partition size by worst compression ratio. If users know the swap partition
> +is enough for compressed snapshot, write 0 to /sys/power/check_swap_size.
> +It is useful for an embedded system with known running softwares.
> +
>  /sys/power/image_size controls the size of the image created by
>  the suspend-to-disk mechanism.  It can be written a string
>  representing a non-negative integer that will be used as an upper
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 1c53f7f..5552473 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -1024,11 +1024,33 @@ static ssize_t reserved_size_store(struct kobject *kobj,
>
>  power_attr(reserved_size);
>
> +static ssize_t check_swap_size_show(struct kobject *kobj, struct kobj_attribute *attr,
> +                              char *buf)
> +{
> +       return sprintf(buf, "%d\n", check_swap_size);
> +}
> +
> +static ssize_t check_swap_size_store(struct kobject *kobj, struct kobj_attribute *attr,
> +                               const char *buf, size_t n)
> +{
> +       int check_size;
> +
> +       if (sscanf(buf, "%d", &check_size) == 1) {
> +               check_swap_size = check_size;
> +               return n;
> +       }
> +
> +       return -EINVAL;
> +}
> +
> +power_attr(check_swap_size);
> +
>  static struct attribute * g[] = {
>        &disk_attr.attr,
>        &resume_attr.attr,
>        &image_size_attr.attr,
>        &reserved_size_attr.attr,
> +       &check_swap_size_attr.attr,
>        NULL,
>  };
>
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 23a2db1..4f0fa78 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -74,6 +74,8 @@ static struct kobj_attribute _name##_attr = { \
>
>  /* Preferred image size in bytes (default 500 MB) */
>  extern unsigned long image_size;
> +/* If 0, skip checking whether the swap size is enough for compressed snapshot */
> +extern int check_swap_size;
>  /* Size of memory reserved for drivers (default SPARE_PAGES x PAGE_SIZE) */
>  extern unsigned long reserved_size;
>  extern int in_suspend;
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index 11a594c..db90195 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -37,6 +37,12 @@
>  #define HIBERNATE_SIG  "S1SUSPEND"
>
>  /*
> + * if users know swap partitions are enough for compressed snapshots,
> + * echo 0 > /sys/power/check_swap_size
> + */
> +int check_swap_size = 1;
> +
> +/*
>  *     The swap map is a data structure used for keeping track of each page
>  *     written to a swap partition.  It consists of many swap_map_page
>  *     structures that contain each an array of MAP_PAGE_ENTRIES swap entries.
> @@ -772,6 +778,9 @@ static int enough_swap(unsigned int nr_pages, unsigned int flags)
>        unsigned int free_swap = count_swap_pages(root_swap, 1);
>        unsigned int required;
>
> +       if (!check_swap_size)
> +               return 1;
> +
>        pr_debug("PM: Free swap pages: %u\n", free_swap);
>
>        required = PAGES_FOR_IO + ((flags & SF_NOCOMPRESS_MODE) ?
> --
> 1.7.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] PM: HIBERNATION: skip the swap size check if the snapshot image size is anticipative
  2011-12-15  8:50 ` Barry Song
@ 2012-01-05 18:46   ` Pavel Machek
  2012-01-06  2:20     ` Barry Song
  0 siblings, 1 reply; 9+ messages in thread
From: Pavel Machek @ 2012-01-05 18:46 UTC (permalink / raw)
  To: Barry Song
  Cc: Barry Song, rjw, linux-pm, workgroup.linux, Xiangzhen Ye,
	linux-kernel, linux-arm-kernel, Barry Song

Hi!

Is the check even useful these days? Should we remove it altogether?

       	     	  	       	     	       Pavel


> Pavel ,Rafael, any feedback?
> -barry
> 
> 2011/11/25 Barry Song <Barry.Song@csr.com>:
> > From: Barry Song <baohua.song@csr.com>
> >
> > Current swsusp requires swap partitions even larger than real saved pages
> > based on the worst compression ratio:
> > but for an embedded system, which has limited storage space, then it might
> > can't give the large partition to save snapshot.
> > In the another way, some embedded systems can definitely know the most size
> > needed for snapshot since they run some specific application lists.
> > So this patch provides the possibility for users to tell kernel even
> > the system has a little snapshot partition, but it is still enough.
> > For example, if the system need to save 120MB memory, origin swsusp will require
> > a 130MB partition to save snapshot. but if users know 30MB is enough for them(
> > compressed image will be less than 30MB), they just make a 30MB partition by
> > echo 0 > /sys/power/check_swap_size
> >
> > Signed-off-by: Barry Song <Baohua.Song@csr.com>
> > Cc: Xiangzhen Ye <Xiangzhen.Ye@csr.com>
> > ---
> >  -v2:drop swap_enough bootargs and use /sys/power/check_swap_size node
> >
> >  Documentation/power/interface.txt |    5 +++++
> >  kernel/power/hibernate.c          |   22 ++++++++++++++++++++++
> >  kernel/power/power.h              |    2 ++
> >  kernel/power/swap.c               |    9 +++++++++
> >  4 files changed, 38 insertions(+), 0 deletions(-)
> >
> > diff --git a/Documentation/power/interface.txt b/Documentation/power/interface.txt
> > index c537834..5e205f0 100644
> > --- a/Documentation/power/interface.txt
> > +++ b/Documentation/power/interface.txt
> > @@ -47,6 +47,11 @@ Writing to this file will accept one of
> >        'testproc'
> >        'test'
> >
> > +/sys/power/check_swap_size controls whether we can skip checking the swap
> > +partition size by worst compression ratio. If users know the swap partition
> > +is enough for compressed snapshot, write 0 to /sys/power/check_swap_size.
> > +It is useful for an embedded system with known running softwares.
> > +
> >  /sys/power/image_size controls the size of the image created by
> >  the suspend-to-disk mechanism.  It can be written a string
> >  representing a non-negative integer that will be used as an upper
> > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> > index 1c53f7f..5552473 100644
> > --- a/kernel/power/hibernate.c
> > +++ b/kernel/power/hibernate.c
> > @@ -1024,11 +1024,33 @@ static ssize_t reserved_size_store(struct kobject *kobj,
> >
> >  power_attr(reserved_size);
> >
> > +static ssize_t check_swap_size_show(struct kobject *kobj, struct kobj_attribute *attr,
> > +                              char *buf)
> > +{
> > +       return sprintf(buf, "%d\n", check_swap_size);
> > +}
> > +
> > +static ssize_t check_swap_size_store(struct kobject *kobj, struct kobj_attribute *attr,
> > +                               const char *buf, size_t n)
> > +{
> > +       int check_size;
> > +
> > +       if (sscanf(buf, "%d", &check_size) == 1) {
> > +               check_swap_size = check_size;
> > +               return n;
> > +       }
> > +
> > +       return -EINVAL;
> > +}
> > +
> > +power_attr(check_swap_size);
> > +
> >  static struct attribute * g[] = {
> >        &disk_attr.attr,
> >        &resume_attr.attr,
> >        &image_size_attr.attr,
> >        &reserved_size_attr.attr,
> > +       &check_swap_size_attr.attr,
> >        NULL,
> >  };
> >
> > diff --git a/kernel/power/power.h b/kernel/power/power.h
> > index 23a2db1..4f0fa78 100644
> > --- a/kernel/power/power.h
> > +++ b/kernel/power/power.h
> > @@ -74,6 +74,8 @@ static struct kobj_attribute _name##_attr = { \
> >
> >  /* Preferred image size in bytes (default 500 MB) */
> >  extern unsigned long image_size;
> > +/* If 0, skip checking whether the swap size is enough for compressed snapshot */
> > +extern int check_swap_size;
> >  /* Size of memory reserved for drivers (default SPARE_PAGES x PAGE_SIZE) */
> >  extern unsigned long reserved_size;
> >  extern int in_suspend;
> > diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> > index 11a594c..db90195 100644
> > --- a/kernel/power/swap.c
> > +++ b/kernel/power/swap.c
> > @@ -37,6 +37,12 @@
> >  #define HIBERNATE_SIG  "S1SUSPEND"
> >
> >  /*
> > + * if users know swap partitions are enough for compressed snapshots,
> > + * echo 0 > /sys/power/check_swap_size
> > + */
> > +int check_swap_size = 1;
> > +
> > +/*
> >  *     The swap map is a data structure used for keeping track of each page
> >  *     written to a swap partition.  It consists of many swap_map_page
> >  *     structures that contain each an array of MAP_PAGE_ENTRIES swap entries.
> > @@ -772,6 +778,9 @@ static int enough_swap(unsigned int nr_pages, unsigned int flags)
> >        unsigned int free_swap = count_swap_pages(root_swap, 1);
> >        unsigned int required;
> >
> > +       if (!check_swap_size)
> > +               return 1;
> > +
> >        pr_debug("PM: Free swap pages: %u\n", free_swap);
> >
> >        required = PAGES_FOR_IO + ((flags & SF_NOCOMPRESS_MODE) ?
> > --
> > 1.7.1

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] PM: HIBERNATION: skip the swap size check if the snapshot image size is anticipative
  2012-01-05 18:46   ` Pavel Machek
@ 2012-01-06  2:20     ` Barry Song
  2012-01-08 21:31       ` Rafael J. Wysocki
  0 siblings, 1 reply; 9+ messages in thread
From: Barry Song @ 2012-01-06  2:20 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Barry Song, rjw, linux-pm, workgroup.linux, Xiangzhen Ye,
	linux-kernel, linux-arm-kernel, Barry Song

2012/1/6 Pavel Machek <pavel@ucw.cz>:
> Hi!
>
> Is the check even useful these days? Should we remove it altogether?

i think we can let users or distributions decide whether it is useful.
On PC, disk space is not an issue, people might run many applications
while doing hibernation, so snapshot is much big. an early check will
improve user experience because people don't need to wait a long time
and find space is not enough.
for embedded system, SoC solutions can know whether the space is
enough since they know what are running while doing hibernation, so
they can skip the check by setting the flag in sysfs.
that is why i had this patch sent.
>
>                                               Pavel
>
>
>> Pavel ,Rafael, any feedback?
>> -barry
>>
>> 2011/11/25 Barry Song <Barry.Song@csr.com>:
>> > From: Barry Song <baohua.song@csr.com>
>> >
>> > Current swsusp requires swap partitions even larger than real saved pages
>> > based on the worst compression ratio:
>> > but for an embedded system, which has limited storage space, then it might
>> > can't give the large partition to save snapshot.
>> > In the another way, some embedded systems can definitely know the most size
>> > needed for snapshot since they run some specific application lists.
>> > So this patch provides the possibility for users to tell kernel even
>> > the system has a little snapshot partition, but it is still enough.
>> > For example, if the system need to save 120MB memory, origin swsusp will require
>> > a 130MB partition to save snapshot. but if users know 30MB is enough for them(
>> > compressed image will be less than 30MB), they just make a 30MB partition by
>> > echo 0 > /sys/power/check_swap_size
>> >
>> > Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> > Cc: Xiangzhen Ye <Xiangzhen.Ye@csr.com>
>> > ---
>> >  -v2:drop swap_enough bootargs and use /sys/power/check_swap_size node
>> >
>> >  Documentation/power/interface.txt |    5 +++++
>> >  kernel/power/hibernate.c          |   22 ++++++++++++++++++++++
>> >  kernel/power/power.h              |    2 ++
>> >  kernel/power/swap.c               |    9 +++++++++
>> >  4 files changed, 38 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/Documentation/power/interface.txt b/Documentation/power/interface.txt
>> > index c537834..5e205f0 100644
>> > --- a/Documentation/power/interface.txt
>> > +++ b/Documentation/power/interface.txt
>> > @@ -47,6 +47,11 @@ Writing to this file will accept one of
>> >        'testproc'
>> >        'test'
>> >
>> > +/sys/power/check_swap_size controls whether we can skip checking the swap
>> > +partition size by worst compression ratio. If users know the swap partition
>> > +is enough for compressed snapshot, write 0 to /sys/power/check_swap_size.
>> > +It is useful for an embedded system with known running softwares.
>> > +
>> >  /sys/power/image_size controls the size of the image created by
>> >  the suspend-to-disk mechanism.  It can be written a string
>> >  representing a non-negative integer that will be used as an upper
>> > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
>> > index 1c53f7f..5552473 100644
>> > --- a/kernel/power/hibernate.c
>> > +++ b/kernel/power/hibernate.c
>> > @@ -1024,11 +1024,33 @@ static ssize_t reserved_size_store(struct kobject *kobj,
>> >
>> >  power_attr(reserved_size);
>> >
>> > +static ssize_t check_swap_size_show(struct kobject *kobj, struct kobj_attribute *attr,
>> > +                              char *buf)
>> > +{
>> > +       return sprintf(buf, "%d\n", check_swap_size);
>> > +}
>> > +
>> > +static ssize_t check_swap_size_store(struct kobject *kobj, struct kobj_attribute *attr,
>> > +                               const char *buf, size_t n)
>> > +{
>> > +       int check_size;
>> > +
>> > +       if (sscanf(buf, "%d", &check_size) == 1) {
>> > +               check_swap_size = check_size;
>> > +               return n;
>> > +       }
>> > +
>> > +       return -EINVAL;
>> > +}
>> > +
>> > +power_attr(check_swap_size);
>> > +
>> >  static struct attribute * g[] = {
>> >        &disk_attr.attr,
>> >        &resume_attr.attr,
>> >        &image_size_attr.attr,
>> >        &reserved_size_attr.attr,
>> > +       &check_swap_size_attr.attr,
>> >        NULL,
>> >  };
>> >
>> > diff --git a/kernel/power/power.h b/kernel/power/power.h
>> > index 23a2db1..4f0fa78 100644
>> > --- a/kernel/power/power.h
>> > +++ b/kernel/power/power.h
>> > @@ -74,6 +74,8 @@ static struct kobj_attribute _name##_attr = { \
>> >
>> >  /* Preferred image size in bytes (default 500 MB) */
>> >  extern unsigned long image_size;
>> > +/* If 0, skip checking whether the swap size is enough for compressed snapshot */
>> > +extern int check_swap_size;
>> >  /* Size of memory reserved for drivers (default SPARE_PAGES x PAGE_SIZE) */
>> >  extern unsigned long reserved_size;
>> >  extern int in_suspend;
>> > diff --git a/kernel/power/swap.c b/kernel/power/swap.c
>> > index 11a594c..db90195 100644
>> > --- a/kernel/power/swap.c
>> > +++ b/kernel/power/swap.c
>> > @@ -37,6 +37,12 @@
>> >  #define HIBERNATE_SIG  "S1SUSPEND"
>> >
>> >  /*
>> > + * if users know swap partitions are enough for compressed snapshots,
>> > + * echo 0 > /sys/power/check_swap_size
>> > + */
>> > +int check_swap_size = 1;
>> > +
>> > +/*
>> >  *     The swap map is a data structure used for keeping track of each page
>> >  *     written to a swap partition.  It consists of many swap_map_page
>> >  *     structures that contain each an array of MAP_PAGE_ENTRIES swap entries.
>> > @@ -772,6 +778,9 @@ static int enough_swap(unsigned int nr_pages, unsigned int flags)
>> >        unsigned int free_swap = count_swap_pages(root_swap, 1);
>> >        unsigned int required;
>> >
>> > +       if (!check_swap_size)
>> > +               return 1;
>> > +
>> >        pr_debug("PM: Free swap pages: %u\n", free_swap);
>> >
>> >        required = PAGES_FOR_IO + ((flags & SF_NOCOMPRESS_MODE) ?
>> > --
>> > 1.7.1
>
-barry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] PM: HIBERNATION: skip the swap size check if the snapshot image size is anticipative
  2012-01-06  2:20     ` Barry Song
@ 2012-01-08 21:31       ` Rafael J. Wysocki
  2012-01-09  2:50         ` Barry Song
  0 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2012-01-08 21:31 UTC (permalink / raw)
  To: Barry Song
  Cc: Pavel Machek, Barry Song, linux-pm, workgroup.linux,
	Xiangzhen Ye, linux-kernel, linux-arm-kernel, Barry Song

On Friday, January 06, 2012, Barry Song wrote:
> 2012/1/6 Pavel Machek <pavel@ucw.cz>:
> > Hi!
> >
> > Is the check even useful these days? Should we remove it altogether?
> 
> i think we can let users or distributions decide whether it is useful.
> On PC, disk space is not an issue, people might run many applications
> while doing hibernation, so snapshot is much big. an early check will
> improve user experience because people don't need to wait a long time
> and find space is not enough.
> for embedded system, SoC solutions can know whether the space is
> enough since they know what are running while doing hibernation, so
> they can skip the check by setting the flag in sysfs.
> that is why i had this patch sent.

I agree with Pavel that it's better to drop the check altogether.

The sysfs switch you're adding doesn't seem to be very useful, as PC
users won't touch it and whoever needs it to be 0, will always set it
that way and won't change it afterwards.

Thanks,
Rafael


> >> 2011/11/25 Barry Song <Barry.Song@csr.com>:
> >> > From: Barry Song <baohua.song@csr.com>
> >> >
> >> > Current swsusp requires swap partitions even larger than real saved pages
> >> > based on the worst compression ratio:
> >> > but for an embedded system, which has limited storage space, then it might
> >> > can't give the large partition to save snapshot.
> >> > In the another way, some embedded systems can definitely know the most size
> >> > needed for snapshot since they run some specific application lists.
> >> > So this patch provides the possibility for users to tell kernel even
> >> > the system has a little snapshot partition, but it is still enough.
> >> > For example, if the system need to save 120MB memory, origin swsusp will require
> >> > a 130MB partition to save snapshot. but if users know 30MB is enough for them(
> >> > compressed image will be less than 30MB), they just make a 30MB partition by
> >> > echo 0 > /sys/power/check_swap_size
> >> >
> >> > Signed-off-by: Barry Song <Baohua.Song@csr.com>
> >> > Cc: Xiangzhen Ye <Xiangzhen.Ye@csr.com>
> >> > ---
> >> >  -v2:drop swap_enough bootargs and use /sys/power/check_swap_size node
> >> >
> >> >  Documentation/power/interface.txt |    5 +++++
> >> >  kernel/power/hibernate.c          |   22 ++++++++++++++++++++++
> >> >  kernel/power/power.h              |    2 ++
> >> >  kernel/power/swap.c               |    9 +++++++++
> >> >  4 files changed, 38 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/Documentation/power/interface.txt b/Documentation/power/interface.txt
> >> > index c537834..5e205f0 100644
> >> > --- a/Documentation/power/interface.txt
> >> > +++ b/Documentation/power/interface.txt
> >> > @@ -47,6 +47,11 @@ Writing to this file will accept one of
> >> >        'testproc'
> >> >        'test'
> >> >
> >> > +/sys/power/check_swap_size controls whether we can skip checking the swap
> >> > +partition size by worst compression ratio. If users know the swap partition
> >> > +is enough for compressed snapshot, write 0 to /sys/power/check_swap_size.
> >> > +It is useful for an embedded system with known running softwares.
> >> > +
> >> >  /sys/power/image_size controls the size of the image created by
> >> >  the suspend-to-disk mechanism.  It can be written a string
> >> >  representing a non-negative integer that will be used as an upper
> >> > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> >> > index 1c53f7f..5552473 100644
> >> > --- a/kernel/power/hibernate.c
> >> > +++ b/kernel/power/hibernate.c
> >> > @@ -1024,11 +1024,33 @@ static ssize_t reserved_size_store(struct kobject *kobj,
> >> >
> >> >  power_attr(reserved_size);
> >> >
> >> > +static ssize_t check_swap_size_show(struct kobject *kobj, struct kobj_attribute *attr,
> >> > +                              char *buf)
> >> > +{
> >> > +       return sprintf(buf, "%d\n", check_swap_size);
> >> > +}
> >> > +
> >> > +static ssize_t check_swap_size_store(struct kobject *kobj, struct kobj_attribute *attr,
> >> > +                               const char *buf, size_t n)
> >> > +{
> >> > +       int check_size;
> >> > +
> >> > +       if (sscanf(buf, "%d", &check_size) == 1) {
> >> > +               check_swap_size = check_size;
> >> > +               return n;
> >> > +       }
> >> > +
> >> > +       return -EINVAL;
> >> > +}
> >> > +
> >> > +power_attr(check_swap_size);
> >> > +
> >> >  static struct attribute * g[] = {
> >> >        &disk_attr.attr,
> >> >        &resume_attr.attr,
> >> >        &image_size_attr.attr,
> >> >        &reserved_size_attr.attr,
> >> > +       &check_swap_size_attr.attr,
> >> >        NULL,
> >> >  };
> >> >
> >> > diff --git a/kernel/power/power.h b/kernel/power/power.h
> >> > index 23a2db1..4f0fa78 100644
> >> > --- a/kernel/power/power.h
> >> > +++ b/kernel/power/power.h
> >> > @@ -74,6 +74,8 @@ static struct kobj_attribute _name##_attr = { \
> >> >
> >> >  /* Preferred image size in bytes (default 500 MB) */
> >> >  extern unsigned long image_size;
> >> > +/* If 0, skip checking whether the swap size is enough for compressed snapshot */
> >> > +extern int check_swap_size;
> >> >  /* Size of memory reserved for drivers (default SPARE_PAGES x PAGE_SIZE) */
> >> >  extern unsigned long reserved_size;
> >> >  extern int in_suspend;
> >> > diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> >> > index 11a594c..db90195 100644
> >> > --- a/kernel/power/swap.c
> >> > +++ b/kernel/power/swap.c
> >> > @@ -37,6 +37,12 @@
> >> >  #define HIBERNATE_SIG  "S1SUSPEND"
> >> >
> >> >  /*
> >> > + * if users know swap partitions are enough for compressed snapshots,
> >> > + * echo 0 > /sys/power/check_swap_size
> >> > + */
> >> > +int check_swap_size = 1;
> >> > +
> >> > +/*
> >> >  *     The swap map is a data structure used for keeping track of each page
> >> >  *     written to a swap partition.  It consists of many swap_map_page
> >> >  *     structures that contain each an array of MAP_PAGE_ENTRIES swap entries.
> >> > @@ -772,6 +778,9 @@ static int enough_swap(unsigned int nr_pages, unsigned int flags)
> >> >        unsigned int free_swap = count_swap_pages(root_swap, 1);
> >> >        unsigned int required;
> >> >
> >> > +       if (!check_swap_size)
> >> > +               return 1;
> >> > +
> >> >        pr_debug("PM: Free swap pages: %u\n", free_swap);
> >> >
> >> >        required = PAGES_FOR_IO + ((flags & SF_NOCOMPRESS_MODE) ?
> >> > --
> >> > 1.7.1
> >
> -barry
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] PM: HIBERNATION: skip the swap size check if the snapshot image size is anticipative
  2012-01-08 21:31       ` Rafael J. Wysocki
@ 2012-01-09  2:50         ` Barry Song
  2012-01-09 21:46           ` Rafael J. Wysocki
  0 siblings, 1 reply; 9+ messages in thread
From: Barry Song @ 2012-01-09  2:50 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Pavel Machek, Barry Song, linux-pm, workgroup.linux,
	Xiangzhen Ye, linux-kernel, linux-arm-kernel, Barry Song

2012/1/9 Rafael J. Wysocki <rjw@sisk.pl>:
> On Friday, January 06, 2012, Barry Song wrote:
>> 2012/1/6 Pavel Machek <pavel@ucw.cz>:
>> > Hi!
>> >
>> > Is the check even useful these days? Should we remove it altogether?
>>
>> i think we can let users or distributions decide whether it is useful.
>> On PC, disk space is not an issue, people might run many applications
>> while doing hibernation, so snapshot is much big. an early check will
>> improve user experience because people don't need to wait a long time
>> and find space is not enough.
>> for embedded system, SoC solutions can know whether the space is
>> enough since they know what are running while doing hibernation, so
>> they can skip the check by setting the flag in sysfs.
>> that is why i had this patch sent.
>
> I agree with Pavel that it's better to drop the check altogether.
>
> The sysfs switch you're adding doesn't seem to be very useful, as PC
> users won't touch it and whoever needs it to be 0, will always set it
> that way and won't change it afterwards.

ok. if we don't have the check, in case swap partition is not enough,
writing failure will happen, system still can restore to normal
status:

for example, in the following test, only 27% data is written with a
small partition, "Restarting tasks ... done" will make system restore
to normal status.

[   11.2080 27%
[   11.403274] PM: Wrote uncompressed 34920 kbytes in 0.65 seconds (53.72 MB/s)
[   11.407649] PM: Wrote compressed 3500 kbytes in 0.65 seconds (5.38 MB/s)
[   11.447176] Restarting tasks ... done.
[   11.448801] ...


>
> Thanks,
> Rafael

-barry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] PM: HIBERNATION: skip the swap size check if the snapshot image size is anticipative
  2012-01-09  2:50         ` Barry Song
@ 2012-01-09 21:46           ` Rafael J. Wysocki
  0 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2012-01-09 21:46 UTC (permalink / raw)
  To: Barry Song
  Cc: Pavel Machek, Barry Song, linux-pm, workgroup.linux,
	Xiangzhen Ye, linux-kernel, linux-arm-kernel, Barry Song

On Monday, January 09, 2012, Barry Song wrote:
> 2012/1/9 Rafael J. Wysocki <rjw@sisk.pl>:
> > On Friday, January 06, 2012, Barry Song wrote:
> >> 2012/1/6 Pavel Machek <pavel@ucw.cz>:
> >> > Hi!
> >> >
> >> > Is the check even useful these days? Should we remove it altogether?
> >>
> >> i think we can let users or distributions decide whether it is useful.
> >> On PC, disk space is not an issue, people might run many applications
> >> while doing hibernation, so snapshot is much big. an early check will
> >> improve user experience because people don't need to wait a long time
> >> and find space is not enough.
> >> for embedded system, SoC solutions can know whether the space is
> >> enough since they know what are running while doing hibernation, so
> >> they can skip the check by setting the flag in sysfs.
> >> that is why i had this patch sent.
> >
> > I agree with Pavel that it's better to drop the check altogether.
> >
> > The sysfs switch you're adding doesn't seem to be very useful, as PC
> > users won't touch it and whoever needs it to be 0, will always set it
> > that way and won't change it afterwards.
> 
> ok. if we don't have the check, in case swap partition is not enough,
> writing failure will happen, system still can restore to normal
> status:
> 
> for example, in the following test, only 27% data is written with a
> small partition, "Restarting tasks ... done" will make system restore
> to normal status.
> 
> [   11.2080 27%
> [   11.403274] PM: Wrote uncompressed 34920 kbytes in 0.65 seconds (53.72 MB/s)
> [   11.407649] PM: Wrote compressed 3500 kbytes in 0.65 seconds (5.38 MB/s)
> [   11.447176] Restarting tasks ... done.
> [   11.448801] ...

That's exactly correct.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-01-09 21:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-25  8:07 [PATCH v2] PM: HIBERNATION: skip the swap size check if the snapshot image size is anticipative Barry Song
2011-11-25 10:21 ` Matthieu CASTET
2011-11-25 13:42   ` Barry Song
2011-12-15  8:50 ` Barry Song
2012-01-05 18:46   ` Pavel Machek
2012-01-06  2:20     ` Barry Song
2012-01-08 21:31       ` Rafael J. Wysocki
2012-01-09  2:50         ` Barry Song
2012-01-09 21:46           ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).