[PATCH -mm] PM: Change ordering of suspend and resume code

[PATCH -mm] PM: Change ordering of suspend and resume code

[Rafael J. Wysocki]

Hi,

As indicated in a recent thread on Linux-PM, it's necessary to call
pm_ops->finish() before devce_resume(), but enable_nonboot_cpus() has to be
called before pm_ops->finish()
(cf. http://lists.osdl.org/pipermail/linux-pm/2006-November/004164.html).
For consistency, it seems reasonable to call disable_nonboot_cpus() after
device_suspend().

This way the suspend code will remain symmetrical with respect to the resume
code and it may allow us to speed up things in the future by suspending and
resuming devices and/or saving the suspend image in many threads.

The following series of patches reorders the suspend and resume code so that
nonboot CPUs are disabled after devices have been suspended and enabled before
the devices are resumed.  It also causes pm_ops->finish() to be called after
enable_nonboot_cpus() wherever necessary.

The first patch changes the ordering of the suspend-to-RAM code and is
untested, because my boxes continue refusing to resume from RAM for other
reasons.  If anyone can, please do me a favour and test it.

The second patch changes the ordering of the built-in suspend-to-disk code,
and the last two patches modify the swsusp userland interface code
accordingly.  As far as the last patch is concerned, I've decided to change
the existing ioctls, so that the patched kernel works with the "old" userland.
I don't think this change breaks anything of importance, but if it does, we
can just add (yet) another ioctl to set the platform mode, although in that
case the userland would have to be changed as well.

All of the patches are against 2.6.20-rc1 and should not be used with any
earlier kernel.

Greetings,
Rafael


-- 
If you don't have the time to read,
you don't have the time or the tools to write.
		- Stephen King


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[PATCH -mm 1/4] PM: Change code ordering in main.c

[Rafael J. Wysocki]

Change the ordering of code in kernel/power/main.c so that device_suspend()
is called before disable_nonboot_cpus() and pm_ops->finish() is called after
enable_nonboot_cpus() and before device_resume(), as indicated by recent
discussion on Linux-PM
(cf. http://lists.osdl.org/pipermail/linux-pm/2006-November/004164.html).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/power/main.c |   34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

Index: linux-2.6.20-rc1/kernel/power/main.c
===================================================================
--- linux-2.6.20-rc1.orig/kernel/power/main.c	2006-12-14 22:35:52.000000000 +0100
+++ linux-2.6.20-rc1/kernel/power/main.c	2006-12-14 23:19:32.000000000 +0100
@@ -43,6 +43,11 @@ void pm_set_ops(struct pm_ops * ops)
 	mutex_unlock(&pm_mutex);
 }
 
+static inline void pm_finish(suspend_state_t state)
+{
+	if (pm_ops->finish)
+		pm_ops->finish(state);
+}
 
 /**
  *	suspend_prepare - Do prep work before entering low-power state.
@@ -63,10 +68,6 @@ static int suspend_prepare(suspend_state
 
 	pm_prepare_console();
 
-	error = disable_nonboot_cpus();
-	if (error)
-		goto Enable_cpu;
-
 	if (freeze_processes()) {
 		error = -EAGAIN;
 		goto Thaw;
@@ -88,18 +89,22 @@ static int suspend_prepare(suspend_state
 	}
 
 	suspend_console();
-	if ((error = device_suspend(PMSG_SUSPEND))) {
+	error = device_suspend(PMSG_SUSPEND);
+	if (error) {
 		printk(KERN_ERR "Some devices failed to suspend\n");
-		goto Finish;
+		goto Resume_devices;
 	}
-	return 0;
- Finish:
-	if (pm_ops->finish)
-		pm_ops->finish(state);
+	error = disable_nonboot_cpus();
+	if (!error)
+		return 0;
+
+	enable_nonboot_cpus();
+ Resume_devices:
+	pm_finish(state);
+	device_resume();
+	resume_console();
  Thaw:
 	thaw_processes();
- Enable_cpu:
-	enable_nonboot_cpus();
 	pm_restore_console();
 	return error;
 }
@@ -134,12 +139,11 @@ int suspend_enter(suspend_state_t state)
 
 static void suspend_finish(suspend_state_t state)
 {
+	enable_nonboot_cpus();
+	pm_finish(state);
 	device_resume();
 	resume_console();
 	thaw_processes();
-	enable_nonboot_cpus();
-	if (pm_ops && pm_ops->finish)
-		pm_ops->finish(state);
 	pm_restore_console();
 }
 


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[PATCH -mm 2/4] swsusp: Change code ordering in disk.c

[Rafael J. Wysocki]

Change the ordering of code in kernel/power/disk.c so that
device_suspend() is called before disable_nonboot_cpus() and
platform_finish() is called after enable_nonboot_cpus() and before
device_resume(), as indicated by the recent discussion on Linux-PM
(cf. http://lists.osdl.org/pipermail/linux-pm/2006-November/004164.html).

The changes here only affect the built-in swsusp.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/cpu.c        |    2 
 kernel/power/disk.c |  110 ++++++++++++++++++++++++++--------------------------
 2 files changed, 57 insertions(+), 55 deletions(-)

Index: linux-2.6.20-rc1/kernel/cpu.c
===================================================================
--- linux-2.6.20-rc1.orig/kernel/cpu.c	2006-12-14 22:35:52.000000000 +0100
+++ linux-2.6.20-rc1/kernel/cpu.c	2006-12-14 23:19:49.000000000 +0100
@@ -309,6 +309,8 @@ void enable_nonboot_cpus(void)
 	mutex_lock(&cpu_add_remove_lock);
 	cpu_hotplug_disabled = 0;
 	mutex_unlock(&cpu_add_remove_lock);
+	if (cpus_empty(frozen_cpus))
+		return;
 
 	printk("Enabling non-boot CPUs ...\n");
 	for_each_cpu_mask(cpu, frozen_cpus) {
Index: linux-2.6.20-rc1/kernel/power/disk.c
===================================================================
--- linux-2.6.20-rc1.orig/kernel/power/disk.c	2006-12-14 22:35:52.000000000 +0100
+++ linux-2.6.20-rc1/kernel/power/disk.c	2006-12-14 23:19:49.000000000 +0100
@@ -85,52 +85,24 @@ static inline void platform_finish(void)
 	}
 }
 
+static void unprepare_processes(void)
+{
+	thaw_processes();
+	pm_restore_console();
+}
+
 static int prepare_processes(void)
 {
 	int error = 0;
 
 	pm_prepare_console();
-
-	error = disable_nonboot_cpus();
-	if (error)
-		goto enable_cpus;
-
 	if (freeze_processes()) {
 		error = -EBUSY;
-		goto thaw;
+		unprepare_processes();
 	}
-
-	if (pm_disk_mode == PM_DISK_TESTPROC) {
-		printk("swsusp debug: Waiting for 5 seconds.\n");
-		mdelay(5000);
-		goto thaw;
-	}
-
-	error = platform_prepare();
-	if (error)
-		goto thaw;
-
-	/* Free memory before shutting down devices. */
-	if (!(error = swsusp_shrink_memory()))
-		return 0;
-
-	platform_finish();
- thaw:
-	thaw_processes();
- enable_cpus:
-	enable_nonboot_cpus();
-	pm_restore_console();
 	return error;
 }
 
