Alan Stern wrote:
On Wed, 22 Sep 2004, David Brownell wrote:
The root cause of the self-deadlocks is that the driver model
code has to be re-entrant, so that logic in suspend/resume
paths can do things that can be done at other times too. That
means the locking mustn't be context-dependent, and the lock
scope must shrink to something sane (like, not held during
potentially long-lasting outcalls).
The context-dependency problems are solved by making the semaphore public,
not private to the PM core. If the core required callers to hold the
lock, and always invoked callbacks with the lock held, then there wouldn't
be any deadlock.
Yeech. Even if it were necessary -- which it is not! -- I say: Yeech.
Public locks are error prone, even in controlled environments; and
I seem to have noticed that Linux isn't particularly controlled!
The scope problem is something else. I'm saying that by increasing the
scope greatly, deadlock can be prevented. Of course this just worsens the
situation with regard to potentially long-lasting callbacks and such. In
fact it makes things as bad as possible: all operations are completely
serialized. That's why I didn't say this was the right thing to do -- I
only said it would prevent deadlock.
That's only part of what "worsens"! :)
I see what you mean about preventing deadlock, but then if every
driver is going to be lock-aware why not make the scopes _small_
so that those structural problems are minimized?
(In fact, it's the push-locking-outside step which is what solves
the reentrancy design issues. It wasn't clear to me from what
you said before that you had such a step.)
By the way, the PM lists aren't necessary. By using the existing list
pointers in the device structure it's possible to do a proper tree
traversal with bounded stack usage. I don't know if anyone's interested
in that...
I've had the same thought about bounded memory, but I don't think
they'd work well without separate PM lists. At least without
adding rude "whole system is serialized" constraints ...
I do want to add PMcore primitives to walk a (sub)tree in constant
stak space, so that USB can rely on that to ensure subtrees (as for
example USB hub to USB-storage to SCSI-Host to SCSI-Disk) can work
even when they jump out of the USB domain.
The existing PM data structures aren't quite up to the job. But if the
children of each parent were linked in a "siblings" list (which struct
device does include but struct dev_pm_info doesn't) it would be possible.
Yes. If pm_info had a list entry, normally empty, then it could
be used to flatten any subtree prior to something (USBcore, PMcore, etc)
walking the list and then removing the entries.
Here's the algorithm for the device tree (not the PM tree). Locking
optional... :-)
I was thinking of something different, something like
this for a prefix walk (as for resume):
walk_subtree(DEV, visitor)
list_head MYLIST = ..initialize me..;
grab flatten lock;
if dev->pm.list not empty
fail;
add DEV to MYLIST using pm.list
list_for_each ENTRY in MYLIST safely
list_for_each CHILD in ENTRY->children
if CHILD->pm.list not empty
fail
append CHILD to mylist using pm.list
drop flatten lock
// front-to-back walk for resume
// back-to-front walk for suspend
list_for_each ENTRY in MYLIST safely
visit(ENTRY)
delete from ENTRY->pm.list
VOILA!
Clearly those "fail" cases need attention; they could maybe
just wait_event. Little glitches: hey, did you notice
that the "PM Tree" is really a forest? Multiple roots.
One difference between that scheme and the one you sketched
is that the "fail" cases exist ... so that when two tasks
are concurrently working on subtrees, they get sequenced
properly. Yes, locktree() does that too... :)
- Dave
void walk_sub_tree(struct device *start_dev) {
struct device *d = start_dev;
for (;;) {
// Pre-order traversal tasks go here
if (!list_empty(d->children)) {
d = d->children.next;
continue;
}
up_one_level:
// Post-order traversal tasks go here
if (d == start_dev)
break;
if (d->parent == d->node.next) {
d = d->parent;
goto up_one_level;
}
d = d->node.next;
}
}
You might want to change the .next's to .prev's for the resume traversal,
to make it the reverse of the suspend traversal.
Alan Stern
-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
