2006-01-23 23:31:20

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH] swsusp: use bytes as image size units

Hi,

This patch makes swsusp use bytes as the image size units, which is needed
for future compatibility.

The patch changes the behavior of the /sys/power/image_size attribute already
present in 2.6.16-rc1, so it is against this kernel.

Please apply (Pavel, please ack).

Greetings,
Rafael


Signed-off-by: Rafael J. Wysocki <[email protected]>

Documentation/power/interface.txt | 2 +-
Documentation/power/swsusp.txt | 2 +-
kernel/power/disk.c | 6 +++---
kernel/power/power.h | 4 ++--
kernel/power/swsusp.c | 8 ++++----
5 files changed, 11 insertions(+), 11 deletions(-)

Index: linux-2.6.16-rc1/kernel/power/power.h
===================================================================
--- linux-2.6.16-rc1.orig/kernel/power/power.h 2006-01-23 15:33:46.000000000 +0100
+++ linux-2.6.16-rc1/kernel/power/power.h 2006-01-23 15:43:40.000000000 +0100
@@ -51,8 +51,8 @@
extern unsigned int nr_copy_pages;
extern struct pbe *pagedir_nosave;

-/* Preferred image size in MB (default 500) */
-extern unsigned int image_size;
+/* Preferred image size in bytes (default 500 MB) */
+extern unsigned long image_size;

extern asmlinkage int swsusp_arch_suspend(void);
extern asmlinkage int swsusp_arch_resume(void);
Index: linux-2.6.16-rc1/kernel/power/swsusp.c
===================================================================
--- linux-2.6.16-rc1.orig/kernel/power/swsusp.c 2006-01-23 15:33:46.000000000 +0100
+++ linux-2.6.16-rc1/kernel/power/swsusp.c 2006-01-23 15:43:40.000000000 +0100
@@ -70,12 +70,12 @@
#include "power.h"

/*
- * Preferred image size in MB (tunable via /sys/power/image_size).
+ * Preferred image size in bytes (tunable via /sys/power/image_size).
* When it is set to N, swsusp will do its best to ensure the image
- * size will not exceed N MB, but if that is impossible, it will
+ * size will not exceed N bytes, but if that is impossible, it will
* try to create the smallest image possible.
*/
-unsigned int image_size = 500;
+unsigned long image_size = 500 * 1024 * 1024;

#ifdef CONFIG_HIGHMEM
unsigned int count_highmem_pages(void);
@@ -590,7 +590,7 @@
if (!tmp)
return -ENOMEM;
pages += tmp;
- } else if (size > (image_size * 1024 * 1024) / PAGE_SIZE) {
+ } else if (size > image_size / PAGE_SIZE) {
tmp = shrink_all_memory(SHRINK_BITE);
pages += tmp;
}
Index: linux-2.6.16-rc1/Documentation/power/swsusp.txt
===================================================================
--- linux-2.6.16-rc1.orig/Documentation/power/swsusp.txt 2006-01-23 15:33:15.000000000 +0100
+++ linux-2.6.16-rc1/Documentation/power/swsusp.txt 2006-01-23 15:43:40.000000000 +0100
@@ -27,7 +27,7 @@

echo platform > /sys/power/disk; echo disk > /sys/power/state

-If you want to limit the suspend image size to N megabytes, do
+If you want to limit the suspend image size to N bytes, do

echo N > /sys/power/image_size

Index: linux-2.6.16-rc1/Documentation/power/interface.txt
===================================================================
--- linux-2.6.16-rc1.orig/Documentation/power/interface.txt 2006-01-23 15:33:15.000000000 +0100
+++ linux-2.6.16-rc1/Documentation/power/interface.txt 2006-01-23 15:43:40.000000000 +0100
@@ -44,7 +44,7 @@
/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
-limit of the image size, in megabytes. The suspend-to-disk mechanism will
+limit of the image size, in bytes. The suspend-to-disk mechanism will
do its best to ensure the image size will not exceed that number. However,
if this turns out to be impossible, it will try to suspend anyway using the
smallest image possible. In particular, if "0" is written to this file, the
Index: linux-2.6.16-rc1/kernel/power/disk.c
===================================================================
--- linux-2.6.16-rc1.orig/kernel/power/disk.c 2006-01-23 15:33:46.000000000 +0100
+++ linux-2.6.16-rc1/kernel/power/disk.c 2006-01-23 15:43:40.000000000 +0100
@@ -367,14 +367,14 @@

static ssize_t image_size_show(struct subsystem * subsys, char *buf)
{
- return sprintf(buf, "%u\n", image_size);
+ return sprintf(buf, "%lu\n", image_size);
}

static ssize_t image_size_store(struct subsystem * subsys, const char * buf, size_t n)
{
- unsigned int size;
+ unsigned long size;

- if (sscanf(buf, "%u", &size) == 1) {
+ if (sscanf(buf, "%lu", &size) == 1) {
image_size = size;
return n;
}


2006-01-23 23:57:22

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] swsusp: use bytes as image size units

On Tue, Jan 24, 2006 at 12:32:26AM +0100, Rafael J. Wysocki wrote:
> Hi,
>
> This patch makes swsusp use bytes as the image size units, which is needed
> for future compatibility.

With what ? I don't see how clipping this range to a maximum of 4GB
will future-proof anything. What happens in a few years time when
I want to suspend my 8GB laptop ?

Dave

2006-01-24 08:07:29

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] swsusp: use bytes as image size units

Hi!

> This patch makes swsusp use bytes as the image size units, which is needed
> for future compatibility.
>
> The patch changes the behavior of the /sys/power/image_size attribute already
> present in 2.6.16-rc1, so it is against this kernel.
>
> Please apply (Pavel, please ack).

ACKed.
Pavel

--
Thanks, Sharp!

2006-01-24 08:09:53

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] swsusp: use bytes as image size units

On Po 23-01-06 18:56:42, Dave Jones wrote:
> On Tue, Jan 24, 2006 at 12:32:26AM +0100, Rafael J. Wysocki wrote:
> > Hi,
> >
> > This patch makes swsusp use bytes as the image size units, which is needed
> > for future compatibility.
>
> With what ? I don't see how clipping this range to a maximum of 4GB
> will future-proof anything. What happens in a few years time when
> I want to suspend my 8GB laptop ?

With rest of suspend code, which uses bytes because depending on
PAGE_SIZE is just too ugly. Notice that image size is smaller than
half of lowmem, by design, so thats okay.

In unlikely case of 8GB 32-bit laptop, either suspend image fits into
lowmem, or you are out of luck... Also notice that kernel is allowed
to use bigger image than size limit if it can not shrink image
further. Current code is equivalent with always having limit of 0.

Pavel
--
Thanks, Sharp!

2006-01-24 08:21:18

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] swsusp: use bytes as image size units

On Tuesday, 24 January 2006 00:56, Dave Jones wrote:
> On Tue, Jan 24, 2006 at 12:32:26AM +0100, Rafael J. Wysocki wrote:
> > Hi,
> >
> > This patch makes swsusp use bytes as the image size units, which is needed
> > for future compatibility.
>
> With what ?

WIth the userland interface.

> I don't see how clipping this range to a maximum of 4GB
> will future-proof anything. What happens in a few years time when
> I want to suspend my 8GB laptop ?

We cannot create an image that's greater than 1/2 of RAM (in general)
or 1/2 of lowmem (on i386) anyway, but this does not limit the size of
RAM of a box you want to suspend. The rest of the RAM contents will
be swapped out before suspend.

Besides on x86-64 unsigned long is 64-bit, so it's not limited to 4 GB.

Greetings,
Rafael