Skip to contentSkip to conversation

swsusp: rework swap handling

RFC PATCH In review Rafael J. Wysocki 3 participants 17 messages Oct 29, 2005 – Oct 30, 2005

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover swsusp: rework swap handling

Hi,

The following series of patches divides swsusp into two functionally independent subsystems:

- the snapshot-handling part responsible for creating and populating the snapshot-related data structure which is the list of page backup entries aka pagedir,

- the swap-handling part responsible for writing the snapshot data to and reading it from swap.

On suspend the snapshot-handling part creates the system snapshot and makes the data stored in the snapshot available to the swap-handling part via an interface function allowing it to transfer the data as a series of consecutive data pages, in a specific order. The swap-handling part writes the data pages to a swap partition is such a way that they can be read in exactly the same order in which they have been saved.

On resume the snapshot-handling part is invoked by the swap-handling part to create the pagedir. Then, the swap-handling part is allowed to send it, with the help of an interface function, data pages that are used to populate the snapshot data structure. It is assumed that the data pages will be sent in the same order in which they have been received by the swap-handling part on suspend. Finally, the system state (from before suspend) is restored by the snaphot-handling part from the data structure handled by it.

Quoted 1 line

From the point of view of the swap-handling part, the contents of the

data pages provided by the snapshot-handling do not matter at all. It handles each data page in the same way without analyzing its contents and the snapshot-handling part is responsible for recognizing the metadata and using them as appropriate. Consequently, in principle the swap-handling part can be replaced with a user-space process and the interface functions used in transferring data between the two parts of swsusp can be replaced with a relatively simple kernel-user interface in the future.

The approach used in this series of patches has some additional benefits:
1) the size of the pagedir is reduced by 1/4 which causes some more memory to be available on resume,
2) the amount of metadata written to swap is reduced by 3/4,
3) the artificial limitation on the pagedir size, imposed by the size of the swsusp_info structure, is lifted,
4) the size of swsusp_info structure is reduced so it can be merged with the swsusp_header structure in the future,
5) the swap-handling part does not use any global variables related to the snapshot data structure,
6) the __nosavedata variables are almost eliminated (on x86-64 the last of them is the in_suspend variable).

I have divided the changes into some more or less logical steps for clarity. Although the code has been designed as proof-of-concept, it is functional and has been tested on x86-64, except for the cryptographic functionality and error paths.

For your convenience the patches are available from:
http://www.sisk.pl/kernel/patches/2.6.14-rc5-mm1/

I will very much appreciate any comments, remarks and suggestions.

Greetings,
Rafael

#1 lore raw

RFC PATCH 1/6 swsusp: rework swsusp_suspend

This is a preliminary step. It makes only the functions in swsusp.c call functions in snapshot.c and not both ways. Basically, it moves the code without changing its functionality.

 kernel/power/power.h    |    2 -
 kernel/power/snapshot.c |   14 +---------
 kernel/power/swsusp.c   |   62 +++++++++++++++++++++++++++---------------------
 3 files changed, 39 insertions(+), 39 deletions(-)
kernel/power/snapshot.c +2 -12
Index: linux-2.6.14-rc5-mm1/kernel/power/snapshot.c
===================================================================
@@ -89,7 +89,7 @@
 }
 
 
-static int save_highmem(void)
+int save_highmem(void)
 {
 	struct zone *zone;
 	int res = 0;
@@ -121,7 +121,7 @@
 	return 0;
 }
 #else
-static int save_highmem(void) { return 0; }
+int save_highmem(void) { return 0; }
 int restore_highmem(void) { return 0; }
 #endif /* CONFIG_HIGHMEM */
 
@@ -383,11 +383,6 @@
 	unsigned nr_pages;
 
 	pr_debug("swsusp: critical section: \n");
-	if (save_highmem()) {
-		printk(KERN_CRIT "swsusp: Not enough free pages for highmem\n");
-		restore_highmem();
-		return -ENOMEM;
-	}
 
 	drain_local_pages();
 	nr_pages = count_data_pages();
@@ -407,11 +402,6 @@
 		return -ENOMEM;
 	}
 
-	if (!enough_swap(nr_pages)) {
-		printk(KERN_ERR "swsusp: Not enough free swap\n");
-		return -ENOSPC;
-	}
-
 	pagedir_nosave = swsusp_alloc(nr_pages);
 	if (!pagedir_nosave)
 		return -ENOMEM;
kernel/power/swsusp.c +36 -26

104 lines not on this page · raw message

kernel/power/power.h +1 -1
Index: linux-2.6.14-rc5-mm1/kernel/power/power.h
===================================================================
@@ -65,8 +65,8 @@
 extern asmlinkage int swsusp_arch_suspend(void);
 extern asmlinkage int swsusp_arch_resume(void);
 
+extern int save_highmem(void);
 extern int restore_highmem(void);
 extern struct pbe * alloc_pagedir(unsigned nr_pages);
 extern void create_pbe_list(struct pbe *pblist, unsigned nr_pages);
 extern void swsusp_free(void);
-extern int enough_swap(unsigned nr_pages);
#2 lore raw

Re: [RFC][PATCH 1/6] swsusp: rework swsusp_suspend

Quoted 6 lines

This is a preliminary step. It makes only the functions in swsusp.c call functions in snapshot.c and not both ways. Basically, it moves the code without changing its functionality.

ACK, nice.
Pavel

Signature · 1 line
Thanks, Sharp!
#3 lore raw

RFC PATCH 2/6 swsusp: move snapshot-handling functions to snapshot.c

In reply to Rafael J. Wysocki

This is another preliminary step. It moves the snapshot-handling functions remaining in swsusp.c to snapshot.c (moving the code without changing the functionality) and makes the next patch be more clear (in my opinion).

 kernel/power/power.h    |    2 
 kernel/power/snapshot.c |  121 ++++++++++++++++++++++++++++++++++++++++++++++
 kernel/power/swsusp.c   |  126 ------------------------------------------------
 3 files changed, 125 insertions(+), 124 deletions(-)
kernel/power/power.h +2 -0
Index: linux-2.6.14-rc5-mm1/kernel/power/power.h
===================================================================
@@ -69,4 +69,6 @@
 extern int restore_highmem(void);
 extern struct pbe * alloc_pagedir(unsigned nr_pages);
 extern void create_pbe_list(struct pbe *pblist, unsigned nr_pages);
