public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH -mm 0/5] Hibernation: Clean up userland interface
@ 2007-09-02 21:15 Rafael J. Wysocki
  2007-09-02 21:17 ` [PATCH -mm 1/5] Hibernation: Introduce SNAPSHOT_GET_IMAGE_SIZE ioctl Rafael J. Wysocki
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2007-09-02 21:15 UTC (permalink / raw)
  To: Pavel Machek; +Cc: pm list

Hi,

This series of patches is intended to eliminate some design problems with the
hibernation userland interface.

It contains the following changes:
* allow the user space utilities to get the size of the image via ioctl
* introduce ioctls to be used instead of SNAPSHOT_PMOPS
* introduce new ioctl definitions to replace the ones that depend on (void *)
* mark the SNAPSHOT_SET_SWAP_FILE ioctl as deprecated
* move the definitions of hibernation ioctls to a separate header file located
  in include/linux/suspend.h
The details are in the changelogs.

Comments welcome.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

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

* [PATCH -mm 1/5] Hibernation: Introduce SNAPSHOT_GET_IMAGE_SIZE ioctl
  2007-09-02 21:15 [PATCH -mm 0/5] Hibernation: Clean up userland interface Rafael J. Wysocki
@ 2007-09-02 21:17 ` Rafael J. Wysocki
  2007-09-03  3:17   ` Pavel Machek
  2007-09-02 21:19 ` [PATCH -mm 2/5] Hibernation: Rework platform support ioctls Rafael J. Wysocki
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2007-09-02 21:17 UTC (permalink / raw)
  To: Pavel Machek; +Cc: pm list

From: Rafael J. Wysocki <rjw@sisk.pl>

Add a new ioctl, SNAPSHOT_GET_IMAGE_SIZE, returning the size of the (just created)
hibernation image, to the hibernation userland interface.

This ioctl is necessary so that the userland utilities using the interface need
not access the hibernation image header, owned by the kernel, in order to obtain
the size of the image.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/userland-swsusp.txt |   12 +++++-------
 kernel/power/power.h                    |    4 +++-
 kernel/power/snapshot.c                 |    7 ++++++-
 kernel/power/user.c                     |   18 ++++++++++++++----
 4 files changed, 28 insertions(+), 13 deletions(-)

