2006-09-20 21:21:59

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH -mm 0/6] swsusp: Add support for swap files

Hi,

The following series of patches makes swsusp support swap files.

For now, it is only possible to suspend to a swap file using the in-kernel
swsusp and the resume cannot be initiated from an initrd.

Greetings,
Rafael


--
You never change things by fighting the existing reality.
R. Buckminster Fuller


2006-09-20 21:20:30

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH -mm 2/6] swsusp: Rearrange swap-handling code

Rearrange the code in kernel/power/swap.c so that the next patch is more
readable.

[This patch only moves the existing code.]

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
kernel/power/swap.c | 219 ++++++++++++++++++++++++++--------------------------
1 file changed, 111 insertions(+), 108 deletions(-)

Index: linux-2.6.18-rc7-mm1/kernel/power/swap.c
===================================================================
--- linux-2.6.18-rc7-mm1.orig/kernel/power/swap.c
+++ linux-2.6.18-rc7-mm1/kernel/power/swap.c
@@ -41,10 +41,120 @@ static struct swsusp_header {
} __attribute__((packed, aligned(PAGE_SIZE))) swsusp_header;

/*
- * Saving part...
+ * General things
*/

static unsigned short root_swap = 0xffff;
+static struct block_device *resume_bdev;
+
+/**
+ * submit - submit BIO request.
+ * @rw: READ or WRITE.
+ * @off physical offset of page.
+ * @page: page we're reading or writing.
+ * @bio_chain: list of pending biod (for async reading)
+ *
+ * Straight from the textbook - allocate and initialize the bio.
+ * If we're reading, make sure the page is marked as dirty.
+ * Then submit it and, if @bio_chain == NULL, wait.
+ */
+static int submit(int rw, pgoff_t page_off, struct page *page,
+ struct bio **bio_chain)
+{
+ struct bio *bio;
+
+ bio = bio_alloc(GFP_ATOMIC, 1);
+ if (!bio)
+ return -ENOMEM;
+ bio->bi_sector = page_off * (PAGE_SIZE >> 9);
+ bio->bi_bdev = resume_bdev;
+ bio->bi_end_io = end_swap_bio_read;
+
+ if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
+ printk("swsusp: ERROR: adding page to bio at %ld\n", page_off);
+ bio_put(bio);
+ return -EFAULT;
+ }
+
+ lock_page(page);
+ bio_get(bio);
+
+ if (bio_chain == NULL) {
+ submit_bio(rw | (1 << BIO_RW_SYNC), bio);
+ wait_on_page_locked(page);
+ if (rw == READ)
+ bio_set_pages_dirty(bio);
+ bio_put(bio);
+ } else {
+ get_page(page);
+ bio->bi_private = *bio_chain;
+ *bio_chain = bio;
+ submit_bio(rw | (1 << BIO_RW_SYNC), bio);
+ }
+ return 0;
+}
+
+static int bio_read_page(pgoff_t page_off, void *addr, struct bio **bio_chain)
+{
+ return submit(READ, page_off, virt_to_page(addr), bio_chain);
+}
+
+static int bio_write_page(pgoff_t page_off, void *addr)
+{
+ return submit(WRITE, page_off, virt_to_page(addr), NULL);
+}
+
+static int wait_on_bio_chain(struct bio **bio_chain)
+{
+ struct bio *bio;
+ struct bio *next_bio;
+ int ret = 0;
+
+ if (bio_chain == NULL)
+ return 0;
+
+ bio = *bio_chain;
+ if (bio == NULL)
+ return 0;
+ while (bio) {
+ struct page *page;
+
+ next_bio = bio->bi_private;
+ page = bio->bi_io_vec[0].bv_page;
+ wait_on_page_locked(page);
+ if (!PageUptodate(page) || PageError(page))
+ ret = -EIO;
+ put_page(page);
+ bio_put(bio);
+ bio = next_bio;
+ }
+ *bio_chain = NULL;
+ return ret;
+}
+
+static void show_speed(struct timeval *start, struct timeval *stop,
+ unsigned nr_pages, char *msg)
+{
+ s64 elapsed_centisecs64;
+ int centisecs;
+ int k;
+ int kps;
+
+ elapsed_centisecs64 = timeval_to_ns(stop) - timeval_to_ns(start);
+ do_div(elapsed_centisecs64, NSEC_PER_SEC / 100);
+ centisecs = elapsed_centisecs64;
+ if (centisecs == 0)
+ centisecs = 1; /* avoid div-by-zero */
+ k = nr_pages * (PAGE_SIZE / 1024);
+ kps = (k * 100) / centisecs;
+ printk("%s %d kbytes in %d.%02d seconds (%d.%02d MB/s)\n", msg, k,
+ centisecs / 100, centisecs % 100,
+ kps / 1000, (kps % 1000) / 10);
+}
+
+/*
+ * Saving part
+ */

static int mark_swapfiles(swp_entry_t start)
{
@@ -166,26 +276,6 @@ static void release_swap_writer(struct s
handle->bitmap = NULL;
}

-static void show_speed(struct timeval *start, struct timeval *stop,
- unsigned nr_pages, char *msg)
-{
- s64 elapsed_centisecs64;
- int centisecs;
- int k;
- int kps;
-
- elapsed_centisecs64 = timeval_to_ns(stop) - timeval_to_ns(start);
- do_div(elapsed_centisecs64, NSEC_PER_SEC / 100);
- centisecs = elapsed_centisecs64;
- if (centisecs == 0)
- centisecs = 1; /* avoid div-by-zero */
- k = nr_pages * (PAGE_SIZE / 1024);
- kps = (k * 100) / centisecs;
- printk("%s %d kbytes in %d.%02d seconds (%d.%02d MB/s)\n", msg, k,
- centisecs / 100, centisecs % 100,
- kps / 1000, (kps % 1000) / 10);
-}
-
static int get_swap_writer(struct swap_map_handle *handle)
{
handle->cur = (struct swap_map_page *)get_zeroed_page(GFP_KERNEL);
@@ -205,34 +295,6 @@ static int get_swap_writer(struct swap_m
return 0;
}

-static int wait_on_bio_chain(struct bio **bio_chain)
-{
- struct bio *bio;
- struct bio *next_bio;
- int ret = 0;
-
- if (bio_chain == NULL)
- return 0;
-
- bio = *bio_chain;
- if (bio == NULL)
- return 0;
- while (bio) {
- struct page *page;
-
- next_bio = bio->bi_private;
- page = bio->bi_io_vec[0].bv_page;
- wait_on_page_locked(page);
- if (!PageUptodate(page) || PageError(page))
- ret = -EIO;
- put_page(page);
- bio_put(bio);
- bio = next_bio;
- }
- *bio_chain = NULL;
- return ret;
-}
-
static int swap_write_page(struct swap_map_handle *handle, void *buf,
struct bio **bio_chain)
{
@@ -384,65 +446,6 @@ int swsusp_write(void)
return error;
}

-static struct block_device *resume_bdev;
-
-/**
- * submit - submit BIO request.
- * @rw: READ or WRITE.
- * @off physical offset of page.
- * @page: page we're reading or writing.
- * @bio_chain: list of pending biod (for async reading)
- *
- * Straight from the textbook - allocate and initialize the bio.
- * If we're reading, make sure the page is marked as dirty.
- * Then submit it and, if @bio_chain == NULL, wait.
- */
-static int submit(int rw, pgoff_t page_off, struct page *page,
- struct bio **bio_chain)
-{
- struct bio *bio;
-
- bio = bio_alloc(GFP_ATOMIC, 1);
- if (!bio)
- return -ENOMEM;
- bio->bi_sector = page_off * (PAGE_SIZE >> 9);
- bio->bi_bdev = resume_bdev;
- bio->bi_end_io = end_swap_bio_read;
-
- if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
- printk("swsusp: ERROR: adding page to bio at %ld\n", page_off);
- bio_put(bio);
- return -EFAULT;
- }
-
- lock_page(page);
- bio_get(bio);
-
- if (bio_chain == NULL) {
- submit_bio(rw | (1 << BIO_RW_SYNC), bio);
- wait_on_page_locked(page);
- if (rw == READ)
- bio_set_pages_dirty(bio);
- bio_put(bio);
- } else {
- get_page(page);
- bio->bi_private = *bio_chain;
- *bio_chain = bio;
- submit_bio(rw | (1 << BIO_RW_SYNC), bio);
- }
- return 0;
-}
-
-static int bio_read_page(pgoff_t page_off, void *addr, struct bio **bio_chain)
-{
- return submit(READ, page_off, virt_to_page(addr), bio_chain);
-}
-
-static int bio_write_page(pgoff_t page_off, void *addr)
-{
- return submit(WRITE, page_off, virt_to_page(addr), NULL);
-}
-
/**
* The following functions allow us to read data using a swap map
* in a file-alike way

2006-09-20 21:21:25

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH -mm 4/6] swsusp: Add resume_offset command line parameter

Add the kernel command line parameter "resume_offset=" allowing us to specify
the offset, in <PAGE_SIZE> units, from the beginning of the partition pointed
to by the "resume=" parameter at which the swap header is located.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
kernel/power/disk.c | 15 +++++++++++++++
kernel/power/power.h | 1 +
kernel/power/swap.c | 15 ++++++++++-----
3 files changed, 26 insertions(+), 5 deletions(-)

Index: linux-2.6.18-rc7-mm1/kernel/power/disk.c
===================================================================
--- linux-2.6.18-rc7-mm1.orig/kernel/power/disk.c
+++ linux-2.6.18-rc7-mm1/kernel/power/disk.c
@@ -27,6 +27,7 @@
static int noresume = 0;
char resume_file[256] = CONFIG_PM_STD_PARTITION;
dev_t swsusp_resume_device;
+sector_t swsusp_resume_block;

/**
* power_down - Shut machine down for hibernate.
@@ -404,6 +405,19 @@ static int __init resume_setup(char *str
return 1;
}

+static int __init resume_offset_setup(char *str)
+{
+ loff_t offset;
+
+ if (noresume)
+ return 1;
+
+ if (sscanf(str, "%llu", &offset) == 1)
+ swsusp_resume_block = offset;
+
+ return 1;
+}
+
static int __init noresume_setup(char *str)
{
noresume = 1;
@@ -411,4 +425,5 @@ static int __init noresume_setup(char *s
}

__setup("noresume", noresume_setup);
+__setup("resume_offset=", resume_offset_setup);
__setup("resume=", resume_setup);
Index: linux-2.6.18-rc7-mm1/kernel/power/power.h
===================================================================
--- linux-2.6.18-rc7-mm1.orig/kernel/power/power.h
+++ linux-2.6.18-rc7-mm1/kernel/power/power.h
@@ -42,6 +42,7 @@ extern const void __nosave_begin, __nosa
extern unsigned long image_size;
extern int in_suspend;
extern dev_t swsusp_resume_device;
+extern sector_t swsusp_resume_block;

extern asmlinkage int swsusp_arch_suspend(void);
extern asmlinkage int swsusp_arch_resume(void);
Index: linux-2.6.18-rc7-mm1/kernel/power/swap.c
===================================================================
--- linux-2.6.18-rc7-mm1.orig/kernel/power/swap.c
+++ linux-2.6.18-rc7-mm1/kernel/power/swap.c
@@ -160,13 +160,14 @@ static int mark_swapfiles(loff_t start)
{
int error;

- bio_read_page(0, &swsusp_header, NULL);
+ bio_read_page(swsusp_resume_block, &swsusp_header, NULL);
if (!memcmp("SWAP-SPACE",swsusp_header.sig, 10) ||
!memcmp("SWAPSPACE2",swsusp_header.sig, 10)) {
memcpy(swsusp_header.orig_sig,swsusp_header.sig, 10);
memcpy(swsusp_header.sig,SWSUSP_SIG, 10);
swsusp_header.image = start;
- error = bio_write_page(0, &swsusp_header, NULL);
+ error = bio_write_page(swsusp_resume_block,
+ &swsusp_header, NULL);
} else {
printk(KERN_ERR "swsusp: Swap header not found!\n");
error = -ENODEV;
@@ -183,7 +184,7 @@ static int swsusp_swap_check(void) /* Th
{
int res;

- res = swap_type_of(swsusp_resume_device, 0);
+ res = swap_type_of(swsusp_resume_device, swsusp_resume_block);
if (res < 0)
return res;

@@ -606,12 +607,16 @@ int swsusp_check(void)
if (!IS_ERR(resume_bdev)) {
set_blocksize(resume_bdev, PAGE_SIZE);
memset(&swsusp_header, 0, sizeof(swsusp_header));
- if ((error = bio_read_page(0, &swsusp_header, NULL)))
+ error = bio_read_page(swsusp_resume_block,
+ &swsusp_header, NULL);
+ if (error)
return error;
+
if (!memcmp(SWSUSP_SIG, swsusp_header.sig, 10)) {
memcpy(swsusp_header.sig, swsusp_header.orig_sig, 10);
/* Reset swap signature now */
- error = bio_write_page(0, &swsusp_header, NULL);
+ error = bio_write_page(swsusp_resume_block,
+ &swsusp_header, NULL);
} else {
return -EINVAL;
}