+extern int check_pagedir(struct pbe *pblist);
+extern struct pbe * swsusp_pagedir_relocate(struct pbe *pblist);
 extern void swsusp_free(void);
kernel/power/snapshot.c +121 -0

124 lines not on this page · raw message

kernel/power/swsusp.c +2 -124

138 lines not on this page · raw message

#4 lore raw

Re: [RFC][PATCH 2/6] swsusp: move snapshot-handling functions to snapshot.c

Quoted 5 lines · Rafael J. Wysocki

This is another preliminary step. It moves the snapshot-handling functions remaining in swsusp.c to snapshot.c (moving the code without changing the functionality) and makes the next patch be more clear (in my opinion).

ACK and thanks.
Pavel

Signature · 1 line
Thanks, Sharp!
#5 lore raw

RFC PATCH 3/6 swsusp: introduce the swap map structure and interface functions

In reply to Rafael J. Wysocki

This is the main part. It introduces a new data structure for the swap-handling part of swsusp (the swap map structure, described in a comment) and new functions for writing the image data to and reading them from swap. It also introduces the interface functions allowing the snapshot-handling part to communicate with the swap-handling part and modifies the struct pbe structure (the swap_address member of it is no longer needed as the swap-handling part uses its own independent data structures).

 include/linux/suspend.h |    6 
 kernel/power/power.h    |   25 +-
 kernel/power/snapshot.c |  174 +++++++++++++----
 kernel/power/swsusp.c   |  475 ++++++++++++++++++++++++++----------------------
 4 files changed, 404 insertions(+), 276 deletions(-)
kernel/power/swsusp.c +259 -216
Index: linux-2.6.14-rc5-mm1/kernel/power/swsusp.c
===================================================================
@@ -33,6 +33,9 @@
  * Andreas Steinmetz <[email protected]>:
  * Added encrypted suspend option
  *
+ * Rafael J. Wysocki <[email protected]>
+ * Added the swap map data structure and reworked the handling of swap
+ *
  * More state savers are welcome. Especially for the scsi layer...
  *
  * For TODOs,FIXMEs also look in Documentation/power/swsusp.txt
@@ -79,24 +82,6 @@
 
 extern char resume_file[];
 
-/* Local variables that should not be affected by save */
-unsigned int nr_copy_pages __nosavedata = 0;
-
-/* Suspend pagedir is allocated before final copy, therefore it
-   must be freed after resume
-
-   Warning: this is evil. There are actually two pagedirs at time of
-   resume. One is "pagedir_save", which is empty frame allocated at
-   time of suspend, that must be freed. Second is "pagedir_nosave",
-   allocated at time of resume, that travels through memory not to
-   collide with anything.
-
-   Warning: this is even more evil than it seems. Pagedirs this file
-   talks about are completely different from page directories used by
-   MMU hardware.
- */
-suspend_pagedir_t *pagedir_nosave __nosavedata = NULL;
-suspend_pagedir_t *pagedir_save;
 
 #define SWSUSP_SIG	"S1SUSPEND"
 
@@ -187,12 +172,12 @@
 	crypto_free_tfm((struct crypto_tfm *)mem);
 }
 
-static __inline__ int crypto_write(struct pbe *p, void *mem)
+static __inline__ int crypto_write(unsigned long addr, swp_entry_t *entry, void *mem)
 {
 	int error = 0;
 	struct scatterlist src, dst;
 
-	src.page   = virt_to_page(p->address);
+	src.page   = virt_to_page((void *)addr);
 	src.offset = 0;
 	src.length = PAGE_SIZE;
 	dst.page   = virt_to_page((void *)&swsusp_header);
@@ -203,23 +188,22 @@
 					PAGE_SIZE);
 
 	if (!error)
-		error = write_page((unsigned long)&swsusp_header,
-				&(p->swap_address));
+		error = write_page((unsigned long)&swsusp_header, entry);
 	return error;
 }
 
-static __inline__ int crypto_read(struct pbe *p, void *mem)
+static __inline__ int crypto_read(unsigned long offset, void *buf, void *mem)
 {
 	int error = 0;
 	struct scatterlist src, dst;
 
-	error = bio_read_page(swp_offset(p->swap_address), (void *)p->address);
+	error = bio_read_page(offset, buf);
 	if (!error) {
 		src.offset = 0;
 		src.length = PAGE_SIZE;
 		dst.offset = 0;
 		dst.length = PAGE_SIZE;
-		src.page = dst.page = virt_to_page((void *)p->address);
+		src.page = dst.page = virt_to_page(buf);
 
 		error = crypto_cipher_decrypt((struct crypto_tfm *)mem, &dst,
 						&src, PAGE_SIZE);
@@ -236,14 +220,14 @@
 {
 }
 
-static __inline__ int crypto_write(struct pbe *p, void *mem)
+static __inline__ int crypto_write(unsigned long addr, swp_entry_t *entry, void *mem)
 {
-	return write_page(p->address, &(p->swap_address));
+	return write_page(addr, entry);
 }
 
-static __inline__ int crypto_read(struct pbe *p, void *mem)
+static __inline__ int crypto_read(unsigned long offset, void *buf, void *mem)
 {
-	return bio_read_page(swp_offset(p->swap_address), (void *)p->address);
+	return bio_read_page(offset, buf);
 }
 #endif
 
@@ -374,59 +358,6 @@
 	return error;
 }
 
-/**
- *	data_free - Free the swap entries used by the saved image.
- *
- *	Walk the list of used swap entries and free each one.
- *	This is only used for cleanup when suspend fails.
- */
-static void data_free(void)
-{
-	swp_entry_t entry;
-	struct pbe * p;
-
-	for_each_pbe(p, pagedir_nosave) {
-		entry = p->swap_address;
-		if (entry.val)
-			swap_free(entry);
-		else
-			break;
-	}
-}
-
-/**
- *	data_write - Write saved image to swap.
- *
- *	Walk the list of pages in the image and sync each one to swap.
- */
-static int data_write(void)
-{
-	int error = 0, i = 0;
-	unsigned int mod = nr_copy_pages / 100;
-	struct pbe *p;
-	void *tfm;
-
-	if ((error = crypto_init(1, &tfm)))
-		return error;
-
-	if (!mod)
-		mod = 1;
-
-	printk( "Writing data to swap (%d pages)...     ", nr_copy_pages );
-	for_each_pbe (p, pagedir_nosave) {
-		if (!(i%mod))
-			printk( "\b\b\b\b%3d%%", i / mod );
-		if ((error = crypto_write(p, tfm))) {
-			crypto_exit(tfm);
-			return error;
-		}
-		i++;
-	}
-	printk("\b\b\b\bdone\n");
-	crypto_exit(tfm);
-	return error;
-}
-
 static void dump_info(void)
 {
 	pr_debug(" swsusp: Version: %u\n",swsusp_info.version_code);
@@ -439,19 +370,19 @@
 	pr_debug(" swsusp: UTS Domain: %s\n",swsusp_info.uts.domainname);
 	pr_debug(" swsusp: CPUs: %d\n",swsusp_info.cpus);
 	pr_debug(" swsusp: Image: %ld Pages\n",swsusp_info.image_pages);
-	pr_debug(" swsusp: Pagedir: %ld Pages\n",swsusp_info.pagedir_pages);
+	pr_debug(" swsusp: Total: %ld Pages\n", swsusp_info.pages);
 }
 
-static void init_header(void)
+static void init_header(unsigned nr_pages, unsigned img_pages)
 {
 	memset(&swsusp_info, 0, sizeof(swsusp_info));
 	swsusp_info.version_code = LINUX_VERSION_CODE;
 	swsusp_info.num_physpages = num_physpages;
 	memcpy(&swsusp_info.uts, &system_utsname, sizeof(system_utsname));
 
-	swsusp_info.suspend_pagedir = pagedir_nosave;
 	swsusp_info.cpus = num_online_cpus();
-	swsusp_info.image_pages = nr_copy_pages;
+	swsusp_info.image_pages = img_pages;
+	swsusp_info.pages = nr_pages;
 }
 
 static int close_swap(void)
@@ -470,40 +401,167 @@
 }
 
 /**
- *	free_pagedir_entries - Free pages used by the page directory.
+ *	Swap map handling functions
  *
- *	This is used during suspend for error recovery.
+ *	The swap map is a data structure used for keeping track of each page
+ *	written to the swap.  It consists of many swp_map_page structures
+ *	that contain each an array of MAP_PAGE_SIZE swap entries.
+ *	These structures are linked together with the help of either the
+ *	.next (in memory) or the .next_swp (in swap) member.
+ *
+ *	The swap map is created during suspend.  At that time we need to keep
+ *	it in memory, because we have to free all of the allocated swap
+ *	entries if an error occurs.  The memory needed is preallocated
+ *	so that we know in advance if there's enough of it.
+ *
+ *	The first swp_map_page structure is filled with the swap entries that
+ *	correspond to the first MAP_PAGE_SIZE data pages written to swap and
+ *	so on.  After the all of the data pages have been written, the order
+ *	of the swp_map_page structures in the map is reversed so that they
+ *	can be read from swap in the original order.  This causes the data
+ *	pages to be loaded in exactly the same order in which they have been
+ *	saved.
+ *
+ *	During resume we only need to use one swp_map_page structure
+ *	at a time, which means that we only need to use two memory pages for
+ *	reading the image - one for reading the swp_map_page structures
+ *	and the second for reading the data pages from swap.
  */
 
-static void free_pagedir_entries(void)
+#define MAP_PAGE_SIZE	((PAGE_SIZE - sizeof(swp_entry_t) - sizeof(void *)) \
+			/ sizeof(swp_entry_t))
+
+struct swp_map_page {
+	swp_entry_t		entries[MAP_PAGE_SIZE];
+	swp_entry_t		next_swp;
+	struct swp_map_page	*next;
+};
+
+typedef struct swp_map_page swp_map_t;
+
+static inline void free_swp_map(swp_map_t *swp_map)
 {
-	int i;
+	swp_map_t *swp;
+
+	while (swp_map) {
+		swp = swp_map->next;
+		free_page((unsigned long)swp_map);
+		swp_map = swp;
+	}
+}
 
-	for (i = 0; i < swsusp_info.pagedir_pages; i++)
-		swap_free(swsusp_info.pagedir[i]);
+static swp_map_t *alloc_swp_map(unsigned nr_pages)
+{
+	swp_map_t *swp_map, *swp;
+	unsigned n = 0;
+
+	if (!nr_pages)
+		return NULL;
+
+	pr_debug("alloc_swp_map(): nr_pages = %d\n", nr_pages);
+	swp_map = (swp_map_t *)get_zeroed_page(GFP_ATOMIC);
+	swp = swp_map;
+	for (n = MAP_PAGE_SIZE; n < nr_pages; n += MAP_PAGE_SIZE) {
+		swp->next = (swp_map_t *)get_zeroed_page(GFP_ATOMIC);
+		swp = swp->next;
+		if (!swp) {
+			free_swp_map(swp_map);
+			return NULL;
+		}
+	}
+	return swp_map;
 }
 
+static inline swp_map_t *reverse_swp_map(swp_map_t *swp_map)
+{
+	swp_map_t *prev, *next;
+
+	prev = NULL;
+	while (swp_map) {
+		next = swp_map->next;
+		swp_map->next = prev;
+		prev = swp_map;
+		swp_map = next;
+	}
+	return prev;
+}
 
 /**
- *	write_pagedir - Write the array of pages holding the page directory.
- *	@last:	Last swap entry we write (needed for header).
+ *	save_swp_map - save the swap map used for tracing the data pages
+ *	stored in swap
  */
 
