[RFC] PM: Add PM_RESUME_PREPARE and PM_POST_RESUME notifiers

[RFC] PM: Add PM_RESUME_PREPARE and PM_POST_RESUME notifiers

[Alan Stern]

Rafael:

How does this patch look?  Any reason not to have the resume notifiers?  
After all, drivers need to know when an image is about to be restored 
just as much as they need to know when one is about to be created.

Assuming this is okay, to whom should I submit it?  There will be a
couple of follow-up patches, one adding the icebox and one converting
the USB threads to use the icebox instead of the freezer.  This sort of
cross-subsystem stuff is difficult to coordinate unless one person
handles everything.

Alan Stern



Index: usb-2.6/Documentation/power/notifiers.txt
===================================================================
--- usb-2.6.orig/Documentation/power/notifiers.txt
+++ usb-2.6/Documentation/power/notifiers.txt
@@ -28,6 +28,14 @@ PM_POST_HIBERNATION	The system memory st
 			hibernation.  Device drivers' .resume() callbacks have
 			been executed and tasks have been thawed.
 
+PM_RESUME_PREPARE	The system is going to restore a hibernation image.
+			If all goes well the restored kernel will issue a
+			PM_POST_HIBERNATION notification.
+
+PM_POST_RESUME		An error occurred during the hibernation resume.
+			Device drivers' .resume() callbacks have been executed
+			and tasks have been thawed.
+
 PM_SUSPEND_PREPARE	The system is preparing for a suspend.
 
 PM_POST_SUSPEND		The system has just resumed or an error occured during