-static void unprepare_processes(void)
-{
-	platform_finish();
-	thaw_processes();
-	enable_nonboot_cpus();
-	pm_restore_console();
-}
-
 /**
  *	pm_suspend_disk - The granpappy of hibernation power management.
  *
@@ -148,29 +120,45 @@ int pm_suspend_disk(void)
 	if (error)
 		return error;
 
-	if (pm_disk_mode == PM_DISK_TESTPROC)
-		return 0;
+	if (pm_disk_mode == PM_DISK_TESTPROC) {
+		printk("swsusp debug: Waiting for 5 seconds.\n");
+		mdelay(5000);
+		goto Thaw;
+	}
+	/* Free memory before shutting down devices. */
+	error = swsusp_shrink_memory();
+	if (error)
+		goto Thaw;
+
+	error = platform_prepare();
+	if (error)
+		goto Thaw;
 
 	suspend_console();
 	error = device_suspend(PMSG_FREEZE);
 	if (error) {
-		resume_console();
-		printk("Some devices failed to suspend\n");
-		goto Thaw;
+		printk(KERN_ERR "PM: Some devices failed to suspend\n");
+		goto Resume_devices;
 	}
+	error = disable_nonboot_cpus();
+	if (error)
+		goto Enable_cpus;
 
 	if (pm_disk_mode == PM_DISK_TEST) {
 		printk("swsusp debug: Waiting for 5 seconds.\n");
 		mdelay(5000);
-		goto Done;
+		goto Enable_cpus;
 	}
 
 	pr_debug("PM: snapshotting memory.\n");
 	in_suspend = 1;