-static int write_pagedir(void)
+static int save_swp_map(swp_map_t *swp_map, swp_entry_t *start)
 {
-	int error = 0;
-	unsigned n = 0;
-	struct pbe * pbe;
+	swp_entry_t entry = (swp_entry_t){0};
+	int error;
 
-	printk( "Writing pagedir...");
-	for_each_pb_page (pbe, pagedir_nosave) {
-		if ((error = write_page((unsigned long)pbe, &swsusp_info.pagedir[n++])))
+	while (swp_map) {
+		swp_map->next_swp = entry;
+		if ((error = write_page((unsigned long)swp_map, &entry)))
 			return error;
+		swp_map = swp_map->next;
 	}
+	*start = entry;
+	return 0;
+}
 
-	swsusp_info.pagedir_pages = n;
-	printk("done (%u pages)\n", n);
-	return error;
+static inline void free_swp_map_entries(swp_map_t *swp_map)
+{
+	while (swp_map) {
+		if (swp_map->next_swp.val)
+			swap_free(swp_map->next_swp);
+		swp_map = swp_map->next;
+	}
+}
+
+/**
+ *	save_image - save the image data provided by the snapshot-handling
+ *	part to swap.  The swap map is used for keeping track of the
+ *	saved pages
+ */
+
+static int save_image(swp_map_t *swp, unsigned nr_pages, void *buf)
+{
+	unsigned n, k;
+	int error;
+	unsigned mod = nr_pages / 100;
+	void *tfm;
+
+	if ((error = crypto_init(1, &tfm)))
+		return error;
+	printk("Writing data to swap (%d pages) ...     ", nr_pages);
+	n = 0;
+	while (swp) {
+		for (k = 0; k < MAP_PAGE_SIZE && n < nr_pages; k++, n++) {
+			error = snapshot_send_page(buf);
+			if (!error)
+				error = crypto_write((unsigned long)buf,
+						swp->entries + k, tfm);
+			if (error) {
+				crypto_exit(tfm);
+				return error;
+			}
+			if (!(n % mod))
+				printk("\b\b\b\b%3d%%", n / mod);
+		}
+		swp = swp->next;
+	}
+	printk("\b\b\b\bdone\n");
+	crypto_exit(tfm);
+	return 0;
+}
+
+static inline void free_pages_entries(swp_map_t *swp)
+{
+	unsigned k;
+
+	while (swp) {
+		for (k = 0; k < MAP_PAGE_SIZE; k++)
+			if (swp->entries[k].val)
+				swap_free(swp->entries[k]);
+		swp = swp->next;
+	}
 }
 
 /**
@@ -532,30 +590,49 @@
  */
 static int write_suspend_image(void)
 {
+	unsigned nr_pages;
 	int error;
+	swp_map_t *swp_map;
+	void *buffer;
 
-	if (!enough_swap(nr_copy_pages)) {
+	nr_pages = snapshot_pages_to_save();
+	if (!enough_swap(nr_pages)) {
 		printk(KERN_ERR "swsusp: Not enough free swap\n");
 		return -ENOSPC;
 	}
 
-	init_header();
-	if ((error = data_write()))
-		goto FreeData;
-
-	if ((error = write_pagedir()))
-		goto FreePagedir;
+	init_header(nr_pages, snapshot_image_pages());
+	buffer = (void *)get_zeroed_page(GFP_ATOMIC);
+	if (!buffer)
+		return -ENOMEM;
 
+	swp_map = alloc_swp_map(nr_pages);
+	if (!swp_map) {
+		free_page((unsigned long)buffer);
+		return -ENOMEM;
+	}
+	snapshot_send_init();
+	if ((error = save_image(swp_map, nr_pages, buffer)))
+		goto Free_pages_entries;
+	if ((error = snapshot_finish()))
+		goto Free_pages_entries;
+	swp_map = reverse_swp_map(swp_map);
+	if ((error = save_swp_map(swp_map, &swsusp_info.start)))
+		goto Free_map_entries;
 	if ((error = close_swap()))
-		goto FreePagedir;
- Done:
+		goto Free_map_entries;
+
+Free_mem:
+	free_swp_map(swp_map);
+	free_page((unsigned long)buffer);
 	memset(key_iv, 0, MAXKEY+MAXIV);
 	return error;
- FreePagedir:
-	free_pagedir_entries();
- FreeData:
-	data_free();
-	goto Done;
+
+Free_map_entries:
+	free_swp_map_entries(swp_map);
+Free_pages_entries:
+	free_pages_entries(swp_map);
+	goto Free_mem;
 }
 
 /* It is important _NOT_ to umount filesystems at this point. We want
@@ -719,6 +796,65 @@
 	return submit(WRITE, page_off, page);
 }
 
+/**
+ *	load_image - Load the image and metadata from swap, using the
+ *	snapshot_recv_page() function provided by the snapshot-handling code
+ *
+ *	We assume that the data has been saved using the swap map handling
+ *	functions above
+ */
+static int load_image(unsigned nr_pages, swp_entry_t start)
+{
+	swp_map_t *swp;
+	void *buf;
+	unsigned n, k;
+	unsigned long offset = swp_offset(start);
+	int error;
+	unsigned mod = nr_pages / 100;
+	void *tfm;
+
+	if (!nr_pages || !offset)
+		return -EINVAL;
+
+	if ((error = crypto_init(0, &tfm)))
+		return error;
+
+	buf = (void *)get_zeroed_page(GFP_ATOMIC);
+	if (!buf) {
+		error = -ENOMEM;
+		goto Crypto_exit;
+	}
+	swp = (swp_map_t *)get_zeroed_page(GFP_ATOMIC);
+	if (!swp) {
+		error = -ENOMEM;
+		goto Free_buf;
+	}
+	printk("Loading data from swap (%d pages) ...     ", nr_pages);
+	n = 0;
+	while (n < nr_pages) {
+		if ((error = crypto_read(offset, (void *)swp, tfm)))
+			goto Free;
+		for (k = 0; k < MAP_PAGE_SIZE && n < nr_pages; k++, n++) {
+			error = bio_read_page(swp_offset(swp->entries[k]), buf);
+			if (!error)
+				error = snapshot_recv_page(buf);
+			if (error)
+				goto Free;
+			if (!(n % mod))
+				printk("\b\b\b\b%3d%%", n / mod);
+		}
+		offset = swp_offset(swp->next_swp);
+	}
+	printk("\b\b\b\bdone\n");
+Free:
+	free_page((unsigned long)swp);
+Free_buf:
+	free_page((unsigned long)buf);
+Crypto_exit:
+	crypto_exit(tfm);
+	return error;
+}
+
 /*
  * Sanity check if this image makes sense with this kernel/swap context
  * I really don't think that it's foolproof but more than nothing..
@@ -761,7 +897,6 @@
 		printk(KERN_ERR "swsusp: Resume mismatch: %s\n",reason);
 		return -EPERM;
 	}
-	nr_copy_pages = swsusp_info.image_pages;
 	return error;
 }
 
@@ -789,82 +924,6 @@
 	return error;
 }
 
-/**
- *	data_read - Read image pages from swap.
- *
- *	You do not need to check for overlaps, check_pagedir()
- *	already did that.
- */
-
-static int data_read(struct pbe *pblist)
-{
-	struct pbe * p;
-	int error = 0;
-	int i = 0;
-	int mod = swsusp_info.image_pages / 100;
-	void *tfm;
-
-	if ((error = crypto_init(0, &tfm)))
-		return error;
-
-	if (!mod)
-		mod = 1;
-
-	printk("swsusp: Reading image data (%lu pages):     ",
-			swsusp_info.image_pages);
-
-	for_each_pbe (p, pblist) {
-		if (!(i % mod))
-			printk("\b\b\b\b%3d%%", i / mod);
-
-		if ((error = crypto_read(p, tfm))) {
-			crypto_exit(tfm);
-			return error;
-		}
-
-		i++;
-	}
-	printk("\b\b\b\bdone\n");
-	crypto_exit(tfm);
-	return error;
-}
-
-/**
- *	read_pagedir - Read page backup list pages from swap
- */
-
-static int read_pagedir(struct pbe *pblist)
-{
-	struct pbe *pbpage, *p;
-	unsigned i = 0;
-	int error;
-
-	if (!pblist)
-		return -EFAULT;
-
-	printk("swsusp: Reading pagedir (%lu pages)\n",
-			swsusp_info.pagedir_pages);
-
-	for_each_pb_page (pbpage, pblist) {
-		unsigned long offset = swp_offset(swsusp_info.pagedir[i++]);
-
-		error = -EFAULT;
-		if (offset) {
-			p = (pbpage + PB_PAGE_SKIP)->next;
-			error = bio_read_page(offset, (void *)pbpage);
-			(pbpage + PB_PAGE_SKIP)->next = p;
-		}
-		if (error)
-			break;
-	}
-
-	if (!error)
-		BUG_ON(i != swsusp_info.pagedir_pages);
-
-	return error;
-}
-
-
 static int check_suspend_image(void)
 {
 	int error = 0;
@@ -880,29 +939,13 @@
 
 static int read_suspend_image(void)
 {
-	int error = 0;
-	struct pbe *p;
-
-	if (!(p = alloc_pagedir(nr_copy_pages)))
-		return -ENOMEM;
-
-	if ((error = read_pagedir(p)))
-		return error;
-
-	create_pbe_list(p, nr_copy_pages);
-
-	pr_debug("swsusp: Relocating pagedir (%lu pages to check)\n",
-			swsusp_info.pagedir_pages);
-	if (!(pagedir_nosave = swsusp_pagedir_relocate(p)))
-		return -ENOMEM;
-
-	/* Allocate memory for the image and read the data from swap */
-
-	error = check_pagedir(pagedir_nosave);
+	int error;
 
+	error = snapshot_recv_init(swsusp_info.pages, swsusp_info.image_pages);
 	if (!error)
-		error = data_read(pagedir_nosave);
-
+		error = load_image(swsusp_info.pages, swsusp_info.start);
+	if (!error)
+		error = snapshot_finish();
 	return error;
 }
 
kernel/power/power.h +11 -14
Index: linux-2.6.14-rc5-mm1/kernel/power/power.h
===================================================================
@@ -9,19 +9,14 @@
 #define SUSPEND_CONSOLE	(MAX_NR_CONSOLES-1)
 #endif
 
-#define MAX_PBES	((PAGE_SIZE - sizeof(struct new_utsname) \
-			- 4 - 3*sizeof(unsigned long) - sizeof(int) \
-			- sizeof(void *)) / sizeof(swp_entry_t))
-
 struct swsusp_info {
 	struct new_utsname	uts;
 	u32			version_code;
 	unsigned long		num_physpages;
 	int			cpus;
 	unsigned long		image_pages;
-	unsigned long		pagedir_pages;
-	suspend_pagedir_t	* suspend_pagedir;
-	swp_entry_t		pagedir[MAX_PBES];
+	unsigned long		pages;
+	swp_entry_t		start;
 } __attribute__((aligned(PAGE_SIZE)));
 
 
@@ -58,17 +53,19 @@
 /* References to section boundaries */
 extern const void __nosave_begin, __nosave_end;
 
-extern unsigned int nr_copy_pages;
-extern suspend_pagedir_t *pagedir_nosave;
-extern suspend_pagedir_t *pagedir_save;
+extern struct pbe *pagedir_nosave;
 
 extern asmlinkage int swsusp_arch_suspend(void);
 extern asmlinkage int swsusp_arch_resume(void);
 
 extern int save_highmem(void);
 extern int restore_highmem(void);
-extern struct pbe * alloc_pagedir(unsigned nr_pages);
-extern void create_pbe_list(struct pbe *pblist, unsigned nr_pages);
-extern int check_pagedir(struct pbe *pblist);
-extern struct pbe * swsusp_pagedir_relocate(struct pbe *pblist);
 extern void swsusp_free(void);
+
+extern unsigned snapshot_pages_to_save(void);
+extern unsigned snapshot_image_pages(void);
+extern void snapshot_send_init(void);
+extern int snapshot_send_page(void *buf);
+extern int snapshot_recv_init(unsigned nr_pages, unsigned img_pages);
+extern int snapshot_recv_page(void *buf);
+extern int snapshot_finish(void);
kernel/power/snapshot.c +133 -41

251 lines not on this page · raw message

include/linux/suspend.h +1 -5
Index: linux-2.6.14-rc5-mm1/include/linux/suspend.h
===================================================================
@@ -14,11 +14,7 @@
 typedef struct pbe {
 	unsigned long address;		/* address of the copy */
 	unsigned long orig_address;	/* original address of page */
-	swp_entry_t swap_address;	
-
-	struct pbe *next;	/* also used as scratch space at
-				 * end of page (see link, diskpage)
-				 */
+	struct pbe *next;
 } suspend_pagedir_t;
 
 #define for_each_pbe(pbe, pblist) \
#6 lore raw

Re: [RFC][PATCH 3/6] swsusp: introduce the swap map structure and interface functions

Hi!

Quoted 7 lines

This is the main part. It introduces a new data structure for the swap-handling part of swsusp (the swap map structure, described in a comment) and new functions for writing the image data to and reading them from swap. It also introduces the interface functions allowing the snapshot-handling part to communicate with the swap-handling part and modifies the struct pbe structure (the swap_address member of it is no longer needed as the swap-handling part uses its own independent data structures).

One small comment. I miss "a" in "swap". Pretty please...

Quoted 3 lines

+ * Rafael J. Wysocki <[email protected]>
+ * Added the swap map data structure and reworked the handling of swap
+ *

Feel free to add yourself to CREDITS, too. CREDITS are going to stay, but this does not really belong here, and may have to be moved somewhere else in future.

Quoted 5 lines

+ * During resume we only need to use one swp_map_page structure
+ * at a time, which means that we only need to use two memory pages for
+ * reading the image - one for reading the swp_map_page structures
+ * and the second for reading the data pages from swap.
*/

Nice...

kernel/power/swsusp.c line 440

Quoted 7 lines
kernel/power/swsusp.c +7 -0
+struct swp_map_page {
+	swp_entry_t		entries[MAP_PAGE_SIZE];
+	swp_entry_t		next_swp;
+	struct swp_map_page	*next;
+};
+
+typedef struct swp_map_page swp_map_t;

Please don't. Just use "struct swap_map_page" instead.

kernel/power/power.h line 66

Quoted 2 lines
kernel/power/power.h +2 -0
+extern unsigned snapshot_pages_to_save(void);
+extern unsigned snapshot_image_pages(void);

Make it "extern unsigned int". (That is in more than one place).

Quoted 1 line

+int snapshot_recv_init(unsigned nr_pages, unsigned img_pages)

Please, usefull words, that's receive_init and image_pages....

Otherwise it looks good. I'll check it once more...

Pavel

Signature · 1 line
Thanks, Sharp!
#7 lore raw

Re: [RFC][PATCH 3/6] swsusp: introduce the swap map structure and interface functions

In reply to Rafael J. Wysocki

Hi!

Quoted 115 lines

Index: linux-2.6.14-rc5-mm1/kernel/power/snapshot.c
===================================================================

kernel/power/snapshot.c +111 -0
+
+unsigned snapshot_pages_to_save(void)
+{
+	return nr_copy_pages + nr_pb_pages;
+}
+
+unsigned snapshot_image_pages(void)
+{
+	return nr_copy_pages;
+}
+
+static unsigned current_page = 0;
+static struct pbe *current_pbe;
+static struct pbe *current_pblist;
+
+void snapshot_send_init(void)
+{
+	current_page = 0;
+	current_pbe = pagedir_nosave;
+	current_pblist = NULL;
+}
+
+static void prepare_next_pb_page(unsigned long *buf)
+{
+	struct pbe *p;
+	unsigned n;
+
+	p  = current_pbe;
+	for (n = 0; n < PAGE_SIZE/sizeof(long) && p; n++) {
+		buf[n] = p->orig_address;
+		p = p->next;
+	}
+	current_pbe = p;
+}
+
+int snapshot_send_page(void *buf)
+{
+	if (current_page >= nr_copy_pages + nr_pb_pages)
+		return -EINVAL;
+	if (current_page < nr_pb_pages) {
+		prepare_next_pb_page(buf);
+		if (!current_pbe)
+			current_pbe = pagedir_nosave;
+	} else {
+		memcpy(buf, (void *)current_pbe->address, PAGE_SIZE);
+		current_pbe = current_pbe->next;
+	}
+	current_page++;
+	return 0;
+}
+
+int snapshot_recv_init(unsigned nr_pages, unsigned img_pages)
+{
+	struct pbe *pblist;
+
+	if (nr_pages <= img_pages)
+		return -EINVAL;
+	/* We have to create a PBE list here */
+	pblist = alloc_pagedir(img_pages);
+	if (!pblist)
+		return -ENOMEM;
+	create_pbe_list(pblist, img_pages);
+	current_pblist = pblist;
+	current_pbe = pblist;
+	current_page = 0;
+	nr_copy_pages = img_pages;
+	nr_pb_pages = nr_pages - img_pages;
+	return 0;
+}
+
+static void load_next_pb_page(unsigned long *buf)
+{
+	struct pbe *p;
+	unsigned n;
+
+	p  = current_pbe;
+	for (n = 0; n < PAGE_SIZE/sizeof(long) && p; n++) {
+		p->orig_address = buf[n];
+		p = p->next;
+	}
+	current_pbe = p;
+}
+
+int snapshot_recv_page(void *buf)
+{
+	if (!current_pblist ||
+	    current_page >= nr_copy_pages + nr_pb_pages)
+		return -EINVAL;
+	if (current_page < nr_pb_pages) {
+		load_next_pb_page(buf);
+		if (!current_pbe) {
+			current_pblist = relocate_pbe_list(current_pblist);
+			if (!current_pblist) {
+				printk(KERN_ERR "\nswsusp: Not enough memory for relocating PBEs\n");
+				return -ENOMEM;
+			}
+			current_pbe = current_pblist;
+		}
+	} else {
+		current_pbe->address = get_safe_page(GFP_ATOMIC);
+		if (!current_pbe->address) {
+			printk(KERN_ERR "\nswsusp: Not enough memory for the image\n");
+			return -ENOMEM;
+		}
+		memcpy((void *)current_pbe->address, buf, PAGE_SIZE);
+		current_pbe = current_pbe->next;
+	}
+	current_page++;
+	return 0;
+}
+

kernel/power/snapshot.c line 638

Quoted 10 lines
kernel/power/snapshot.c +10 -0
+int snapshot_finish(void)
+{
+	if (current_pbe)
+		return -EINVAL;
+	current_page = 0;
+	if (current_pblist)
+		pagedir_nosave = current_pblist;
+	current_pblist = NULL;
+	return 0;
+}

No, sorry, I don't like that. I like existing interface better. It should be something like...

system_snapshot(), returns structure to save and nr_pages.

system_restore() takes struct pbe * (or something like that) and nr_pages, and takes care.

snapshot_send_init()...snapshot_finish() interface is too complex in my eyes, and will be hard to get right.
Pavel

Signature · 1 line
Thanks, Sharp!
#8 lore raw

RFC PATCH 4/6 swsusp: move swap check out of swsusp_suspend

In reply to Rafael J. Wysocki

This is a non-essential step making the next patch possible. No functionality changes.

 kernel/power/swsusp.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)
