Hibernation: Clean up userland interface
Subsystems (2)
cover Hibernation: Clean up userland interface
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
Signature · 1 line
"Premature optimization is the root of all evil." - Donald Knuth
PATCH 1/5 Hibernation: Introduce SNAPSHOT_GET_IMAGE_SIZE ioctl
From: Rafael J. Wysocki <[email protected]>
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.
--- 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(-)
kernel/power/power.h +3 -1
@@ -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
kernel/power/user.c +14 -4
@@ -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 :
kernel/power/snapshot.c +6 -1
@@ -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);
Documentation/power/userland-swsusp.txt +5 -7
@@ -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
Re: [PATCH -mm 1/5] Hibernation: Introduce SNAPSHOT_GET_IMAGE_SIZE ioctl
Quoted 10 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
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 <[email protected]>
ACK.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH 2/5 Hibernation: Rework platform support ioctls
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
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.
--- Documentation/power/userland-swsusp.txt | 24 ++++--------------- kernel/power/power.h | 9 ++----- kernel/power/user.c | 39 +++++++++++++++++++++++--------- 3 files changed, 38 insertions(+), 34 deletions(-)
kernel/power/power.h +3 -6
@@ -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;
kernel/power/user.c +29 -10
@@ -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 :
Documentation/power/userland-swsusp.txt +6 -18
@@ -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
Re: [PATCH -mm 2/5] Hibernation: Rework platform support ioctls
Hi!
Quoted 10 lines
Modify the hibernation userland interface by adding two new ioctls to it,
SNAPSHOT_PLATFORM_SUPPORTandSNAPSHOT_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_PMOPSioctl, which from now is regarded as obsolete and will be removed in the future.Signed-off-by Rafael J. Wysocki <[email protected]>
ACK.
Quoted 5 lines
+/*
+ * NOTE: TheSNAPSHOT_PMOPSioctl 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?
Quoted 15 lines
Documentation/power/userland-swsusp.txt +0
-11@@ -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
Documentation/power/userland-swsusp.txt SNAPSHOT_S2RAM - suspend to RAM; using t · line 106
Quoted 10 lines
Documentation/power/userland-swsusp.txt +0
-7- 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
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH 3/5 Hibernation: Correct definitions of some ioctls
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
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.
--- Documentation/power/userland-swsusp.txt | 24 ++++++++++++------------ kernel/power/power.h | 8 ++++---- kernel/power/user.c | 16 ++++++++++++++-- 3 files changed, 30 insertions(+), 18 deletions(-)
kernel/power/power.h +4 -4
@@ -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;
kernel/power/user.c +14 -2
@@ -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 ; }
Documentation/power/userland-swsusp.txt +12 -12
@@ -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).
Re: [PATCH -mm 3/5] Hibernation: Correct definitions of some ioctls
Hi!
Quoted 6 lines
From: Rafael J. Wysocki <[email protected]>
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
kernel/power/power.h struct resume_swap_area { · line 154
Quoted 8 lines
kernel/power/power.h +0
-1@@ -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.
kernel/power/power.h struct resume_swap_area { · line 164
Quoted 12 lines
kernel/power/power.h +2
-3-#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?
kernel/power/power.h struct resume_swap_area { · line 165
Quoted 1 line
kernel/power/power.h +1
-0+#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
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH -mm 3/5] Hibernation: Correct definitions of some ioctls
Quoted 11 lines · Pavel Machek
Hi!
Quoted 6 lines
From: Rafael J. Wysocki <[email protected]>
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
Quoted 11 lines
kernel/power/power.h struct resume_swap_area { · line 154
Quoted 8 lines
kernel/power/power.h +0
-1@@ -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_sizeis 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.
Quoted 8 lines
kernel/power/power.h struct resume_swap_area { · line 161
Quoted 8 lines
kernel/power/power.h +0
-2-#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.
Quoted 6 lines
kernel/power/power.h struct resume_swap_area {
Quoted 4 lines
kernel/power/power.h +2
-1#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.
Quoted 4 lines
kernel/power/power.h struct resume_swap_area { · line 165
Quoted 1 line
kernel/power/power.h +1
-0+#define SNAPSHOT_ALLOC_SWAP_PAGE _IOR(SNAPSHOT_IOC_MAGIC, 19, loff_t)Hmm, this looks like
_IOWto me, too, plus it doesput_userwith 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
PATCH 4/5 Hibernation: Mark SNAPSHOT_SET_SWAP_FILE ioctl as deprecated
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
Mark the obsolete SNAPSHOT_SET_SWAP_FILE ioctl belonging to the hibernation userland interface as deprecated.
--- Documentation/power/userland-swsusp.txt | 9 --------- kernel/power/power.h | 1 - kernel/power/user.c | 9 +++++---- 3 files changed, 5 insertions(+), 14 deletions(-)
kernel/power/power.h +0 -1
@@ -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)
kernel/power/user.c +5 -4
@@ -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,
Documentation/power/userland-swsusp.txt +0 -9
@@ -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)
Re: [PATCH -mm 4/5] Hibernation: Mark SNAPSHOT_SET_SWAP_FILE ioctl as deprecated
Quoted 6 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
Mark the obsolete
SNAPSHOT_SET_SWAP_FILEioctl belonging to the hibernation userland interface as deprecated.Signed-off-by Rafael J. Wysocki <[email protected]>
ACK.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH 5/5 Hibernation: Introduce exportable suspend ioctls header
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
Move the definitions of hibernation ioctls to a separate header file in include/linux, which can be exported to the user space.
--- Documentation/power/userland-swsusp.txt | 11 ++++------ include/linux/suspend_ioctls.h | 33 ++++++++++++++++++++++++++++++++ kernel/power/power.h | 29 ---------------------------- 3 files changed, 39 insertions(+), 34 deletions(-)
include/linux/suspend_ioctls.h new file +33 -0
@@ -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 */
kernel/power/power.h +1 -28
@@ -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 snextern 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;
Documentation/power/userland-swsusp.txt +5 -6
@@ -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)
Re: [PATCH -mm 5/5] Hibernation: Introduce exportable suspend ioctls header
Quoted 7 lines
From: Rafael J. Wysocki <[email protected]>
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 <[email protected]>
ACK, but this depends on previous patches that need fixes. Perhaps this should be moved to the begging of series?
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH -mm 5/5] Hibernation: Introduce exportable suspend ioctls header
Quoted 10 lines · Pavel Machek
Quoted 7 lines
From: Rafael J. Wysocki <[email protected]>
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 <[email protected]>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
Re: [PATCH -mm 5/5] Hibernation: Introduce exportable suspend ioctls header
In reply to Rafael J. WysockiQuoted 4 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
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
Re: [PATCH -mm 5/5] Hibernation: Introduce exportable suspend ioctls header
linux/Kbuild
Quoted 11 lines · Johannes Berg
Quoted 4 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
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:
linux/Kbuild +1
-0+header-y += suspend_ioctls.h
Good point, thanks.
Greetings,
Rafael