[PATCH 5/5] PM / Hibernate: group swap ops

Previous thread: Re: [PATCH] staging/otus: Add null check and fix coding style issue by Simon Horman on Tuesday, April 27, 2010 - 1:16 am. (1 message)

Next thread: Re: 2.6.33-2.6.34-rc5 suspend issues by Nico Schottelius on Tuesday, April 27, 2010 - 1:32 am. (9 messages)
From: Jiri Slaby
Date: Tuesday, April 27, 2010 - 1:15 am

It will be used in suspend code and serves as an easy wrap around
copy_from_user. Similar to simple_read_from_buffer, it takes care
of transfers with proper lengths depending on available and count
parameters and advances ppos appropriately.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Nigel Cunningham <ncunningham@crca.org.au>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
---
 fs/libfs.c         |   35 +++++++++++++++++++++++++++++++++++
 include/linux/fs.h |    2 ++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c
index ea9a6cc..cfab42a 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -547,6 +547,40 @@ ssize_t simple_read_from_buffer(void __user *to, size_t count, loff_t *ppos,
 }
 
 /**
+ * simple_write_to_buffer - copy data from user space to the buffer
+ * @to: the buffer to write to
+ * @available: the size of the buffer
+ * @ppos: the current position in the buffer
+ * @from: the user space buffer to read from
+ * @count: the maximum number of bytes to read
+ *
+ * The simple_write_to_buffer() function reads up to @count bytes from the user
+ * space address starting at @from into the buffer @to at offset @ppos.
+ *
+ * On success, the number of bytes written is returned and the offset @ppos is
+ * advanced by this number, or negative value is returned on error.
+ **/
+ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
+		const void __user *from, size_t count)
+{
+	loff_t pos = *ppos;
+	size_t ret;
+
+	if (pos < 0)
+		return -EINVAL;
+	if (pos >= available || !count)
+		return 0;
+	if (count > available - pos)
+		count = available - pos;
+	ret = copy_from_user(to + pos, from, count);
+	if (ret == count)
+		return -EFAULT;
+	count -= ret;
+	*ppos = pos + count;
+	return count;
+}
+
+/**
  * memory_read_from_buffer - copy data from the buffer
  * @to: the kernel space buffer to read to
  * @count: the maximum number ...
From: Jiri Slaby
Date: Tuesday, April 27, 2010 - 1:15 am

From: Jiri Slaby <jirislaby@gmail.com>

Remove support of reads with offset. This means snapshot_read/write_next
now does not accept count parameter. It allows to clean up the functions
and snapshot handle which no longer needs to care about offsets.

/dev/snapshot handler is converted to simple_{read_from,write_to}_buffer
which take care of offsets.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Nigel Cunningham <ncunningham@crca.org.au>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
---
 kernel/power/power.h    |   18 +-----
 kernel/power/snapshot.c |  145 ++++++++++++++++++-----------------------------
 kernel/power/swap.c     |    8 +-
 kernel/power/user.c     |   37 ++++++++----
 4 files changed, 88 insertions(+), 120 deletions(-)

diff --git a/kernel/power/power.h b/kernel/power/power.h
index 46c5a26..b1e207d 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -97,24 +97,12 @@ extern int hibernate_preallocate_memory(void);
  */
 
 struct snapshot_handle {
-	loff_t		offset;	/* number of the last byte ready for reading
-				 * or writing in the sequence
-				 */
 	unsigned int	cur;	/* number of the block of PAGE_SIZE bytes the
 				 * next operation will refer to (ie. current)
 				 */
-	unsigned int	cur_offset;	/* offset with respect to the current
-					 * block (for the next operation)
-					 */
-	unsigned int	prev;	/* number of the block of PAGE_SIZE bytes that
-				 * was the current one previously
-				 */
 	void		*buffer;	/* address of the block to read from
 					 * or write to
 					 */
-	unsigned int	buf_offset;	/* location to read from or write to,
-					 * given as a displacement from 'buffer'
-					 */
 	int		sync_read;	/* Set to one to notify the caller of
 					 * snapshot_write_next() that it may
 					 * need to call wait_on_bio_chain()
@@ -125,12 +113,12 @@ struct snapshot_handle {
  * snapshot_read_next()/snapshot_write_next() is allowed to
  * read/write data after the function returns
  */
-#define ...
From: Jiri Slaby
Date: Tuesday, April 27, 2010 - 1:15 am

Move block I/O operations to a separate file. It is because it will
be used later not only by the swap writer.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Nigel Cunningham <ncunningham@crca.org.au>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
---
 kernel/power/Makefile   |    3 +-
 kernel/power/block_io.c |  103 +++++++++++++++++++++++++++++++++++
 kernel/power/power.h    |    9 +++
 kernel/power/swap.c     |  136 +++++++++--------------------------------------
 4 files changed, 139 insertions(+), 112 deletions(-)
 create mode 100644 kernel/power/block_io.c

diff --git a/kernel/power/Makefile b/kernel/power/Makefile
index 4319181..524e058 100644
--- a/kernel/power/Makefile
+++ b/kernel/power/Makefile
@@ -8,7 +8,8 @@ obj-$(CONFIG_PM_SLEEP)		+= console.o
 obj-$(CONFIG_FREEZER)		+= process.o
 obj-$(CONFIG_SUSPEND)		+= suspend.o
 obj-$(CONFIG_PM_TEST_SUSPEND)	+= suspend_test.o
-obj-$(CONFIG_HIBERNATION)	+= hibernate.o snapshot.o swap.o user.o
+obj-$(CONFIG_HIBERNATION)	+= hibernate.o snapshot.o swap.o user.o \
+				   block_io.o
 obj-$(CONFIG_HIBERNATION_NVS)	+= hibernate_nvs.o
 
 obj-$(CONFIG_MAGIC_SYSRQ)	+= poweroff.o
diff --git a/kernel/power/block_io.c b/kernel/power/block_io.c
new file mode 100644
index 0000000..3ba57e0
--- /dev/null
+++ b/kernel/power/block_io.c
@@ -0,0 +1,103 @@
+/*
+ * This file provides functions for block I/O operations on swap/file.
+ *
+ * Copyright (C) 1998,2001-2005 Pavel Machek <pavel@suse.cz>
+ * Copyright (C) 2006 Rafael J. Wysocki <rjw@sisk.pl>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/bio.h>
+#include <linux/kernel.h>
+#include <linux/pagemap.h>
+#include <linux/swap.h>
+
+#include "power.h"
+
+/**
+ *	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 ...
From: Pavel Machek
Date: Tuesday, April 27, 2010 - 10:52 pm

It is slightly sad that this is now not only static, but also

Does the documentation usually go to the header in these cases?

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

From: Jiri Slaby
Date: Wednesday, April 28, 2010 - 11:22 pm

Oops, it's a cut&paste from swap.c:
$ grep -r pavel.*suse kernel/power/
kernel/power/snapshot.c: * Copyright (C) 1998-2005 Pavel Machek
<pavel@suse.cz>
kernel/power/swap.c: * Copyright (C) 1998,2001-2005 Pavel Machek
<pavel@suse.cz>
kernel/power/hibernate.c: * Copyright (c) 2004 Pavel Machek <pavel@suse.cz>

-- 
js
--

From: Jiri Slaby
Date: Tuesday, April 27, 2010 - 1:15 am

From: Jiri Slaby <jirislaby@gmail.com>

The first sector knowledge is swap-only specific. Move it into the
swap handle. This will be needed for later non-swap specific code
moving into snapshot.c.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Nigel Cunningham <ncunningham@crca.org.au>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 kernel/power/swap.c |   76 +++++++++++++++++++++++++-------------------------
 1 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 1b1ab6f..63e8062 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -29,6 +29,40 @@
 
 #define SWSUSP_SIG	"S1SUSPEND"
 
+/*
+ *	The swap map is a data structure used for keeping track of each page
+ *	written to a swap partition.  It consists of many swap_map_page
+ *	structures that contain each an array of MAP_PAGE_SIZE swap entries.
+ *	These structures are stored on the swap and linked together with the
+ *	help of the .next_swap member.
+ *
+ *	The swap map is created during suspend.  The swap map pages are
+ *	allocated and populated one at a time, so we only need one memory
+ *	page to set up the entire structure.
+ *
+ *	During resume we also only need to use one swap_map_page structure
+ *	at a time.
+ */
+
+#define MAP_PAGE_ENTRIES	(PAGE_SIZE / sizeof(sector_t) - 1)
+
+struct swap_map_page {
+	sector_t entries[MAP_PAGE_ENTRIES];
+	sector_t next_swap;
+};
+
+/**
+ *	The swap_map_handle structure is used for handling swap in
+ *	a file-alike way
+ */
+
+struct swap_map_handle {
+	struct swap_map_page *cur;
+	sector_t cur_swap;
+	sector_t first_sector;
+	unsigned int k;
+};
+
 struct swsusp_header {
 	char reserved[PAGE_SIZE - 20 - sizeof(sector_t) - sizeof(int)];
 	sector_t image;
@@ -151,7 +185,7 @@ struct block_device *hib_resume_bdev;
  * Saving part
  */
 
-static int mark_swapfiles(sector_t start, unsigned int flags)
+static int mark_swapfiles(struct swap_map_handle ...
From: Jiri Slaby
Date: Tuesday, April 27, 2010 - 1:15 am

Move all the swap processing into one function. It will make swap
calls from a non-swap code easier.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Nigel Cunningham <ncunningham@crca.org.au>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
---
 kernel/power/swap.c |  117 ++++++++++++++++++++++++++++++++-------------------
 1 files changed, 74 insertions(+), 43 deletions(-)

diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 63e8062..b0bb217 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -208,9 +208,10 @@ static int mark_swapfiles(struct swap_map_handle *handle, unsigned int flags)
 /**
  *	swsusp_swap_check - check if the resume device is a swap device
  *	and get its index (if so)
+ *
+ *	This is called before saving image
  */
-
-static int swsusp_swap_check(void) /* This is called before saving image */
+static int swsusp_swap_check(void)
 {
 	int res;
 
@@ -269,17 +270,33 @@ static void release_swap_writer(struct swap_map_handle *handle)
 
 static int get_swap_writer(struct swap_map_handle *handle)
 {
+	int ret;
+
+	ret = swsusp_swap_check();
+	if (ret) {
+		if (ret != -ENOSPC)
+			printk(KERN_ERR "PM: Cannot find swap device, try "
+					"swapon -a.\n");
+		return ret;
+	}
 	handle->cur = (struct swap_map_page *)get_zeroed_page(GFP_KERNEL);
-	if (!handle->cur)
-		return -ENOMEM;
+	if (!handle->cur) {
+		ret = -ENOMEM;
+		goto err_close;
+	}
 	handle->cur_swap = alloc_swapdev_block(root_swap);
 	if (!handle->cur_swap) {
-		release_swap_writer(handle);
-		return -ENOSPC;
+		ret = -ENOSPC;
+		goto err_rel;
 	}
 	handle->k = 0;
 	handle->first_sector = handle->cur_swap;
 	return 0;
+err_rel:
+	release_swap_writer(handle);
+err_close:
+	swsusp_close(FMODE_WRITE);
+	return ret;
 }
 
 static int swap_write_page(struct swap_map_handle *handle, void *buf,
@@ -322,6 +339,24 @@ static int flush_swap_writer(struct swap_map_handle *handle)
 		return -EINVAL;
 }
 
+static int swap_writer_finish(struct swap_map_handle ...
From: Rafael J. Wysocki
Date: Tuesday, April 27, 2010 - 2:58 pm

Thanks for the patches, I'll review them as soon as I can.

Rafael
--

From: Rafael J. Wysocki
Date: Wednesday, April 28, 2010 - 3:16 pm

They look good.  If there are no objections from the other people, I'll add
them to suspend-2.6/linux-next in the next few days.

Rafael
--

From: Rafael J. Wysocki
Date: Saturday, May 1, 2010 - 3:00 pm

All patches in this series applied to suspend-2.6/linux-next.

I fixed up the Pavel's address in the copyright notice in kernel/power/block_io.c.

Thanks,
Rafael
--

From: Rafael J. Wysocki
Date: Wednesday, April 28, 2010 - 3:10 pm

Will anyone object if I take this one into the suspend-2.6 tree?


--

Previous thread: Re: [PATCH] staging/otus: Add null check and fix coding style issue by Simon Horman on Tuesday, April 27, 2010 - 1:16 am. (1 message)

Next thread: Re: 2.6.33-2.6.34-rc5 suspend issues by Nico Schottelius on Tuesday, April 27, 2010 - 1:32 am. (9 messages)