kernel/power/swsusp.c +5 -8
Index: linux-2.6.14-rc5-mm1/kernel/power/swsusp.c
===================================================================
@@ -644,12 +644,15 @@
 {
 	int error;
 
+	if ((error = swsusp_swap_check())) {
+		printk(KERN_ERR "swsusp: cannot find swap device, try swapon -a.\n");
+		return error;
+	}
 	lock_swapdevices();
 	error = write_suspend_image();
 	/* This will unlock ignored swap devices since writing is finished */
 	lock_swapdevices();
 	return error;
-
 }
 
 
@@ -672,11 +675,6 @@
 		goto Enable_irqs;
 	}
 
-	if ((error = swsusp_swap_check())) {
-		printk(KERN_ERR "swsusp: cannot find swap device, try swapon -a.\n");
-		goto Power_up;
-	}
-
 	if ((error = save_highmem())) {
 		printk(KERN_ERR "swsusp: Not enough free pages for highmem\n");
 		goto Restore_highmem;
@@ -689,7 +687,6 @@
 	restore_processor_state();
 Restore_highmem:
 	restore_highmem();
-Power_up:
 	device_power_up();
 Enable_irqs:
 	local_irq_enable();
@@ -916,7 +913,7 @@
 		 * Reset swap signature now.
 		 */
 		error = bio_write_page(0, &swsusp_header);
-	} else { 
+	} else {
 		return -EINVAL;
 	}
 	if (!error)
#9 lore raw

Re: [RFC][PATCH 4/6] swsusp: move swap check out of swsusp_suspend

Hi!

Quoted 2 lines

This is a non-essential step making the next patch possible. No functionality changes.

If you can push this before 3/6, that would be nice.

Pavel

Signature · 1 line
Thanks, Sharp!
#10 lore raw

Re: [RFC][PATCH 4/6] swsusp: move swap check out of swsusp_suspend

Hi,

Quoted 6 lines · Pavel Machek

Hi!

Quoted 2 lines

This is a non-essential step making the next patch possible. No functionality changes.

If you can push this before 3/6, that would be nice.

Sure. I think I'll send the two patches you have already acked and this one to Andrew as a separate series. Then I'll get back to the 3/6 etc.

Greetings,
Rafael

#11 lore raw

Re: [RFC][PATCH 4/6] swsusp: move swap check out of swsusp_suspend

Hi!

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

This is a non-essential step making the next patch possible. No functionality changes.

If you can push this before 3/6, that would be nice.

Sure. I think I'll send the two patches you have already acked and this one to Andrew as a separate series. Then I'll get back to the 3/6 etc.

Splitting swsusp.c into swsusp.c and swap.c looked nice, too, btw. We may finally get rid of "swsusp" name :-). But, in the long term, I'd like to see swap reading/writing support removed from kernel. That 2.8.0 or something, through...
Pavel