Index: linux-2.6.23-rc4-mm1/kernel/power/power.h
===================================================================
--- linux-2.6.23-rc4-mm1.orig/kernel/power/power.h
+++ linux-2.6.23-rc4-mm1/kernel/power/power.h
@@ -130,6 +130,7 @@ struct snapshot_handle {
 #define data_of(handle)	((handle).buffer + (handle).buf_offset)
 
 extern unsigned int snapshot_additional_pages(struct zone *zone);
+extern unsigned long snapshot_get_image_size(void);
 extern int snapshot_read_next(struct snapshot_handle *handle, size_t count);
 extern int snapshot_write_next(struct snapshot_handle *handle, size_t count);
 extern void snapshot_write_finalize(struct snapshot_handle *handle);
@@ -160,7 +161,8 @@ struct resume_swap_area {
 #define SNAPSHOT_PMOPS			_IOW(SNAPSHOT_IOC_MAGIC, 12, unsigned int)
 #define SNAPSHOT_SET_SWAP_AREA		_IOW(SNAPSHOT_IOC_MAGIC, 13, \
 							struct resume_swap_area)
-#define SNAPSHOT_IOC_MAXNR	13
+#define SNAPSHOT_GET_IMAGE_SIZE		_IOR(SNAPSHOT_IOC_MAGIC, 14, loff_t)
+#define SNAPSHOT_IOC_MAXNR	14
 
 #define PMOPS_PREPARE	1
 #define PMOPS_ENTER	2
Index: linux-2.6.23-rc4-mm1/kernel/power/user.c
===================================================================
--- linux-2.6.23-rc4-mm1.orig/kernel/power/user.c
+++ linux-2.6.23-rc4-mm1/kernel/power/user.c
@@ -133,7 +133,7 @@ static int snapshot_ioctl(struct inode *
 {
 	int error = 0;
 	struct snapshot_data *data;
-	loff_t avail;
+	loff_t size;
 	sector_t offset;
 
 	if (_IOC_TYPE(cmd) != SNAPSHOT_IOC_MAGIC)
@@ -210,10 +210,20 @@ static int snapshot_ioctl(struct inode *
 		image_size = arg;
 		break;
 
+	case SNAPSHOT_GET_IMAGE_SIZE:
+		if (!data->ready) {
+			error = -ENODATA;
+			break;
+		}
+		size = snapshot_get_image_size();
+		size <<= PAGE_SHIFT;
+		error = put_user(size, (loff_t __user *)arg);
+		break;
+
 	case SNAPSHOT_AVAIL_SWAP:
-		avail = count_swap_pages(data->swap, 1);
-		avail <<= PAGE_SHIFT;
-		error = put_user(avail, (loff_t __user *)arg);
+		size = count_swap_pages(data->swap, 1);
+		size <<= PAGE_SHIFT;
+		error = put_user(size, (loff_t __user *)arg);
 		break;
 
 	case SNAPSHOT_GET_SWAP_PAGE:
Index: linux-2.6.23-rc4-mm1/kernel/power/snapshot.c
===================================================================
--- linux-2.6.23-rc4-mm1.orig/kernel/power/snapshot.c
+++ linux-2.6.23-rc4-mm1/kernel/power/snapshot.c
@@ -1263,12 +1263,17 @@ static char *check_image_kernel(struct s
 }
 #endif /* CONFIG_ARCH_HIBERNATION_HEADER */
 
+unsigned long snapshot_get_image_size(void)
+{
+	return nr_copy_pages + nr_meta_pages + 1;
+}
+
 static int init_header(struct swsusp_info *info)
 {
 	memset(info, 0, sizeof(struct swsusp_info));
 	info->num_physpages = num_physpages;
 	info->image_pages = nr_copy_pages;
-	info->pages = nr_copy_pages + nr_meta_pages + 1;
+	info->pages = snapshot_get_image_size();
 	info->size = info->pages;
 	info->size <<= PAGE_SHIFT;
 	return init_header_complete(info);
Index: linux-2.6.23-rc4-mm1/Documentation/power/userland-swsusp.txt
===================================================================
--- linux-2.6.23-rc4-mm1.orig/Documentation/power/userland-swsusp.txt
+++ linux-2.6.23-rc4-mm1/Documentation/power/userland-swsusp.txt
@@ -54,6 +54,8 @@ SNAPSHOT_SET_IMAGE_SIZE - set the prefer
 	this number, but if it turns out to be impossible, the kernel will
 	create the smallest image possible)
 
+SNAPSHOT_GET_IMAGE_SIZE - return the actual size of the hibernation image
+
 SNAPSHOT_AVAIL_SWAP - return the amount of available swap in bytes (the last
 	argument should be a pointer to an unsigned int variable that will
 	contain the result if the call is successful).
@@ -136,13 +138,9 @@ required, as they can use, for example, 
 a file on a partition that is unmounted before SNAPSHOT_ATOMIC_SNAPSHOT and
 mounted afterwards.
 
-These utilities SHOULD NOT make any assumptions regarding the ordering of
-data within the snapshot image, except for the image header that MAY be
-assumed to start with an swsusp_info structure, as specified in
-kernel/power/power.h.  This structure MAY be used by the userland utilities
-to obtain some information about the snapshot image, such as the size
-of the snapshot image, including the metadata and the header itself,
-contained in the .size member of swsusp_info.
+These utilities MUST NOT make any assumptions regarding the ordering of
+data within the snapshot image.  The contents of the image are entirely owned
+by the kernel and its structure may be changed in future kernel releases.
 
 The snapshot image MUST be written to the kernel unaltered (ie. all of the image
 data, metadata and header MUST be written in _exactly_ the same amount, form

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

* [PATCH -mm 2/5] Hibernation: Rework platform support ioctls
  2007-09-02 21:15 [PATCH -mm 0/5] Hibernation: Clean up userland interface Rafael J. Wysocki
  2007-09-02 21:17 ` [PATCH -mm 1/5] Hibernation: Introduce SNAPSHOT_GET_IMAGE_SIZE ioctl Rafael J. Wysocki
@ 2007-09-02 21:19 ` Rafael J. Wysocki
  2007-09-06  1:57   ` Pavel Machek
  2007-09-02 21:21 ` [PATCH -mm 3/5] Hibernation: Correct definitions of some ioctls Rafael J. Wysocki
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2007-09-02 21:19 UTC (permalink / raw)
  To: Pavel Machek; +Cc: pm list

From: Rafael J. Wysocki <rjw@sisk.pl>

Modify the hibernation userland interface by adding two new ioctls to it,
SNAPSHOT_PLATFORM_SUPPORT and SNAPSHOT_POWER_OFF, that can be used,
respectively, to switch the hibernation platform support on/off and to make the
kernel transition the system to the hibernation state (eg. ACPI S4) using the
platform (eg. ACPI) driver.

These ioctls are intended to replace the misdesigned SNAPSHOT_PMOPS ioctl,
which from now is regarded as obsolete and will be removed in the future.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/userland-swsusp.txt |   24 ++++---------------
 kernel/power/power.h                    |    9 ++-----
 kernel/power/user.c                     |   39 +++++++++++++++++++++++---------
 3 files changed, 38 insertions(+), 34 deletions(-)

Index: linux-2.6.23-rc4-mm1/kernel/power/power.h
===================================================================
--- linux-2.6.23-rc4-mm1.orig/kernel/power/power.h
+++ linux-2.6.23-rc4-mm1/kernel/power/power.h
@@ -158,15 +158,12 @@ struct resume_swap_area {
 #define SNAPSHOT_FREE_SWAP_PAGES	_IO(SNAPSHOT_IOC_MAGIC, 9)
 #define SNAPSHOT_SET_SWAP_FILE		_IOW(SNAPSHOT_IOC_MAGIC, 10, unsigned int)
 #define SNAPSHOT_S2RAM			_IO(SNAPSHOT_IOC_MAGIC, 11)
-#define SNAPSHOT_PMOPS			_IOW(SNAPSHOT_IOC_MAGIC, 12, unsigned int)
 #define SNAPSHOT_SET_SWAP_AREA		_IOW(SNAPSHOT_IOC_MAGIC, 13, \
 							struct resume_swap_area)
 #define SNAPSHOT_GET_IMAGE_SIZE		_IOR(SNAPSHOT_IOC_MAGIC, 14, loff_t)
-#define SNAPSHOT_IOC_MAXNR	14
-
-#define PMOPS_PREPARE	1
-#define PMOPS_ENTER	2
-#define PMOPS_FINISH	3
+#define SNAPSHOT_PLATFORM_SUPPORT	_IOR(SNAPSHOT_IOC_MAGIC, 15, int)
+#define SNAPSHOT_POWER_OFF		_IO(SNAPSHOT_IOC_MAGIC, 16)
+#define SNAPSHOT_IOC_MAXNR	16
 
 /* If unset, the snapshot device cannot be open. */
 extern atomic_t snapshot_device_available;
Index: linux-2.6.23-rc4-mm1/kernel/power/user.c
===================================================================
--- linux-2.6.23-rc4-mm1.orig/kernel/power/user.c
+++ linux-2.6.23-rc4-mm1/kernel/power/user.c
@@ -28,6 +28,18 @@
 
 #include "power.h"
 
+/*
+ * NOTE: The SNAPSHOT_PMOPS ioctl is obsolete and will be removed in the
+ * future.  It is only preserved here for compatibility with existing userland
+ * utilities.
+ */
+#define SNAPSHOT_PMOPS		_IOW(SNAPSHOT_IOC_MAGIC, 12, unsigned int)
+
+#define PMOPS_PREPARE	1
+#define PMOPS_ENTER	2
+#define PMOPS_FINISH	3
+
+
 #define SNAPSHOT_MINOR	231
 
 static struct snapshot_data {
@@ -36,7 +48,7 @@ static struct snapshot_data {
 	int mode;
 	char frozen;
 	char ready;
-	char platform_suspend;
+	char platform_support;
 } snapshot_state;
 
 atomic_t snapshot_device_available = ATOMIC_INIT(1);
@@ -70,7 +82,7 @@ static int snapshot_open(struct inode *i
 	}
 	data->frozen = 0;
 	data->ready = 0;
-	data->platform_suspend = 0;
+	data->platform_support = 0;
 
 	return 0;
 }
@@ -183,7 +195,7 @@ static int snapshot_ioctl(struct inode *
 			error = -EPERM;
 			break;
 		}
-		error = hibernation_snapshot(data->platform_suspend);
+		error = hibernation_snapshot(data->platform_support);
 		if (!error)
 			error = put_user(in_suspend, (unsigned int __user *)arg);
 		if (!error)
@@ -197,7 +209,7 @@ static int snapshot_ioctl(struct inode *
 			error = -EPERM;
 			break;
 		}
-		error = hibernation_restore(data->platform_suspend);
+		error = hibernation_restore(data->platform_support);
 		break;
 
 	case SNAPSHOT_FREE:
@@ -285,26 +297,33 @@ static int snapshot_ioctl(struct inode *
 		mutex_unlock(&pm_mutex);
 		break;
 
-	case SNAPSHOT_PMOPS:
+	case SNAPSHOT_PLATFORM_SUPPORT:
+		data->platform_support = !!arg;
+		break;
+
+	case SNAPSHOT_POWER_OFF:
+		if (data->platform_support)
+			error = hibernation_platform_enter();
+		break;
+
+	case SNAPSHOT_PMOPS: /* This ioctl is deprecated */
 		error = -EINVAL;
 
 		switch (arg) {
 
 		case PMOPS_PREPARE:
-			data->platform_suspend = 1;
+			data->platform_support = 1;
 			error = 0;
 			break;
 
 		case PMOPS_ENTER:
-			if (data->platform_suspend)
+			if (data->platform_support)
 				error = hibernation_platform_enter();
-
 			break;
 
 		case PMOPS_FINISH:
-			if (data->platform_suspend)
+			if (data->platform_support)
 				error = 0;
-
 			break;
 
 		default:
Index: linux-2.6.23-rc4-mm1/Documentation/power/userland-swsusp.txt
===================================================================
--- linux-2.6.23-rc4-mm1.orig/Documentation/power/userland-swsusp.txt
+++ linux-2.6.23-rc4-mm1/Documentation/power/userland-swsusp.txt
@@ -85,6 +85,12 @@ SNAPSHOT_SET_SWAP_AREA - set the resume 
 	recommended to always use this call, because the code to set the resume
 	partition may be removed from future kernels
 
+SNAPSHOT_PLATFORM_SUPPORT - enable/disable the hibernation platform support,
+	depending on the argument value (enable, if the argument is nonzero)
+
+SNAPSHOT_POWER_OFF - make the kernel transition the system to the hibernation
+	state (eg. ACPI S4) using the platform (eg. ACPI) driver
+
 SNAPSHOT_S2RAM - suspend to RAM; using this call causes the kernel to
 	immediately enter the suspend-to-RAM state, so this call must always
 	be preceded by the SNAPSHOT_FREEZE call and it is also necessary
@@ -95,24 +101,6 @@ SNAPSHOT_S2RAM - suspend to RAM; using t
 	to resume the system from RAM if there's enough battery power or restore
 	its state on the basis of the saved suspend image otherwise)
 
-SNAPSHOT_PMOPS - enable the usage of the hibernation_ops->prepare,
-	hibernate_ops->enter and hibernation_ops->finish methods (the in-kernel
-	swsusp knows these as the "platform method") which are needed on many
-	machines to (among others) speed up the resume by letting the BIOS skip
-	some steps or to let the system recognise the correct state of the
-	hardware after the resume (in particular on many machines this ensures
-	that unplugged AC adapters get correctly detected and that kacpid does
-	not run wild after the resume).  The last ioctl() argument can take one
-	of the three values, defined in kernel/power/power.h:
-	PMOPS_PREPARE - make the kernel carry out the
-		hibernation_ops->prepare() operation
-	PMOPS_ENTER - make the kernel power off the system by calling
-		hibernation_ops->enter()
-	PMOPS_FINISH - make the kernel carry out the
-		hibernation_ops->finish() operation
-	Note that the actual constants are misnamed because they surface
-	internal kernel implementation details that have changed.
-
 The device's read() operation can be used to transfer the snapshot image from
 the kernel.  It has the following limitations:
 - you cannot read() more than one virtual memory page at a time

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

* [PATCH -mm 3/5] Hibernation: Correct definitions of some ioctls
  2007-09-02 21:15 [PATCH -mm 0/5] Hibernation: Clean up userland interface Rafael J. Wysocki
  2007-09-02 21:17 ` [PATCH -mm 1/5] Hibernation: Introduce SNAPSHOT_GET_IMAGE_SIZE ioctl Rafael J. Wysocki
  2007-09-02 21:19 ` [PATCH -mm 2/5] Hibernation: Rework platform support ioctls Rafael J. Wysocki
@ 2007-09-02 21:21 ` Rafael J. Wysocki
  2007-09-03  3:25   ` Pavel Machek
  2007-09-02 21:22 ` [PATCH -mm 4/5] Hibernation: Mark SNAPSHOT_SET_SWAP_FILE ioctl as deprecated Rafael J. Wysocki
  2007-09-02 21:28 ` [PATCH -mm 5/5] Hibernation: Introduce exportable suspend ioctls header Rafael J. Wysocki
  4 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2007-09-02 21:21 UTC (permalink / raw)
  To: Pavel Machek; +Cc: pm list

From: Rafael J. Wysocki <rjw@sisk.pl>

Three ioctl numbers belonging to the hibernation userland interface,
SNAPSHOT_ATOMIC_SNAPSHOT, SNAPSHOT_AVAIL_SWAP, SNAPSHOT_GET_SWAP_PAGE,
are defined in a wrong way (eg. not portable).  Provide new ioctl numbers for
these ioctls and mark the existing ones as deprecated.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/userland-swsusp.txt |   24 ++++++++++++------------
 kernel/power/power.h                    |    8 ++++----
 kernel/power/user.c                     |   16 ++++++++++++++--
 3 files changed, 30 insertions(+), 18 deletions(-)

Index: linux-2.6.23-rc4-mm1/kernel/power/power.h
===================================================================
--- linux-2.6.23-rc4-mm1.orig/kernel/power/power.h
+++ linux-2.6.23-rc4-mm1/kernel/power/power.h
@@ -149,12 +149,9 @@ struct resume_swap_area {
 #define SNAPSHOT_IOC_MAGIC	'3'
 #define SNAPSHOT_FREEZE			_IO(SNAPSHOT_IOC_MAGIC, 1)
 #define SNAPSHOT_UNFREEZE		_IO(SNAPSHOT_IOC_MAGIC, 2)
-#define SNAPSHOT_ATOMIC_SNAPSHOT	_IOW(SNAPSHOT_IOC_MAGIC, 3, void *)
 #define SNAPSHOT_ATOMIC_RESTORE		_IO(SNAPSHOT_IOC_MAGIC, 4)
 #define SNAPSHOT_FREE			_IO(SNAPSHOT_IOC_MAGIC, 5)
 #define SNAPSHOT_SET_IMAGE_SIZE		_IOW(SNAPSHOT_IOC_MAGIC, 6, unsigned long)
-#define SNAPSHOT_AVAIL_SWAP		_IOR(SNAPSHOT_IOC_MAGIC, 7, void *)
-#define SNAPSHOT_GET_SWAP_PAGE		_IOR(SNAPSHOT_IOC_MAGIC, 8, void *)
 #define SNAPSHOT_FREE_SWAP_PAGES	_IO(SNAPSHOT_IOC_MAGIC, 9)
 #define SNAPSHOT_SET_SWAP_FILE		_IOW(SNAPSHOT_IOC_MAGIC, 10, unsigned int)
 #define SNAPSHOT_S2RAM			_IO(SNAPSHOT_IOC_MAGIC, 11)
@@ -163,7 +160,10 @@ struct resume_swap_area {
 #define SNAPSHOT_GET_IMAGE_SIZE		_IOR(SNAPSHOT_IOC_MAGIC, 14, loff_t)
 #define SNAPSHOT_PLATFORM_SUPPORT	_IOR(SNAPSHOT_IOC_MAGIC, 15, int)
 #define SNAPSHOT_POWER_OFF		_IO(SNAPSHOT_IOC_MAGIC, 16)
-#define SNAPSHOT_IOC_MAXNR	16
+#define SNAPSHOT_CREATE_IMAGE		_IOW(SNAPSHOT_IOC_MAGIC, 17, int)
+#define SNAPSHOT_AVAIL_SWAP_SIZE	_IOR(SNAPSHOT_IOC_MAGIC, 18, loff_t)
+#define SNAPSHOT_ALLOC_SWAP_PAGE	_IOR(SNAPSHOT_IOC_MAGIC, 19, loff_t)
+#define SNAPSHOT_IOC_MAXNR	19
 
 /* If unset, the snapshot device cannot be open. */
 extern atomic_t snapshot_device_available;
Index: linux-2.6.23-rc4-mm1/kernel/power/user.c
===================================================================
--- linux-2.6.23-rc4-mm1.orig/kernel/power/user.c
+++ linux-2.6.23-rc4-mm1/kernel/power/user.c
@@ -39,6 +39,15 @@
 #define PMOPS_ENTER	2
 #define PMOPS_FINISH	3
 
+/*
+ * NOTE: The following ioctl definitions are wrong and have been replaced with
+ * correct ones.  They are only preserved here for compatibility with existing
+ * userland utilities and will be removed in the future.
+ */
+#define SNAPSHOT_ATOMIC_SNAPSHOT	_IOW(SNAPSHOT_IOC_MAGIC, 3, void *)
+#define SNAPSHOT_AVAIL_SWAP		_IOR(SNAPSHOT_IOC_MAGIC, 7, void *)
+#define SNAPSHOT_GET_SWAP_PAGE		_IOR(SNAPSHOT_IOC_MAGIC, 8, void *)
+
 
 #define SNAPSHOT_MINOR	231
 
@@ -190,6 +199,7 @@ static int snapshot_ioctl(struct inode *
 		data->frozen = 0;
 		break;
 
+	case SNAPSHOT_CREATE_IMAGE:
 	case SNAPSHOT_ATOMIC_SNAPSHOT:
 		if (data->mode != O_RDONLY || !data->frozen  || data->ready) {
 			error = -EPERM;
@@ -197,7 +207,7 @@ static int snapshot_ioctl(struct inode *
 		}
 		error = hibernation_snapshot(data->platform_support);
 		if (!error)
-			error = put_user(in_suspend, (unsigned int __user *)arg);
+			error = put_user(in_suspend, (int __user *)arg);
 		if (!error)
 			data->ready = 1;
 		break;
@@ -232,12 +242,14 @@ static int snapshot_ioctl(struct inode *
 		error = put_user(size, (loff_t __user *)arg);
 		break;
 
+	case SNAPSHOT_AVAIL_SWAP_SIZE:
 	case SNAPSHOT_AVAIL_SWAP:
 		size = count_swap_pages(data->swap, 1);
 		size <<= PAGE_SHIFT;
 		error = put_user(size, (loff_t __user *)arg);
 		break;
 
+	case SNAPSHOT_ALLOC_SWAP_PAGE:
 	case SNAPSHOT_GET_SWAP_PAGE:
 		if (data->swap < 0 || data->swap >= MAX_SWAPFILES) {
 			error = -ENODEV;
@@ -246,7 +258,7 @@ static int snapshot_ioctl(struct inode *
 		offset = alloc_swapdev_block(data->swap);
 		if (offset) {
 			offset <<= PAGE_SHIFT;
-			error = put_user(offset, (sector_t __user *)arg);
+			error = put_user(offset, (loff_t __user *)arg);
 		} else {
 			error = -ENOSPC;
 		}
Index: linux-2.6.23-rc4-mm1/Documentation/power/userland-swsusp.txt
===================================================================
--- linux-2.6.23-rc4-mm1.orig/Documentation/power/userland-swsusp.txt
+++ linux-2.6.23-rc4-mm1/Documentation/power/userland-swsusp.txt
@@ -27,17 +27,17 @@ once at a time.
 The ioctl() commands recognized by the device are:
 
 SNAPSHOT_FREEZE - freeze user space processes (the current process is
-	not frozen); this is required for SNAPSHOT_ATOMIC_SNAPSHOT
+	not frozen); this is required for SNAPSHOT_CREATE_IMAGE
 	and SNAPSHOT_ATOMIC_RESTORE to succeed
 
 SNAPSHOT_UNFREEZE - thaw user space processes frozen by SNAPSHOT_FREEZE
 
-SNAPSHOT_ATOMIC_SNAPSHOT - create a snapshot of the system memory; the
+SNAPSHOT_CREATE_IMAGE - create a snapshot of the system memory; the
 	last argument of ioctl() should be a pointer to an int variable,
 	the value of which will indicate whether the call returned after
 	creating the snapshot (1) or after restoring the system memory state
 	from it (0) (after resume the system finds itself finishing the
-	SNAPSHOT_ATOMIC_SNAPSHOT ioctl() again); after the snapshot
+	SNAPSHOT_CREATE_IMAGE ioctl() again); after the snapshot
 	has been created the read() operation can be used to transfer
 	it out of the kernel
 
@@ -56,16 +56,16 @@ SNAPSHOT_SET_IMAGE_SIZE - set the prefer
 
 SNAPSHOT_GET_IMAGE_SIZE - return the actual size of the hibernation image
 
-SNAPSHOT_AVAIL_SWAP - return the amount of available swap in bytes (the last
-	argument should be a pointer to an unsigned int variable that will
+SNAPSHOT_AVAIL_SWAP_SIZE - return the amount of available swap in bytes (the
+	last argument should be a pointer to an unsigned int variable that will
 	contain the result if the call is successful).
 
-SNAPSHOT_GET_SWAP_PAGE - allocate a swap page from the resume partition
+SNAPSHOT_ALLOC_SWAP_PAGE - allocate a swap page from the resume partition
 	(the last argument should be a pointer to a loff_t variable that
 	will contain the swap page offset if the call is successful)
 
-SNAPSHOT_FREE_SWAP_PAGES - free all swap pages allocated with
-	SNAPSHOT_GET_SWAP_PAGE
+SNAPSHOT_FREE_SWAP_PAGES - free all swap pages allocated by
+	SNAPSHOT_ALLOC_SWAP_PAGE
 
 SNAPSHOT_SET_SWAP_FILE - set the resume partition (the last ioctl() argument
 	should specify the device's major and minor numbers in the old
@@ -112,7 +112,7 @@ The device's write() operation is used f
 into the kernel.  It has the same limitations as the read() operation.
 
 The release() operation frees all memory allocated for the snapshot image
-and all swap pages allocated with SNAPSHOT_GET_SWAP_PAGE (if any).
+and all swap pages allocated with SNAPSHOT_ALLOC_SWAP_PAGE (if any).
 Thus it is not necessary to use either SNAPSHOT_FREE or
 SNAPSHOT_FREE_SWAP_PAGES before closing the device (in fact it will also
 unfreeze user space processes frozen by SNAPSHOT_UNFREEZE if they are
@@ -123,7 +123,7 @@ snapshot image from/to the kernel will u
 partition, or a swap file as storage space (if a swap file is used, the resume
 partition is the partition that holds this file).  However, this is not really
 required, as they can use, for example, a special (blank) suspend partition or
-a file on a partition that is unmounted before SNAPSHOT_ATOMIC_SNAPSHOT and
+a file on a partition that is unmounted before SNAPSHOT_CREATE_IMAGE and
 mounted afterwards.
 
 These utilities MUST NOT make any assumptions regarding the ordering of
@@ -145,7 +145,7 @@ means, such as checksums, to ensure the 
 The suspending and resuming utilities MUST lock themselves in memory,
 preferrably using mlockall(), before calling SNAPSHOT_FREEZE.
 
-The suspending utility MUST check the value stored by SNAPSHOT_ATOMIC_SNAPSHOT
+The suspending utility MUST check the value stored by SNAPSHOT_CREATE_IMAGE
 in the memory location pointed to by the last argument of ioctl() and proceed
 in accordance with it:
 1. 	If the value is 1 (ie. the system memory snapshot has just been
@@ -159,7 +159,7 @@ in accordance with it:
 		image has been saved.
 	(b)	The suspending utility SHOULD NOT attempt to perform any
 		file system operations (including reads) on the file systems
-		that were mounted before SNAPSHOT_ATOMIC_SNAPSHOT has been
+		that were mounted before SNAPSHOT_CREATE_IMAGE has been
 		called.  However, it MAY mount a file system that was not
 		mounted at that time and perform some operations on it (eg.
 		use it for saving the image).

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

* [PATCH -mm 4/5] Hibernation: Mark SNAPSHOT_SET_SWAP_FILE ioctl as deprecated
  2007-09-02 21:15 [PATCH -mm 0/5] Hibernation: Clean up userland interface Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2007-09-02 21:21 ` [PATCH -mm 3/5] Hibernation: Correct definitions of some ioctls Rafael J. Wysocki
@ 2007-09-02 21:22 ` Rafael J. Wysocki
  2007-09-06  2:08   ` Pavel Machek
  2007-09-02 21:28 ` [PATCH -mm 5/5] Hibernation: Introduce exportable suspend ioctls header Rafael J. Wysocki
  4 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2007-09-02 21:22 UTC (permalink / raw)
  To: Pavel Machek; +Cc: pm list

From: Rafael J. Wysocki <rjw@sisk.pl>

Mark the obsolete SNAPSHOT_SET_SWAP_FILE ioctl belonging to the hibernation
userland interface as deprecated.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/userland-swsusp.txt |    9 ---------
 kernel/power/power.h                    |    1 -
 kernel/power/user.c                     |    9 +++++----
 3 files changed, 5 insertions(+), 14 deletions(-)

Index: linux-2.6.23-rc4-mm1/kernel/power/power.h
===================================================================
--- linux-2.6.23-rc4-mm1.orig/kernel/power/power.h
+++ linux-2.6.23-rc4-mm1/kernel/power/power.h
@@ -153,7 +153,6 @@ struct resume_swap_area {
 #define SNAPSHOT_FREE			_IO(SNAPSHOT_IOC_MAGIC, 5)
 #define SNAPSHOT_SET_IMAGE_SIZE		_IOW(SNAPSHOT_IOC_MAGIC, 6, unsigned long)
 #define SNAPSHOT_FREE_SWAP_PAGES	_IO(SNAPSHOT_IOC_MAGIC, 9)
-#define SNAPSHOT_SET_SWAP_FILE		_IOW(SNAPSHOT_IOC_MAGIC, 10, unsigned int)
 #define SNAPSHOT_S2RAM			_IO(SNAPSHOT_IOC_MAGIC, 11)
 #define SNAPSHOT_SET_SWAP_AREA		_IOW(SNAPSHOT_IOC_MAGIC, 13, \
 							struct resume_swap_area)
Index: linux-2.6.23-rc4-mm1/kernel/power/user.c
===================================================================
--- linux-2.6.23-rc4-mm1.orig/kernel/power/user.c
+++ linux-2.6.23-rc4-mm1/kernel/power/user.c
@@ -29,10 +29,11 @@
 #include "power.h"
 
 /*
- * NOTE: The SNAPSHOT_PMOPS ioctl is obsolete and will be removed in the
- * future.  It is only preserved here for compatibility with existing userland
- * utilities.
+ * NOTE: The SNAPSHOT_SET_SWAP_FILE and SNAPSHOT_PMOPS ioctls are obsolete and
+ * will be removed in the future.  They are only preserved here for
+ * compatibility with existing userland utilities.
  */
+#define SNAPSHOT_SET_SWAP_FILE	_IOW(SNAPSHOT_IOC_MAGIC, 10, unsigned int)
 #define SNAPSHOT_PMOPS		_IOW(SNAPSHOT_IOC_MAGIC, 12, unsigned int)
 
 #define PMOPS_PREPARE	1
@@ -272,7 +273,7 @@ static int snapshot_ioctl(struct inode *
 		free_all_swap_pages(data->swap);
 		break;
 
-	case SNAPSHOT_SET_SWAP_FILE:
+	case SNAPSHOT_SET_SWAP_FILE: /* This ioctl is deprecated */
 		if (!swsusp_swap_in_use()) {
 			/*
 			 * User space encodes device types as two-byte values,
Index: linux-2.6.23-rc4-mm1/Documentation/power/userland-swsusp.txt
===================================================================
--- linux-2.6.23-rc4-mm1.orig/Documentation/power/userland-swsusp.txt
+++ linux-2.6.23-rc4-mm1/Documentation/power/userland-swsusp.txt
@@ -67,11 +67,6 @@ SNAPSHOT_ALLOC_SWAP_PAGE - allocate a sw
 SNAPSHOT_FREE_SWAP_PAGES - free all swap pages allocated by
 	SNAPSHOT_ALLOC_SWAP_PAGE
 
-SNAPSHOT_SET_SWAP_FILE - set the resume partition (the last ioctl() argument
-	should specify the device's major and minor numbers in the old
-	two-byte format, as returned by the stat() function in the .st_rdev
-	member of the stat structure)
-
 SNAPSHOT_SET_SWAP_AREA - set the resume partition and the offset (in <PAGE_SIZE>
 	units) from the beginning of the partition at which the swap header is
 	located (the last ioctl() argument should point to a struct
@@ -80,10 +75,6 @@ SNAPSHOT_SET_SWAP_AREA - set the resume 
 	and the offset); for swap partitions the offset is always 0, but it is
 	different to zero for swap files (please see
 	Documentation/swsusp-and-swap-files.txt for details).
-	The SNAPSHOT_SET_SWAP_AREA ioctl() is considered as a replacement for
-	SNAPSHOT_SET_SWAP_FILE which is regarded as obsolete.   It is
-	recommended to always use this call, because the code to set the resume
-	partition may be removed from future kernels
 
 SNAPSHOT_PLATFORM_SUPPORT - enable/disable the hibernation platform support,
 	depending on the argument value (enable, if the argument is nonzero)

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

* [PATCH -mm 5/5] Hibernation: Introduce exportable suspend ioctls header
  2007-09-02 21:15 [PATCH -mm 0/5] Hibernation: Clean up userland interface Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2007-09-02 21:22 ` [PATCH -mm 4/5] Hibernation: Mark SNAPSHOT_SET_SWAP_FILE ioctl as deprecated Rafael J. Wysocki
@ 2007-09-02 21:28 ` Rafael J. Wysocki
  2007-09-03  3:29   ` Pavel Machek
  2007-09-03  9:43   ` Johannes Berg
  4 siblings, 2 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2007-09-02 21:28 UTC (permalink / raw)
  To: Pavel Machek; +Cc: pm list

From: Rafael J. Wysocki <rjw@sisk.pl>

Move the definitions of hibernation ioctls to a separate header file in
include/linux, which can be exported to the user space.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/userland-swsusp.txt |   11 ++++------
 include/linux/suspend_ioctls.h          |   33 ++++++++++++++++++++++++++++++++
 kernel/power/power.h                    |   29 ----------------------------
 3 files changed, 39 insertions(+), 34 deletions(-)

Index: linux-2.6.23-rc4-mm1/include/linux/suspend_ioctls.h
===================================================================
--- /dev/null
+++ linux-2.6.23-rc4-mm1/include/linux/suspend_ioctls.h
@@ -0,0 +1,33 @@
+#ifndef _LINUX_SUSPEND_IOCTLS_H
+#define _LINUX_SUSPEND_IOCTLS_H
+
+/*
+ * This structure is used to pass the values needed for the identification
+ * of the resume swap area from a user space to the kernel via the
+ * SNAPSHOT_SET_SWAP_AREA ioctl
+ */
+struct resume_swap_area {
+	loff_t offset;
+	u_int32_t dev;
+} __attribute__((packed));
+
+#define SNAPSHOT_IOC_MAGIC	'3'
+#define SNAPSHOT_FREEZE			_IO(SNAPSHOT_IOC_MAGIC, 1)
+#define SNAPSHOT_UNFREEZE		_IO(SNAPSHOT_IOC_MAGIC, 2)
+#define SNAPSHOT_ATOMIC_RESTORE		_IO(SNAPSHOT_IOC_MAGIC, 4)
+#define SNAPSHOT_FREE			_IO(SNAPSHOT_IOC_MAGIC, 5)
+#define SNAPSHOT_SET_IMAGE_SIZE		_IOW(SNAPSHOT_IOC_MAGIC, 6, \
+							unsigned long)
+#define SNAPSHOT_FREE_SWAP_PAGES	_IO(SNAPSHOT_IOC_MAGIC, 9)
+#define SNAPSHOT_S2RAM			_IO(SNAPSHOT_IOC_MAGIC, 11)
+#define SNAPSHOT_SET_SWAP_AREA		_IOW(SNAPSHOT_IOC_MAGIC, 13, \
+							struct resume_swap_area)
+#define SNAPSHOT_GET_IMAGE_SIZE		_IOR(SNAPSHOT_IOC_MAGIC, 14, loff_t)
+#define SNAPSHOT_PLATFORM_SUPPORT	_IOR(SNAPSHOT_IOC_MAGIC, 15, int)
+#define SNAPSHOT_POWER_OFF		_IO(SNAPSHOT_IOC_MAGIC, 16)
+#define SNAPSHOT_CREATE_IMAGE		_IOW(SNAPSHOT_IOC_MAGIC, 17, int)
+#define SNAPSHOT_AVAIL_SWAP_SIZE	_IOR(SNAPSHOT_IOC_MAGIC, 18, loff_t)
+#define SNAPSHOT_ALLOC_SWAP_PAGE	_IOR(SNAPSHOT_IOC_MAGIC, 19, loff_t)
+#define SNAPSHOT_IOC_MAXNR	19
+
+#endif /* _LINUX_SUSPEND_IOCTLS_H */
Index: linux-2.6.23-rc4-mm1/kernel/power/power.h
===================================================================
--- linux-2.6.23-rc4-mm1.orig/kernel/power/power.h
+++ linux-2.6.23-rc4-mm1/kernel/power/power.h
@@ -1,4 +1,5 @@
 #include <linux/suspend.h>
+#include <linux/suspend_ioctls.h>
 #include <linux/utsname.h>
 
 struct swsusp_info {
@@ -136,34 +137,6 @@ extern int snapshot_write_next(struct sn
 extern void snapshot_write_finalize(struct snapshot_handle *handle);
 extern int snapshot_image_loaded(struct snapshot_handle *handle);
 
-/*
- * This structure is used to pass the values needed for the identification
- * of the resume swap area from a user space to the kernel via the
- * SNAPSHOT_SET_SWAP_AREA ioctl
- */
-struct resume_swap_area {
-	loff_t offset;
-	u_int32_t dev;
-} __attribute__((packed));
-
-#define SNAPSHOT_IOC_MAGIC	'3'
-#define SNAPSHOT_FREEZE			_IO(SNAPSHOT_IOC_MAGIC, 1)
-#define SNAPSHOT_UNFREEZE		_IO(SNAPSHOT_IOC_MAGIC, 2)
-#define SNAPSHOT_ATOMIC_RESTORE		_IO(SNAPSHOT_IOC_MAGIC, 4)
-#define SNAPSHOT_FREE			_IO(SNAPSHOT_IOC_MAGIC, 5)
-#define SNAPSHOT_SET_IMAGE_SIZE		_IOW(SNAPSHOT_IOC_MAGIC, 6, unsigned long)
-#define SNAPSHOT_FREE_SWAP_PAGES	_IO(SNAPSHOT_IOC_MAGIC, 9)
-#define SNAPSHOT_S2RAM			_IO(SNAPSHOT_IOC_MAGIC, 11)
-#define SNAPSHOT_SET_SWAP_AREA		_IOW(SNAPSHOT_IOC_MAGIC, 13, \
-							struct resume_swap_area)
-#define SNAPSHOT_GET_IMAGE_SIZE		_IOR(SNAPSHOT_IOC_MAGIC, 14, loff_t)
-#define SNAPSHOT_PLATFORM_SUPPORT	_IOR(SNAPSHOT_IOC_MAGIC, 15, int)
-#define SNAPSHOT_POWER_OFF		_IO(SNAPSHOT_IOC_MAGIC, 16)
-#define SNAPSHOT_CREATE_IMAGE		_IOW(SNAPSHOT_IOC_MAGIC, 17, int)
-#define SNAPSHOT_AVAIL_SWAP_SIZE	_IOR(SNAPSHOT_IOC_MAGIC, 18, loff_t)
-#define SNAPSHOT_ALLOC_SWAP_PAGE	_IOR(SNAPSHOT_IOC_MAGIC, 19, loff_t)
-#define SNAPSHOT_IOC_MAXNR	19
-
 /* If unset, the snapshot device cannot be open. */
 extern atomic_t snapshot_device_available;
 
Index: linux-2.6.23-rc4-mm1/Documentation/power/userland-swsusp.txt
===================================================================
--- linux-2.6.23-rc4-mm1.orig/Documentation/power/userland-swsusp.txt
+++ linux-2.6.23-rc4-mm1/Documentation/power/userland-swsusp.txt
@@ -14,7 +14,7 @@ are going to develop your own suspend/re
 
 The interface consists of a character device providing the open(),
 release(), read(), and write() operations as well as several ioctl()
-commands defined in kernel/power/power.h.  The major and minor
+commands defined in include/linux/suspend_ioctls.h .  The major and minor
 numbers of the device are, respectively, 10 and 231, and they can
 be read from /sys/class/misc/snapshot/dev.
 
@@ -70,11 +70,10 @@ SNAPSHOT_FREE_SWAP_PAGES - free all swap
 SNAPSHOT_SET_SWAP_AREA - set the resume partition and the offset (in <PAGE_SIZE>
 	units) from the beginning of the partition at which the swap header is
 	located (the last ioctl() argument should point to a struct
-	resume_swap_area, as defined in kernel/power/power.h, containing the
-	resume device specification, as for the SNAPSHOT_SET_SWAP_FILE ioctl(),
-	and the offset); for swap partitions the offset is always 0, but it is
-	different to zero for swap files (please see
-	Documentation/swsusp-and-swap-files.txt for details).
+	resume_swap_area, as defined in include/linux/suspend_ioctls.h,
+	containing the resume device specification and the offset); for swap
+	partitions the offset is always 0, but it is different to zero for swap
+	files (please see Documentation/swsusp-and-swap-files.txt for details).
 
 SNAPSHOT_PLATFORM_SUPPORT - enable/disable the hibernation platform support,
 	depending on the argument value (enable, if the argument is nonzero)

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

* Re: [PATCH -mm 1/5] Hibernation: Introduce SNAPSHOT_GET_IMAGE_SIZE ioctl
  2007-09-02 21:17 ` [PATCH -mm 1/5] Hibernation: Introduce SNAPSHOT_GET_IMAGE_SIZE ioctl Rafael J. Wysocki
@ 2007-09-03  3:17   ` Pavel Machek
  0 siblings, 0 replies; 15+ messages in thread
From: Pavel Machek @ 2007-09-03  3:17 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: pm list

On Sun 2007-09-02 23:17:41, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Add a new ioctl, SNAPSHOT_GET_IMAGE_SIZE, returning the size of the (just created)
> hibernation image, to the hibernation userland interface.
> 
> This ioctl is necessary so that the userland utilities using the interface need
> not access the hibernation image header, owned by the kernel, in order to obtain
> the size of the image.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

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

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

* Re: [PATCH -mm 3/5] Hibernation: Correct definitions of some ioctls
  2007-09-02 21:21 ` [PATCH -mm 3/5] Hibernation: Correct definitions of some ioctls Rafael J. Wysocki
@ 2007-09-03  3:25   ` Pavel Machek
  2007-09-03 21:14     ` Rafael J. Wysocki
  0 siblings, 1 reply; 15+ messages in thread
From: Pavel Machek @ 2007-09-03  3:25 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: pm list

Hi!

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Three ioctl numbers belonging to the hibernation userland interface,
> SNAPSHOT_ATOMIC_SNAPSHOT, SNAPSHOT_AVAIL_SWAP, SNAPSHOT_GET_SWAP_PAGE,
> are defined in a wrong way (eg. not portable).  Provide new ioctl numbers for
> these ioctls and mark the existing ones as deprecated.

It looks to me like

> @@ -149,12 +149,9 @@ struct resume_swap_area {
>  #define SNAPSHOT_IOC_MAGIC	'3'
>  #define SNAPSHOT_FREEZE			_IO(SNAPSHOT_IOC_MAGIC, 1)
>  #define SNAPSHOT_UNFREEZE		_IO(SNAPSHOT_IOC_MAGIC, 2)
> -#define SNAPSHOT_ATOMIC_SNAPSHOT	_IOW(SNAPSHOT_IOC_MAGIC, 3, void *)
>  #define SNAPSHOT_ATOMIC_RESTORE		_IO(SNAPSHOT_IOC_MAGIC, 4)
>  #define SNAPSHOT_FREE			_IO(SNAPSHOT_IOC_MAGIC, 5)
>  #define SNAPSHOT_SET_IMAGE_SIZE  _IOW(SNAPSHOT_IOC_MAGIC, 6, unsigned long)

at least set_image_size is wrong - too: we do not use argument as
unsigned long *, and we certainly don't write to it.

> -#define SNAPSHOT_AVAIL_SWAP		_IOR(SNAPSHOT_IOC_MAGIC, 7, void *)
> -#define SNAPSHOT_GET_SWAP_PAGE		_IOR(SNAPSHOT_IOC_MAGIC, 8, void *)
>  #define SNAPSHOT_FREE_SWAP_PAGES	_IO(SNAPSHOT_IOC_MAGIC, 9)
>  #define SNAPSHOT_SET_SWAP_FILE		_IOW(SNAPSHOT_IOC_MAGIC, 10, unsigned int)
>  #define SNAPSHOT_S2RAM			_IO(SNAPSHOT_IOC_MAGIC, 11)
> @@ -163,7 +160,10 @@ struct resume_swap_area {
>  #define SNAPSHOT_GET_IMAGE_SIZE		_IOR(SNAPSHOT_IOC_MAGIC, 14, loff_t)
>  #define SNAPSHOT_PLATFORM_SUPPORT	_IOR(SNAPSHOT_IOC_MAGIC, 15, int)
>  #define SNAPSHOT_POWER_OFF		_IO(SNAPSHOT_IOC_MAGIC, 16)
> -#define SNAPSHOT_IOC_MAXNR	16
> +#define SNAPSHOT_CREATE_IMAGE		_IOW(SNAPSHOT_IOC_MAGIC, 17, int)
> +#define SNAPSHOT_AVAIL_SWAP_SIZE	_IOR(SNAPSHOT_IOC_MAGIC, 18, -#loff_t)

Should this be _IOW, because we return that value to userspace?

> +#define SNAPSHOT_ALLOC_SWAP_PAGE	_IOR(SNAPSHOT_IOC_MAGIC, 19, loff_t)

Hmm, this looks like _IOW to me, too, plus it does put_user with u64
(at least in my copy, may have been hacked too much).

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

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

* Re: [PATCH -mm 5/5] Hibernation: Introduce exportable suspend ioctls header
  2007-09-02 21:28 ` [PATCH -mm 5/5] Hibernation: Introduce exportable suspend ioctls header Rafael J. Wysocki
@ 2007-09-03  3:29   ` Pavel Machek
  2007-09-03 21:15     ` Rafael J. Wysocki
  2007-09-03  9:43   ` Johannes Berg
  1 sibling, 1 reply; 15+ messages in thread
From: Pavel Machek @ 2007-09-03  3:29 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: pm list


> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Move the definitions of hibernation ioctls to a separate header file in
> include/linux, which can be exported to the user space.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

ACK, but this depends on previous patches that need fixes. Perhaps
this should be moved to the begging of series?

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

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

* Re: [PATCH -mm 5/5] Hibernation: Introduce exportable suspend ioctls header
  2007-09-02 21:28 ` [PATCH -mm 5/5] Hibernation: Introduce exportable suspend ioctls header Rafael J. Wysocki
  2007-09-03  3:29   ` Pavel Machek
@ 2007-09-03  9:43   ` Johannes Berg
  2007-09-03 21:15     ` Rafael J. Wysocki
  1 sibling, 1 reply; 15+ messages in thread
From: Johannes Berg @ 2007-09-03  9:43 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: pm list


[-- Attachment #1.1: Type: text/plain, Size: 380 bytes --]

On Sun, 2007-09-02 at 23:28 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Move the definitions of hibernation ioctls to a separate header file in
> include/linux, which can be exported to the user space.

Missing the actual export to userspace:

--- include/linux/Kbuild
+++ include/linux/Kbuild
+header-y += suspend_ioctls.h

johannes

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



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

* Re: [PATCH -mm 3/5] Hibernation: Correct definitions of some ioctls
  2007-09-03  3:25   ` Pavel Machek
@ 2007-09-03 21:14     ` Rafael J. Wysocki
  0 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2007-09-03 21:14 UTC (permalink / raw)
  To: Pavel Machek; +Cc: pm list

On Monday, 3 September 2007 05:25, Pavel Machek wrote:
> Hi!
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Three ioctl numbers belonging to the hibernation userland interface,
> > SNAPSHOT_ATOMIC_SNAPSHOT, SNAPSHOT_AVAIL_SWAP, SNAPSHOT_GET_SWAP_PAGE,
> > are defined in a wrong way (eg. not portable).  Provide new ioctl numbers for
> > these ioctls and mark the existing ones as deprecated.
> 
> It looks to me like
> 
> > @@ -149,12 +149,9 @@ struct resume_swap_area {
> >  #define SNAPSHOT_IOC_MAGIC	'3'
> >  #define SNAPSHOT_FREEZE			_IO(SNAPSHOT_IOC_MAGIC, 1)
> >  #define SNAPSHOT_UNFREEZE		_IO(SNAPSHOT_IOC_MAGIC, 2)
> > -#define SNAPSHOT_ATOMIC_SNAPSHOT	_IOW(SNAPSHOT_IOC_MAGIC, 3, void *)
> >  #define SNAPSHOT_ATOMIC_RESTORE		_IO(SNAPSHOT_IOC_MAGIC, 4)
> >  #define SNAPSHOT_FREE			_IO(SNAPSHOT_IOC_MAGIC, 5)
> >  #define SNAPSHOT_SET_IMAGE_SIZE  _IOW(SNAPSHOT_IOC_MAGIC, 6, unsigned long)
> 
> at least set_image_size is wrong - too: we do not use argument as
> unsigned long *, and we certainly don't write to it.

Yes, it is.  I think it should be _IO(SNAPSHOT_IOC_MAGIC, a_number)

I'll update the patch to cover this.

> > -#define SNAPSHOT_AVAIL_SWAP		_IOR(SNAPSHOT_IOC_MAGIC, 7, void *)
> > -#define SNAPSHOT_GET_SWAP_PAGE		_IOR(SNAPSHOT_IOC_MAGIC, 8, void *)
> >  #define SNAPSHOT_FREE_SWAP_PAGES	_IO(SNAPSHOT_IOC_MAGIC, 9)
> >  #define SNAPSHOT_SET_SWAP_FILE		_IOW(SNAPSHOT_IOC_MAGIC, 10, unsigned int)
> >  #define SNAPSHOT_S2RAM			_IO(SNAPSHOT_IOC_MAGIC, 11)
> > @@ -163,7 +160,10 @@ struct resume_swap_area {
> >  #define SNAPSHOT_GET_IMAGE_SIZE		_IOR(SNAPSHOT_IOC_MAGIC, 14, loff_t)
> >  #define SNAPSHOT_PLATFORM_SUPPORT	_IOR(SNAPSHOT_IOC_MAGIC, 15, int)

This one also should be _IO(...), BTW.

> >  #define SNAPSHOT_POWER_OFF		_IO(SNAPSHOT_IOC_MAGIC, 16)
> > -#define SNAPSHOT_IOC_MAXNR	16
> > +#define SNAPSHOT_CREATE_IMAGE		_IOW(SNAPSHOT_IOC_MAGIC, 17, int)
> > +#define SNAPSHOT_AVAIL_SWAP_SIZE	_IOR(SNAPSHOT_IOC_MAGIC, 18, -#loff_t)
> 
> Should this be _IOW, because we return that value to userspace?

No.  The direction is from the userland POV, AFAICS.

> > +#define SNAPSHOT_ALLOC_SWAP_PAGE	_IOR(SNAPSHOT_IOC_MAGIC, 19, loff_t)
> 
> Hmm, this looks like _IOW to me, too, plus it does put_user with u64
> (at least in my copy, may have been hacked too much).

In -rc5 it uses loff_t, and I believe that the direction is correct.

Greetings,
Rafael

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

* Re: [PATCH -mm 5/5] Hibernation: Introduce exportable suspend ioctls header
  2007-09-03  3:29   ` Pavel Machek
@ 2007-09-03 21:15     ` Rafael J. Wysocki
  0 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2007-09-03 21:15 UTC (permalink / raw)
  To: Pavel Machek; +Cc: pm list

On Monday, 3 September 2007 05:29, Pavel Machek wrote:
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Move the definitions of hibernation ioctls to a separate header file in
> > include/linux, which can be exported to the user space.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> ACK, but this depends on previous patches that need fixes. Perhaps
> this should be moved to the begging of series?

Hm, the whole point of some patches in the series is to prepare for this one ...

Greetings,
Rafael

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

* Re: [PATCH -mm 5/5] Hibernation: Introduce exportable suspend ioctls header
  2007-09-03  9:43   ` Johannes Berg
@ 2007-09-03 21:15     ` Rafael J. Wysocki
  0 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2007-09-03 21:15 UTC (permalink / raw)
  To: Johannes Berg; +Cc: pm list

On Monday, 3 September 2007 11:43, Johannes Berg wrote:
> On Sun, 2007-09-02 at 23:28 +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Move the definitions of hibernation ioctls to a separate header file in
> > include/linux, which can be exported to the user space.
> 
> Missing the actual export to userspace:
> 
> --- include/linux/Kbuild
> +++ include/linux/Kbuild
> +header-y += suspend_ioctls.h

Good point, thanks.

Greetings,
Rafael

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

* Re: [PATCH -mm 2/5] Hibernation: Rework platform support ioctls
  2007-09-02 21:19 ` [PATCH -mm 2/5] Hibernation: Rework platform support ioctls Rafael J. Wysocki
@ 2007-09-06  1:57   ` Pavel Machek
  0 siblings, 0 replies; 15+ messages in thread
From: Pavel Machek @ 2007-09-06  1:57 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: pm list

Hi!
> Modify the hibernation userland interface by adding two new ioctls to it,
> SNAPSHOT_PLATFORM_SUPPORT and SNAPSHOT_POWER_OFF, that can be used,
> respectively, to switch the hibernation platform support on/off and to make the
> kernel transition the system to the hibernation state (eg. ACPI S4) using the
> platform (eg. ACPI) driver.
> 
> These ioctls are intended to replace the misdesigned SNAPSHOT_PMOPS ioctl,
> which from now is regarded as obsolete and will be removed in the future.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

ACK.

> +/*
> + * NOTE: The SNAPSHOT_PMOPS ioctl is obsolete and will be removed in the
> + * future.  It is only preserved here for compatibility with existing userland
> + * utilities.
> + */

Perhaps that paragraph from Documentation should be moved here, so
that people know what this obsolete ioctl() is expected to do?

> @@ -95,24 +101,6 @@ SNAPSHOT_S2RAM - suspend to RAM; using t
>  	to resume the system from RAM if there's enough battery power or restore
>  	its state on the basis of the saved suspend image otherwise)
>  
> -SNAPSHOT_PMOPS - enable the usage of the hibernation_ops->prepare,
> -	hibernate_ops->enter and hibernation_ops->finish methods (the in-kernel
> -	swsusp knows these as the "platform method") which are needed on many
> -	machines to (among others) speed up the resume by letting the BIOS skip
> -	some steps or to let the system recognise the correct state of the
> -	hardware after the resume (in particular on many machines this ensures
> -	that unplugged AC adapters get correctly detected and that kacpid does
> -	not run wild after the resume).  The last ioctl() argument can take one
> -	of the three values, defined in kernel/power/power.h:
> -	PMOPS_PREPARE - make the kernel carry out the
> -		hibernation_ops->prepare() operation
> -	PMOPS_ENTER - make the kernel power off the system by calling
> -		hibernation_ops->enter()
> -	PMOPS_FINISH - make the kernel carry out the
> -		hibernation_ops->finish() operation
> -	Note that the actual constants are misnamed because they surface
> -	internal kernel implementation details that have changed.
> -
>  The device's read() operation can be used to transfer the snapshot image from
>  the kernel.  It has the following limitations:
>  - you cannot read() more than one virtual memory page at a time

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

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

* Re: [PATCH -mm 4/5] Hibernation: Mark SNAPSHOT_SET_SWAP_FILE ioctl as deprecated
  2007-09-02 21:22 ` [PATCH -mm 4/5] Hibernation: Mark SNAPSHOT_SET_SWAP_FILE ioctl as deprecated Rafael J. Wysocki
@ 2007-09-06  2:08   ` Pavel Machek
  0 siblings, 0 replies; 15+ messages in thread
From: Pavel Machek @ 2007-09-06  2:08 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: pm list

On Sun 2007-09-02 23:22:26, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Mark the obsolete SNAPSHOT_SET_SWAP_FILE ioctl belonging to the hibernation
> userland interface as deprecated.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

ACK.
									Pavel

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

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

end of thread, other threads:[~2007-09-06  2:08 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-09-02 21:15 [PATCH -mm 0/5] Hibernation: Clean up userland interface Rafael J. Wysocki
2007-09-02 21:17 ` [PATCH -mm 1/5] Hibernation: Introduce SNAPSHOT_GET_IMAGE_SIZE ioctl Rafael J. Wysocki
2007-09-03  3:17   ` Pavel Machek
2007-09-02 21:19 ` [PATCH -mm 2/5] Hibernation: Rework platform support ioctls Rafael J. Wysocki
2007-09-06  1:57   ` Pavel Machek
2007-09-02 21:21 ` [PATCH -mm 3/5] Hibernation: Correct definitions of some ioctls Rafael J. Wysocki
2007-09-03  3:25   ` Pavel Machek
2007-09-03 21:14     ` Rafael J. Wysocki
2007-09-02 21:22 ` [PATCH -mm 4/5] Hibernation: Mark SNAPSHOT_SET_SWAP_FILE ioctl as deprecated Rafael J. Wysocki
2007-09-06  2:08   ` Pavel Machek
2007-09-02 21:28 ` [PATCH -mm 5/5] Hibernation: Introduce exportable suspend ioctls header Rafael J. Wysocki
2007-09-03  3:29   ` Pavel Machek
2007-09-03 21:15     ` Rafael J. Wysocki
2007-09-03  9:43   ` Johannes Berg
2007-09-03 21:15     ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox