parallel suspend/resume

parallel suspend/resume

[Marcelo Tosatti]

Hi,

I spent some time working with the parallel suspend/resume idea, even
wrote a simple patch (attached), but that obviously does not scale to
large number of devices.

What would be a better approach? 

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 05dc876..45f5851 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -62,6 +62,8 @@ int device_pm_add(struct device * dev)
 	down(&dpm_list_sem);
 	list_add_tail(&dev->power.entry, &dpm_active);
 	device_pm_set_parent(dev, dev->parent);
+	init_waitqueue_head(&dev->power.powered_on);
+	INIT_LIST_HEAD(&dev->power.entry_sus);
 	if ((error = dpm_sysfs_add(dev)))
 		list_del(&dev->power.entry);
 	up(&dpm_list_sem);
diff --git a/drivers/base/power/resume.c b/drivers/base/power/resume.c
index a2c6418..a3de7d6 100644
--- a/drivers/base/power/resume.c
+++ b/drivers/base/power/resume.c
@@ -9,10 +9,13 @@
  */
 
 #include <linux/device.h>
+#include <linux/sched.h>
+#include <linux/kthread.h>
 #include <linux/resume-trace.h>
 #include "../base.h"
 #include "power.h"
 
+LIST_HEAD(suspend_threads);
 
 /**
  *	resume_device - Restore state for one device.
@@ -40,24 +43,61 @@ int resume_device(struct device * dev)
 	if (dev->bus && dev->bus->resume) {
 		dev_dbg(dev,"resuming\n");
 		error = dev->bus->resume(dev);
+		dev_dbg(dev,"finish\n");
 	}
 
 	if (!error && dev->type && dev->type->resume) {
 		dev_dbg(dev,"resuming\n");
 		error = dev->type->resume(dev);
+		dev_dbg(dev,"finish\n");
 	}
 
 	if (!error && dev->class && dev->class->resume) {
 		dev_dbg(dev,"class resume\n");
 		error = dev->class->resume(dev);
+		dev_dbg(dev,"finish\n");
 	}
 
+	dev->power.power_state = PMSG_ON;
+
 	up(&dev->sem);
 
 	TRACE_RESUME(error);
+
 	return error;
 }
 
+static int resume_device_work(void *data)
+{
+	int error;
+	struct device *dev = (struct device *)data;
+
+	if (dev->power.pm_parent
+			&& dev->power.pm_parent->power.power_state.event) {
+		dev_err(dev, "PM: resume from %d, parent %s still %d\n",
+			dev->power.power_state.event,
+			dev->power.pm_parent->bus_id,
+			dev->power.pm_parent->power.power_state.event);
+		wait_event_interruptible(dev->power.pm_parent->power.powered_on,
+					!dev->power.pm_parent->power.power_state.event);
+	}
+
+	error = resume_device(dev);
+	if (!error) 
+		wake_up_all(&dev->power.powered_on);
+
+	put_device(dev);
+	return error;
+}
+
+int span_resume_thread(struct device *dev)
+{
+	dev->power.resume_thread = kthread_run(resume_device_work, dev,
+				"resume-%s", dev_driver_string(dev));
+	list_add(&dev->power.entry_sus, &suspend_threads);
+	return 0;
+}
+
 
 static int resume_device_early(struct device * dev)
 {
@@ -80,6 +120,8 @@ static int resume_device_early(struct de
  */
 void dpm_resume(void)
 {
+	struct dev_pm_info *dev;
+
 	down(&dpm_list_sem);
 	while(!list_empty(&dpm_off)) {
 		struct list_head * entry = dpm_off.next;
@@ -90,11 +132,15 @@ void dpm_resume(void)
 
 		up(&dpm_list_sem);
 		if (!dev->power.prev_state.event)
-			resume_device(dev);
+			span_resume_thread(dev);
 		down(&dpm_list_sem);
-		put_device(dev);
 	}
 	up(&dpm_list_sem);
+
+	list_for_each_entry(dev, &suspend_threads, entry_sus)
+		wait_event_interruptible(dev->powered_on,
+					!dev->power_state.event);
+	INIT_LIST_HEAD(&suspend_threads);
 }
 
 