-	if ((error = swsusp_suspend()))
-		goto Done;
+	error = swsusp_suspend();
+	if (error)
+		goto Enable_cpus;
 
 	if (in_suspend) {
+		enable_nonboot_cpus();
+		platform_finish();
 		device_resume();
 		resume_console();
 		pr_debug("PM: writing image.\n");
@@ -186,7 +174,10 @@ int pm_suspend_disk(void)
 	}
 
 	swsusp_free();
- Done:
+ Enable_cpus:
+	enable_nonboot_cpus();
+ Resume_devices:
+	platform_finish();
 	device_resume();
 	resume_console();
  Thaw:
@@ -235,41 +226,50 @@ static int software_resume(void)
 
 	pr_debug("PM: Checking swsusp image.\n");
 
-	if ((error = swsusp_check()))
+	error = swsusp_check();
+	if (error)
 		goto Done;
 
 	pr_debug("PM: Preparing processes for restore.\n");
 
-	if ((error = prepare_processes())) {
+	error = prepare_processes();
+	if (error) {
 		swsusp_close();
 		goto Done;
 	}
 
 	pr_debug("PM: Reading swsusp image.\n");
 
-	if ((error = swsusp_read())) {
+	error = swsusp_read();
+	if (error) {
 		swsusp_free();
 		goto Thaw;
 	}
 
 	pr_debug("PM: Preparing devices for restore.\n");
 
-	suspend_console();
-	if ((error = device_suspend(PMSG_PRETHAW))) {
-		resume_console();
-		printk("Some devices failed to suspend\n");
+	error = platform_prepare();
+	if (error) {
 		swsusp_free();
 		goto Thaw;
 	}
+	suspend_console();
+	error = device_suspend(PMSG_PRETHAW);
+	if (error)
+		goto Free;
 
-	mb();
+	error = disable_nonboot_cpus();
+	if (!error)
+		swsusp_resume();
 
-	pr_debug("PM: Restoring saved image.\n");
-	swsusp_resume();
-	pr_debug("PM: Restore failed, recovering.n");
+	enable_nonboot_cpus();
+ Free:
+	swsusp_free();
+	platform_finish();
 	device_resume();
 	resume_console();
  Thaw:
+	printk(KERN_ERR "PM: Restore failed, recovering.\n");
 	unprepare_processes();
  Done:
 	/* For success case, the suspend path will release the lock */


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[PATCH -mm 3/4] swsusp: Change code ordering in user.c

[Rafael J. Wysocki]

Change the ordering of code in kernel/power/user.c so that device_suspend()
is called before disable_nonboot_cpus() and device_resume() is called after
enable_nonboot_cpus().  This is needed to make the userland suspend call
pm_ops->finish() after enable_nonboot_cpus() and before device_resume(),
as indicated by the recent discussion on Linux-PM
(cf. http://lists.osdl.org/pipermail/linux-pm/2006-November/004164.html).

The changes here only affect the userland interface of swsusp.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/power/user.c |   92 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 58 insertions(+), 34 deletions(-)

Index: linux-2.6.20-rc1/kernel/power/user.c
===================================================================
--- linux-2.6.20-rc1.orig/kernel/power/user.c	2006-12-14 22:35:52.000000000 +0100
+++ linux-2.6.20-rc1/kernel/power/user.c	2006-12-14 23:20:10.000000000 +0100
@@ -122,6 +122,59 @@ static ssize_t snapshot_write(struct fil
 	return res;
 }
 
+static inline int snapshot_suspend(void)
+{
+	int error;
+
+	mutex_lock(&pm_mutex);
+	/* Free memory before shutting down devices. */
+	error = swsusp_shrink_memory();
+	if (error)
+		goto Finish;
+
+	suspend_console();
+	error = device_suspend(PMSG_FREEZE);
+	if (error)
+		goto Resume_devices;
+
+	error = disable_nonboot_cpus();
+	if (!error) {
+		in_suspend = 1;
+		error = swsusp_suspend();
+	}
+	enable_nonboot_cpus();
+ Resume_devices:
+	device_resume();
+	resume_console();
+ Finish:
+	mutex_unlock(&pm_mutex);
+	return error;
+}
+
+static inline int snapshot_restore(void)
+{
+	int error;
+
+	mutex_lock(&pm_mutex);
+	pm_prepare_console();
+	suspend_console();
+	error = device_suspend(PMSG_PRETHAW);
+	if (error)
+		goto Resume_devices;
+
+	error = disable_nonboot_cpus();
+	if (!error)
+		error = swsusp_resume();
+
+	enable_nonboot_cpus();
+ Resume_devices:
+	device_resume();
+	resume_console();
+	pm_restore_console();
+	mutex_unlock(&pm_mutex);
+	return error;
+}
+
 static int snapshot_ioctl(struct inode *inode, struct file *filp,
                           unsigned int cmd, unsigned long arg)
 {
@@ -145,14 +198,9 @@ static int snapshot_ioctl(struct inode *
 		if (data->frozen)
 			break;
 		mutex_lock(&pm_mutex);
-		error = disable_nonboot_cpus();
-		if (!error) {
-			error = freeze_processes();
-			if (error) {
-				thaw_processes();
-				enable_nonboot_cpus();
-				error = -EBUSY;
-			}
+		if (freeze_processes()) {
+			thaw_processes();
+			error = -EBUSY;
 		}
 		mutex_unlock(&pm_mutex);
 		if (!error)
@@ -164,7 +212,6 @@ static int snapshot_ioctl(struct inode *
 			break;
 		mutex_lock(&pm_mutex);
 		thaw_processes();
-		enable_nonboot_cpus();
 		mutex_unlock(&pm_mutex);
 		data->frozen = 0;
 		break;
@@ -174,20 +221,7 @@ static int snapshot_ioctl(struct inode *
 			error = -EPERM;
 			break;
 		}
-		mutex_lock(&pm_mutex);
-		/* Free memory before shutting down devices. */
-		error = swsusp_shrink_memory();
-		if (!error) {
-			suspend_console();
-			error = device_suspend(PMSG_FREEZE);
-			if (!error) {
-				in_suspend = 1;
-				error = swsusp_suspend();
-				device_resume();
-			}
-			resume_console();
-		}
-		mutex_unlock(&pm_mutex);
+		error = snapshot_suspend();
 		if (!error)
 			error = put_user(in_suspend, (unsigned int __user *)arg);
 		if (!error)
@@ -201,17 +235,7 @@ static int snapshot_ioctl(struct inode *
 			error = -EPERM;
 			break;
 		}
-		mutex_lock(&pm_mutex);
-		pm_prepare_console();
-		suspend_console();
-		error = device_suspend(PMSG_PRETHAW);
-		if (!error) {
-			error = swsusp_resume();
-			device_resume();
-		}
-		resume_console();
-		pm_restore_console();
-		mutex_unlock(&pm_mutex);
+		error = snapshot_restore();
 		break;
 
 	case SNAPSHOT_FREE:


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[PATCH -mm 4/4] swsusp: Change pm_ops handling by userland interface

[Rafael J. Wysocki]

Make the userland interface of swsusp call pm_ops->finish() after
enable_nonboot_cpus() and before resume_device(),
as indicated by the recent discussion on Linux-PM
(cf. http://lists.osdl.org/pipermail/linux-pm/2006-November/004164.html).

This patch changes the SNAPSHOT_PMOPS ioctl so that its first function,
PMOPS_PREPARE, only sets a switch turning the platform suspend mode on, and
its last function, PMOPS_PREPARE, only checks if the platform mode is enabled.
This should allow the older userland tools to work with new kernels without
any modifications.

The changes here only affect the userland interface of swsusp.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/power/user.c |   64 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 51 insertions(+), 13 deletions(-)

Index: linux-2.6.20-rc1/kernel/power/user.c
===================================================================
--- linux-2.6.20-rc1.orig/kernel/power/user.c	2006-12-14 23:20:10.000000000 +0100
+++ linux-2.6.20-rc1/kernel/power/user.c	2006-12-14 23:20:18.000000000 +0100
@@ -37,6 +37,7 @@ static struct snapshot_data {
 	int mode;
 	char frozen;
 	char ready;
+	char platform_suspend;
 } snapshot_state;
 
 static atomic_t device_available = ATOMIC_INIT(1);
@@ -66,6 +67,7 @@ static int snapshot_open(struct inode *i
 	data->bitmap = NULL;
 	data->frozen = 0;
 	data->ready = 0;
+	data->platform_suspend = 0;
 
 	return 0;
 }
@@ -122,7 +124,23 @@ static ssize_t snapshot_write(struct fil
 	return res;
 }
 
-static inline int snapshot_suspend(void)
+static inline int platform_prepare(void)
+{
+	int error = 0;
+
+	if (pm_ops && pm_ops->prepare)
+		error = pm_ops->prepare(PM_SUSPEND_DISK);
+
+	return error;
+}
+
+static inline void platform_finish(void)
+{
+	if (pm_ops && pm_ops->finish)
+		pm_ops->finish(PM_SUSPEND_DISK);
+}
+
+static inline int snapshot_suspend(int platform_suspend)
 {
 	int error;
 
@@ -132,6 +150,11 @@ static inline int snapshot_suspend(void)
 	if (error)
 		goto Finish;
 
+	if (platform_suspend) {
+		error = platform_prepare();
+		if (error)
+			goto Finish;
+	}
 	suspend_console();
 	error = device_suspend(PMSG_FREEZE);
 	if (error)
@@ -144,6 +167,9 @@ static inline int snapshot_suspend(void)
 	}
 	enable_nonboot_cpus();
  Resume_devices:
+	if (platform_suspend)
+		platform_finish();
+
 	device_resume();
 	resume_console();
  Finish:
@@ -151,12 +177,17 @@ static inline int snapshot_suspend(void)
 	return error;
 }
 
-static inline int snapshot_restore(void)
+static inline int snapshot_restore(int platform_suspend)
 {
 	int error;
 
 	mutex_lock(&pm_mutex);
 	pm_prepare_console();
+	if (platform_suspend) {
+		error = platform_prepare();
+		if (error)
+			goto Finish;
+	}
 	suspend_console();
 	error = device_suspend(PMSG_PRETHAW);
 	if (error)
@@ -168,8 +199,12 @@ static inline int snapshot_restore(void)
 
 	enable_nonboot_cpus();
  Resume_devices:
+	if (platform_suspend)
+		platform_finish();
+
 	device_resume();
 	resume_console();
+ Finish:
 	pm_restore_console();
 	mutex_unlock(&pm_mutex);
 	return error;
@@ -221,7 +256,7 @@ static int snapshot_ioctl(struct inode *
 			error = -EPERM;
 			break;
 		}
-		error = snapshot_suspend();
+		error = snapshot_suspend(data->platform_suspend);
 		if (!error)
 			error = put_user(in_suspend, (unsigned int __user *)arg);
 		if (!error)
@@ -235,7 +270,7 @@ static int snapshot_ioctl(struct inode *
 			error = -EPERM;
 			break;
 		}
-		error = snapshot_restore();
+		error = snapshot_restore(data->platform_suspend);
 		break;
 
 	case SNAPSHOT_FREE:
@@ -342,28 +377,31 @@ static int snapshot_ioctl(struct inode *
 		break;
 
 	case SNAPSHOT_PMOPS:
+		error = -EINVAL;
+
 		switch (arg) {
 
 		case PMOPS_PREPARE:
-			if (pm_ops->prepare) {
-				error = pm_ops->prepare(PM_SUSPEND_DISK);
-			}
+			data->platform_suspend = 1;
+			error = 0;
 			break;
 
 		case PMOPS_ENTER:
-			kernel_shutdown_prepare(SYSTEM_SUSPEND_DISK);
-			error = pm_ops->enter(PM_SUSPEND_DISK);
+			if (data->platform_suspend) {
+				kernel_shutdown_prepare(SYSTEM_SUSPEND_DISK);
+				error = pm_ops->enter(PM_SUSPEND_DISK);
+				error = 0;
+			}
 			break;
 
 		case PMOPS_FINISH:
-			if (pm_ops && pm_ops->finish) {
-				pm_ops->finish(PM_SUSPEND_DISK);
-			}
+			if (data->platform_suspend)
+				error = 0;
+
 			break;
 
 		default:
 			printk(KERN_ERR "SNAPSHOT_PMOPS: invalid argument %ld\n", arg);
-			error = -EINVAL;
 
 		}
 		break;

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: [PATCH -mm] PM: Change ordering of suspend and resume code

[Stefan Seyfried]

Hi,

On Sun, Dec 17, 2006 at 06:58:15PM +0100, Rafael J. Wysocki wrote:
> Hi,
> 
> As indicated in a recent thread on Linux-PM, it's necessary to call
> pm_ops->finish() before devce_resume(), but enable_nonboot_cpus() has to be
> called before pm_ops->finish()
> (cf. http://lists.osdl.org/pipermail/linux-pm/2006-November/004164.html).
> For consistency, it seems reasonable to call disable_nonboot_cpus() after
> device_suspend().
> 
> This way the suspend code will remain symmetrical with respect to the resume
> code and it may allow us to speed up things in the future by suspending and
> resuming devices and/or saving the suspend image in many threads.

Good. A thread to do progress bars :-)
 
> The first patch changes the ordering of the suspend-to-RAM code and is
> untested, because my boxes continue refusing to resume from RAM for other
> reasons.  If anyone can, please do me a favour and test it.
> 
> The second patch changes the ordering of the built-in suspend-to-disk code,
> and the last two patches modify the swsusp userland interface code
> accordingly.  As far as the last patch is concerned, I've decided to change
> the existing ioctls, so that the patched kernel works with the "old" userland.
> I don't think this change breaks anything of importance, but if it does, we
> can just add (yet) another ioctl to set the platform mode, although in that
> case the userland would have to be changed as well.
> 
> All of the patches are against 2.6.20-rc1 and should not be used with any
> earlier kernel.

I'll test them, but probably not this year :-(
I'll only get to it during the first week of January.
-- 
Stefan Seyfried
QA / R&D Team Mobile Devices        |              "Any ideas, John?"
SUSE LINUX Products GmbH, Nürnberg  | "Well, surrounding them's out." 
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -mm] PM: Change ordering of suspend and resume code

[Rafael J. Wysocki]

Hi,

On Monday, 18 December 2006 08:42, Stefan Seyfried wrote:
> Hi,
> 
> On Sun, Dec 17, 2006 at 06:58:15PM +0100, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > As indicated in a recent thread on Linux-PM, it's necessary to call
> > pm_ops->finish() before devce_resume(), but enable_nonboot_cpus() has to be
> > called before pm_ops->finish()
> > (cf. http://lists.osdl.org/pipermail/linux-pm/2006-November/004164.html).
> > For consistency, it seems reasonable to call disable_nonboot_cpus() after
> > device_suspend().
> > 
> > This way the suspend code will remain symmetrical with respect to the resume
> > code and it may allow us to speed up things in the future by suspending and
> > resuming devices and/or saving the suspend image in many threads.
> 
> Good. A thread to do progress bars :-)

I thought about something like multiple compression/encryption threads, but
well ... ;-)

> > The first patch changes the ordering of the suspend-to-RAM code and is
> > untested, because my boxes continue refusing to resume from RAM for other
> > reasons.  If anyone can, please do me a favour and test it.
> > 
> > The second patch changes the ordering of the built-in suspend-to-disk code,
> > and the last two patches modify the swsusp userland interface code
> > accordingly.  As far as the last patch is concerned, I've decided to change
> > the existing ioctls, so that the patched kernel works with the "old" userland.
> > I don't think this change breaks anything of importance, but if it does, we
> > can just add (yet) another ioctl to set the platform mode, although in that
> > case the userland would have to be changed as well.
> > 
> > All of the patches are against 2.6.20-rc1 and should not be used with any
> > earlier kernel.
> 
> I'll test them,

Thanks.

> but probably not this year :-( 
> I'll only get to it during the first week of January.

That will be fine.

Greetings,
Rafael


-- 
If you don't have the time to read,
you don't have the time or the tools to write.
		- Stephen King

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: [PATCH -mm] PM: Change ordering of suspend and resume code

[Pavel Machek]

Hi!

> As indicated in a recent thread on Linux-PM, it's necessary to call
> pm_ops->finish() before devce_resume(), but enable_nonboot_cpus() has to be
> called before pm_ops->finish()
> (cf. http://lists.osdl.org/pipermail/linux-pm/2006-November/004164.html).
> For consistency, it seems reasonable to call disable_nonboot_cpus() after
> device_suspend().
> 
> This way the suspend code will remain symmetrical with respect to the resume
> code and it may allow us to speed up things in the future by suspending and
> resuming devices and/or saving the suspend image in many threads.
...
> The following series of patches reorders the suspend and resume code so that
> nonboot CPUs are disabled after devices have been suspended and enabled before
> the devices are resumed.  It also causes pm_ops->finish() to be called after
> enable_nonboot_cpus() wherever necessary.

Series looks okay to me... but it will need _long_ testing in
-mm. (Consider this ACK).

> The first patch changes the ordering of the suspend-to-RAM code and is
> untested, because my boxes continue refusing to resume from RAM for other
> reasons.  If anyone can, please do me a favour and test it.

I did a bit of testing, and it seems to still work, both s2ram and
swsusp. (uswsusp untested).
									Pavel

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

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: [Suspend-devel] [PATCH -mm] PM: Change ordering of suspend and resume code

[Rafael J. Wysocki]

Hi,

On Wednesday, 20 December 2006 00:30, Pavel Machek wrote:
> Hi!
> 
> > As indicated in a recent thread on Linux-PM, it's necessary to call
> > pm_ops->finish() before devce_resume(), but enable_nonboot_cpus() has to be
> > called before pm_ops->finish()
> > (cf. http://lists.osdl.org/pipermail/linux-pm/2006-November/004164.html).
> > For consistency, it seems reasonable to call disable_nonboot_cpus() after
> > device_suspend().
> > 
> > This way the suspend code will remain symmetrical with respect to the resume
> > code and it may allow us to speed up things in the future by suspending and
> > resuming devices and/or saving the suspend image in many threads.
> ...
> > The following series of patches reorders the suspend and resume code so that
> > nonboot CPUs are disabled after devices have been suspended and enabled before
> > the devices are resumed.  It also causes pm_ops->finish() to be called after
> > enable_nonboot_cpus() wherever necessary.
> 
> Series looks okay to me... but it will need _long_ testing in
> -mm. (Consider this ACK).

Thanks.

> > The first patch changes the ordering of the suspend-to-RAM code and is
> > untested, because my boxes continue refusing to resume from RAM for other
> > reasons.  If anyone can, please do me a favour and test it.
> 
> I did a bit of testing, and it seems to still work, both s2ram and
> swsusp. (uswsusp untested).

I've been testing uswsusp for some time and it doesn't seem to break (so far :-)).

Greetings,
Rafael


-- 
If you don't have the time to read,
you don't have the time or the tools to write.
		- Stephen King
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html