Signature · 1 line
Thanks, Sharp!
#12 lore raw

RFC PATCH 5/6 swsusp: move swap-handling functions to separate file

In reply to Rafael J. Wysocki

This patch is to show that the swap-handling part of swsusp is really independent and it can be moved entirely to a separate file. It introduces the file swap.c containing all of the swap-handling code.

After the change swsusp.c contains the functions that in my opinion do not belong to either the snapshot-handling part or the swap-handling part (swsusp_suspend(), swsusp_resume() and the functions related to highmem).

 kernel/power/Makefile   |    2 
 kernel/power/power.h    |    2 
 kernel/power/snapshot.c |   93 ----
 kernel/power/swap.c     |  915 +++++++++++++++++++++++++++++++++++++++++++++++
 kernel/power/swsusp.c   |  923 +++---------------------------------------------
 5 files changed, 988 insertions(+), 947 deletions(-)
kernel/power/Makefile +1 -1
Index: linux-2.6.14-rc5-mm1/kernel/power/Makefile
===================================================================
@@ -4,7 +4,7 @@
 endif
 
 obj-y				:= main.o process.o console.o pm.o
-obj-$(CONFIG_SOFTWARE_SUSPEND)	+= swsusp.o disk.o snapshot.o
+obj-$(CONFIG_SOFTWARE_SUSPEND)	+= swsusp.o disk.o snapshot.o swap.o
 
 obj-$(CONFIG_SUSPEND_SMP)	+= smp.o
 
kernel/power/power.h +0 -2
Index: linux-2.6.14-rc5-mm1/kernel/power/power.h
===================================================================
@@ -58,8 +58,6 @@
 extern asmlinkage int swsusp_arch_suspend(void);
 extern asmlinkage int swsusp_arch_resume(void);
 
-extern int save_highmem(void);
-extern int restore_highmem(void);
 extern void swsusp_free(void);
 
 extern unsigned snapshot_pages_to_save(void);
kernel/power/swap.c new file +915 -0

915 lines not on this page · raw message

kernel/power/snapshot.c +0 -93

99 lines not on this page · raw message

kernel/power/swsusp.c +72 -851

950 lines not on this page · raw message

#13 lore raw

Re: [RFC][PATCH 5/6] swsusp: move swap-handling functions to separate file

Hi!

Quoted 7 lines

This patch is to show that the swap-handling part of swsusp is really independent and it can be moved entirely to a separate file. It introduces the file swap.c containing all of the swap-handling code.

After the change swsusp.c contains the functions that in my opinion do not belong to either the snapshot-handling part or the swap-handling part (swsusp_suspend(), swsusp_resume() and the functions related to highmem).

Highmem handling should go to snapshot.c. Other parts do not need to know about it.

linux-2.6.14-rc5-mm1/kernel/power/swap.c

kernel/power/swap.c line 2

Quoted 6 lines

===================================================================