2006-09-20 21:20:52

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH -mm 6/6] swsusp: Document support for swap files

Document the "resume_offset=" command line parameter as well as the way in
which swap files are supported by swsusp.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
Documentation/kernel-parameters.txt | 6 +++
Documentation/power/swsusp-and-swap-files.txt | 46 ++++++++++++++++++++++++++
Documentation/power/swsusp.txt | 20 +++--------
3 files changed, 58 insertions(+), 14 deletions(-)

Index: linux-2.6.18-rc7-mm1/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.18-rc7-mm1.orig/Documentation/kernel-parameters.txt
+++ linux-2.6.18-rc7-mm1/Documentation/kernel-parameters.txt
@@ -1370,6 +1370,12 @@ and is between 256 and 4096 characters.
resume= [SWSUSP]
Specify the partition device for software suspend

+ resume_offset= [SWSUSP]
+ Specify the offset from the beginning of the partition
+ given by "resume=" at which the swap header is located,
+ in <PAGE_SIZE> units (needed only for swap files).
+ See Documentation/power/swsusp-and-swap-files.txt
+
rhash_entries= [KNL,NET]
Set number of hash buckets for route cache

Index: linux-2.6.18-rc7-mm1/Documentation/power/swsusp-and-swap-files.txt
===================================================================
--- /dev/null
+++ linux-2.6.18-rc7-mm1/Documentation/power/swsusp-and-swap-files.txt
@@ -0,0 +1,46 @@
+Using swap files with software suspend (swsusp)
+ (C) 2006 Rafael J. Wysocki <[email protected]>
+
+The Linux kernel handles swap files almost in the same way as it handles swap
+partitions and there are only two differences between these two types of swap
+areas:
+(1) swap files need not be contiguous,
+(2) the header of a swap file is not in the first block of the partition that
+holds it. From the swsusp's point of view (1) is not a problem, because it is
+already taken care of by the swap-handling code, but (2) has to be taken into
+consideration.
+
+In principle the location of a swap file's header may be determined with the
+help of appropriate filesystem driver. Unfortunately, however, it requires the
+filesystem holding the swap file to be mounted, and if this filesystem is
+journaled, it cannot be mounted during resume from disk. For this reason to
+identify a swap file swsusp uses the name of the partition that holds the file
+and the offset from the beginning of the partition at which the swap file's
+header is located. For convenience, this offset is expressed in <PAGE_SIZE>
+units.
+
+In order to use a swap file with swsusp, you need to:
+
+1) Create the swap file and make it active, eg.
+
+# dd if=/dev/zero of=<swap_file_path> bs=1024 count=<swap_file_size_in_k>
+# mkswap <swap_file_path>
+# swapon <swap_file_path>
+
+Then, the kernel will place the following line in the dmesg:
+
+Adding <swap_file_size_in_k - 1>k swap on <swap_file_path>. Priority:-2 extents:<nr_extents> across:<span_in_k>k offset:<swap_file_offset>
+
+where <nr_extents>, <span_in_k>, <swap_file_offset> are numbers that describe
+the swap file configuration. The last one, <swap_file_offset>, is needed by
+swsusp.
+
+2) Add the following parameters to the kernel command line:
+
+resume=<swap_file_partition> resume_offset=<swap_file_offset>
+
+where <swap_file_partition> is the partition on which the swap file is located.
+
+Now, swsusp will use the swap file in the same way in which it would use a swap
+partition. [Of course this means that the resume from a swap file cannot be
+initiated from whithin an initrd of initramfs image.]
Index: linux-2.6.18-rc7-mm1/Documentation/power/swsusp.txt
===================================================================
--- linux-2.6.18-rc7-mm1.orig/Documentation/power/swsusp.txt
+++ linux-2.6.18-rc7-mm1/Documentation/power/swsusp.txt
@@ -297,20 +297,12 @@ system is shut down or suspended. Additi
suspend image to prevent sensitive data from being stolen after
resume.

-Q: Why can't we suspend to a swap file?
+Q: Can I suspend to a swap file?

-A: Because accessing swap file needs the filesystem mounted, and
-filesystem might do something wrong (like replaying the journal)
-during mount.
-
-There are few ways to get that fixed:
-
-1) Probably could be solved by modifying every filesystem to support
-some kind of "really read-only!" option. Patches welcome.
-
-2) suspend2 gets around that by storing absolute positions in on-disk
-image (and blocksize), with resume parameter pointing directly to
-suspend header.
+A: Generally, yes, you can. However, it requires you to use the "resume=" and
+"resume_offset=" kernel command line parameters, so the resume from a swap file
+cannot be initiated from an initrd or initramfs image. See
+swsusp-and-swap-files.txt for details.

Q: Is there a maximum system RAM size that is supported by swsusp?

2006-09-20 21:20:53

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH -mm 5/6] mm: Print first block offset for swap areas

In order to use a swap file with swsusp we need to know the offset at which
its swap header is located. However, the swap header is always located in the
first block of the swap file and it's quite easy to make sys_swapon() print
the offset of the swap file's (or swap partition's) first block.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
mm/swapfile.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