diff --git a/drivers/base/power/suspend.c b/drivers/base/power/suspend.c
index 42d2b86..3f6e9b0 100644
--- a/drivers/base/power/suspend.c
+++ b/drivers/base/power/suspend.c
@@ -169,8 +169,10 @@ int device_suspend(pm_message_t state)
 		/* Check if the device got removed */
 		if (!list_empty(&dev->power.entry)) {
 			/* Move it to the dpm_off list */
-			if (!error)
+			if (!error) {
+				dev->power.power_state = PMSG_SUSPEND;
 				list_move(&dev->power.entry, &dpm_off);
+			}
 		}
 		if (error)
 			printk(KERN_ERR "Could not suspend device %s: "
diff --git a/include/linux/pm.h b/include/linux/pm.h
index b2c4fde..4bfa8da 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -24,6 +24,7 @@ #define _LINUX_PM_H
 #ifdef __KERNEL__
 
 #include <linux/list.h>
+#include <linux/wait.h>
 #include <asm/atomic.h>
 
 /*
@@ -271,6 +272,9 @@ #ifdef	CONFIG_PM
 	void			* saved_state;
 	struct device		* pm_parent;
 	struct list_head	entry;
+	struct task_struct 	* resume_thread;
+	wait_queue_head_t 	powered_on;
+	struct list_head entry_sus;
 #endif
 };
 

Re: parallel suspend/resume

[Alan Stern]

On Mon, 8 Oct 2007, Marcelo Tosatti wrote:

> Hi,
> 
> I spent some time working with the parallel suspend/resume idea, even
> wrote a simple patch (attached), but that obviously does not scale to
> large number of devices.
> 
> What would be a better approach? 

In general, parallel suspend/resume isn't easy to do correctly.  There 
are lots of constraints on the order in which devices can be suspended, 
many of them not explicitly stated anywhere.  I don't think it is 
feasible to encapsulate the constraint information in the PM core; if 
anything, it should be left up to the individual subsystems to suspend 
or resume their devices in parallel.

Alan Stern

Re: parallel suspend/resume

[Pavel Machek]

On Mon 2007-10-08 15:46:39, Marcelo Tosatti wrote:
> Hi,
> 
> I spent some time working with the parallel suspend/resume idea, even
> wrote a simple patch (attached), but that obviously does not scale to
> large number of devices.
> 
> What would be a better approach? 

No idea, but thread per device does not seem _that_ bad. Debuggability
worries me more.

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

Re: parallel suspend/resume

[Alan Stern]

On Fri, 7 Dec 2007, Pavel Machek wrote:

> On Mon 2007-10-08 15:46:39, Marcelo Tosatti wrote:
> > Hi,
> > 
> > I spent some time working with the parallel suspend/resume idea, even
> > wrote a simple patch (attached), but that obviously does not scale to
> > large number of devices.
> > 
> > What would be a better approach? 
> 
> No idea, but thread per device does not seem _that_ bad. Debuggability
> worries me more.

What about ordering constraints?  A lot of them aren't explicit.  We 
just depend on the fact that devices get suspended in reverse order of 
registration.  You can't do that in parallel.

Alan Stern

Re: parallel suspend/resume

[David Brownell]

> What about ordering constraints?  A lot of them aren't explicit.  We 
> just depend on the fact that devices get suspended in reverse order of 
> registration.  You can't do that in parallel.

Those constraints should *become* explicit ... not stay implicit!

FWIW the appended patch removes that rude "order of registration"
policy, so that the suspend/resume list matches the device tree.
It's behaved OK on PCs and, in light duty, a few development boards;
I've carried it around most of this year.

I'd expect this to eventually turn up some cases where Linux is
right now doing a bad job of exposing devices with multiple such
ordering constraint ... power management via I2C seems likely to
turn up a few of them.  We'd need some automatic way to detect
and report such problems so that they could be fixed.

- Dave


============= SNIP!
Change how the PM list is constructed, so that devices are added right
after their parents (when they have one) rather than at the end of the
list.  This preserves sequencing guarantees, but enables sequencing of
suspend/resume operations by more important characteristics than "when
device happened to enumerate" ... e.g. clocksources and clockevents
at a clearly defined point during suspend and resume, power management
chips nearer their I2C parent.

This patch has a potential downside for devices that have multiple
power dependencies and which "just happened to work" before.  Or
looking at it the other way around:  a potential upside for devices
that didn't work before.

---
 drivers/base/power/main.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

--- g26.orig/drivers/base/power/main.c	2007-11-29 20:06:04.000000000 -0800
+++ g26/drivers/base/power/main.c	2007-11-29 20:11:38.000000000 -0800
@@ -40,11 +40,17 @@ int (*platform_enable_wakeup)(struct dev
 
 void device_pm_add(struct device *dev)
 {
-	pr_debug("PM: Adding info for %s:%s\n",
+	struct device *parent = dev->parent;
+
+	pr_debug("PM: Adding info for %s:%s after %s\n",
 		 dev->bus ? dev->bus->name : "No Bus",
-		 kobject_name(&dev->kobj));
+		 kobject_name(&dev->kobj),
+		 parent ? parent->bus_id : "(no parent)");
 	mutex_lock(&dpm_list_mtx);
-	list_add_tail(&dev->power.entry, &dpm_active);
+	if (parent)
+		list_add(&dev->power.entry, &parent->power.entry);
+	else
+		list_add_tail(&dev->power.entry, &dpm_active);
 	mutex_unlock(&dpm_list_mtx);
 }
 

Re: parallel suspend/resume

[Alan Stern]

On Fri, 7 Dec 2007, David Brownell wrote:

> > What about ordering constraints?  A lot of them aren't explicit.  We 
> > just depend on the fact that devices get suspended in reverse order of 
> > registration.  You can't do that in parallel.
> 
> Those constraints should *become* explicit ... not stay implicit!

Why do I get the feeling we've gone through this before?  :-)

> FWIW the appended patch removes that rude "order of registration"

"Rude"?  What's rude about it?  It seems most logical to me: Since the
devices obviously worked in the intermediate stages of registration,
when the early-to-register devices were available and the
late-to-register devices weren't, duplicating those same intermediate
stages (with the same sets of devices available) should also work
during suspend.

> policy, so that the suspend/resume list matches the device tree.

In what respect doesn't it match the device tree now?

As far as I know, the only way in which the list wouldn't match the
tree is when the tree gets reorganized by device_move(), which doesn't
change the list at all.  (This is quite possibly a bug.)  Your patch 
doesn't affect that.

> It's behaved OK on PCs and, in light duty, a few development boards;
> I've carried it around most of this year.

I predict it will cause failure of the USB-Persist mechanism when a 
non-high-speed device is plugged into a dual high/full-speed port.  
This is because when an EHCI root hub is resumed after a loss of power, 
the driver looks for ports that had been owned by the companion 
controllers and tries to hand them over again -- but if the companion 
controllers haven't been resumed yet the handover won't work.

However to be fair, about the best one can say for the current way this 
situation is handled is that it works more or less by coincidence (EHCI 
controllers are required by the spec to have higher function numbers 
than their companion controllers on a PCI board, and PCI devices are 
enumerated in increasing order by function number).  I have to agree 
that this ordering dependency really should be made explicit.

> I'd expect this to eventually turn up some cases where Linux is
> right now doing a bad job of exposing devices with multiple such
> ordering constraint ... power management via I2C seems likely to
> turn up a few of them.  We'd need some automatic way to detect
> and report such problems so that they could be fixed.

If you really want to sniff out dependencies, you should add each new
device to the list at a _random_ position, subject only to the
constraint that it must not precede its parent!

Alan Stern

Re: parallel suspend/resume

[Oliver Neukum]

Am Freitag, 7. Dezember 2007 19:01:12 schrieb David Brownell:
> FWIW the appended patch removes that rude "order of registration"
> policy, so that the suspend/resume list matches the device tree.
> It's behaved OK on PCs and, in light duty, a few development boards;
> I've carried it around most of this year.

As it is a tree, why not store it as such?

	Regards
		Oliver

Re: parallel suspend/resume

[Pavel Machek]

On Sat 2007-12-08 09:00:44, Oliver Neukum wrote:
> Am Freitag, 7. Dezember 2007 19:01:12 schrieb David Brownell:
> > FWIW the appended patch removes that rude "order of registration"
> > policy, so that the suspend/resume list matches the device tree.
> > It's behaved OK on PCs and, in light duty, a few development boards;
> > I've carried it around most of this year.
> 
> As it is a tree, why not store it as such?

IIRC because we do not want recursive tree walkers in the kernel --
stack limits.

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

Re: parallel suspend/resume

[James Courtier-Dutton]

Pavel Machek wrote:
> On Sat 2007-12-08 09:00:44, Oliver Neukum wrote:
>   
>> Am Freitag, 7. Dezember 2007 19:01:12 schrieb David Brownell:
>>     
>>> FWIW the appended patch removes that rude "order of registration"
>>> policy, so that the suspend/resume list matches the device tree.
>>> It's behaved OK on PCs and, in light duty, a few development boards;
>>> I've carried it around most of this year.
>>>       
>> As it is a tree, why not store it as such?
>>     
>
> IIRC because we do not want recursive tree walkers in the kernel --
> stack limits.
>
>   

Surely it is possible to code a tree walker that is not a recursive 
function?
I believe you can use a simple loop and the heap to store state that 
would otherwise be stored on the stack in a recursive function.

Re: parallel suspend/resume

[Oliver Neukum]

Am Samstag, 8. Dezember 2007 11:21:43 schrieb Pavel Machek:
> On Sat 2007-12-08 09:00:44, Oliver Neukum wrote:
> > Am Freitag, 7. Dezember 2007 19:01:12 schrieb David Brownell:
> > > FWIW the appended patch removes that rude "order of registration"
> > > policy, so that the suspend/resume list matches the device tree.
> > > It's behaved OK on PCs and, in light duty, a few development boards;
> > > I've carried it around most of this year.
> > 
> > As it is a tree, why not store it as such?
> 
> IIRC because we do not want recursive tree walkers in the kernel --
> stack limits.

Then use a non-recursive algorithm.

	Regards
		Oliver

Re: parallel suspend/resume

[Oliver Neukum]

Am Samstag, 8. Dezember 2007 12:08:41 schrieb James Courtier-Dutton:
> Pavel Machek wrote:
> > On Sat 2007-12-08 09:00:44, Oliver Neukum wrote:
> >   
> >> Am Freitag, 7. Dezember 2007 19:01:12 schrieb David Brownell:
> >>     
> >>> FWIW the appended patch removes that rude "order of registration"
> >>> policy, so that the suspend/resume list matches the device tree.
> >>> It's behaved OK on PCs and, in light duty, a few development boards;
> >>> I've carried it around most of this year.
> >>>       
> >> As it is a tree, why not store it as such?
> >>     
> >
> > IIRC because we do not want recursive tree walkers in the kernel --
> > stack limits.
> >
> >   
> 
> Surely it is possible to code a tree walker that is not a recursive 
> function?
> I believe you can use a simple loop and the heap to store state that 
> would otherwise be stored on the stack in a recursive function.

If you store a pointer to the parent in each node, you can do with
static memory of only the current node and the previous node.

	Regards
		Oliver

Re: parallel suspend/resume

[Alan Stern]

On Sat, 8 Dec 2007, Oliver Neukum wrote:

> Am Freitag, 7. Dezember 2007 19:01:12 schrieb David Brownell:
> > FWIW the appended patch removes that rude "order of registration"
> > policy, so that the suspend/resume list matches the device tree.
> > It's behaved OK on PCs and, in light duty, a few development boards;
> > I've carried it around most of this year.
> 
> As it is a tree, why not store it as such?

There's no need to "store" the tree ordering specially, since all the
pointers already exist.  The question is: In what order should the tree
be traversed?  About the only explicit constraint we have now is that
children must be suspended before their parents, but there undoubtedly
are plenty of undocumented implicit constraints (maybe some of them
aren't known to anybody at all).

Given the vast number of possible orders, and given that the only order 
we _know_ works correctly is reverse order of registration, I don't see 
any big reason to change.  Speeding things up by parallel suspension 
would be a valid reason, but it needs to be done with a great deal of 
care.

Alan Stern

Re: parallel suspend/resume

[Alan Stern]

On Fri, 7 Dec 2007, David Brownell wrote:

> FWIW the appended patch removes that rude "order of registration"
> policy, so that the suspend/resume list matches the device tree.
> It's behaved OK on PCs and, in light duty, a few development boards;
> I've carried it around most of this year.

Actually I'm surprised that it works with USB devices.

As they are registered, you patch adds new devices to the list
immediately after their parents.  This means that the children of a
particular device will be listed in reverse order of registration,
right?

So if you have a USB hub, the hub's children will be added following 
the hub, and the hub's interface will come after all the children since 
it gets registered before them.  Hence during suspend, the interface's 
suspend method will be called before any of the children are suspended.  
But hub_suspend() will fail if there are unsuspended children.

This is an example of another implicit order dependency: A hub's 
interface must be suspended after all the other children of that hub.  
It's an artifact of the way USB devices are represented in the device 
hierarchy; strictly speaking they should be children of the hub's 
interface, not of the hub itself.

Alan Stern

Re: parallel suspend/resume

[Rafael J. Wysocki]

On Saturday, 8 of December 2007, Alan Stern wrote:
> On Sat, 8 Dec 2007, Oliver Neukum wrote:
> 
> > Am Freitag, 7. Dezember 2007 19:01:12 schrieb David Brownell:
> > > FWIW the appended patch removes that rude "order of registration"
> > > policy, so that the suspend/resume list matches the device tree.
> > > It's behaved OK on PCs and, in light duty, a few development boards;
> > > I've carried it around most of this year.
> > 
> > As it is a tree, why not store it as such?
> 
> There's no need to "store" the tree ordering specially, since all the
> pointers already exist.  The question is: In what order should the tree
> be traversed?  About the only explicit constraint we have now is that
> children must be suspended before their parents, but there undoubtedly
> are plenty of undocumented implicit constraints (maybe some of them
> aren't known to anybody at all).

Yes, that makes me nervous every time someone suggests to change the ordering
of suspending devices.

> Given the vast number of possible orders, and given that the only order 
> we _know_ works correctly is reverse order of registration, I don't see 
> any big reason to change.  Speeding things up by parallel suspension 
> would be a valid reason, but it needs to be done with a great deal of 
> care.

Agreed.

Greetings,
Rafael

Re: parallel suspend/resume

[Rafael J. Wysocki]

On Friday, 7 of December 2007, Alan Stern wrote:
> On Fri, 7 Dec 2007, David Brownell wrote:
> 
> > > What about ordering constraints?  A lot of them aren't explicit.  We 
> > > just depend on the fact that devices get suspended in reverse order of 
> > > registration.  You can't do that in parallel.
> > 
> > Those constraints should *become* explicit ... not stay implicit!
> 
> Why do I get the feeling we've gone through this before?  :-)
> 
> > FWIW the appended patch removes that rude "order of registration"
> 
> "Rude"?  What's rude about it?  It seems most logical to me: Since the
> devices obviously worked in the intermediate stages of registration,
> when the early-to-register devices were available and the
> late-to-register devices weren't, duplicating those same intermediate
> stages (with the same sets of devices available) should also work
> during suspend.
> 
> > policy, so that the suspend/resume list matches the device tree.
> 
> In what respect doesn't it match the device tree now?
> 
> As far as I know, the only way in which the list wouldn't match the
> tree is when the tree gets reorganized by device_move(), which doesn't
> change the list at all.  (This is quite possibly a bug.)

IMO, this _is_ a bug.  I think device_move() should move the device in question
to the end of the list along with its children.

That might cause some breakage in the short run, but we'll have to do it at one
point.

Greetings,
Rafael

Re: parallel suspend/resume

[Alan Stern]

On Sat, 8 Dec 2007, Alan Stern wrote:

> On Fri, 7 Dec 2007, David Brownell wrote:
> 
> > FWIW the appended patch removes that rude "order of registration"
> > policy, so that the suspend/resume list matches the device tree.
> > It's behaved OK on PCs and, in light duty, a few development boards;
> > I've carried it around most of this year.
> 
> Actually I'm surprised that it works with USB devices.
> 
> As they are registered, you patch adds new devices to the list
> immediately after their parents.  This means that the children of a
> particular device will be listed in reverse order of registration,
> right?
> 
> So if you have a USB hub, the hub's children will be added following 
> the hub, and the hub's interface will come after all the children since 
> it gets registered before them.  Hence during suspend, the interface's 
> suspend method will be called before any of the children are suspended.  
> But hub_suspend() will fail if there are unsuspended children.

I take it back.  Interfaces get suspended along with their device;
hence it doesn't matter in what order the interface is listed with
respect to the children.

Alan Stern

Re: parallel suspend/resume

[Oliver Neukum]

Am Samstag, 8. Dezember 2007 16:43:25 schrieb Alan Stern:
> On Sat, 8 Dec 2007, Oliver Neukum wrote:
> 
> > Am Freitag, 7. Dezember 2007 19:01:12 schrieb David Brownell:
> > > FWIW the appended patch removes that rude "order of registration"
> > > policy, so that the suspend/resume list matches the device tree.
> > > It's behaved OK on PCs and, in light duty, a few development boards;
> > > I've carried it around most of this year.
> > 
> > As it is a tree, why not store it as such?
> 
> There's no need to "store" the tree ordering specially, since all the
> pointers already exist.  The question is: In what order should the tree

Nevertheless, we currently have a list. Why? To reverse the temporal order
only?

> be traversed?  About the only explicit constraint we have now is that
> children must be suspended before their parents, but there undoubtedly
> are plenty of undocumented implicit constraints (maybe some of them
> aren't known to anybody at all).

We will need to know them for runtime pm.

> Given the vast number of possible orders, and given that the only order 
> we _know_ works correctly is reverse order of registration, I don't see 

modus advocati diaboli:

Suppose I have a system with a FibreChannel disk. Now I hot plug another
FibreChannel controller and connect it to the disk. Then I disconnect the disk
from the original controller. What will happen if I suspend the system?

> any big reason to change.  Speeding things up by parallel suspension 
> would be a valid reason, but it needs to be done with a great deal of 
> care.

catch-22.

	Regards
		Oliver

Re: parallel suspend/resume

[Alan Stern]

On Sun, 9 Dec 2007, Oliver Neukum wrote:

> Am Samstag, 8. Dezember 2007 16:43:25 schrieb Alan Stern:
> > On Sat, 8 Dec 2007, Oliver Neukum wrote:
> > 
> > > Am Freitag, 7. Dezember 2007 19:01:12 schrieb David Brownell:
> > > > FWIW the appended patch removes that rude "order of registration"
> > > > policy, so that the suspend/resume list matches the device tree.
> > > > It's behaved OK on PCs and, in light duty, a few development boards;
> > > > I've carried it around most of this year.
> > > 
> > > As it is a tree, why not store it as such?
> > 
> > There's no need to "store" the tree ordering specially, since all the
> > pointers already exist.  The question is: In what order should the tree
> 
> Nevertheless, we currently have a list. Why? To reverse the temporal order
> only?

As far as I know, yes.  Maybe also to simplify the traversal; it takes 
less code to follow a list than to go through a tree.

> > be traversed?  About the only explicit constraint we have now is that
> > children must be suspended before their parents, but there undoubtedly
> > are plenty of undocumented implicit constraints (maybe some of them
> > aren't known to anybody at all).
> 
> We will need to know them for runtime pm.

Maybe.  But that knowledge will be localized in drivers and not 
consolidated in the PM core, since that's where runtime PM is handled.  
What we don't have is any way for drivers to export the knowledge.

> > Given the vast number of possible orders, and given that the only order 
> > we _know_ works correctly is reverse order of registration, I don't see 
> 
> modus advocati diaboli:
> 
> Suppose I have a system with a FibreChannel disk. Now I hot plug another
> FibreChannel controller and connect it to the disk. Then I disconnect the disk
> from the original controller. What will happen if I suspend the system?

That seems a little strange.  What happens during the intermediate
stage when you have a single disk connected to the computer by two I/O
pathways?  Will the OS think it is really two disks?  Will it get
confused by seeing on-disk data structures in unstable incomplete
states and subject to apparently random updates?

A better question is what happens when a Bluetooth communications
driver creates a new tty device and then uses device_move() to put the
Bluetooth device under the newer tty.  Will the tty get suspended
before its Bluetooth child?

Alan Stern

Re: parallel suspend/resume

[David Brownell]

On Friday 07 December 2007, Alan Stern wrote:
> On Fri, 7 Dec 2007, David Brownell wrote:
> 
> > > What about ordering constraints?  A lot of them aren't explicit.  We 
> > > just depend on the fact that devices get suspended in reverse order of 
> > > registration.  You can't do that in parallel.
> > 
> > Those constraints should *become* explicit ... not stay implicit!
> 
> Why do I get the feeling we've gone through this before?  :-)

Why indeed.


> > FWIW the appended patch removes that rude "order of registration"
> 
> "Rude"?  What's rude about it?  It seems most logical to me: Since the
> devices obviously worked in the intermediate stages of registration,
> ...

Rude because it keeps things implicit which should be explicit.


> > policy, so that the suspend/resume list matches the device tree.
> 
> In what respect doesn't it match the device tree now?

It's neither a depth-first nor a breadth-first traversal; it's
more of a random node visitation based on what devices were
initialized when.

Such a random visitation seems like a clear problem when a
goal is to introduce parallelism ...


> As far as I know, the only way in which the list wouldn't match the
> tree is when the tree gets reorganized by device_move(), which doesn't
> change the list at all.  (This is quite possibly a bug.)  Your patch 
> doesn't affect that.

Right, device_move() should update the list.  Different issue.


> > It's behaved OK on PCs and, in light duty, a few development boards;
> > I've carried it around most of this year.
> 
> ...  I have to agree 
> that this ordering dependency really should be made explicit.

Good.  :)


> > I'd expect this to eventually turn up some cases where Linux is
> > right now doing a bad job of exposing devices with multiple such
> > ordering constraint ... power management via I2C seems likely to
> > turn up a few of them.  We'd need some automatic way to detect
> > and report such problems so that they could be fixed.
> 
> If you really want to sniff out dependencies, you should add each new
> device to the list at a _random_ position, subject only to the
> constraint that it must not precede its parent!

Nope; what we need is the ability to finger "this specific device
has dependencies that aren't explicit".  Being able to introduce
random failures isn't really helpful.  It's the difference between
being able to use lockdep vs. having to wait until a random locking
failure crops up in a way that can usefully be diagnosed.

- Dave

Re: parallel suspend/resume

[Alan Stern]

On Sun, 9 Dec 2007, David Brownell wrote:

> > > FWIW the appended patch removes that rude "order of registration"
> > 
> > "Rude"?  What's rude about it?  It seems most logical to me: Since the
> > devices obviously worked in the intermediate stages of registration,
> > ...
> 
> Rude because it keeps things implicit which should be explicit.

Your patch isn't really much better (IMHO).  While its order may be
less "random" in the sense that it is determined by the device tree,
the tree itself is somewhat random and not fully determined by the
physical hardware layout.  It is affected by the order in which drivers
are loaded, or the order in which PCI cards are probed, and so on.

And in any case, having a fixed order doesn't move us any closer to the 
goal of making all the dependencies explicit.

> Such a random visitation seems like a clear problem when a
> goal is to introduce parallelism ...

So does a strict depth-first visitation.  Or indeed, any fixed 
ordering, since parallelism will involve indeterminacy.

> Nope; what we need is the ability to finger "this specific device
> has dependencies that aren't explicit".  Being able to introduce
> random failures isn't really helpful.  It's the difference between
> being able to use lockdep vs. having to wait until a random locking
> failure crops up in a way that can usefully be diagnosed.

I agree.  Even better would be to say "this specific device has this 
specific dependency".  But it's not easy to come up with strategies for 
finding this information.  There's no clear-cut analog to lockdep.

Alan Stern