Index: usb-2.6/include/linux/notifier.h
===================================================================
--- usb-2.6.orig/include/linux/notifier.h
+++ usb-2.6/include/linux/notifier.h
@@ -230,6 +230,8 @@ static inline int notifier_to_errno(int 
 #define PM_POST_HIBERNATION	0x0002 /* Hibernation finished */
 #define PM_SUSPEND_PREPARE	0x0003 /* Going to suspend the system */
 #define PM_POST_SUSPEND		0x0004 /* Suspend finished */
+#define PM_RESUME_PREPARE	0x0005 /* Going to resume from hibernation */
+#define PM_POST_RESUME		0x0006 /* Resume failed */
 
 /* Console keyboard events.
  * Note: KBD_KEYCODE is always sent before KBD_UNBOUND_KEYCODE, KBD_UNICODE and
Index: usb-2.6/kernel/power/disk.c
===================================================================
--- usb-2.6.orig/kernel/power/disk.c
+++ usb-2.6/kernel/power/disk.c
@@ -489,6 +489,10 @@ static int software_resume(void)
 		goto Unlock;
 	}
 
+	error = pm_notifier_call_chain(PM_RESUME_PREPARE);
+	if (error)
+		goto Exit;
+
 	error = create_basic_memory_bitmaps();
 	if (error)
 		goto Finish;
@@ -512,6 +516,8 @@ static int software_resume(void)
  Done:
 	free_basic_memory_bitmaps();
  Finish:
+	pm_notifier_call_chain(PM_POST_RESUME);
+ Exit:
 	atomic_inc(&snapshot_device_available);
 	/* For success case, the suspend path will release the lock */
  Unlock:

Re: [RFC] PM: Add PM_RESUME_PREPARE and PM_POST_RESUME notifiers

[Rafael J. Wysocki]

On Monday, 29 October 2007 22:30, Alan Stern wrote:
> Rafael:
> 
> How does this patch look?  Any reason not to have the resume notifiers?  

Yes.

The userland interface already uses PM_HIBERNATION_PREPARE and
PM_POST_HIBERNATION for restore too, so if anything, we should call these
from software_resume() either.

As a rule of thumb, if you're going to change kernel/power/disk.c, have a look
at kernel/power/user.c and see if analogous changes are needed in there.

> After all, drivers need to know when an image is about to be restored 
> just as much as they need to know when one is about to be created.
> 
> Assuming this is okay, to whom should I submit it?

Well, it isn't, but as far as the patch flow is concerned, please post suspend
patches to linux-pm and I'll forward them to Len Brown.  Next, they will go
through the suspend branch of the ACPI tree.

This doesn't apply to patches that change drivers/base/power and should go to
Greg, but I can forward them to him too.

> There will be a couple of follow-up patches, one adding the icebox and one
> converting the USB threads to use the icebox instead of the freezer.  This
> sort of cross-subsystem stuff is difficult to coordinate unless one person
> handles everything.

The overall idea is to collect patches in the suspend branch of the ACPI tree,
unless that is against some higher priority rules.

Greetings,
Rafael

Re: [RFC] PM: Add PM_RESUME_PREPARE and PM_POST_RESUME notifiers

[Alan Stern]

On Mon, 29 Oct 2007, Rafael J. Wysocki wrote:

> On Monday, 29 October 2007 22:30, Alan Stern wrote:
> > Rafael:
> > 
> > How does this patch look?  Any reason not to have the resume notifiers?  
> 
> Yes.
> 
> The userland interface already uses PM_HIBERNATION_PREPARE and
> PM_POST_HIBERNATION for restore too, so if anything, we should call these
> from software_resume() either.

I could use those same notifiers, but are you sure that's a good idea?  
Drivers might want to do different things at the beginning of
hibernation and the beginning of a restore.

Alternatively, the user interface can be changed.  The current 
organization is slightly illogical; there should be different ioctls 
for prepare-to-create-snapshot and prepare-to-restore-snapshot instead 
of a single SNAPSHOT_FREEZE for both.  How about adding RESTORE_FREEZE 
and RESTORE_UNFREEZE; does this sound good?

Alan Stern

P.S.: While I'm updating things, the thought occurs that 
PM_RESTORE_PREPARE and PM_POST_RESTORE would be better names than 
PM_RESUME_PREPARE and PM_POST_RESUME.

Re: [RFC] PM: Add PM_RESUME_PREPARE and PM_POST_RESUME notifiers

[Rafael J. Wysocki]

On Tuesday, 30 October 2007 15:52, Alan Stern wrote:
> On Mon, 29 Oct 2007, Rafael J. Wysocki wrote:
> 
> > On Monday, 29 October 2007 22:30, Alan Stern wrote:
> > > Rafael:
> > > 
> > > How does this patch look?  Any reason not to have the resume notifiers?  
> > 
> > Yes.
> > 
> > The userland interface already uses PM_HIBERNATION_PREPARE and
> > PM_POST_HIBERNATION for restore too, so if anything, we should call these
> > from software_resume() either.
> 
> I could use those same notifiers, but are you sure that's a good idea?  

I'm not sure, but also I have no counter examples.

> Drivers might want to do different things at the beginning of
> hibernation and the beginning of a restore.
> 
> Alternatively, the user interface can be changed.  The current 
> organization is slightly illogical; there should be different ioctls 
> for prepare-to-create-snapshot and prepare-to-restore-snapshot instead 
> of a single SNAPSHOT_FREEZE for both.  How about adding RESTORE_FREEZE 
> and RESTORE_UNFREEZE; does this sound good?

Hm, we could define separate FREEZE ioctls for restore, but if they end up
doing the same as the analogous snapshot ones, they'll be somewhat redundant
...

> Alan Stern
> 
> P.S.: While I'm updating things, the thought occurs that 
> PM_RESTORE_PREPARE and PM_POST_RESTORE would be better names than 
> PM_RESUME_PREPARE and PM_POST_RESUME.

Agreed.

Greetings,
Rafael

Re: [RFC] PM: Add PM_RESUME_PREPARE and PM_POST_RESUME notifiers

[Alan Stern]

On Tue, 30 Oct 2007, Rafael J. Wysocki wrote:

> > Drivers might want to do different things at the beginning of
> > hibernation and the beginning of a restore.
> > 
> > Alternatively, the user interface can be changed.  The current 
> > organization is slightly illogical; there should be different ioctls 
> > for prepare-to-create-snapshot and prepare-to-restore-snapshot instead 
> > of a single SNAPSHOT_FREEZE for both.  How about adding RESTORE_FREEZE 
> > and RESTORE_UNFREEZE; does this sound good?
> 
> Hm, we could define separate FREEZE ioctls for restore, but if they end up
> doing the same as the analogous snapshot ones, they'll be somewhat redundant
> ...

How do you like this version of the patch then?  Redundancy is kept to 
a minimum.

(Strictly speaking, we should have two different notification codes for 
PM_POST_HIBERNATION: one for use after the atomic snapshot has been 
created and one for use after it has been restored.  But I'm not going 
to worry about that right now.)

Alan Stern



Index: usb-2.6/Documentation/power/notifiers.txt
===================================================================
--- usb-2.6.orig/Documentation/power/notifiers.txt
+++ usb-2.6/Documentation/power/notifiers.txt
@@ -28,6 +28,14 @@ PM_POST_HIBERNATION	The system memory st
 			hibernation.  Device drivers' .resume() callbacks have
 			been executed and tasks have been thawed.
 
+PM_RESTORE_PREPARE	The system is going to restore a hibernation image.
+			If all goes well the restored kernel will issue a
+			PM_POST_HIBERNATION notification.
+
+PM_POST_RESTORE		An error occurred during the hibernation restore.
+			Device drivers' .resume() callbacks have been executed
+			and tasks have been thawed.
+
 PM_SUSPEND_PREPARE	The system is preparing for a suspend.
 
 PM_POST_SUSPEND		The system has just resumed or an error occured during
Index: usb-2.6/include/linux/notifier.h
===================================================================
--- usb-2.6.orig/include/linux/notifier.h
+++ usb-2.6/include/linux/notifier.h
@@ -230,6 +230,8 @@ static inline int notifier_to_errno(int 
 #define PM_POST_HIBERNATION	0x0002 /* Hibernation finished */
 #define PM_SUSPEND_PREPARE	0x0003 /* Going to suspend the system */
 #define PM_POST_SUSPEND		0x0004 /* Suspend finished */
+#define PM_RESTORE_PREPARE	0x0005 /* Going to restore a saved image */
+#define PM_POST_RESTORE		0x0006 /* Restore failed */
 
 /* Console keyboard events.
  * Note: KBD_KEYCODE is always sent before KBD_UNBOUND_KEYCODE, KBD_UNICODE and
Index: usb-2.6/kernel/power/disk.c
===================================================================
--- usb-2.6.orig/kernel/power/disk.c
+++ usb-2.6/kernel/power/disk.c
@@ -489,6 +489,10 @@ static int software_resume(void)
 		goto Unlock;
 	}
 
+	error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
+	if (error)
+		goto Exit;
+
 	error = create_basic_memory_bitmaps();
 	if (error)
 		goto Finish;
@@ -512,6 +516,8 @@ static int software_resume(void)
  Done:
 	free_basic_memory_bitmaps();
  Finish:
+	pm_notifier_call_chain(PM_POST_RESTORE);
+ Exit:
 	atomic_inc(&snapshot_device_available);
 	/* For success case, the suspend path will release the lock */
  Unlock:
Index: usb-2.6/Documentation/power/userland-swsusp.txt
===================================================================
--- usb-2.6.orig/Documentation/power/userland-swsusp.txt
+++ usb-2.6/Documentation/power/userland-swsusp.txt
@@ -26,11 +26,12 @@ 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
-	and SNAPSHOT_ATOMIC_RESTORE to succeed
+SNAPSHOT_FREEZE - freeze user space processes in preparation for creating
+	an atomic snapshot (the current process is not frozen); this is
+	required for SNAPSHOT_ATOMIC_SNAPSHOT to succeed
 
 SNAPSHOT_UNFREEZE - thaw user space processes frozen by SNAPSHOT_FREEZE
+	after the snapshot has been created or restored
 
 SNAPSHOT_ATOMIC_SNAPSHOT - create a snapshot of the system memory; the
 	last argument of ioctl() should be a pointer to an int variable,
@@ -41,6 +42,15 @@ SNAPSHOT_ATOMIC_SNAPSHOT - create a snap
 	has been created the read() operation can be used to transfer
 	it out of the kernel
 
+SNAPSHOT_RESTORE_FREEZE - freeze user space processes in preparation for
+	restoring an atomic snapshot (the current pocess is not frozen);
+	this is required for SNAPSHOT_ATOMIC_RESTORE to succeed
+
+SNAPSHOT_RESTORE_UNFREEZE - thaw user space processes frozen by
+	SNAPSHOT_RESTORE_FREEZE; this should be used when
+	SNAPSHOT_ATOMIC_RESTORE fails (if it succeeds the current process
+	won't exist any more)
+
 SNAPSHOT_ATOMIC_RESTORE - restore the system memory state from the
 	uploaded snapshot image; before calling it you should transfer
 	the system memory snapshot back to the kernel using the write()
@@ -157,7 +167,8 @@ mechanism and the userland utilities usi
 means, such as checksums, to ensure the integrity of the snapshot image.
 
 The suspending and resuming utilities MUST lock themselves in memory,
-preferrably using mlockall(), before calling SNAPSHOT_FREEZE.
+preferrably using mlockall(), before calling SNAPSHOT_FREEZE or
+SNAPSHOT_RESTORE_FREEZE.
 
 The suspending utility MUST check the value stored by SNAPSHOT_ATOMIC_SNAPSHOT
 in the memory location pointed to by the last argument of ioctl() and proceed
Index: usb-2.6/kernel/power/power.h
===================================================================
--- usb-2.6.orig/kernel/power/power.h
+++ usb-2.6/kernel/power/power.h
@@ -160,7 +160,9 @@ 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_RESTORE_FREEZE		_IO(SNAPSHOT_IOC_MAGIC, 14)
+#define SNAPSHOT_RESTORE_UNFREEZE	_IO(SNAPSHOT_IOC_MAGIC, 15)
+#define SNAPSHOT_IOC_MAXNR	15
 
 #define PMOPS_PREPARE	1
 #define PMOPS_ENTER	2
Index: usb-2.6/kernel/power/user.c
===================================================================
--- usb-2.6.orig/kernel/power/user.c
+++ usb-2.6/kernel/power/user.c
@@ -128,6 +128,42 @@ static ssize_t snapshot_write(struct fil
 	return res;
 }
 
+static int user_freeze(struct snapshot_data *data, int pre_msg, int err_msg)
+{
+	int error = 0;
+
+	if (data->frozen)
+		return error;
+	mutex_lock(&pm_mutex);
+	error = pm_notifier_call_chain(pre_msg);
+	if (!error) {
+		printk("Syncing filesystems ... ");
+		sys_sync();
+		printk("done.\n");
+
+		error = freeze_processes();
+		if (error)
+			thaw_processes();
+	}
+	if (error)
+		pm_notifier_call_chain(err_msg);
+	mutex_unlock(&pm_mutex);
+	if (!error)
+		data->frozen = 1;
+	return error;
+}
+
+static void user_unfreeze(struct snapshot_data *data, int post_msg)
+{
+	if (!data->frozen || data->ready)
+		return;
+	mutex_lock(&pm_mutex);
+	thaw_processes();
+	pm_notifier_call_chain(post_msg);
+	mutex_unlock(&pm_mutex);
+	data->frozen = 0;
+}
+
 static int snapshot_ioctl(struct inode *inode, struct file *filp,
                           unsigned int cmd, unsigned long arg)
 {
@@ -148,34 +184,20 @@ static int snapshot_ioctl(struct inode *
 	switch (cmd) {
 
 	case SNAPSHOT_FREEZE:
-		if (data->frozen)
-			break;
-		mutex_lock(&pm_mutex);
-		error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
-		if (!error) {
-			printk("Syncing filesystems ... ");
-			sys_sync();
-			printk("done.\n");
-
-			error = freeze_processes();
-			if (error)
-				thaw_processes();
-		}
-		if (error)
-			pm_notifier_call_chain(PM_POST_HIBERNATION);
-		mutex_unlock(&pm_mutex);
-		if (!error)
-			data->frozen = 1;
+		error = user_freeze(data, PM_HIBERNATION_PREPARE,
+				PM_POST_HIBERNATION);
+		break;
+
+	case SNAPSHOT_RESTORE_FREEZE:
+		error = user_freeze(data, PM_RESTORE_PREPARE, PM_POST_RESTORE);
 		break;
 
 	case SNAPSHOT_UNFREEZE:
-		if (!data->frozen || data->ready)
-			break;
-		mutex_lock(&pm_mutex);
-		thaw_processes();
-		pm_notifier_call_chain(PM_POST_HIBERNATION);
-		mutex_unlock(&pm_mutex);
-		data->frozen = 0;
+		user_unfreeze(data, PM_POST_HIBERNATION);
+		break;
+
+	case SNAPSHOT_RESTORE_UNFREEZE:
+		user_unfreeze(data, PM_POST_RESTORE);
 		break;
 
 	case SNAPSHOT_ATOMIC_SNAPSHOT:

Re: [RFC] PM: Add PM_RESUME_PREPARE and PM_POST_RESUME notifiers

[Rafael J. Wysocki]

On Wednesday, 31 October 2007 22:11, Alan Stern wrote:
> On Tue, 30 Oct 2007, Rafael J. Wysocki wrote:
> 
> > > Drivers might want to do different things at the beginning of
> > > hibernation and the beginning of a restore.
> > > 
> > > Alternatively, the user interface can be changed.  The current 
> > > organization is slightly illogical; there should be different ioctls 
> > > for prepare-to-create-snapshot and prepare-to-restore-snapshot instead 
> > > of a single SNAPSHOT_FREEZE for both.  How about adding RESTORE_FREEZE 
> > > and RESTORE_UNFREEZE; does this sound good?
> > 
> > Hm, we could define separate FREEZE ioctls for restore, but if they end up
> > doing the same as the analogous snapshot ones, they'll be somewhat redundant
> > ...
> 
> How do you like this version of the patch then?  Redundancy is kept to 
> a minimum.
> 
> (Strictly speaking, we should have two different notification codes for 
> PM_POST_HIBERNATION: one for use after the atomic snapshot has been 
> created and one for use after it has been restored.  But I'm not going 
> to worry about that right now.)

IIRC, the notifiers are not called after creating the image.  Do you think that
they should be called at that time?  If so, then why?

As far as the patch is concerned, I'd prefer not to add new ioctls(), mainly
because there's userland out there that doesn't know about them and we can't
make people switch overnight.  I'd prever to move the notifier calls to
snapshot_open() and snapshot_release().

Greetings,
Rafael

Re: [RFC] PM: Add PM_RESUME_PREPARE and PM_POST_RESUME notifiers

[Alan Stern]

On Wed, 31 Oct 2007, Rafael J. Wysocki wrote:

> > (Strictly speaking, we should have two different notification codes for 
> > PM_POST_HIBERNATION: one for use after the atomic snapshot has been 
> > created and one for use after it has been restored.  But I'm not going 
> > to worry about that right now.)
> 
> IIRC, the notifiers are not called after creating the image.  Do you think that
> they should be called at that time?  If so, then why?

My mistake.  It's easy to forget that even though devices get resumed 
for writing after the image is created, tasks don't get thawed.  And 
since tasks don't get thawed, the notifiers shouldn't be called.

> As far as the patch is concerned, I'd prefer not to add new ioctls(), mainly
> because there's userland out there that doesn't know about them and we can't
> make people switch overnight.  I'd prever to move the notifier calls to
> snapshot_open() and snapshot_release().

Okay, here's a patch to do things that way.

Alan Stern



Index: usb-2.6/Documentation/power/notifiers.txt
===================================================================
--- usb-2.6.orig/Documentation/power/notifiers.txt
+++ usb-2.6/Documentation/power/notifiers.txt
@@ -28,6 +28,14 @@ PM_POST_HIBERNATION	The system memory st
 			hibernation.  Device drivers' .resume() callbacks have
 			been executed and tasks have been thawed.
 
+PM_RESTORE_PREPARE	The system is going to restore a hibernation image.
+			If all goes well the restored kernel will issue a
+			PM_POST_HIBERNATION notification.
+
+PM_POST_RESTORE		An error occurred during the hibernation restore.
+			Device drivers' .resume() callbacks have been executed
+			and tasks have been thawed.
+
 PM_SUSPEND_PREPARE	The system is preparing for a suspend.
 
 PM_POST_SUSPEND		The system has just resumed or an error occured during
Index: usb-2.6/include/linux/notifier.h
===================================================================
--- usb-2.6.orig/include/linux/notifier.h
+++ usb-2.6/include/linux/notifier.h
@@ -230,6 +230,8 @@ static inline int notifier_to_errno(int 
 #define PM_POST_HIBERNATION	0x0002 /* Hibernation finished */
 #define PM_SUSPEND_PREPARE	0x0003 /* Going to suspend the system */
 #define PM_POST_SUSPEND		0x0004 /* Suspend finished */
+#define PM_RESTORE_PREPARE	0x0005 /* Going to restore a saved image */
+#define PM_POST_RESTORE		0x0006 /* Restore failed */
 
 /* Console keyboard events.
  * Note: KBD_KEYCODE is always sent before KBD_UNBOUND_KEYCODE, KBD_UNICODE and
Index: usb-2.6/kernel/power/disk.c
===================================================================
--- usb-2.6.orig/kernel/power/disk.c
+++ usb-2.6/kernel/power/disk.c
@@ -489,6 +489,10 @@ static int software_resume(void)
 		goto Unlock;
 	}
 
+	error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
+	if (error)
+		goto Exit;
+
 	error = create_basic_memory_bitmaps();
 	if (error)
 		goto Finish;
@@ -512,6 +516,8 @@ static int software_resume(void)
  Done:
 	free_basic_memory_bitmaps();
  Finish:
+	pm_notifier_call_chain(PM_POST_RESTORE);
+ Exit:
 	atomic_inc(&snapshot_device_available);
 	/* For success case, the suspend path will release the lock */
  Unlock:
Index: usb-2.6/kernel/power/user.c
===================================================================
--- usb-2.6.orig/kernel/power/user.c
+++ usb-2.6/kernel/power/user.c
@@ -44,6 +44,7 @@ atomic_t snapshot_device_available = ATO
 static int snapshot_open(struct inode *inode, struct file *filp)
 {
 	struct snapshot_data *data;
+	int error;
 
 	if (!atomic_add_unless(&snapshot_device_available, -1, 0))
 		return -EBUSY;
@@ -64,9 +65,15 @@ static int snapshot_open(struct inode *i
 		data->swap = swsusp_resume_device ?
 			swap_type_of(swsusp_resume_device, 0, NULL) : -1;
 		data->mode = O_RDONLY;
+		error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
 	} else {
 		data->swap = -1;
 		data->mode = O_WRONLY;
+		error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
+	}
+	if (error) {
+		atomic_inc(&snapshot_device_available);
+		return error;
 	}
 	data->frozen = 0;
 	data->ready = 0;
@@ -88,6 +95,8 @@ static int snapshot_release(struct inode
 		thaw_processes();
 		mutex_unlock(&pm_mutex);
 	}
+	pm_notifier_call_chain(data->mode == O_WRONLY ?
+			PM_POST_HIBERNATION : PM_POST_RESTORE);
 	atomic_inc(&snapshot_device_available);
 	return 0;
 }
@@ -151,18 +160,13 @@ static int snapshot_ioctl(struct inode *
 		if (data->frozen)
 			break;
 		mutex_lock(&pm_mutex);
-		error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
-		if (!error) {
-			printk("Syncing filesystems ... ");
-			sys_sync();
-			printk("done.\n");
-
-			error = freeze_processes();
-			if (error)
-				thaw_processes();
-		}
+		printk("Syncing filesystems ... ");
+		sys_sync();
+		printk("done.\n");
+
+		error = freeze_processes();
 		if (error)
-			pm_notifier_call_chain(PM_POST_HIBERNATION);
+			thaw_processes();
 		mutex_unlock(&pm_mutex);
 		if (!error)
 			data->frozen = 1;
@@ -173,7 +177,6 @@ static int snapshot_ioctl(struct inode *
 			break;
 		mutex_lock(&pm_mutex);
 		thaw_processes();
-		pm_notifier_call_chain(PM_POST_HIBERNATION);
 		mutex_unlock(&pm_mutex);
 		data->frozen = 0;
 		break;

Re: [RFC] PM: Add PM_RESUME_PREPARE and PM_POST_RESUME notifiers

[Rafael J. Wysocki]

On Thursday, 1 November 2007 15:40, Alan Stern wrote:
> On Wed, 31 Oct 2007, Rafael J. Wysocki wrote:
> 
> > > (Strictly speaking, we should have two different notification codes for 
> > > PM_POST_HIBERNATION: one for use after the atomic snapshot has been 
> > > created and one for use after it has been restored.  But I'm not going 
> > > to worry about that right now.)
> > 
> > IIRC, the notifiers are not called after creating the image.  Do you think that
> > they should be called at that time?  If so, then why?
> 
> My mistake.  It's easy to forget that even though devices get resumed 
> for writing after the image is created, tasks don't get thawed.  And 
> since tasks don't get thawed, the notifiers shouldn't be called.
> 
> > As far as the patch is concerned, I'd prefer not to add new ioctls(), mainly
> > because there's userland out there that doesn't know about them and we can't
> > make people switch overnight.  I'd prever to move the notifier calls to
> > snapshot_open() and snapshot_release().
> 
> Okay, here's a patch to do things that way.

Thanks.  [I was going to write that myself, but you were faster. :-)]

The patch looks good.

Please sign it off and provide a changelog and I'll forward it to Len.

Greetings,
Rafael


> Index: usb-2.6/Documentation/power/notifiers.txt
> ===================================================================
> --- usb-2.6.orig/Documentation/power/notifiers.txt
> +++ usb-2.6/Documentation/power/notifiers.txt
> @@ -28,6 +28,14 @@ PM_POST_HIBERNATION	The system memory st
>  			hibernation.  Device drivers' .resume() callbacks have
>  			been executed and tasks have been thawed.
>  
> +PM_RESTORE_PREPARE	The system is going to restore a hibernation image.
> +			If all goes well the restored kernel will issue a
> +			PM_POST_HIBERNATION notification.
> +
> +PM_POST_RESTORE		An error occurred during the hibernation restore.
> +			Device drivers' .resume() callbacks have been executed
> +			and tasks have been thawed.
> +
>  PM_SUSPEND_PREPARE	The system is preparing for a suspend.
>  
>  PM_POST_SUSPEND		The system has just resumed or an error occured during
> Index: usb-2.6/include/linux/notifier.h
> ===================================================================
> --- usb-2.6.orig/include/linux/notifier.h
> +++ usb-2.6/include/linux/notifier.h
> @@ -230,6 +230,8 @@ static inline int notifier_to_errno(int 
>  #define PM_POST_HIBERNATION	0x0002 /* Hibernation finished */
>  #define PM_SUSPEND_PREPARE	0x0003 /* Going to suspend the system */
>  #define PM_POST_SUSPEND		0x0004 /* Suspend finished */
> +#define PM_RESTORE_PREPARE	0x0005 /* Going to restore a saved image */
> +#define PM_POST_RESTORE		0x0006 /* Restore failed */
>  
>  /* Console keyboard events.
>   * Note: KBD_KEYCODE is always sent before KBD_UNBOUND_KEYCODE, KBD_UNICODE and
> Index: usb-2.6/kernel/power/disk.c
> ===================================================================
> --- usb-2.6.orig/kernel/power/disk.c
> +++ usb-2.6/kernel/power/disk.c
> @@ -489,6 +489,10 @@ static int software_resume(void)
>  		goto Unlock;
>  	}
>  
> +	error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
> +	if (error)
> +		goto Exit;
> +
>  	error = create_basic_memory_bitmaps();
>  	if (error)
>  		goto Finish;
> @@ -512,6 +516,8 @@ static int software_resume(void)
>   Done:
>  	free_basic_memory_bitmaps();
>   Finish:
> +	pm_notifier_call_chain(PM_POST_RESTORE);
> + Exit:
>  	atomic_inc(&snapshot_device_available);
>  	/* For success case, the suspend path will release the lock */
>   Unlock:
> Index: usb-2.6/kernel/power/user.c
> ===================================================================
> --- usb-2.6.orig/kernel/power/user.c
> +++ usb-2.6/kernel/power/user.c
> @@ -44,6 +44,7 @@ atomic_t snapshot_device_available = ATO
>  static int snapshot_open(struct inode *inode, struct file *filp)
>  {
>  	struct snapshot_data *data;
> +	int error;
>  
>  	if (!atomic_add_unless(&snapshot_device_available, -1, 0))
>  		return -EBUSY;
> @@ -64,9 +65,15 @@ static int snapshot_open(struct inode *i
>  		data->swap = swsusp_resume_device ?
>  			swap_type_of(swsusp_resume_device, 0, NULL) : -1;
>  		data->mode = O_RDONLY;
> +		error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
>  	} else {
>  		data->swap = -1;
>  		data->mode = O_WRONLY;
> +		error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
> +	}
> +	if (error) {
> +		atomic_inc(&snapshot_device_available);
> +		return error;
>  	}
>  	data->frozen = 0;
>  	data->ready = 0;
> @@ -88,6 +95,8 @@ static int snapshot_release(struct inode
>  		thaw_processes();
>  		mutex_unlock(&pm_mutex);
>  	}
> +	pm_notifier_call_chain(data->mode == O_WRONLY ?
> +			PM_POST_HIBERNATION : PM_POST_RESTORE);
>  	atomic_inc(&snapshot_device_available);
>  	return 0;
>  }
> @@ -151,18 +160,13 @@ static int snapshot_ioctl(struct inode *
>  		if (data->frozen)
>  			break;
>  		mutex_lock(&pm_mutex);
> -		error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
> -		if (!error) {
> -			printk("Syncing filesystems ... ");
> -			sys_sync();
> -			printk("done.\n");
> -
> -			error = freeze_processes();
> -			if (error)
> -				thaw_processes();
> -		}
> +		printk("Syncing filesystems ... ");
> +		sys_sync();
> +		printk("done.\n");
> +
> +		error = freeze_processes();
>  		if (error)
> -			pm_notifier_call_chain(PM_POST_HIBERNATION);
> +			thaw_processes();
>  		mutex_unlock(&pm_mutex);
>  		if (!error)
>  			data->frozen = 1;
> @@ -173,7 +177,6 @@ static int snapshot_ioctl(struct inode *
>  			break;
>  		mutex_lock(&pm_mutex);
>  		thaw_processes();
> -		pm_notifier_call_chain(PM_POST_HIBERNATION);
>  		mutex_unlock(&pm_mutex);
>  		data->frozen = 0;
>  		break;

[PATCH] PM: Add PM_RESTORE_PREPARE and PM_POST_RESTORE notifiers

[Alan Stern]

This patch (as1006) adds PM_RESTORE_PREPARE and PM_POST_RESTORE
notifiers to the PM core, to be used in analogy with the existing
PM_HIBERNATION_PREPARE and PM_POST_HIBERNATION notifiers.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>

---

Index: usb-2.6/Documentation/power/notifiers.txt
===================================================================
--- usb-2.6.orig/Documentation/power/notifiers.txt
+++ usb-2.6/Documentation/power/notifiers.txt
@@ -28,6 +28,14 @@ PM_POST_HIBERNATION	The system memory st
 			hibernation.  Device drivers' .resume() callbacks have
 			been executed and tasks have been thawed.
 
+PM_RESTORE_PREPARE	The system is going to restore a hibernation image.
+			If all goes well the restored kernel will issue a
+			PM_POST_HIBERNATION notification.
+
+PM_POST_RESTORE		An error occurred during the hibernation restore.
+			Device drivers' .resume() callbacks have been executed
+			and tasks have been thawed.
+
 PM_SUSPEND_PREPARE	The system is preparing for a suspend.
 
 PM_POST_SUSPEND		The system has just resumed or an error occured during
Index: usb-2.6/include/linux/notifier.h
===================================================================
--- usb-2.6.orig/include/linux/notifier.h
+++ usb-2.6/include/linux/notifier.h
@@ -230,6 +230,8 @@ static inline int notifier_to_errno(int 
 #define PM_POST_HIBERNATION	0x0002 /* Hibernation finished */
 #define PM_SUSPEND_PREPARE	0x0003 /* Going to suspend the system */
 #define PM_POST_SUSPEND		0x0004 /* Suspend finished */
+#define PM_RESTORE_PREPARE	0x0005 /* Going to restore a saved image */
+#define PM_POST_RESTORE		0x0006 /* Restore failed */
 
 /* Console keyboard events.
  * Note: KBD_KEYCODE is always sent before KBD_UNBOUND_KEYCODE, KBD_UNICODE and
Index: usb-2.6/kernel/power/disk.c
===================================================================
--- usb-2.6.orig/kernel/power/disk.c
+++ usb-2.6/kernel/power/disk.c
@@ -489,6 +489,10 @@ static int software_resume(void)
 		goto Unlock;
 	}
 
+	error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
+	if (error)
+		goto Exit;
+
 	error = create_basic_memory_bitmaps();
 	if (error)
 		goto Finish;
@@ -512,6 +516,8 @@ static int software_resume(void)
  Done:
 	free_basic_memory_bitmaps();
  Finish:
+	pm_notifier_call_chain(PM_POST_RESTORE);
+ Exit:
 	atomic_inc(&snapshot_device_available);
 	/* For success case, the suspend path will release the lock */
  Unlock:
Index: usb-2.6/kernel/power/user.c
===================================================================
--- usb-2.6.orig/kernel/power/user.c
+++ usb-2.6/kernel/power/user.c
@@ -44,6 +44,7 @@ atomic_t snapshot_device_available = ATO
 static int snapshot_open(struct inode *inode, struct file *filp)
 {
 	struct snapshot_data *data;
+	int error;
 
 	if (!atomic_add_unless(&snapshot_device_available, -1, 0))
 		return -EBUSY;
@@ -64,9 +65,15 @@ static int snapshot_open(struct inode *i
 		data->swap = swsusp_resume_device ?
 			swap_type_of(swsusp_resume_device, 0, NULL) : -1;
 		data->mode = O_RDONLY;
+		error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
 	} else {
 		data->swap = -1;
 		data->mode = O_WRONLY;
+		error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
+	}
+	if (error) {
+		atomic_inc(&snapshot_device_available);
+		return error;
 	}
 	data->frozen = 0;
 	data->ready = 0;
@@ -88,6 +95,8 @@ static int snapshot_release(struct inode
 		thaw_processes();
 		mutex_unlock(&pm_mutex);
 	}
+	pm_notifier_call_chain(data->mode == O_WRONLY ?
+			PM_POST_HIBERNATION : PM_POST_RESTORE);
 	atomic_inc(&snapshot_device_available);
 	return 0;
 }
@@ -151,18 +160,13 @@ static int snapshot_ioctl(struct inode *
 		if (data->frozen)
 			break;
 		mutex_lock(&pm_mutex);
-		error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
-		if (!error) {
-			printk("Syncing filesystems ... ");
-			sys_sync();
-			printk("done.\n");
-
-			error = freeze_processes();
-			if (error)
-				thaw_processes();
-		}
+		printk("Syncing filesystems ... ");
+		sys_sync();
+		printk("done.\n");
+
+		error = freeze_processes();
 		if (error)
-			pm_notifier_call_chain(PM_POST_HIBERNATION);
+			thaw_processes();
 		mutex_unlock(&pm_mutex);
 		if (!error)
 			data->frozen = 1;
@@ -173,7 +177,6 @@ static int snapshot_ioctl(struct inode *
 			break;
 		mutex_lock(&pm_mutex);
 		thaw_processes();
-		pm_notifier_call_chain(PM_POST_HIBERNATION);
 		mutex_unlock(&pm_mutex);
 		data->frozen = 0;
 		break;