Index: linux-2.6.18-rc7-mm1/mm/swapfile.c
===================================================================
--- linux-2.6.18-rc7-mm1.orig/mm/swapfile.c
+++ linux-2.6.18-rc7-mm1/mm/swapfile.c
@@ -1047,7 +1047,8 @@ add_swap_extent(struct swap_info_struct
* This is extremely effective. The average number of iterations in
* map_swap_page() has been measured at about 0.3 per page. - akpm.
*/
-static int setup_swap_extents(struct swap_info_struct *sis, sector_t *span)
+static int setup_swap_extents(struct swap_info_struct *sis, sector_t *span,
+ sector_t *start)
{
struct inode *inode;
unsigned blocks_per_page;
@@ -1060,6 +1061,7 @@ static int setup_swap_extents(struct swa
int nr_extents = 0;
int ret;

+ *start = 0;
inode = sis->swap_file->f_mapping->host;
if (S_ISBLK(inode->i_mode)) {
ret = add_swap_extent(sis, 0, sis->max, 0);
@@ -1114,6 +1116,8 @@ static int setup_swap_extents(struct swa
lowest_block = first_block;
if (first_block > highest_block)
highest_block = first_block;
+ } else {
+ *start = first_block;
}

/*
@@ -1407,7 +1411,7 @@ asmlinkage long sys_swapon(const char __
int swap_header_version;
unsigned int nr_good_pages = 0;
int nr_extents = 0;
- sector_t span;
+ sector_t span, start;
unsigned long maxpages = 1;
int swapfilesize;
unsigned short *swap_map;
@@ -1608,7 +1612,7 @@ asmlinkage long sys_swapon(const char __
p->swap_map[0] = SWAP_MAP_BAD;
p->max = maxpages;
p->pages = nr_good_pages;
- nr_extents = setup_swap_extents(p, &span);
+ nr_extents = setup_swap_extents(p, &span, &start);
if (nr_extents < 0) {
error = nr_extents;
goto bad_swap;
@@ -1628,9 +1632,10 @@ asmlinkage long sys_swapon(const char __
total_swap_pages += nr_good_pages;

printk(KERN_INFO "Adding %uk swap on %s. "
- "Priority:%d extents:%d across:%lluk\n",
+ "Priority:%d extents:%d across:%lluk offset:%llu\n",
nr_good_pages<<(PAGE_SHIFT-10), name, p->prio,
- nr_extents, (unsigned long long)span<<(PAGE_SHIFT-10));
+ nr_extents, (unsigned long long)span<<(PAGE_SHIFT-10),
+ (unsigned long long)start);

/* insert swap space into swap_list: */
prev = -1;

2006-09-20 21:20:29

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH -mm 1/6] swsusp: Use device and offset to intentify swap areas

The Linux kernel handles swap files almost in the same way as it handles swap
partitions and there are only two differences between these two types of swap
areas:
(1) swap files need not be contiguous,
(2) the header of a swap file is not in the first block of the partition that
holds it. From the swsusp's point of view (1) is not a problem, because it is
already taken care of by the swap-handling code, but (2) has to be taken into
consideration.

In principle the location of a swap file's header may be determined with the
help of appropriate filesystem driver. Unfortunately, however, it requires the
filesystem holding the swap file to be mounted, and if this filesystem is
journaled, it cannot be mounted during a resume from disk. For this reason
we need some other means by which swap areas can be identified.

For example, to identify a swap area we can use the partition that holds the
area and the offset from the beginning of this partition at which the swap
header is located.

The following patch allows swsusp to identify swap areas this way. It changes
swap_type_of() so that it takes an additional argument representing an offset
of the swap header within the partition represented by its first argument.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
include/linux/swap.h | 2 +-
kernel/power/swap.c | 2 +-
kernel/power/user.c | 5 +++--
mm/swapfile.c | 38 ++++++++++++++++++++++++++------------
4 files changed, 31 insertions(+), 16 deletions(-)

Index: linux-2.6.18-rc7-mm1/mm/swapfile.c
===================================================================
--- linux-2.6.18-rc7-mm1.orig/mm/swapfile.c
+++ linux-2.6.18-rc7-mm1/mm/swapfile.c
@@ -427,34 +427,48 @@ void free_swap_and_cache(swp_entry_t ent

#ifdef CONFIG_SOFTWARE_SUSPEND
/*
- * Find the swap type that corresponds to given device (if any)
+ * Find the swap type that corresponds to given device (if any).
*
- * This is needed for software suspend and is done in such a way that inode
- * aliasing is allowed.
+ * @offset - number of the PAGE_SIZE-sized block of the device, starting
+ * from 0, in which the swap header is expected to be located.
+ *
+ * This is needed for the suspend to disk (aka swsusp).
*/
-int swap_type_of(dev_t device)
+int swap_type_of(dev_t device, sector_t offset)
{
+ struct block_device *bdev = NULL;
int i;

+ if (device)
+ bdev = bdget(device);
+
spin_lock(&swap_lock);
for (i = 0; i < nr_swapfiles; i++) {
- struct inode *inode;
+ struct swap_info_struct *sis = swap_info + i;

- if (!(swap_info[i].flags & SWP_WRITEOK))
+ if (!(sis->flags & SWP_WRITEOK))
continue;

- if (!device) {
+ if (!bdev) {
spin_unlock(&swap_lock);
return i;
}
- inode = swap_info[i].swap_file->f_dentry->d_inode;
- if (S_ISBLK(inode->i_mode) &&
- device == MKDEV(imajor(inode), iminor(inode))) {
- spin_unlock(&swap_lock);
- return i;
+ if (bdev == sis->bdev) {
+ struct swap_extent *se;
+
+ se = list_entry(sis->extent_list.next,
+ struct swap_extent, list);
+ if (se->start_block == offset) {
+ spin_unlock(&swap_lock);
+ bdput(bdev);
+ return i;
+ }
}
}
spin_unlock(&swap_lock);
+ if (bdev)
+ bdput(bdev);
+
return -ENODEV;
}

Index: linux-2.6.18-rc7-mm1/include/linux/swap.h
===================================================================
--- linux-2.6.18-rc7-mm1.orig/include/linux/swap.h
+++ linux-2.6.18-rc7-mm1/include/linux/swap.h
@@ -249,7 +249,7 @@ extern int swap_duplicate(swp_entry_t);
extern int valid_swaphandles(swp_entry_t, unsigned long *);
extern void swap_free(swp_entry_t);
extern void free_swap_and_cache(swp_entry_t);
-extern int swap_type_of(dev_t);
+extern int swap_type_of(dev_t, sector_t);
extern unsigned int count_swap_pages(int, int);
extern sector_t map_swap_page(struct swap_info_struct *, pgoff_t);
extern struct swap_info_struct *get_swap_info_struct(unsigned);
Index: linux-2.6.18-rc7-mm1/kernel/power/swap.c
===================================================================
--- linux-2.6.18-rc7-mm1.orig/kernel/power/swap.c
+++ linux-2.6.18-rc7-mm1/kernel/power/swap.c
@@ -74,7 +74,7 @@ static int mark_swapfiles(swp_entry_t st

static int swsusp_swap_check(void) /* This is called before saving image */
{
- int res = swap_type_of(swsusp_resume_device);
+ int res = swap_type_of(swsusp_resume_device, 0);

if (res >= 0) {
root_swap = res;
Index: linux-2.6.18-rc7-mm1/kernel/power/user.c
===================================================================
--- linux-2.6.18-rc7-mm1.orig/kernel/power/user.c
+++ linux-2.6.18-rc7-mm1/kernel/power/user.c
@@ -54,7 +54,8 @@ static int snapshot_open(struct inode *i
filp->private_data = data;
memset(&data->handle, 0, sizeof(struct snapshot_handle));
if ((filp->f_flags & O_ACCMODE) == O_RDONLY) {
- data->swap = swsusp_resume_device ? swap_type_of(swsusp_resume_device) : -1;
+ data->swap = swsusp_resume_device ?
+ swap_type_of(swsusp_resume_device, 0) : -1;
data->mode = O_RDONLY;
} else {
data->swap = -1;
@@ -264,7 +265,7 @@ static int snapshot_ioctl(struct inode *
* so we need to recode them
*/
if (old_decode_dev(arg)) {
- data->swap = swap_type_of(old_decode_dev(arg));
+ data->swap = swap_type_of(old_decode_dev(arg), 0);
if (data->swap < 0)
error = -ENODEV;
} else {

2006-09-20 21:21:25

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH -mm 3/6] swsusp: Use block device offsets to identify swap locations

Make swsusp use block device offsets instead of swap offsets to identify swap
locations and make it use the same code paths for writing as well as for
reading data.

This allows us to use the same code for handling swap files and swap
partitions and to simplify the code, eg. by dropping rw_swap_page_sync().

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
include/linux/swap.h | 3 -
kernel/power/power.h | 2
kernel/power/swap.c | 128 +++++++++++++++++++++++++-------------------------
kernel/power/swsusp.c | 10 +--
kernel/power/user.c | 2
mm/page_io.c | 45 -----------------
mm/swapfile.c | 17 ++++++
7 files changed, 91 insertions(+), 116 deletions(-)

Index: linux-2.6.18-rc7-mm1/include/linux/swap.h
===================================================================
--- linux-2.6.18-rc7-mm1.orig/include/linux/swap.h
+++ linux-2.6.18-rc7-mm1/include/linux/swap.h
@@ -219,8 +219,6 @@ extern void swap_unplug_io_fn(struct bac
/* linux/mm/page_io.c */
extern int swap_readpage(struct file *, struct page *);
extern int swap_writepage(struct page *page, struct writeback_control *wbc);
-extern int rw_swap_page_sync(int rw, swp_entry_t entry, struct page *page,
- struct bio **bio_chain);
extern int end_swap_bio_read(struct bio *bio, unsigned int bytes_done, int err);

/* linux/mm/swap_state.c */
@@ -252,6 +250,7 @@ extern void free_swap_and_cache(swp_entr
extern int swap_type_of(dev_t, sector_t);
extern unsigned int count_swap_pages(int, int);
extern sector_t map_swap_page(struct swap_info_struct *, pgoff_t);
+extern sector_t swapdev_block(int, pgoff_t);
extern struct swap_info_struct *get_swap_info_struct(unsigned);
extern int can_share_swap_page(struct page *);
extern int remove_exclusive_swap_page(struct page *);
Index: linux-2.6.18-rc7-mm1/mm/swapfile.c
===================================================================
--- linux-2.6.18-rc7-mm1.orig/mm/swapfile.c
+++ linux-2.6.18-rc7-mm1/mm/swapfile.c
@@ -945,6 +945,23 @@ sector_t map_swap_page(struct swap_info_
}
}

+#ifdef CONFIG_SOFTWARE_SUSPEND
+/*
+ * Get the (PAGE_SIZE) block corrsponding to given offset on the swapdev
+ * corrsponding to given index in swap_info (swap type).
+ */
+sector_t swapdev_block(int swap_type, pgoff_t offset)
+{
+ struct swap_info_struct *sis;
+
+ if (swap_type >= nr_swapfiles)
+ return 0;
+
+ sis = swap_info + swap_type;
+ return (sis->flags & SWP_WRITEOK) ? map_swap_page(sis, offset) : 0;
+}
+#endif /* CONFIG_SOFTWARE_SUSPEND */
+
/*
* Free all of a swapdev's extent information
*/
Index: linux-2.6.18-rc7-mm1/kernel/power/power.h
===================================================================
--- linux-2.6.18-rc7-mm1.orig/kernel/power/power.h
+++ linux-2.6.18-rc7-mm1/kernel/power/power.h
@@ -141,7 +141,7 @@ struct bitmap_page {

extern void free_bitmap(struct bitmap_page *bitmap);
extern struct bitmap_page *alloc_bitmap(unsigned int nr_bits);
-extern unsigned long alloc_swap_page(int swap, struct bitmap_page *bitmap);
+extern loff_t alloc_swapdev_block(int swap, struct bitmap_page *bitmap);
extern void free_all_swap_pages(int swap, struct bitmap_page *bitmap);

extern int swsusp_check(void);
Index: linux-2.6.18-rc7-mm1/kernel/power/swsusp.c
===================================================================
--- linux-2.6.18-rc7-mm1.orig/kernel/power/swsusp.c
+++ linux-2.6.18-rc7-mm1/kernel/power/swsusp.c
@@ -134,18 +134,18 @@ static int bitmap_set(struct bitmap_page
return 0;
}

-unsigned long alloc_swap_page(int swap, struct bitmap_page *bitmap)
+loff_t alloc_swapdev_block(int swap, struct bitmap_page *bitmap)
{
unsigned long offset;

offset = swp_offset(get_swap_page_of_type(swap));
if (offset) {
- if (bitmap_set(bitmap, offset)) {
+ if (bitmap_set(bitmap, offset))
swap_free(swp_entry(swap, offset));
- offset = 0;
- }
+ else
+ return swapdev_block(swap, offset);
}
- return offset;
+ return 0;
}

void free_all_swap_pages(int swap, struct bitmap_page *bitmap)
Index: linux-2.6.18-rc7-mm1/kernel/power/user.c
===================================================================
--- linux-2.6.18-rc7-mm1.orig/kernel/power/user.c
+++ linux-2.6.18-rc7-mm1/kernel/power/user.c
@@ -239,7 +239,7 @@ static int snapshot_ioctl(struct inode *
break;
}
}
- offset = alloc_swap_page(data->swap, data->bitmap);
+ offset = alloc_swapdev_block(data->swap, data->bitmap);
if (offset) {
offset <<= PAGE_SHIFT;
error = put_user(offset, (loff_t __user *)arg);
Index: linux-2.6.18-rc7-mm1/kernel/power/swap.c
===================================================================
--- linux-2.6.18-rc7-mm1.orig/kernel/power/swap.c
+++ linux-2.6.18-rc7-mm1/kernel/power/swap.c
@@ -34,8 +34,8 @@ extern char resume_file[];
#define SWSUSP_SIG "S1SUSPEND"

static struct swsusp_header {
- char reserved[PAGE_SIZE - 20 - sizeof(swp_entry_t)];
- swp_entry_t image;
+ char reserved[PAGE_SIZE - 20 - sizeof(loff_t)];
+ loff_t image;
char orig_sig[10];
char sig[10];
} __attribute__((packed, aligned(PAGE_SIZE))) swsusp_header;
@@ -99,9 +99,9 @@ static int bio_read_page(pgoff_t page_of
return submit(READ, page_off, virt_to_page(addr), bio_chain);
}

-static int bio_write_page(pgoff_t page_off, void *addr)
+static int bio_write_page(pgoff_t page_off, void *addr, struct bio **bio_chain)
{
- return submit(WRITE, page_off, virt_to_page(addr), NULL);
+ return submit(WRITE, page_off, virt_to_page(addr), bio_chain);
}

static int wait_on_bio_chain(struct bio **bio_chain)
@@ -156,22 +156,19 @@ static void show_speed(struct timeval *s
* Saving part
*/

-static int mark_swapfiles(swp_entry_t start)
+static int mark_swapfiles(loff_t start)
{
int error;

- rw_swap_page_sync(READ, swp_entry(root_swap, 0),
- virt_to_page((unsigned long)&swsusp_header), NULL);
+ bio_read_page(0, &swsusp_header, NULL);
if (!memcmp("SWAP-SPACE",swsusp_header.sig, 10) ||
!memcmp("SWAPSPACE2",swsusp_header.sig, 10)) {
memcpy(swsusp_header.orig_sig,swsusp_header.sig, 10);
memcpy(swsusp_header.sig,SWSUSP_SIG, 10);
swsusp_header.image = start;
- error = rw_swap_page_sync(WRITE, swp_entry(root_swap, 0),
- virt_to_page((unsigned long)&swsusp_header),
- NULL);
+ error = bio_write_page(0, &swsusp_header, NULL);
} else {
- pr_debug("swsusp: Partition is not swap space.\n");
+ printk(KERN_ERR "swsusp: Swap header not found!\n");
error = -ENODEV;
}
return error;
@@ -184,13 +181,19 @@ static int mark_swapfiles(swp_entry_t st

static int swsusp_swap_check(void) /* This is called before saving image */
{
- int res = swap_type_of(swsusp_resume_device, 0);
+ int res;

- if (res >= 0) {
- root_swap = res;
- return 0;
- }
- return res;
+ res = swap_type_of(swsusp_resume_device, 0);
+ if (res < 0)
+ return res;
+
+ root_swap = res;
+ resume_bdev = open_by_devnum(swsusp_resume_device, FMODE_WRITE);
+ if (IS_ERR(resume_bdev))
+ return PTR_ERR(resume_bdev);
+
+ set_blocksize(resume_bdev, PAGE_SIZE);
+ return 0;
}

/**
@@ -200,36 +203,26 @@ static int swsusp_swap_check(void) /* Th
* @bio_chain: Link the next write BIO here
*/

-static int write_page(void *buf, unsigned long offset, struct bio **bio_chain)
+static int write_page(void *buf, loff_t offset, struct bio **bio_chain)
{
- swp_entry_t entry;
- int error = -ENOSPC;
+ void *src;

- if (offset) {
- struct page *page = virt_to_page(buf);
+ if (!offset)
+ return -ENOSPC;

- if (bio_chain) {
- /*
- * Whether or not we successfully allocated a copy page,
- * we take a ref on the page here. It gets undone in
- * wait_on_bio_chain().
- */
- struct page *page_copy;
- page_copy = alloc_page(GFP_ATOMIC);
- if (page_copy == NULL) {
- WARN_ON_ONCE(1);
- bio_chain = NULL; /* Go synchronous */
- get_page(page);
- } else {
- memcpy(page_address(page_copy),
- page_address(page), PAGE_SIZE);
- page = page_copy;
- }
+ if (bio_chain) {
+ src = (void *)__get_free_page(GFP_ATOMIC);
+ if (src) {
+ memcpy(src, buf, PAGE_SIZE);
+ } else {
+ WARN_ON_ONCE(1);
+ bio_chain = NULL; /* Go synchronous */
+ src = buf;
}
- entry = swp_entry(root_swap, offset);
- error = rw_swap_page_sync(WRITE, entry, page, bio_chain);
+ } else {
+ src = buf;
}
- return error;
+ return bio_write_page(offset, src, bio_chain);
}

/*
@@ -247,11 +240,11 @@ static int write_page(void *buf, unsigne
* at a time.
*/

-#define MAP_PAGE_ENTRIES (PAGE_SIZE / sizeof(long) - 1)
+#define MAP_PAGE_ENTRIES (PAGE_SIZE / sizeof(loff_t) - 1)

struct swap_map_page {
- unsigned long entries[MAP_PAGE_ENTRIES];
- unsigned long next_swap;
+ loff_t entries[MAP_PAGE_ENTRIES];
+ loff_t next_swap;
};

/**
@@ -261,7 +254,7 @@ struct swap_map_page {

struct swap_map_handle {
struct swap_map_page *cur;
- unsigned long cur_swap;
+ loff_t cur_swap;
struct bitmap_page *bitmap;
unsigned int k;
};
@@ -286,7 +279,7 @@ static int get_swap_writer(struct swap_m
release_swap_writer(handle);
return -ENOMEM;
}
- handle->cur_swap = alloc_swap_page(root_swap, handle->bitmap);
+ handle->cur_swap = alloc_swapdev_block(root_swap, handle->bitmap);
if (!handle->cur_swap) {
release_swap_writer(handle);
return -ENOSPC;
@@ -299,11 +292,11 @@ static int swap_write_page(struct swap_m
struct bio **bio_chain)
{
int error = 0;
- unsigned long offset;
+ loff_t offset;

if (!handle->cur)
return -EINVAL;
- offset = alloc_swap_page(root_swap, handle->bitmap);
+ offset = alloc_swapdev_block(root_swap, handle->bitmap);
error = write_page(buf, offset, bio_chain);
if (error)
return error;
@@ -312,7 +305,7 @@ static int swap_write_page(struct swap_m
error = wait_on_bio_chain(bio_chain);
if (error)
goto out;
- offset = alloc_swap_page(root_swap, handle->bitmap);
+ offset = alloc_swapdev_block(root_swap, handle->bitmap);
if (!offset)
return -ENOSPC;
handle->cur->next_swap = offset;
@@ -412,37 +405,47 @@ int swsusp_write(void)
struct swsusp_info *header;
int error;

- if ((error = swsusp_swap_check())) {
+ error = swsusp_swap_check();
+ if (error) {
printk(KERN_ERR "swsusp: Cannot find swap device, try "
"swapon -a.\n");
return error;
}
memset(&snapshot, 0, sizeof(struct snapshot_handle));
error = snapshot_read_next(&snapshot, PAGE_SIZE);
- if (error < PAGE_SIZE)
- return error < 0 ? error : -EFAULT;
+ if (error < PAGE_SIZE) {
+ if (error >= 0)
+ error = -EFAULT;
+
+ goto out;
+ }
header = (struct swsusp_info *)data_of(snapshot);
if (!enough_swap(header->pages)) {
printk(KERN_ERR "swsusp: Not enough free swap\n");
- return -ENOSPC;
+ error = -ENOSPC;
+ goto out;
}
error = get_swap_writer(&handle);
if (!error) {
- unsigned long start = handle.cur_swap;
+ loff_t start = handle.cur_swap;
+
error = swap_write_page(&handle, header, NULL);
if (!error)
error = save_image(&handle, &snapshot,
header->pages - 1);
+
if (!error) {
flush_swap_writer(&handle);
printk("S");
- error = mark_swapfiles(swp_entry(root_swap, start));
+ error = mark_swapfiles(start);
printk("|\n");
}
}
if (error)
free_all_swap_pages(root_swap, handle.bitmap);
release_swap_writer(&handle);
+out:
+ swsusp_close();
return error;
}

@@ -458,17 +461,18 @@ static void release_swap_reader(struct s
handle->cur = NULL;
}

-static int get_swap_reader(struct swap_map_handle *handle,
- swp_entry_t start)
+static int get_swap_reader(struct swap_map_handle *handle, loff_t start)
{
int error;

- if (!swp_offset(start))
+ if (!start)
return -EINVAL;
+
handle->cur = (struct swap_map_page *)get_zeroed_page(GFP_ATOMIC);
if (!handle->cur)
return -ENOMEM;
- error = bio_read_page(swp_offset(start), handle->cur, NULL);
+
+ error = bio_read_page(start, handle->cur, NULL);
if (error) {
release_swap_reader(handle);
return error;
@@ -480,7 +484,7 @@ static int get_swap_reader(struct swap_m
static int swap_read_page(struct swap_map_handle *handle, void *buf,
struct bio **bio_chain)
{
- unsigned long offset;
+ loff_t offset;
int error;

if (!handle->cur)
@@ -607,7 +611,7 @@ int swsusp_check(void)
if (!memcmp(SWSUSP_SIG, swsusp_header.sig, 10)) {
memcpy(swsusp_header.sig, swsusp_header.orig_sig, 10);
/* Reset swap signature now */
- error = bio_write_page(0, &swsusp_header);
+ error = bio_write_page(0, &swsusp_header, NULL);
} else {
return -EINVAL;
}
Index: linux-2.6.18-rc7-mm1/mm/page_io.c
===================================================================
--- linux-2.6.18-rc7-mm1.orig/mm/page_io.c
+++ linux-2.6.18-rc7-mm1/mm/page_io.c
@@ -147,48 +147,3 @@ int swap_readpage(struct file *file, str
out:
return ret;
}
-
-#ifdef CONFIG_SOFTWARE_SUSPEND
-/*
- * A scruffy utility function to read or write an arbitrary swap page
- * and wait on the I/O. The caller must have a ref on the page.
- *
- * We use end_swap_bio_read() even for writes, because it happens to do what
- * we want.
- */
-int rw_swap_page_sync(int rw, swp_entry_t entry, struct page *page,
- struct bio **bio_chain)
-{
- struct bio *bio;
- int ret = 0;
- int bio_rw;
-
- lock_page(page);
-
- bio = get_swap_bio(GFP_KERNEL, entry.val, page, end_swap_bio_read);
- if (bio == NULL) {
- unlock_page(page);
- ret = -ENOMEM;
- goto out;
- }
-
- bio_rw = rw;
- if (!bio_chain)
- bio_rw |= (1 << BIO_RW_SYNC);
- if (bio_chain)
- bio_get(bio);
- submit_bio(bio_rw, bio);
- if (bio_chain == NULL) {
- wait_on_page_locked(page);
-
- if (!PageUptodate(page) || PageError(page))
- ret = -EIO;
- }
- if (bio_chain) {
- bio->bi_private = *bio_chain;
- *bio_chain = bio;
- }
-out:
- return ret;
-}
-#endif

2006-09-21 21:27:16

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH -mm 1/6] swsusp: Use device and offset to intentify swap areas

Hi!

> The Linux kernel handles swap files almost in the same way as it handles swap
> partitions and there are only two differences between these two types of swap
> areas:
> (1) swap files need not be contiguous,
> (2) the header of a swap file is not in the first block of the partition that
> holds it. From the swsusp's point of view (1) is not a problem, because it is
> already taken care of by the swap-handling code, but (2) has to be taken into
> consideration.
>
> In principle the location of a swap file's header may be determined with the
> help of appropriate filesystem driver. Unfortunately, however, it requires the
> filesystem holding the swap file to be mounted, and if this filesystem is
> journaled, it cannot be mounted during a resume from disk. For this reason
> we need some other means by which swap areas can be identified.
>
> For example, to identify a swap area we can use the partition that holds the
> area and the offset from the beginning of this partition at which the swap
> header is located.
>
> The following patch allows swsusp to identify swap areas this way. It changes
> swap_type_of() so that it takes an additional argument representing an offset
> of the swap header within the partition represented by its first argument.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>

ACK.

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

2006-09-21 21:28:21

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH -mm 2/6] swsusp: Rearrange swap-handling code

On Wed 2006-09-20 21:34:38, Rafael J. Wysocki wrote:
> Rearrange the code in kernel/power/swap.c so that the next patch is more
> readable.
>
> [This patch only moves the existing code.]
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>

ACK.

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

2006-09-21 21:30:25

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH -mm 3/6] swsusp: Use block device offsets to identify swap locations

Hi!

> Make swsusp use block device offsets instead of swap offsets to identify swap
> locations and make it use the same code paths for writing as well as for
> reading data.
>
> This allows us to use the same code for handling swap files and swap
> partitions and to simplify the code, eg. by dropping rw_swap_page_sync().
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>

ACK. You may want to fix the comments, but that's probably separate
patch.


> Index: linux-2.6.18-rc7-mm1/mm/swapfile.c
> ===================================================================
> --- linux-2.6.18-rc7-mm1.orig/mm/swapfile.c
> +++ linux-2.6.18-rc7-mm1/mm/swapfile.c
> @@ -945,6 +945,23 @@ sector_t map_swap_page(struct swap_info_
> }
> }
>
> +#ifdef CONFIG_SOFTWARE_SUSPEND
> +/*
> + * Get the (PAGE_SIZE) block corrsponding to given offset on the swapdev

corresponding?

> + * corrsponding to given index in swap_info (swap type).

here too...


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

2006-09-21 21:31:47

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH -mm 4/6] swsusp: Add resume_offset command line parameter

Hi!

> Add the kernel command line parameter "resume_offset=" allowing us to specify
> the offset, in <PAGE_SIZE> units, from the beginning of the partition pointed
> to by the "resume=" parameter at which the swap header is located.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>

Okay, I'd prefer not to add aditional features to in-kernel swsusp,
but this is just not big enough to reject.

ACK.

(What is the solution for uswsusp?)
Pavel

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

2006-09-21 21:32:54

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH -mm 5/6] mm: Print first block offset for swap areas

On Wed 2006-09-20 21:54:38, Rafael J. Wysocki wrote:
> In order to use a swap file with swsusp we need to know the offset at which
> its swap header is located. However, the swap header is always located in the
> first block of the swap file and it's quite easy to make sys_swapon() print
> the offset of the swap file's (or swap partition's) first block.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>

ACK.

(main point of this is that it changes user-visible printk, but that's
probably okay, and way better than changing /proc files...)
Pavel
> @@ -1628,9 +1632,10 @@ asmlinkage long sys_swapon(const char __
> total_swap_pages += nr_good_pages;
>
> printk(KERN_INFO "Adding %uk swap on %s. "
> - "Priority:%d extents:%d across:%lluk\n",
> + "Priority:%d extents:%d across:%lluk offset:%llu\n",
> nr_good_pages<<(PAGE_SHIFT-10), name, p->prio,
> - nr_extents, (unsigned long long)span<<(PAGE_SHIFT-10));
> + nr_extents, (unsigned long long)span<<(PAGE_SHIFT-10),
> + (unsigned long long)start);
>
> /* insert swap space into swap_list: */
> prev = -1;

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

2006-09-21 21:33:34

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH -mm 6/6] swsusp: Document support for swap files

Hi!

> Document the "resume_offset=" command line parameter as well as the way in
> which swap files are supported by swsusp.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>

ACK and thanks!
Pavel

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

2006-09-21 22:11:20

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH -mm 5/6] mm: Print first block offset for swap areas

On Thu, Sep 21, 2006 at 11:32:53PM +0200, Pavel Machek wrote:
> On Wed 2006-09-20 21:54:38, Rafael J. Wysocki wrote:
> > In order to use a swap file with swsusp we need to know the offset at which
> > its swap header is located. However, the swap header is always located in the
> > first block of the swap file and it's quite easy to make sys_swapon() print
> > the offset of the swap file's (or swap partition's) first block.
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
>
> ACK.
>
> (main point of this is that it changes user-visible printk, but that's
> probably okay, and way better than changing /proc files...)

As a user-interface, "read the number from dmesg that swapon printk'd
and add that to your boot command line" is pretty damned awful.
It means that it's next to impossible for a distro installer to
support suspend-to-swapfile. It's also incredibly fragile to rely
on that file always remaining in that part of the disk.

Dave

2006-09-21 22:18:34

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH -mm 3/6] swsusp: Use block device offsets to identify swap locations

On Thursday, 21 September 2006 23:30, Pavel Machek wrote:
> Hi!
>
> > Make swsusp use block device offsets instead of swap offsets to identify swap
> > locations and make it use the same code paths for writing as well as for
> > reading data.
> >
> > This allows us to use the same code for handling swap files and swap
> > partitions and to simplify the code, eg. by dropping rw_swap_page_sync().
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
>
> ACK. You may want to fix the comments, but that's probably separate
> patch.
>
>
> > Index: linux-2.6.18-rc7-mm1/mm/swapfile.c
> > ===================================================================
> > --- linux-2.6.18-rc7-mm1.orig/mm/swapfile.c
> > +++ linux-2.6.18-rc7-mm1/mm/swapfile.c
> > @@ -945,6 +945,23 @@ sector_t map_swap_page(struct swap_info_
> > }
> > }
> >
> > +#ifdef CONFIG_SOFTWARE_SUSPEND
> > +/*
> > + * Get the (PAGE_SIZE) block corrsponding to given offset on the swapdev
>
> corresponding?
>
> > + * corrsponding to given index in swap_info (swap type).
>
> here too...

Ouch, thanks.

I think I'll repost the whole series if Andrew doesn't pick it up, because
I made a typo in the name of the first patch (shame, shame).

Greetings,
Rafael


--
You never change things by fighting the existing reality.
R. Buckminster Fuller

2006-09-21 22:18:34

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH -mm 4/6] swsusp: Add resume_offset command line parameter

On Thursday, 21 September 2006 23:31, Pavel Machek wrote:
> Hi!
>
> > Add the kernel command line parameter "resume_offset=" allowing us to specify
> > the offset, in <PAGE_SIZE> units, from the beginning of the partition pointed
> > to by the "resume=" parameter at which the swap header is located.
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
>
> Okay, I'd prefer not to add aditional features to in-kernel swsusp,
> but this is just not big enough to reject.
>
> ACK.
>
> (What is the solution for uswsusp?)

We need an additional ioctl to set both the swap partition and "resume offset"
at a time (the one we currently have allows us only to set the partition and I
don't want to change it because of the backwards compatibility, but I think
I'll change its name ;-) ).

I'd like to add this ioctl along with the one needed to support the "platform"
method of powering off, because it will require some similar documentation
changes (most importantly, the description of the interface in
Documentation/ABI which I'd like to add once and not to tamper with
afterwards).

Greetings,
Rafael


--
You never change things by fighting the existing reality.
R. Buckminster Fuller

2006-09-21 22:43:36

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH -mm 5/6] mm: Print first block offset for swap areas

On Friday, 22 September 2006 00:10, Dave Jones wrote:
> On Thu, Sep 21, 2006 at 11:32:53PM +0200, Pavel Machek wrote:
> > On Wed 2006-09-20 21:54:38, Rafael J. Wysocki wrote:
> > > In order to use a swap file with swsusp we need to know the offset at which
> > > its swap header is located. However, the swap header is always located in the
> > > first block of the swap file and it's quite easy to make sys_swapon() print
> > > the offset of the swap file's (or swap partition's) first block.
> > >
> > > Signed-off-by: Rafael J. Wysocki <[email protected]>
> >
> > ACK.
> >
> > (main point of this is that it changes user-visible printk, but that's
> > probably okay, and way better than changing /proc files...)
>
> As a user-interface, "read the number from dmesg that swapon printk'd
> and add that to your boot command line" is pretty damned awful.

Yes, it is. However, the only solution I've invented so far is to print
that number in /proc/swaps, but that would be changing the
kernel-user interface quite badly ...

> It means that it's next to impossible for a distro installer to support
> suspend-to-swapfile.

Not necessarily. After creating the swap file the installer can bmap it
and figure out the offset from there (it will require some rescaling, but the
code for that is in mm/swapfile.c).

> It's also incredibly fragile to rely on that file always remaining in that
> part of the disk.

Well, if you don't delete it, it'll stay there. ;-)

The entire problem is we can't use file names during resume, because we can't
mount filesystems at that time, so we need to represent the swap header's
location in a filesystem-independent way.

Greetings,
Rafael


--
You never change things by fighting the existing reality.
R. Buckminster Fuller

2006-09-21 23:58:41

by Jason Lunz

[permalink] [raw]
Subject: Re: [PATCH -mm 5/6] mm: Print first block offset for swap areas

> The entire problem is we can't use file names during resume, because
> we can't mount filesystems at that time, so we need to represent the
> swap header's location in a filesystem-independent way.

grub reads files without mounting the filesystem. And it has to find the
entire file, not just the beginning. Maybe swsusp could use that
technique? If not the in-kernel one, surely the userland version
could.

Jason

2006-09-22 01:01:57

by Nigel Cunningham

[permalink] [raw]
Subject: Re: [PATCH -mm 0/6] swsusp: Add support for swap files

Hi.

On Wed, 2006-09-20 at 21:20 +0200, Rafael J. Wysocki wrote:
> Hi,
>
> The following series of patches makes swsusp support swap files.
>
> For now, it is only possible to suspend to a swap file using the in-kernel
> swsusp and the resume cannot be initiated from an initrd.

I'm trying to understand 'resume cannot be initiated from an initrd'.
Does that mean if you want to use this functionality, you have to have
everything needed compiled in to the kernel, and it's not compatible
with LVM and so on?

Regards,

Nigel

2006-09-22 05:23:31

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH -mm 0/6] swsusp: Add support for swap files

On Fri 2006-09-22 11:01:53, Nigel Cunningham wrote:
> Hi.
>
> On Wed, 2006-09-20 at 21:20 +0200, Rafael J. Wysocki wrote:
> > Hi,
> >
> > The following series of patches makes swsusp support swap files.
> >
> > For now, it is only possible to suspend to a swap file using the in-kernel
> > swsusp and the resume cannot be initiated from an initrd.
>
> I'm trying to understand 'resume cannot be initiated from an initrd'.
> Does that mean if you want to use this functionality, you have to have
> everything needed compiled in to the kernel, and it's not compatible
> with LVM and so on?

Not in this version of patch; for resume from initrd, ioctl()
interface needs to be added (*).
Pavel
(*) Actually.. of course resume from file from initrd is possible
*now*, probably without this patch series, but that would be bmap and
doing it by hand from userland.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2006-09-22 10:54:16

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH -mm 5/6] mm: Print first block offset for swap areas

On Friday, 22 September 2006 01:58, Jason Lunz wrote:
> > The entire problem is we can't use file names during resume, because
> > we can't mount filesystems at that time, so we need to represent the
> > swap header's location in a filesystem-independent way.
>
> grub reads files without mounting the filesystem. And it has to find the
> entire file, not just the beginning. Maybe swsusp could use that
> technique? If not the in-kernel one, surely the userland version
> could.

This is filesystem-dependent. AFAICT not all filesystems are supported
by GRUB.

Greetings,
Rafael


--
You never change things by fighting the existing reality.
R. Buckminster Fuller

2006-09-22 11:26:04

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH -mm 0/6] swsusp: Add support for swap files

On Friday, 22 September 2006 07:23, Pavel Machek wrote:
> On Fri 2006-09-22 11:01:53, Nigel Cunningham wrote:
> > Hi.
> >
> > On Wed, 2006-09-20 at 21:20 +0200, Rafael J. Wysocki wrote:
> > > Hi,
> > >
> > > The following series of patches makes swsusp support swap files.
> > >
> > > For now, it is only possible to suspend to a swap file using the in-kernel
> > > swsusp and the resume cannot be initiated from an initrd.
> >
> > I'm trying to understand 'resume cannot be initiated from an initrd'.
> > Does that mean if you want to use this functionality, you have to have
> > everything needed compiled in to the kernel, and it's not compatible
> > with LVM and so on?
>
> Not in this version of patch; for resume from initrd, ioctl()
> interface needs to be added (*).

Yup. This is not technically impossible, but the patches don't add an
interface needed for this purpose.

Initially I thought of a sysfs-based one, but it didn't seem to be a good
solution. I'm going to add an ioctl() to /dev/snapshot that will allow us
to set the "resume offset" from an application.

> Pavel
> (*) Actually.. of course resume from file from initrd is possible
> *now*, probably without this patch series, but that would be bmap and
> doing it by hand from userland.

Well, not from a swap file. To use a swap file for suspending we need a
kernel to tell us which page "slots" are available to us (otherwise we could
overwrite some swapped-out pages).

We could use a regular (non-swap) file like this but that would require us to
use some dangerous code (ie. one that writes directly to blocks belonging to
certain file bypassing the filesystem). IMHO this isn't worth it, provided
the kernel's swap-handling code can do this for us and is known to work. ;-)

Greetings,
Rafael


--
You never change things by fighting the existing reality.
R. Buckminster Fuller

2006-09-22 13:34:25

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH -mm 4/6] swsusp: Add resume_offset command line parameter

Hi!

> > > Add the kernel command line parameter "resume_offset=" allowing us to specify
> > > the offset, in <PAGE_SIZE> units, from the beginning of the partition pointed
> > > to by the "resume=" parameter at which the swap header is located.
> > >
> > > Signed-off-by: Rafael J. Wysocki <[email protected]>
> >
> > Okay, I'd prefer not to add aditional features to in-kernel swsusp,
> > but this is just not big enough to reject.
> >
> > ACK.
> >
> > (What is the solution for uswsusp?)
>
> We need an additional ioctl to set both the swap partition and "resume offset"
> at a time (the one we currently have allows us only to set the partition and I
> don't want to change it because of the backwards compatibility, but I think
> I'll change its name ;-) ).
>
> I'd like to add this ioctl along with the one needed to support the "platform"
> method of powering off, because it will require some similar documentation
> changes (most importantly, the description of the interface in
> Documentation/ABI which I'd like to add once and not to tamper with
> afterwards).

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

2006-09-22 14:13:59

by Jason Lunz

[permalink] [raw]
Subject: Re: [PATCH -mm 5/6] mm: Print first block offset for swap areas

On Fri, Sep 22, 2006 at 12:57:11PM +0200, Rafael J. Wysocki wrote:
> This is filesystem-dependent. AFAICT not all filesystems are supported
> by GRUB.

of course, but it shows the technique is viable. Grub is
widespread, and if it's good enough for so many x86 users to boot with
then the same approach ought to be adequate for resume, no?

Jason

2006-09-22 14:18:24

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH -mm 5/6] mm: Print first block offset for swap areas

On Fri 2006-09-22 10:13:47, Jason Lunz wrote:
> On Fri, Sep 22, 2006 at 12:57:11PM +0200, Rafael J. Wysocki wrote:
> > This is filesystem-dependent. AFAICT not all filesystems are supported
> > by GRUB.
>
> of course, but it shows the technique is viable. Grub is
> widespread, and if it's good enough for so many x86 users to boot with
> then the same approach ought to be adequate for resume, no?

Well, most people "solve" this by having their boot partition on ext2,
no?

Anyway, yes, you can do libext2 magic... in uswsusp..

Or you can just patch "real_read_only" mode to journalling
filesystem. That should be easy enough, and that patch would be
welcome for other reasons (data recovery), too. Please do it :-).
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2006-09-22 14:30:05

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH -mm 5/6] mm: Print first block offset for swap areas

On Friday, 22 September 2006 16:13, Jason Lunz wrote:
> On Fri, Sep 22, 2006 at 12:57:11PM +0200, Rafael J. Wysocki wrote:
> > This is filesystem-dependent. AFAICT not all filesystems are supported
> > by GRUB.
>
> of course, but it shows the technique is viable. Grub is
> widespread, and if it's good enough for so many x86 users to boot with
> then the same approach ought to be adequate for resume, no?

Well, I'd like to avoid making _any_ assumptions regarding the partition
on which the "resume" swap file is located.

Also I think we can implement a simpler approach (ie. one that requires less
code changes in both the kernel and userland) first and then look for a
"nicer" one.

Greetings,
Rafael


--
You never change things by fighting the existing reality.
R. Buckminster Fuller

2006-09-22 14:33:20

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH -mm 5/6] mm: Print first block offset for swap areas

Hi!

> > > This is filesystem-dependent. AFAICT not all filesystems are supported
> > > by GRUB.
> >
> > of course, but it shows the technique is viable. Grub is
> > widespread, and if it's good enough for so many x86 users to boot with
> > then the same approach ought to be adequate for resume, no?
>
> Well, I'd like to avoid making _any_ assumptions regarding the partition
> on which the "resume" swap file is located.
>
> Also I think we can implement a simpler approach (ie. one that requires less
> code changes in both the kernel and userland) first and then look for a
> "nicer" one.

Agreed.

(And someone else can create those "realreadonly" patches in the
meantime.)
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2006-09-22 14:35:47

by Jason Lunz

[permalink] [raw]
Subject: Re: [PATCH -mm 5/6] mm: Print first block offset for swap areas

> Well, most people "solve" this by having their boot partition on ext2,
> no?

my grub appears to handle ext2/3, reiser3, xfs, jfs, fat, and minix.

> Anyway, yes, you can do libext2 magic... in uswsusp..

a hybrid approach might work, with grub-like support for common filesystems and
the ability to specify the resume_offset on the kernel command line as a fallback.

Jason

2006-09-22 14:37:39

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH -mm 5/6] mm: Print first block offset for swap areas

On Fri 2006-09-22 10:35:39, Jason Lunz wrote:
> > Well, most people "solve" this by having their boot partition on ext2,
> > no?
>
> my grub appears to handle ext2/3, reiser3, xfs, jfs, fat, and minix.
>
> > Anyway, yes, you can do libext2 magic... in uswsusp..
>
> a hybrid approach might work, with grub-like support for common filesystems and
> the ability to specify the resume_offset on the kernel command line as a fallback.

Let's do "realreadonly" instead. It is right thing to do.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2006-09-22 22:18:51

by Nigel Cunningham

[permalink] [raw]
Subject: Re: [PATCH -mm 0/6] swsusp: Add support for swap files

Hi.

On Fri, 2006-09-22 at 13:28 +0200, Rafael J. Wysocki wrote:
> On Friday, 22 September 2006 07:23, Pavel Machek wrote:
> > On Fri 2006-09-22 11:01:53, Nigel Cunningham wrote:
> > > Hi.
> > >
> > > On Wed, 2006-09-20 at 21:20 +0200, Rafael J. Wysocki wrote:
> > > > Hi,
> > > >
> > > > The following series of patches makes swsusp support swap files.
> > > >
> > > > For now, it is only possible to suspend to a swap file using the in-kernel
> > > > swsusp and the resume cannot be initiated from an initrd.
> > >
> > > I'm trying to understand 'resume cannot be initiated from an initrd'.
> > > Does that mean if you want to use this functionality, you have to have
> > > everything needed compiled in to the kernel, and it's not compatible
> > > with LVM and so on?
> >
> > Not in this version of patch; for resume from initrd, ioctl()
> > interface needs to be added (*).
>
> Yup. This is not technically impossible, but the patches don't add an
> interface needed for this purpose.
>
> Initially I thought of a sysfs-based one, but it didn't seem to be a good
> solution. I'm going to add an ioctl() to /dev/snapshot that will allow us
> to set the "resume offset" from an application.
> problem I not
> > Pavel
> > (*) Actually.. of course resume from file from initrd is possible
> > *now*, probably without this patch series, but that would be bmap and
> > doing it by hand from userland.
>
> Well, not from a swap file. To use a swap file for suspending we need a
> kernel to tell us which page "slots" are available to us (otherwise we could
> overwrite some swapped-out pages).
>
> We could use a regular (non-swap) file like this but that would require us to
> use some dangerous code (ie. one that writes directly to blocks belonging to
> certain file bypassing the filesystem). IMHO this isn't worth it, provided
> the kernel's swap-handling code can do this for us and is known to work. ;-)

It's not that dangerous once you debug it. This is what Suspend2 does -
all I/O is done using bmapping and bios with direct sector numbers,
regardless of where you're reading from/writing to. The main difficulty
I saw was with XFS, where the device block size and filesystem block
size can differ, but even then, it's just a matter of making sure you
get the right one in the right place.

I would encourage you in this direction because it also adds way more
flexibility. If swap is a thing of the past, the only reason for people
to have swap space now is to suspend to disk. If you can write to a swap
file, they don't need a swap partition and more. If you can write to an
ordinary file, they can know that even if they are in a low memory
situation and swap is being used, there's no race condition between
freeing up memory to meet the conditions for suspending to disk, and
allocating storage for writing the actual image.

Regards,

Nigel

2006-09-23 22:16:10

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH -mm 0/6] swsusp: Add support for swap files

Hi,

On Saturday, 23 September 2006 00:18, Nigel Cunningham wrote:
> Hi.
>
> On Fri, 2006-09-22 at 13:28 +0200, Rafael J. Wysocki wrote:
> > On Friday, 22 September 2006 07:23, Pavel Machek wrote:
> > > On Fri 2006-09-22 11:01:53, Nigel Cunningham wrote:
> > > > Hi.
> > > >
> > > > On Wed, 2006-09-20 at 21:20 +0200, Rafael J. Wysocki wrote:
> > > > > Hi,
> > > > >
> > > > > The following series of patches makes swsusp support swap files.
> > > > >
> > > > > For now, it is only possible to suspend to a swap file using the in-kernel
> > > > > swsusp and the resume cannot be initiated from an initrd.
> > > >
> > > > I'm trying to understand 'resume cannot be initiated from an initrd'.
> > > > Does that mean if you want to use this functionality, you have to have
> > > > everything needed compiled in to the kernel, and it's not compatible
> > > > with LVM and so on?
> > >
> > > Not in this version of patch; for resume from initrd, ioctl()
> > > interface needs to be added (*).
> >
> > Yup. This is not technically impossible, but the patches don't add an
> > interface needed for this purpose.
> >
> > Initially I thought of a sysfs-based one, but it didn't seem to be a good
> > solution. I'm going to add an ioctl() to /dev/snapshot that will allow us
> > to set the "resume offset" from an application.
> > problem I not
> > > Pavel
> > > (*) Actually.. of course resume from file from initrd is possible
> > > *now*, probably without this patch series, but that would be bmap and
> > > doing it by hand from userland.
> >
> > Well, not from a swap file. To use a swap file for suspending we need a
> > kernel to tell us which page "slots" are available to us (otherwise we could
> > overwrite some swapped-out pages).
> >
> > We could use a regular (non-swap) file like this but that would require us to
> > use some dangerous code (ie. one that writes directly to blocks belonging to
> > certain file bypassing the filesystem). IMHO this isn't worth it, provided
> > the kernel's swap-handling code can do this for us and is known to work. ;-)
>
> It's not that dangerous once you debug it.

Certainly. Still, let me repeat: if there is some code that does pretty much
the same and has _already_ been debugged, I prefer using it to writing some
new code, debugging it etc.

> This is what Suspend2 does -
> all I/O is done using bmapping and bios with direct sector numbers,
> regardless of where you're reading from/writing to. The main difficulty
> I saw was with XFS, where the device block size and filesystem block
> size can differ, but even then, it's just a matter of making sure you
> get the right one in the right place.
>
> I would encourage you in this direction because it also adds way more
> flexibility. If swap is a thing of the past, the only reason for people
> to have swap space now is to suspend to disk. If you can write to a swap
> file, they don't need a swap partition and more.

Obviously. That's what the patches are for. :-)

> If you can write to an
> ordinary file, they can know that even if they are in a low memory
> situation and swap is being used, there's no race condition between
> freeing up memory to meet the conditions for suspending to disk, and
> allocating storage for writing the actual image.

Well, I wouldn't call that a race condition.

By using an ordinary file to save the suspend image, you effectively divide
the whole storage needed for suspending in two independent parts. Still
the sum of these two parts has to be sufficient to save the total amount of
data that cannot be discarded residing in memory before the suspend. IMO
it doesn't really matter if this storage is divided or not, because to total
size of it has to be sufficient regardless.

Greetings,
Rafael


--
You never change things by fighting the existing reality.
R. Buckminster Fuller

2006-09-23 22:49:52

by Nigel Cunningham

[permalink] [raw]
Subject: Re: [PATCH -mm 0/6] swsusp: Add support for swap files

Hi.

On Sun, 2006-09-24 at 00:18 +0200, Rafael J. Wysocki wrote:
> > > We could use a regular (non-swap) file like this but that would require us to
> > > use some dangerous code (ie. one that writes directly to blocks belonging to
> > > certain file bypassing the filesystem). IMHO this isn't worth it, provided
> > > the kernel's swap-handling code can do this for us and is known to work. ;-)
> >
> > It's not that dangerous once you debug it.
>
> Certainly. Still, let me repeat: if there is some code that does pretty much
> the same and has _already_ been debugged, I prefer using it to writing some
> new code, debugging it etc.

Look at Suspend2 then. I know you won't want it in exactly that form,
but it's there and have been tested and debugged.

> > This is what Suspend2 does -
> > all I/O is done using bmapping and bios with direct sector numbers,
> > regardless of where you're reading from/writing to. The main difficulty
> > I saw was with XFS, where the device block size and filesystem block
> > size can differ, but even then, it's just a matter of making sure you
> > get the right one in the right place.
> >
> > I would encourage you in this direction because it also adds way more
> > flexibility. If swap is a thing of the past, the only reason for people
> > to have swap space now is to suspend to disk. If you can write to a swap
> > file, they don't need a swap partition and more.
>
> Obviously. That's what the patches are for. :-)
>
> > If you can write to an
> > ordinary file, they can know that even if they are in a low memory
> > situation and swap is being used, there's no race condition between
> > freeing up memory to meet the conditions for suspending to disk, and
> > allocating storage for writing the actual image.
>
> Well, I wouldn't call that a race condition.

You want to write the image to swap (any kind) but you need to use swap
to free up enough memory so that you can make the image and write it.
But wait, in freeing enough memory, you reduced the amount of swap
available for your image, so now you need to free more...

> By using an ordinary file to save the suspend image, you effectively divide
> the whole storage needed for suspending in two independent parts. Still
> the sum of these two parts has to be sufficient to save the total amount of
> data that cannot be discarded residing in memory before the suspend. IMO
> it doesn't really matter if this storage is divided or not, because to total
> size of it has to be sufficient regardless.

You're forgetting, I think, that for Suspend2, we're not usually
swapping anything out. It would only be under very unusual memory load
that we'd be swapping memory out, and that would only be after caches
were discarded, which would probably address the situation itself.
There's therefore no division of the image.

Regards,

Nigel

2006-09-23 23:00:00

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH -mm 0/6] swsusp: Add support for swap files

Hi!

> > > > We could use a regular (non-swap) file like this but that would require us to
> > > > use some dangerous code (ie. one that writes directly to blocks belonging to
> > > > certain file bypassing the filesystem). IMHO this isn't worth it, provided
> > > > the kernel's swap-handling code can do this for us and is known to work. ;-)
> > >
> > > It's not that dangerous once you debug it.
> >
> > Certainly. Still, let me repeat: if there is some code that does pretty much
> > the same and has _already_ been debugged, I prefer using it to writing some
> > new code, debugging it etc.
>
> Look at Suspend2 then. I know you won't want it in exactly that form,
> but it's there and have been tested and debugged.

Well, but any testing/debugging would have to be invalidated for a
merge, sorry. suspend2 has diverged too much.

"normal files" are not big priority for us, and you could probably do
them in userland just now.... Anyway, diff -u for kernel or preferably
for uswsusp parts would be welcome.

Telling us how much suspend2 rocks with each and every mail... is not
that welcome.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2006-09-23 23:24:21

by Nigel Cunningham

[permalink] [raw]
Subject: Re: [PATCH -mm 0/6] swsusp: Add support for swap files

Hi.

On Sun, 2006-09-24 at 00:59 +0200, Pavel Machek wrote:
> Hi!
>
> > > > > We could use a regular (non-swap) file like this but that would require us to
> > > > > use some dangerous code (ie. one that writes directly to blocks belonging to
> > > > > certain file bypassing the filesystem). IMHO this isn't worth it, provided
> > > > > the kernel's swap-handling code can do this for us and is known to work. ;-)
> > > >
> > > > It's not that dangerous once you debug it.
> > >
> > > Certainly. Still, let me repeat: if there is some code that does pretty much
> > > the same and has _already_ been debugged, I prefer using it to writing some
> > > new code, debugging it etc.
> >
> > Look at Suspend2 then. I know you won't want it in exactly that form,
> > but it's there and have been tested and debugged.
>
> Well, but any testing/debugging would have to be invalidated for a
> merge, sorry. suspend2 has diverged too much.
>
> "normal files" are not big priority for us, and you could probably do
> them in userland just now.... Anyway, diff -u for kernel or preferably
> for uswsusp parts would be welcome.
>
> Telling us how much suspend2 rocks with each and every mail... is not
> that welcome.

That's not what I was doing. I was saying that Rafael could, if he
wanted, at least look at how Suspend2 has done it and use that (if he
thinks its helpful) as a basis for improving or whatever. If he does
essentially the same thing for swsusp, he could again look at suspend2
loops etc to help in double checking his algorithm and so on.

Regards,

Nigel