[PATCH 0/8] PM: Asynchronous suspen and resume

[PATCH 0/8] PM: Asynchronous suspen and resume

[Rafael J. Wysocki]

Hi,

This has been discussed to some extent, but without the last three patches that
actually enable the feature for the PCI, USB and SCSI subsystems.

First, the motivation.  At least on some machines (well, let's face it, most
likely on all of them) resume (from suspend to RAM) takes more time than
booting the kernel, which is not really surprising given that it's done
completely synchronously, even on systems with many CPUs, and which sucks
(especially in the case of hibernation, where devices are "suspended" three
times and "resumed" two times in each cycle).  Testing shows that with the
patches below applied the suspend time can be reduced by half and the resume
time even more (on one of my test systems it takes ~4 s without the patches
and ~1 s with them, which is a huge gain).

Second, why at the core level.  In short, this is the most straightforward way
to take dependencies between devices, following from the structure of the
device tree, into account.  Namely, a device can only be suspended if all
of its children have been suspended and it can only be resumed after its
parent.  Moreover, devices are resumed in the order in which they have been
registered (and are suspended in the reverse order), which is reflected by the
ordering of the list of devices used by the PM core, dpm_list.  Generally
speaking, it's not really advisable to change the ordering of this list
arbitrarily, because that may lead into problems with some devices and only
the PM core knows what the "right" ordering actually is.

So, the idea is to execute the entire suspend and resume callbacks of devices
in parallel with the main suspend/resume thread and possibly in parallel with
each other, but do that only for the devices known to take substantial amounts
of time to suspend or resume and for the devices that depend on them this
way or another (ie. that are their parents or children, or parents of their
parents and so on).  [It generally wouldn't make sense to suspend and resume
a device asynchronously if at least one of its children or its parent were
suspended and resumed synchronously, because in that case the main
suspend/resume thread would have to wait for the "asynchronous" devices'
callbacks to return before handling their "synchronous relatives".]

Now, testing shows that the slowest to suspend or resume are PCI, USB and SCSI
devices, so it makes sense to make them suspend and resume asynchronously,
which is done by the last three patches.  The dangerous part is that for things
to be efficient we need to make the whole subtrees of the device tree suspend
and resume asynchronously and it can only be verified by testing if things work
or break.  So far, I haven't seen any breakage resulting from this patchset on
any of my test machines, even though I have only a few of them.  That's why the
third patch leaves the switch blocking this feature in the "enabled" position.
Still, if it turns out that these changes introduce any problems on the other
people's machines, the default can be easily changed to "disabled".

Finally, the patches.

[1/8] - Nothing really interesting, modifies debug printk()s to show the parent
        name in addition to the information about the device being handled at
        the moment.  This is quite useful if one wants to learn about the device
        tree structure from the logs.

[2/8] - The actual implementation of the asynchronous suspend/resume at the
        PM core level.  However, it doesn't change the functionality as long as
        no devices have power.async_suspend set.

[3/8] - Adds the sysfs switch to disable/enable the asychronous suspend/resume
        at the high level.  It's /sys/power/pm_async and it's equal to 1 by
        default, which means "enabled".

[4/8] - Adds switches to disable/enable the asychronous suspend/resume for
        individual devices.  This really is regaded as a debugging feature,
        so it's enabled by means of a separate .config option.

[5/8] - Optimisation reducing resume time even more.  Actually, on some systems
        it's necessary to get _any_ resume speedup, but then it can be _huge_.

[6/8] - Set power.async_suspend for PCI devices.

[7/8] - Set power.async_suspend for USB devices.

[8/8] - Set power.async_suspend for SCSI devices.

Patches [2/8] - [5/8] have already been discussed in detail, [6/8] - [8/8] are
new (well, the PCI patch was discussed too, but then I didn't have the testing
data showing that PCI devices can be the slowest ones to suspend or resume
in some configurations).

Hopefully, I didn't get anything wrong this time, but if I did, please let me know.

The patches are available from the async branch of the suspend-2.6 tree at
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/suspend-2.6.git

Thanks,
Rafael

[PATCH 1/8] PM: Add parent information to timing messages

[Rafael J. Wysocki]

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