kernel/power/swap.c new file +2 -0
@@ -0,0 +1,915 @@
+/*
+ * linux/kernel/power/snapshot.c

wrong name.

Quoted 4 lines
kernel/power/swap.c new file +4 -0
+static void dump_info(void)
+{
+	pr_debug(" swsusp: Version: %u\n",swsusp_info.version_code);
+	pr_debug(" swsusp: Num Pages: %ld\n",swsusp_info.num_physpages);

kernel/power/swap.c line 343

Quoted 10 lines
kernel/power/swap.c new file +10 -0
+	pr_debug(" swsusp: UTS Sys: %s\n",swsusp_info.uts.sysname);
+	pr_debug(" swsusp: UTS Node: %s\n",swsusp_info.uts.nodename);
+	pr_debug(" swsusp: UTS Release: %s\n",swsusp_info.uts.release);
+	pr_debug(" swsusp: UTS Version: %s\n",swsusp_info.uts.version);
+	pr_debug(" swsusp: UTS Machine: %s\n",swsusp_info.uts.machine);
+	pr_debug(" swsusp: UTS Domain: %s\n",swsusp_info.uts.domainname);
+	pr_debug(" swsusp: CPUs: %d\n",swsusp_info.cpus);
+	pr_debug(" swsusp: Image: %ld Pages\n",swsusp_info.image_pages);
+	pr_debug(" swsusp: Total: %ld Pages\n", swsusp_info.pages);
+}

I'd rather get rid of this, or at least made it *way* more terse.

Ok, that probably belongs to separate patch.

Pavel

Signature · 1 line
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         
#14 lore raw

RFC PATCH 6/6 swsusp: improve freeing of memory

In reply to Rafael J. Wysocki

This is the bonus patch not functionally related to the previous patches that speeds up swsusp substantially (on my box swsusp is two times faster with it) and makes the system be much more responsive after resume.

However, it only makes sense if the limit imposed by the size of the swsusp_info structure is lifted, which is done in one of the previous patches.

 include/linux/suspend.h |    2 -
 kernel/power/disk.c     |   30 ++----------------------
 kernel/power/power.h    |    1 
 kernel/power/snapshot.c |    6 +---
 kernel/power/swsusp.c   |   60 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 67 insertions(+), 32 deletions(-)
kernel/power/disk.c +3 -27
Index: linux-2.6.14-rc5-mm1/kernel/power/disk.c
===================================================================
@@ -24,6 +24,7 @@
 
 extern suspend_disk_method_t pm_disk_mode;
 
+extern int swsusp_shrink_memory(void);
 extern int swsusp_suspend(void);
 extern int swsusp_write(void);
 extern int swsusp_check(void);
@@ -73,31 +74,6 @@
 static int in_suspend __nosavedata = 0;
 
 
-/**
- *	free_some_memory -  Try to free as much memory as possible
- *
- *	... but do not OOM-kill anyone
- *
- *	Notice: all userland should be stopped at this point, or
- *	livelock is possible.
- */
-
-static void free_some_memory(void)
-{
-	unsigned int i = 0;
-	unsigned int tmp;
-	unsigned long pages = 0;
-	char *p = "-\\|/";
-
-	printk("Freeing memory...  ");
-	while ((tmp = shrink_all_memory(10000))) {
-		pages += tmp;
-		printk("\b%c", p[i++ % 4]);
-	}
-	printk("\bdone (%li pages freed)\n", pages);
-}
-
-
 static inline void platform_finish(void)
 {
 	if (pm_disk_mode == PM_DISK_PLATFORM) {
@@ -127,8 +103,8 @@
 	}
 
 	/* Free memory before shutting down devices. */
-	free_some_memory();
-	return 0;
+	if (!(error = swsusp_shrink_memory()))
+		return 0;
 thaw:
 	thaw_processes();
 	enable_nonboot_cpus();
kernel/power/power.h +1 -0
Index: linux-2.6.14-rc5-mm1/kernel/power/power.h
===================================================================
@@ -58,6 +58,7 @@
 extern asmlinkage int swsusp_arch_suspend(void);
 extern asmlinkage int swsusp_arch_resume(void);
 
+extern unsigned count_data_pages(void);
 extern void swsusp_free(void);
 
 extern unsigned snapshot_pages_to_save(void);
kernel/power/snapshot.c +2 -4
Index: linux-2.6.14-rc5-mm1/kernel/power/snapshot.c
===================================================================
@@ -67,17 +67,15 @@
 	BUG_ON(PageReserved(page) && PageNosave(page));
 	if (PageNosave(page))
 		return 0;
-	if (PageReserved(page) && pfn_is_nosave(pfn)) {
-		pr_debug("[nosave pfn 0x%lx]", pfn);
+	if (PageReserved(page) && pfn_is_nosave(pfn))
 		return 0;
-	}
 	if (PageNosaveFree(page))
 		return 0;
 
 	return 1;
 }
 
-static unsigned count_data_pages(void)
+unsigned count_data_pages(void)
 {
 	struct zone *zone;
 	unsigned long zone_pfn;
kernel/power/swsusp.c +60 -0

76 lines not on this page · raw message

include/linux/suspend.h +1 -1
Index: linux-2.6.14-rc5-mm1/include/linux/suspend.h
===================================================================
@@ -73,6 +73,6 @@
  * XXX: We try to keep some more pages free so that I/O operations succeed
  * without paging. Might this be more?
  */
-#define PAGES_FOR_IO	512
+#define PAGES_FOR_IO	1024
 
 #endif /* _LINUX_SWSUSP_H */
#15 lore raw
In reply to Rafael J. Wysocki

Hi!

Quoted 4 lines

I have divided the changes into some more or less logical steps for clarity. Although the code has been designed as proof-of-concept, it is functional and has been tested on x86-64, except for the cryptographic functionality and error paths.

Don't worry about crypto paths too much. It is my fault, I should not have taken them in the first place. Just ask ast for testing when you are reasonably confident it will work.
Pavel

Signature · 1 line
Thanks, Sharp!
#16 lore raw

Hi!

Quoted 8 lines
Quoted 4 lines

I have divided the changes into some more or less logical steps for clarity. Although the code has been designed as proof-of-concept, it is functional and has been tested on x86-64, except for the cryptographic functionality and error paths.

Don't worry about crypto paths too much. It is my fault, I should not have taken them in the first place. Just ask ast for testing when you are reasonably confident it will work.

We exchanged few emails with ast, and he'll probably not kill me if I remove crypto swsusp support. That's good news ;-).
Pavel

Signature · 1 line
Thanks, Sharp!