Add parent information to the messages printed by the suspend/resume
core when initcall_debug is set.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/main.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -271,8 +271,9 @@ static int pm_noirq_op(struct device *de
 	ktime_t calltime, delta, rettime;
 
 	if (initcall_debug) {
-		pr_info("calling  %s_i+ @ %i\n",
-				dev_name(dev), task_pid_nr(current));
+		pr_info("calling  %s+ @ %i, parent: %s\n",
+				dev_name(dev), task_pid_nr(current),
+				dev->parent ? dev_name(dev->parent) : "none");
 		calltime = ktime_get();
 	}
 

[PATCH 2/8] PM: Asynchronous suspend and resume of devices

[Rafael J. Wysocki]

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

Theoretically, the total time of system sleep transitions (suspend
to RAM, hibernation) can be reduced by running suspend and resume
callbacks of device drivers in parallel with each other.  However,
there are dependencies between devices such that we're not allowed
to suspend the parent of a device before suspending the device
itself.  Analogously, we're not allowed to resume a device before
resuming its parent.

The most straightforward way to take these dependencies into accout
is to start the async threads used for suspending and resuming
devices at the core level, so that async_schedule() is called for
each suspend and resume callback supposed to be executed
asynchronously.

For this purpose, introduce a new device flag, power.async_suspend,
used to mark the devices whose suspend and resume callbacks are to be
executed asynchronously (ie. in parallel with the main suspend/resume
thread and possibly in parallel with each other) and helper function
device_enable_async_suspend() allowing one to set power.async_suspend
for given device (power.async_suspend is unset by default for all
devices).  For each device with the power.async_suspend flag set the
PM core will use async_schedule() to execute its suspend and resume
callbacks.

The async threads started for different devices as a result of
calling async_schedule() are synchronized with each other and with
the main suspend/resume thread with the help of completions, in the
following way:
(1) There is a completion, power.completion, for each device object.
(2) Each device's completion is reset before calling async_schedule()
    for the device or, in the case of devices with the
    power.async_suspend flags unset, before executing the device's
    suspend and resume callbacks.
(3) During suspend, right before running the bus type, device type
    and device class suspend callbacks for the device, the PM core
    waits for the completions of all the device's children to be
    completed.
(4) During resume, right before running the bus type, device type and
    device class resume callbacks for the device, the PM core waits
    for the completion of the device's parent to be completed.
(5) The PM core completes power.completion for each device right
    after the bus type, device type and device class suspend (or
    resume) callbacks executed for the device have returned.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/main.c    |  115 ++++++++++++++++++++++++++++++++++++++++---
 include/linux/device.h       |    6 ++
 include/linux/pm.h           |    3 +
 include/linux/resume-trace.h |    7 ++
 4 files changed, 125 insertions(+), 6 deletions(-)

Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -26,6 +26,7 @@
 #include <linux/spinlock.h>
 #include <linux/wait.h>
 #include <linux/timer.h>
+#include <linux/completion.h>
 
 /*
  * Callbacks for platform drivers to implement.
@@ -412,9 +413,11 @@ struct dev_pm_info {
 	pm_message_t		power_state;
 	unsigned int		can_wakeup:1;
 	unsigned int		should_wakeup:1;
+	unsigned		async_suspend:1;
 	enum dpm_state		status;		/* Owned by the PM core */
 #ifdef CONFIG_PM_SLEEP
 	struct list_head	entry;
+	struct completion	completion;
 #endif
 #ifdef CONFIG_PM_RUNTIME
 	struct timer_list	suspend_timer;
Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -25,6 +25,7 @@
 #include <linux/resume-trace.h>
 #include <linux/interrupt.h>
 #include <linux/sched.h>
+#include <linux/async.h>
 
 #include "../base.h"
 #include "power.h"
@@ -42,6 +43,7 @@
 LIST_HEAD(dpm_list);
 
 static DEFINE_MUTEX(dpm_list_mtx);
+static pm_message_t pm_transition;
 
 /*
  * Set once the preparation of devices for a PM transition has started, reset
@@ -56,6 +58,7 @@ static bool transition_started;
 void device_pm_init(struct device *dev)
 {
 	dev->power.status = DPM_ON;
+	init_completion(&dev->power.completion);
 	pm_runtime_init(dev);
 }
 
@@ -111,6 +114,7 @@ void device_pm_remove(struct device *dev
 	pr_debug("PM: Removing info for %s:%s\n",
 		 dev->bus ? dev->bus->name : "No Bus",
 		 kobject_name(&dev->kobj));
+	complete_all(&dev->power.completion);
 	mutex_lock(&dpm_list_mtx);
 	list_del_init(&dev->power.entry);
 	mutex_unlock(&dpm_list_mtx);
@@ -188,6 +192,31 @@ static void initcall_debug_report(struct
 }
 
 /**
+ * dpm_wait - Wait for a PM operation to complete.
+ * @dev: Device to wait for.
+ * @async: If unset, wait only if the device's power.async_suspend flag is set.
+ */
+static void dpm_wait(struct device *dev, bool async)
+{
+	if (!dev)
+		return;
+
+	if (async || dev->power.async_suspend)
+		wait_for_completion(&dev->power.completion);
+}
+
+static int dpm_wait_fn(struct device *dev, void *async_ptr)
+{
+	dpm_wait(dev, *((bool *)async_ptr));
+	return 0;
+}
+
+static void dpm_wait_for_children(struct device *dev, bool async)
+{
+       device_for_each_child(dev, &async, dpm_wait_fn);
+}
+
+/**
  * pm_op - Execute the PM operation appropriate for given PM event.
  * @dev: Device to handle.
  * @ops: PM operations to choose from.
@@ -466,17 +495,19 @@ static int legacy_resume(struct device *
 }
 
 /**
- * device_resume - Execute "resume" callbacks for given device.
+ * __device_resume - Execute "resume" callbacks for given device.
  * @dev: Device to handle.
  * @state: PM transition of the system being carried out.
+ * @async: If true, the device is being resumed asynchronously.
  */
-static int device_resume(struct device *dev, pm_message_t state)
+static int __device_resume(struct device *dev, pm_message_t state, bool async)
 {
 	int error = 0;
 
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
 
+	dpm_wait(dev->parent, async);
 	down(&dev->sem);
 
 	if (dev->bus) {
@@ -511,11 +542,36 @@ static int device_resume(struct device *
 	}
  End:
 	up(&dev->sem);
+	complete_all(&dev->power.completion);
 
 	TRACE_RESUME(error);
 	return error;
 }
 
+static void async_resume(void *data, async_cookie_t cookie)
+{
+	struct device *dev = (struct device *)data;
+	int error;
+
+	error = __device_resume(dev, pm_transition, true);
+	if (error)
+		pm_dev_err(dev, pm_transition, " async", error);
+	put_device(dev);
+}
+
+static int device_resume(struct device *dev)
+{
+	INIT_COMPLETION(dev->power.completion);
+
+	if (dev->power.async_suspend && !pm_trace_is_enabled()) {
+		get_device(dev);
+		async_schedule(async_resume, dev);
+		return 0;
+	}
+
+	return __device_resume(dev, pm_transition, false);
+}
+
 /**
  * dpm_resume - Execute "resume" callbacks for non-sysdev devices.
  * @state: PM transition of the system being carried out.
@@ -530,6 +586,7 @@ static void dpm_resume(pm_message_t stat
 
 	INIT_LIST_HEAD(&list);
 	mutex_lock(&dpm_list_mtx);
+	pm_transition = state;
 	while (!list_empty(&dpm_list)) {
 		struct device *dev = to_device(dpm_list.next);
 
@@ -540,7 +597,7 @@ static void dpm_resume(pm_message_t stat
 			dev->power.status = DPM_RESUMING;
 			mutex_unlock(&dpm_list_mtx);
 
-			error = device_resume(dev, state);
+			error = device_resume(dev);
 
 			mutex_lock(&dpm_list_mtx);
 			if (error)
@@ -555,6 +612,7 @@ static void dpm_resume(pm_message_t stat
 	}
 	list_splice(&list, &dpm_list);
 	mutex_unlock(&dpm_list_mtx);
+	async_synchronize_full();
 	dpm_show_time(starttime, state, NULL);
 }
 
@@ -732,17 +790,24 @@ static int legacy_suspend(struct device 
 	return error;
 }
 
+static int async_error;
+
 /**
  * device_suspend - Execute "suspend" callbacks for given device.
  * @dev: Device to handle.
  * @state: PM transition of the system being carried out.
+ * @async: If true, the device is being suspended asynchronously.
  */
-static int device_suspend(struct device *dev, pm_message_t state)
+static int __device_suspend(struct device *dev, pm_message_t state, bool async)
 {
 	int error = 0;
 
+	dpm_wait_for_children(dev, async);
 	down(&dev->sem);
 
+	if (async_error)
+		goto End;
+
 	if (dev->class) {
 		if (dev->class->pm) {
 			pm_dev_dbg(dev, state, "class ");
@@ -773,12 +838,44 @@ static int device_suspend(struct device 
 			error = legacy_suspend(dev, state, dev->bus->suspend);
 		}
 	}
+
+	if (!error)
+		dev->power.status = DPM_OFF;
+
  End:
 	up(&dev->sem);
+	complete_all(&dev->power.completion);
 
 	return error;
 }
 
+static void async_suspend(void *data, async_cookie_t cookie)
+{
+	struct device *dev = (struct device *)data;
+	int error;
+
+	error = __device_suspend(dev, pm_transition, true);
+	if (error) {
+		pm_dev_err(dev, pm_transition, " async", error);
+		async_error = error;
+	}
+
+	put_device(dev);
+}
+
+static int device_suspend(struct device *dev)
+{
+	INIT_COMPLETION(dev->power.completion);
+
+	if (dev->power.async_suspend) {
+		get_device(dev);
+		async_schedule(async_suspend, dev);
+		return 0;
+	}
+
+	return __device_suspend(dev, pm_transition, false);
+}
+
 /**
  * dpm_suspend - Execute "suspend" callbacks for all non-sysdev devices.
  * @state: PM transition of the system being carried out.
@@ -791,13 +888,15 @@ static int dpm_suspend(pm_message_t stat
 
 	INIT_LIST_HEAD(&list);
 	mutex_lock(&dpm_list_mtx);
+	pm_transition = state;
+	async_error = 0;
 	while (!list_empty(&dpm_list)) {
 		struct device *dev = to_device(dpm_list.prev);
 
 		get_device(dev);
 		mutex_unlock(&dpm_list_mtx);
 
-		error = device_suspend(dev, state);
+		error = device_suspend(dev);
 
 		mutex_lock(&dpm_list_mtx);
 		if (error) {
@@ -805,13 +904,17 @@ static int dpm_suspend(pm_message_t stat
 			put_device(dev);
 			break;
 		}
-		dev->power.status = DPM_OFF;
 		if (!list_empty(&dev->power.entry))
 			list_move(&dev->power.entry, &list);
 		put_device(dev);
+		if (async_error)
+			break;
 	}
 	list_splice(&list, dpm_list.prev);
 	mutex_unlock(&dpm_list_mtx);
+	async_synchronize_full();
+	if (!error)
+		error = async_error;
 	if (!error)
 		dpm_show_time(starttime, state, NULL);
 	return error;
Index: linux-2.6/include/linux/resume-trace.h
===================================================================
--- linux-2.6.orig/include/linux/resume-trace.h
+++ linux-2.6/include/linux/resume-trace.h
@@ -6,6 +6,11 @@
 
 extern int pm_trace_enabled;
 
+static inline int pm_trace_is_enabled(void)
+{
+       return pm_trace_enabled;
+}
+
 struct device;
 extern void set_trace_device(struct device *);
 extern void generate_resume_trace(const void *tracedata, unsigned int user);
@@ -17,6 +22,8 @@ extern void generate_resume_trace(const 
 
 #else
 
+static inline int pm_trace_is_enabled(void) { return 0; }
+
 #define TRACE_DEVICE(dev) do { } while (0)
 #define TRACE_RESUME(dev) do { } while (0)
 
Index: linux-2.6/include/linux/device.h
===================================================================
--- linux-2.6.orig/include/linux/device.h
+++ linux-2.6/include/linux/device.h
@@ -472,6 +472,12 @@ static inline int device_is_registered(s
 	return dev->kobj.state_in_sysfs;
 }
 
+static inline void device_enable_async_suspend(struct device *dev)
+{
+	if (dev->power.status == DPM_ON)
+		dev->power.async_suspend = true;
+}
+
 void driver_init(void);
 
 /*

[PATCH 3/8] PM: Add a switch for disabling/enabling asynchronous suspend/resume

[Rafael J. Wysocki]

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

Add sysfs attribute /sys/power/pm_async allowing the user space to
disable/enable asynchronous suspend/resume of devices.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/ABI/testing/sysfs-power |   13 +++++++++++++
 drivers/base/power/main.c             |    7 ++++---
 drivers/base/power/power.h            |    6 +++---
 kernel/power/main.c                   |   31 ++++++++++++++++++++++++++++++-
 4 files changed, 50 insertions(+), 7 deletions(-)

Index: linux-2.6/kernel/power/main.c
===================================================================
--- linux-2.6.orig/kernel/power/main.c
+++ linux-2.6/kernel/power/main.c
@@ -44,6 +44,32 @@ int pm_notifier_call_chain(unsigned long
 			== NOTIFY_BAD) ? -EINVAL : 0;
 }
 
+/* If set, devices may be suspended and resumed asynchronously. */
+int pm_async_enabled = 1;
+
+static ssize_t pm_async_show(struct kobject *kobj, struct kobj_attribute *attr,
+			     char *buf)
+{
+	return sprintf(buf, "%d\n", pm_async_enabled);
+}
+
+static ssize_t pm_async_store(struct kobject *kobj, struct kobj_attribute *attr,
+			      const char *buf, size_t n)
+{
+	unsigned long val;
+
+	if (strict_strtoul(buf, 10, &val))
+		return -EINVAL;
+
+	if (val > 1)
+		return -EINVAL;
+
+	pm_async_enabled = val;
+	return n;
+}
+
+power_attr(pm_async);
+
 #ifdef CONFIG_PM_DEBUG
 int pm_test_level = TEST_NONE;
 
@@ -208,9 +234,12 @@ static struct attribute * g[] = {
 #ifdef CONFIG_PM_TRACE
 	&pm_trace_attr.attr,
 #endif
-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_PM_DEBUG)
+#ifdef CONFIG_PM_SLEEP
+	&pm_async_attr.attr,
+#ifdef CONFIG_PM_DEBUG
 	&pm_test_attr.attr,
 #endif
+#endif
 	NULL,
 };
 
Index: linux-2.6/drivers/base/power/power.h
===================================================================
--- linux-2.6.orig/drivers/base/power/power.h
+++ linux-2.6/drivers/base/power/power.h
@@ -12,10 +12,10 @@ static inline void pm_runtime_remove(str
 
 #ifdef CONFIG_PM_SLEEP
 
-/*
- * main.c
- */
+/* kernel/power/main.c */
+extern int pm_async_enabled;
 
+/* drivers/base/power/main.c */
 extern struct list_head dpm_list;	/* The active device list */
 
 static inline struct device *to_device(struct list_head *entry)
Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -201,7 +201,7 @@ static void dpm_wait(struct device *dev,
 	if (!dev)
 		return;
 
-	if (async || dev->power.async_suspend)
+	if (async || (pm_async_enabled && dev->power.async_suspend))
 		wait_for_completion(&dev->power.completion);
 }
 
@@ -563,7 +563,8 @@ static int device_resume(struct device *
 {
 	INIT_COMPLETION(dev->power.completion);
 
-	if (dev->power.async_suspend && !pm_trace_is_enabled()) {
+	if (pm_async_enabled && dev->power.async_suspend
+	    && !pm_trace_is_enabled()) {
 		get_device(dev);
 		async_schedule(async_resume, dev);
 		return 0;
@@ -867,7 +868,7 @@ static int device_suspend(struct device 
 {
 	INIT_COMPLETION(dev->power.completion);
 
-	if (dev->power.async_suspend) {
+	if (pm_async_enabled && dev->power.async_suspend) {
 		get_device(dev);
 		async_schedule(async_suspend, dev);
 		return 0;
Index: linux-2.6/Documentation/ABI/testing/sysfs-power
===================================================================
--- linux-2.6.orig/Documentation/ABI/testing/sysfs-power
+++ linux-2.6/Documentation/ABI/testing/sysfs-power
@@ -101,3 +101,16 @@ Description:
 
 		CAUTION: Using it will cause your machine's real-time (CMOS)
 		clock to be set to a random invalid time after a resume.
+
+What:		/sys/power/pm_async
+Date:		January 2009
+Contact:	Rafael J. Wysocki <rjw@sisk.pl>
+Description:
+		The /sys/power/pm_async file controls the switch allowing the
+		user space to enable or disable asynchronous suspend and resume
+		of devices.  If enabled, this feature will cause some device
+		drivers' suspend and resume callbacks to be executed in parallel
+		with each other and with the main suspend thread.  It is enabled
+		if this file contains "1", which is the default.  It may be
+		disabled by writing "0" to this file, in which case all devices
+		will be suspended and resumed synchronously.

[PATCH 4/8] PM: Add facility for advanced testing of async suspend/resume

[Rafael J. Wysocki]

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

Add configuration switch CONFIG_PM_ADVANCED_DEBUG for compiling in
extra PM debugging/testing code allowing one to access some
PM-related attributes of devices from the user space via sysfs.

If CONFIG_PM_ADVANCED_DEBUG is set, add sysfs attribute power/async
for every device allowing the user space to access the device's
power.async_suspend flag and modify it, if desired.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/ABI/testing/sysfs-devices-power |   26 +++++++++++++
 drivers/base/power/sysfs.c                    |   49 ++++++++++++++++++++++++++
 include/linux/device.h                        |   11 +++++
 kernel/power/Kconfig                          |   14 +++++++
 4 files changed, 100 insertions(+)

Index: linux-2.6/drivers/base/power/sysfs.c
===================================================================
--- linux-2.6.orig/drivers/base/power/sysfs.c
+++ linux-2.6/drivers/base/power/sysfs.c
@@ -54,6 +54,24 @@
  *	wakeup events internally (unless they are disabled), keeping
  *	their hardware in low power modes whenever they're unused.  This
  *	saves runtime power, without requiring system-wide sleep states.
+ *
+ *	async - Report/change current async suspend setting for the device
+ *
+ *	Asynchronous suspend and resume of the device during system-wide power
+ *	state transitions can be enabled by writing "enabled" to this file.
+ *	Analogously, if "disabled" is written to this file, the device will be
+ *	suspended and resumed synchronously.
+ *
+ *	All devices have one of the following two values for power/async:
+ *
+ *	 + "enabled\n" to permit the asynchronous suspend/resume of the device;
+ *	 + "disabled\n" to forbid it;
+ *
+ *	NOTE: It generally is unsafe to permit the asynchronous suspend/resume
+ *	of a device unless it is certain that all of the PM dependencies of the
+ *	device are known to the PM core.  However, for some devices this
+ *	attribute is set to "enabled" by bus type code or device drivers and in
+ *	that cases it should be safe to leave the default value.
  */
 
 static const char enabled[] = "enabled";
@@ -125,12 +143,43 @@ wake_store(struct device * dev, struct d
 
 static DEVICE_ATTR(wakeup, 0644, wake_show, wake_store);
 
+#ifdef CONFIG_PM_SLEEP_ADVANCED_DEBUG
+static ssize_t async_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	return sprintf(buf, "%s\n",
+			device_async_suspend_enabled(dev) ? enabled : disabled);
+}
+
+static ssize_t async_store(struct device *dev, struct device_attribute *attr,
+			   const char *buf, size_t n)
+{
+	char *cp;
+	int len = n;
+
+	cp = memchr(buf, '\n', n);
+	if (cp)
+		len = cp - buf;
+	if (len == sizeof enabled - 1 && strncmp(buf, enabled, len) == 0)
+		device_enable_async_suspend(dev);
+	else if (len == sizeof disabled - 1 && strncmp(buf, disabled, len) == 0)
+		device_disable_async_suspend(dev);
+	else
+		return -EINVAL;
+	return n;
+}
+
+static DEVICE_ATTR(async, 0644, async_show, async_store);
+#endif /* CONFIG_PM_SLEEP_ADVANCED_DEBUG */
 
 static struct attribute * power_attrs[] = {
 #ifdef CONFIG_PM_RUNTIME
 	&dev_attr_control.attr,
 #endif
 	&dev_attr_wakeup.attr,
+#ifdef CONFIG_PM_SLEEP_ADVANCED_DEBUG
+	&dev_attr_async.attr,
+#endif
 	NULL,
 };
 static struct attribute_group pm_attr_group = {
Index: linux-2.6/include/linux/device.h
===================================================================
--- linux-2.6.orig/include/linux/device.h
+++ linux-2.6/include/linux/device.h
@@ -478,6 +478,17 @@ static inline void device_enable_async_s
 		dev->power.async_suspend = true;
 }
 
+static inline void device_disable_async_suspend(struct device *dev)
+{
+	if (dev->power.status == DPM_ON)
+		dev->power.async_suspend = false;
+}
+
+static inline bool device_async_suspend_enabled(struct device *dev)
+{
+	return !!dev->power.async_suspend;
+}
+
 void driver_init(void);
 
 /*
Index: linux-2.6/kernel/power/Kconfig
===================================================================
--- linux-2.6.orig/kernel/power/Kconfig
+++ linux-2.6/kernel/power/Kconfig
@@ -27,6 +27,15 @@ config PM_DEBUG
 	code. This is helpful when debugging and reporting PM bugs, like
 	suspend support.
 
+config PM_ADVANCED_DEBUG
+	bool "Extra PM attributes in sysfs for low-level debugging/testing"
+	depends on PM_DEBUG
+	default n
+	---help---
+	Add extra sysfs attributes allowing one to access some Power Management
+	fields of device objects from user space.  If you are not a kernel
+	developer interested in debugging/testing Power Management, say "no".
+
 config PM_VERBOSE
 	bool "Verbose Power Management debugging"
 	depends on PM_DEBUG
@@ -85,6 +94,11 @@ config PM_SLEEP
 	depends on SUSPEND || HIBERNATION || XEN_SAVE_RESTORE
 	default y
 
+config PM_SLEEP_ADVANCED_DEBUG
+	bool
+	depends on PM_ADVANCED_DEBUG
+	default n
+
 config SUSPEND
 	bool "Suspend to RAM and standby"
 	depends on PM && ARCH_SUSPEND_POSSIBLE
Index: linux-2.6/Documentation/ABI/testing/sysfs-devices-power
===================================================================
--- linux-2.6.orig/Documentation/ABI/testing/sysfs-devices-power
+++ linux-2.6/Documentation/ABI/testing/sysfs-devices-power
@@ -51,3 +51,29 @@ Description:
 		drivers.  Changing this attribute to "on" prevents the driver
 		from power managing the device at run time.  Doing that while
 		the device is suspended causes it to be woken up.
+
+What:		/sys/devices/.../power/async
+Date:		January 2009
+Contact:	Rafael J. Wysocki <rjw@sisk.pl>
+Description:
+		The /sys/devices/.../async attribute allows the user space to
+		enable or diasble the device's suspend and resume callbacks to
+		be executed asynchronously (ie. in separate threads, in parallel
+		with the main suspend/resume thread) during system-wide power
+		transitions (eg. suspend to RAM, hibernation).
+
+		All devices have one of the following two values for the
+		power/async file:
+
+		+ "enabled\n" to permit the asynchronous suspend/resume;
+		+ "disabled\n" to forbid it;
+
+		The value of this attribute may be changed by writing either
+		"enabled", or "disabled" to it.
+
+		It generally is unsafe to permit the asynchronous suspend/resume
+		of a device unless it is certain that all of the PM dependencies
+		of the device are known to the PM core.  However, for some
+		devices this attribute is set to "enabled" by bus type code or
+		device drivers and in that cases it should be safe to leave the
+		default value.

[PATCH 5/8] PM: Start asynchronous resume threads upfront

[Rafael J. Wysocki]

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

It has been shown by testing that total device resume time can be
reduced significantly (by as much as 50% or more) if the async
threads executing some devices' resume routines are all started
before the main resume thread starts to handle the "synchronous"
devices.

This is a consequence of the fact that the slowest devices tend to be
located at the end of dpm_list, so their resume routines are started
very late.  Consequently, they have to wait for all the preceding
"synchronous" devices before their resume routines can be started
by the main resume thread, even if they are "asynchronous".  By
starting their async threads upfront we effectively move those
devices towards the beginning of dpm_list, without breaking their
ordering with respect to their parents and children.  As a result,
their resume routines are started much earlier and we are able to
save much more device resume time this way.

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

Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -495,12 +495,12 @@ static int legacy_resume(struct device *
 }
 
 /**
- * __device_resume - Execute "resume" callbacks for given device.
+ * device_resume - Execute "resume" callbacks for given device.
  * @dev: Device to handle.
  * @state: PM transition of the system being carried out.
  * @async: If true, the device is being resumed asynchronously.
  */
-static int __device_resume(struct device *dev, pm_message_t state, bool async)
+static int device_resume(struct device *dev, pm_message_t state, bool async)
 {
 	int error = 0;
 
@@ -510,6 +510,8 @@ static int __device_resume(struct device
 	dpm_wait(dev->parent, async);
 	down(&dev->sem);
 
+	dev->power.status = DPM_RESUMING;
+
 	if (dev->bus) {
 		if (dev->bus->pm) {
 			pm_dev_dbg(dev, state, "");
@@ -553,24 +555,16 @@ static void async_resume(void *data, asy
 	struct device *dev = (struct device *)data;
 	int error;
 
-	error = __device_resume(dev, pm_transition, true);
+	error = device_resume(dev, pm_transition, true);
 	if (error)
 		pm_dev_err(dev, pm_transition, " async", error);
 	put_device(dev);
 }
 
-static int device_resume(struct device *dev)
+static bool is_async(struct device *dev)
 {
-	INIT_COMPLETION(dev->power.completion);
-
-	if (pm_async_enabled && dev->power.async_suspend
-	    && !pm_trace_is_enabled()) {
-		get_device(dev);
-		async_schedule(async_resume, dev);
-		return 0;
-	}
-
-	return __device_resume(dev, pm_transition, false);
+	return dev->power.async_suspend && pm_async_enabled
+		&& !pm_trace_is_enabled();
 }
 
 /**
@@ -583,22 +577,33 @@ static int device_resume(struct device *
 static void dpm_resume(pm_message_t state)
 {
 	struct list_head list;
+	struct device *dev;
 	ktime_t starttime = ktime_get();
 
 	INIT_LIST_HEAD(&list);
 	mutex_lock(&dpm_list_mtx);
 	pm_transition = state;
-	while (!list_empty(&dpm_list)) {
-		struct device *dev = to_device(dpm_list.next);
 
+	list_for_each_entry(dev, &dpm_list, power.entry) {
+		if (dev->power.status < DPM_OFF)
+			continue;
+
+		INIT_COMPLETION(dev->power.completion);
+		if (is_async(dev)) {
+			get_device(dev);
+			async_schedule(async_resume, dev);
+		}
+	}
+
+	while (!list_empty(&dpm_list)) {
+		dev = to_device(dpm_list.next);
 		get_device(dev);
-		if (dev->power.status >= DPM_OFF) {
+		if (dev->power.status >= DPM_OFF && !is_async(dev)) {
 			int error;
 
-			dev->power.status = DPM_RESUMING;
 			mutex_unlock(&dpm_list_mtx);
 
-			error = device_resume(dev);
+			error = device_resume(dev, state, false);
 
 			mutex_lock(&dpm_list_mtx);
 			if (error)

[PATCH 6/8] PM: Allow PCI devices to suspend/resume asynchronously

[Rafael J. Wysocki]

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

Set power.async_suspend for all PCI devices and PCIe port services,
so that they can be suspended and resumed in parallel with other
devices they don't depend on in a known way (i.e. devices which are
not their parents or children).

This only affects the "regular" suspend and resume stages, which
means in particular that the restoration of the PCI devices' standard
configuration registers during resume will still be carried out
synchronously (at the "early" resume stage).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pci/pci.c               |    1 +
 drivers/pci/pcie/portdrv_core.c |    1 +
 drivers/pci/probe.c             |    1 +
 3 files changed, 3 insertions(+)

Index: linux-2.6/drivers/pci/pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci.c
+++ linux-2.6/drivers/pci/pci.c
@@ -1541,6 +1541,7 @@ void pci_pm_init(struct pci_dev *dev)
 	int pm;
 	u16 pmc;
 
+	device_enable_async_suspend(&dev->dev);
 	dev->wakeup_prepared = false;
 	dev->pm_cap = 0;
 
Index: linux-2.6/drivers/pci/pcie/portdrv_core.c
===================================================================
--- linux-2.6.orig/drivers/pci/pcie/portdrv_core.c
+++ linux-2.6/drivers/pci/pcie/portdrv_core.c
@@ -285,6 +285,7 @@ static int pcie_device_init(struct pci_d
 		     pci_name(pdev),
 		     get_descriptor_id(pdev->pcie_type, service));
 	device->parent = &pdev->dev;
+	device_enable_async_suspend(device);
 
 	retval = device_register(device);
 	if (retval)
Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -1200,6 +1200,7 @@ struct pci_bus * pci_create_bus(struct d
 	if (error)
 		goto dev_reg_err;
 	b->bridge = get_device(dev);
+	device_enable_async_suspend(b->bridge);
 
 	if (!parent)
 		set_dev_node(b->bridge, pcibus_to_node(b));

[PATCH 7/8] PM: Allow USB devices to suspend/resume asynchronously

[Rafael J. Wysocki]

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

Set power.async_suspend for USB devices, endpoints and interfaces,
allowing them to be suspended and resumed asynchronously during
system sleep transitions.

The power.async_suspend flag is also set for devices that don't have
suspend or resume callbacks, because otherwise they would make the
main suspend/resume thread wait for their "asynchronous" children
(during suspend) or parents (during resume), effectively negating the
possible gains from executing these devices' suspend and resume
callbacks asynchronously.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/usb/core/endpoint.c |    1 +
 drivers/usb/core/hcd-pci.c  |    1 +
 drivers/usb/core/hub.c      |    1 +
 drivers/usb/core/message.c  |    1 +
 4 files changed, 4 insertions(+)

Index: linux-2.6/drivers/usb/core/hub.c
===================================================================
--- linux-2.6.orig/drivers/usb/core/hub.c
+++ linux-2.6/drivers/usb/core/hub.c
@@ -1817,6 +1817,7 @@ int usb_new_device(struct usb_device *ud
 	/* Tell the world! */
 	announce_device(udev);
 
+	device_enable_async_suspend(&udev->dev);
 	/* Register the device.  The device driver is responsible
 	 * for configuring the device and invoking the add-device
 	 * notifier chain (used by usbfs and possibly others).
Index: linux-2.6/drivers/usb/core/message.c
===================================================================
--- linux-2.6.orig/drivers/usb/core/message.c
+++ linux-2.6/drivers/usb/core/message.c
@@ -1867,6 +1867,7 @@ free_interfaces:
 			"adding %s (config #%d, interface %d)\n",
 			dev_name(&intf->dev), configuration,
 			intf->cur_altsetting->desc.bInterfaceNumber);
+		device_enable_async_suspend(&intf->dev);
 		ret = device_add(&intf->dev);
 		if (ret != 0) {
 			dev_err(&dev->dev, "device_add(%s) --> %d\n",
Index: linux-2.6/drivers/usb/core/endpoint.c
===================================================================
--- linux-2.6.orig/drivers/usb/core/endpoint.c
+++ linux-2.6/drivers/usb/core/endpoint.c
@@ -186,6 +186,7 @@ int usb_create_ep_devs(struct device *pa
 	ep_dev->dev.parent = parent;
 	ep_dev->dev.release = ep_device_release;
 	dev_set_name(&ep_dev->dev, "ep_%02x", endpoint->desc.bEndpointAddress);
+	device_enable_async_suspend(&ep_dev->dev);
 
 	retval = device_register(&ep_dev->dev);
 	if (retval)

[PATCH 8/8] PM: Allow SCSI devices to suspend/resume asynchronously

[Rafael J. Wysocki]

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

Set power.async_suspend for all SCSI devices, targets and hosts, so
that they can be suspended and resumed in parallel with the main
suspend/resume thread and possibly with other devices they don't
depend on in a known way (i.e. devices which are not their parents or
children).

The power.async_suspend flag is also set for devices that don't have
suspend or resume callbacks, because otherwise they would make the
main suspend/resume thread wait for their "asynchronous" children
(during suspend) or parents (during resume), effectively negating the
possible gains from executing these devices' suspend and resume
callbacks asynchronously.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/scsi/hosts.c      |    4 ++++
 drivers/scsi/scsi_sysfs.c |    4 ++++
 2 files changed, 8 insertions(+)

Index: linux-2.6/drivers/scsi/hosts.c
===================================================================
--- linux-2.6.orig/drivers/scsi/hosts.c
+++ linux-2.6/drivers/scsi/hosts.c
@@ -215,6 +215,8 @@ int scsi_add_host_with_dma(struct Scsi_H
 		shost->shost_gendev.parent = dev ? dev : &platform_bus;
 	shost->dma_dev = dma_dev;
 
+	device_enable_async_suspend(&shost->shost_gendev);
+
 	error = device_add(&shost->shost_gendev);
 	if (error)
 		goto out;
@@ -222,6 +224,8 @@ int scsi_add_host_with_dma(struct Scsi_H
 	scsi_host_set_state(shost, SHOST_RUNNING);
 	get_device(shost->shost_gendev.parent);
 
+	device_enable_async_suspend(&shost->shost_dev);
+
 	error = device_add(&shost->shost_dev);
 	if (error)
 		goto out_del_gendev;
Index: linux-2.6/drivers/scsi/scsi_sysfs.c
===================================================================
--- linux-2.6.orig/drivers/scsi/scsi_sysfs.c
+++ linux-2.6/drivers/scsi/scsi_sysfs.c
@@ -847,6 +847,8 @@ static int scsi_target_add(struct scsi_t
 	if (starget->state != STARGET_CREATED)
 		return 0;
 
+	device_enable_async_suspend(&starget->dev);
+
 	error = device_add(&starget->dev);
 	if (error) {
 		dev_err(&starget->dev, "target device_add failed, error %d\n", error);
@@ -886,11 +888,13 @@ int scsi_sysfs_add_sdev(struct scsi_devi
 		return error;
 
 	transport_configure_device(&starget->dev);
+	device_enable_async_suspend(&sdev->sdev_gendev);
 	error = device_add(&sdev->sdev_gendev);
 	if (error) {
 		printk(KERN_INFO "error 1\n");
 		goto out_remove;
 	}
+	device_enable_async_suspend(&sdev->sdev_dev);
 	error = device_add(&sdev->sdev_dev);
 	if (error) {
 		printk(KERN_INFO "error 2\n");

Re: [PATCH 2/8] PM: Asynchronous suspend and resume of devices

[Alan Stern]

On Sun, 24 Jan 2010, Rafael J. Wysocki wrote:

> The async threads started for different devices as a result of
> calling async_schedule() are synchronized with each other and with
> the main suspend/resume thread with the help of completions, in the
> following way:
> (1) There is a completion, power.completion, for each device object.
> (2) Each device's completion is reset before calling async_schedule()
>     for the device or, in the case of devices with the
>     power.async_suspend flags unset, before executing the device's
>     suspend and resume callbacks.
> (3) During suspend, right before running the bus type, device type
>     and device class suspend callbacks for the device, the PM core
>     waits for the completions of all the device's children to be
>     completed.
> (4) During resume, right before running the bus type, device type and
>     device class resume callbacks for the device, the PM core waits
>     for the completion of the device's parent to be completed.
> (5) The PM core completes power.completion for each device right
>     after the bus type, device type and device class suspend (or
>     resume) callbacks executed for the device have returned.

...

>  /**
> + * dpm_wait - Wait for a PM operation to complete.
> + * @dev: Device to wait for.
> + * @async: If unset, wait only if the device's power.async_suspend flag is set.
> + */
> +static void dpm_wait(struct device *dev, bool async)
> +{
> +	if (!dev)
> +		return;
> +
> +	if (async || dev->power.async_suspend)
> +		wait_for_completion(&dev->power.completion);
> +}

There needs to be a public interface to this function available for 
drivers that have non-tree constraints.  The arguments should be the 
device to wait for and the device doing the waiting (needed only to 
determine whether the caller is running synchronously or not).

This will be necessary for USB.  In fact, your current code (with patch 
7/8 applied) is subject to a race that will sometimes cause a 
non-high-speed USB device to fail to resume from hibernation.

Alan Stern

Re: [PATCH 2/8] PM: Asynchronous suspend and resume of devices

[Rafael J. Wysocki]

[CC list trimmed.]

On Sunday 24 January 2010, Alan Stern wrote:
> On Sun, 24 Jan 2010, Rafael J. Wysocki wrote:
> 
> > The async threads started for different devices as a result of
> > calling async_schedule() are synchronized with each other and with
> > the main suspend/resume thread with the help of completions, in the
> > following way:
> > (1) There is a completion, power.completion, for each device object.
> > (2) Each device's completion is reset before calling async_schedule()
> >     for the device or, in the case of devices with the
> >     power.async_suspend flags unset, before executing the device's
> >     suspend and resume callbacks.
> > (3) During suspend, right before running the bus type, device type
> >     and device class suspend callbacks for the device, the PM core
> >     waits for the completions of all the device's children to be
> >     completed.
> > (4) During resume, right before running the bus type, device type and
> >     device class resume callbacks for the device, the PM core waits
> >     for the completion of the device's parent to be completed.
> > (5) The PM core completes power.completion for each device right
> >     after the bus type, device type and device class suspend (or
> >     resume) callbacks executed for the device have returned.
> 
> ...
> 
> >  /**
> > + * dpm_wait - Wait for a PM operation to complete.
> > + * @dev: Device to wait for.
> > + * @async: If unset, wait only if the device's power.async_suspend flag is set.
> > + */
> > +static void dpm_wait(struct device *dev, bool async)
> > +{
> > +	if (!dev)
> > +		return;
> > +
> > +	if (async || dev->power.async_suspend)
> > +		wait_for_completion(&dev->power.completion);
> > +}
> 
> There needs to be a public interface to this function available for 
> drivers that have non-tree constraints.  The arguments should be the 
> device to wait for and the device doing the waiting (needed only to 
> determine whether the caller is running synchronously or not).

Something like the patch below?

> This will be necessary for USB.  In fact, your current code (with patch 
> 7/8 applied) is subject to a race that will sometimes cause a 
> non-high-speed USB device to fail to resume from hibernation.

That's the kind of information I need at this point.  What exactly should be
added to patch 7/8 to avoid this problem?

Rafael

---
 drivers/base/power/main.c |   11 +++++++++++
 include/linux/pm.h        |    2 ++
 2 files changed, 13 insertions(+)

Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -1050,3 +1050,14 @@ void __suspend_report_result(const char 
 		printk(KERN_ERR "%s(): %pF returns %d\n", function, fn, ret);
 }
 EXPORT_SYMBOL_GPL(__suspend_report_result);
+
+/**
+ * device_pm_wait_for_dev - Wait for suspend/resume of a device to complete.
+ * @dev: Device to wait for.
+ * @subordinate: Device that needs to wait for @dev.
+ */
+void device_pm_wait_for_dev(struct device *subordinate, struct device *dev)
+{
+	dpm_wait(dev, subordinate->power.async_suspend);
+}
+EXPORT_SYMBOL_GPL(device_pm_wait_for_dev);
Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -512,6 +512,7 @@ extern void __suspend_report_result(cons
 		__suspend_report_result(__func__, fn, ret);		\
 	} while (0)
 
+extern void device_pm_wait_for_dev(struct device *sub, struct device *dev);
 #else /* !CONFIG_PM_SLEEP */
 
 #define device_pm_lock() do {} while (0)
@@ -524,6 +525,7 @@ static inline int dpm_suspend_start(pm_m
 
 #define suspend_report_result(fn, ret)		do {} while (0)
 
+static inline void device_pm_wait_for_dev(struct device *a, struct device *b) {}
 #endif /* !CONFIG_PM_SLEEP */
 
 /* How to reorder dpm_list after device_move() */

Re: [PATCH 2/8] PM: Asynchronous suspend and resume of devices

[Alan Stern]

On Sun, 24 Jan 2010, Rafael J. Wysocki wrote:

> > There needs to be a public interface to this function available for 
> > drivers that have non-tree constraints.  The arguments should be the 
> > device to wait for and the device doing the waiting (needed only to 
> > determine whether the caller is running synchronously or not).
> 
> Something like the patch below?

Yes, exactly.  Thanks.

> > This will be necessary for USB.  In fact, your current code (with patch 
> > 7/8 applied) is subject to a race that will sometimes cause a 
> > non-high-speed USB device to fail to resume from hibernation.
> 
> That's the kind of information I need at this point.  What exactly should be
> added to patch 7/8 to avoid this problem?

I don't think there's anything you can do about it, really.  It's an
obscure USB issue: If an EHCI controller was reset or loses power
during a system sleep (which always happens with hibernation and
sometimes happens with suspend-to-RAM, depending on the chipset and
firmware), then the EHCI controller must be resumed after all its
sibling companion OHCI/UHCI controllers, and USB devices below the
companion root hubs must be resumed after the EHCI root hub.

With synchronous resume this happens automatically because the devices
in question are always registered in the appropriate order.  But the
only way to meet these constraints with async resume is to add specific
code to the USB core to enforce the non-tree orderings.

I intend to write this code, but merging it will be a little tricky.  
You'll have to coordinate with Greg KH.

If you have a non-high-speed USB device, you can test to see if this
problem is not just a figment of my imagination.  To violate the first
constraint, add an ssleep(1) to the beginning of ohci_pci_resume() in
drivers/usb/host/ohci-pci.c or to the beginning of uhci_pci_resume() in
drivers/usb/host/uhci-hcd.c (depending on whether your companion
controllers are OHCI or UHCI).  To violate the second constraint, add
an ssleep(1) to the beginning of ehci_bus_resume() in
drivers/usb/host/ehci-hub.c.  Either constraint violation should cause
the device to be disconnected and then reconnected during an async
resume from hibernation, as opposed to simply being reset.

Alan Stern

Re: [PATCH 2/8] PM: Asynchronous suspend and resume of devices

[Rafael J. Wysocki]

On Sunday 24 January 2010, Alan Stern wrote:
> On Sun, 24 Jan 2010, Rafael J. Wysocki wrote:
> 
> > > There needs to be a public interface to this function available for 
> > > drivers that have non-tree constraints.  The arguments should be the 
> > > device to wait for and the device doing the waiting (needed only to 
> > > determine whether the caller is running synchronously or not).
> > 
> > Something like the patch below?
> 
> Yes, exactly.  Thanks.

OK, I'll put it into the patchset after 5/8.

> > > This will be necessary for USB.  In fact, your current code (with patch 
> > > 7/8 applied) is subject to a race that will sometimes cause a 
> > > non-high-speed USB device to fail to resume from hibernation.
> > 
> > That's the kind of information I need at this point.  What exactly should be
> > added to patch 7/8 to avoid this problem?
> 
> I don't think there's anything you can do about it, really.  It's an
> obscure USB issue: If an EHCI controller was reset or loses power
> during a system sleep (which always happens with hibernation and
> sometimes happens with suspend-to-RAM, depending on the chipset and
> firmware), then the EHCI controller must be resumed after all its
> sibling companion OHCI/UHCI controllers, and USB devices below the
> companion root hubs must be resumed after the EHCI root hub.
> 
> With synchronous resume this happens automatically because the devices
> in question are always registered in the appropriate order.  But the
> only way to meet these constraints with async resume is to add specific
> code to the USB core to enforce the non-tree orderings.
> 
> I intend to write this code, but merging it will be a little tricky.  
> You'll have to coordinate with Greg KH.

OK, I don't think that's a big deal.  I can defer patch 7/8 until that code has
been merged.

> If you have a non-high-speed USB device, you can test to see if this
> problem is not just a figment of my imagination.  To violate the first
> constraint, add an ssleep(1) to the beginning of ohci_pci_resume() in
> drivers/usb/host/ohci-pci.c or to the beginning of uhci_pci_resume() in
> drivers/usb/host/uhci-hcd.c (depending on whether your companion
> controllers are OHCI or UHCI).  To violate the second constraint, add
> an ssleep(1) to the beginning of ehci_bus_resume() in
> drivers/usb/host/ehci-hub.c.  Either constraint violation should cause
> the device to be disconnected and then reconnected during an async
> resume from hibernation, as opposed to simply being reset.

I'll try that, but my mkinitrd automatically puts the USB drivers into
initramfs.  I guess I'll need to do some research to really verify it. :-)

Rafael

Re: [PATCH 2/8] PM: Asynchronous suspend and resume of devices

[Alan Stern]

On Mon, 25 Jan 2010, Rafael J. Wysocki wrote:

> > I intend to write this code, but merging it will be a little tricky.  
> > You'll have to coordinate with Greg KH.
> 
> OK, I don't think that's a big deal.  I can defer patch 7/8 until that code has
> been merged.

You may have to delay 6/8 as well, since the controllers are PCI
devices.  Writing the new code shouldn't take too long, though.

Is there a good way to iterate through all PCI devices in a particular
slot, or should it be done by going through all PCI devices and
ignoring those in other slots?  Calling pci_get_slot() multiple times
doesn't look very efficient.


> I'll try that, but my mkinitrd automatically puts the USB drivers into
> initramfs.  I guess I'll need to do some research to really verify it. :-)

Then when you install the test kernel, mkinitrd should build a
corresponding initramfs image with the modified drivers, right?  
Otherwise there would be a version mismatch error when the init code
tried to load the old drivers into the new kernel.

If nothing else works, you can simply unload the standard uhci-hcd,
ohci-hcd, and ehci-hcd drivers and then modprobe the modified versions
before starting the hibernation.

Alan Stern

Re: [PATCH 2/8] PM: Asynchronous suspend and resume of devices

[Rafael J. Wysocki]

On Monday 25 January 2010, Alan Stern wrote:
> On Mon, 25 Jan 2010, Rafael J. Wysocki wrote:
> 
> > > I intend to write this code, but merging it will be a little tricky.  
> > > You'll have to coordinate with Greg KH.
> > 
> > OK, I don't think that's a big deal.  I can defer patch 7/8 until that code has
> > been merged.
> 
> You may have to delay 6/8 as well, since the controllers are PCI
> devices.  Writing the new code shouldn't take too long, though.

No problem with that.

Alternatively, if Greg agrees, I can add your patches modifying this into this
series.  Greg?

I wonder, though.  Since the controllers are PCI devices, we put them into D0
and restore their standard config spaces in the dpm_list order at the "early"
resume stage.  Doesn't that help here?

> Is there a good way to iterate through all PCI devices in a particular
> slot, or should it be done by going through all PCI devices and
> ignoring those in other slots?  Calling pci_get_slot() multiple times
> doesn't look very efficient.

Hmm, I don't know (added Jesse and Linux PCI to the CC list again).

> > I'll try that, but my mkinitrd automatically puts the USB drivers into
> > initramfs.  I guess I'll need to do some research to really verify it. :-)
> 
> Then when you install the test kernel, mkinitrd should build a
> corresponding initramfs image with the modified drivers, right?  
> Otherwise there would be a version mismatch error when the init code
> tried to load the old drivers into the new kernel.

Yes, but then they are loaded during restore before the image is read and
the hardware is initialized.  Would it still fail in that case?

> If nothing else works, you can simply unload the standard uhci-hcd,
> ohci-hcd, and ehci-hcd drivers and then modprobe the modified versions
> before starting the hibernation.

I guess I'll just build a special "restore" kernel without USB support.

Rafael

Re: [PATCH 2/8] PM: Asynchronous suspend and resume of devices

[Alan Stern]

On Mon, 25 Jan 2010, Rafael J. Wysocki wrote:

> > You may have to delay 6/8 as well, since the controllers are PCI
> > devices.  Writing the new code shouldn't take too long, though.
> 
> No problem with that.
> 
> Alternatively, if Greg agrees, I can add your patches modifying this into this
> series.  Greg?
> 
> I wonder, though.  Since the controllers are PCI devices, we put them into D0
> and restore their standard config spaces in the dpm_list order at the "early"
> resume stage.  Doesn't that help here?

No.  The problem is that the "late" resume routines in the host
controller drivers reinitialize the hardware, and existing connections
(i.e., from before the system sleep) may not be handled properly if the
controllers are initialized in the wrong order.


> > > I'll try that, but my mkinitrd automatically puts the USB drivers into
> > > initramfs.  I guess I'll need to do some research to really verify it. :-)
> > 
> > Then when you install the test kernel, mkinitrd should build a
> > corresponding initramfs image with the modified drivers, right?  
> > Otherwise there would be a version mismatch error when the init code
> > tried to load the old drivers into the new kernel.
> 
> Yes, but then they are loaded during restore before the image is read and
> the hardware is initialized.  Would it still fail in that case?

It wouldn't matter.  What the boot kernel does shouldn't affect what 
happens after the image starts running.

BTW, I should mention that these problems don't always occur 100% of 
the time, even when the order constraints are violated.  There may
still be other races that can affect the result.  But try it and see...

Alan Stern

Re: [PATCH 1/8] PM: Add parent information to timing messages

[Pavel Machek]

On Sun 2010-01-24 00:34:36, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Add parent information to the messages printed by the suspend/resume
> core when initcall_debug is set.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

ACK.

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