Prev: futex: Take mmap_sem for get_user_pages in fault_in_user_writeable
Next: [PATCH v9 0/8] Loongson: YeeLoong: add platform drivers
From: Alan Stern on 8 Dec 2009 21:30 On Tue, 8 Dec 2009, Rafael J. Wysocki wrote: > > Well, one difficulty. It arises only because we are contemplating > > having the PM core fire up the async tasks, rather than having the > > drivers' suspend routines launch them (the way your original proposal > > did -- the difficulty does not arise there). > > > > Suppose A and B are unrelated devices and we need to impose the > > off-tree constraint that A suspends after B. With children taking > > their parent's lock, the way to prevent A from suspending too soon is > > by having B's suspend routine acquire A's lock. > > > > But B's suspend routine runs entirely in an async task, because that > > task is started by the PM core and it does the method call. Hence by > > the time B's suspend routine is called, A may already have begun > > suspending -- it's too late to take A's lock. To make the locking > > work, B would have to acquire A's lock _before_ B's async task starts. > > Since the PM core is unaware of the off-tree dependency, there's no > > simple way to make it work. > > Do not set async_suspend for B and instead start your own async thread > from its suspend callback. The parent-children synchronization is done by the > core anyway (at least I'd do it that way), so the only thing you need to worry > about is the extra dependency. I don't like that because it introduces "artificial" dependencies: It makes B depend on all the preceding synchronous suspends, even totally unrelated ones. But yes, it would work. > I would be slightly more comfortable using completions, but the rwsem-based > approach is fine with me as well. On the principle of making things as easy and foolproof as possible for driver authors, I also favor completions since it makes dealing with non-tree dependencies easier. However either way would be okay. I do have to handle some non-tree dependencies in USB, but oddly enough they affect only resume, not suspend. So this "who starts the async task" issue doesn't apply. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on 8 Dec 2009 21:40 On Tue, 8 Dec 2009, Linus Torvalds wrote: > It's just that I think the "looping over children" is ugly, when I think > that by doing it the other way around you can make the code simpler and > only depend on the PM device list and a simple parent pointer access. I agree that it is uglier. The only advantage is in handling asynchronous non-tree suspend dependencies, of which we probably won't have very many. In fact, I don't know of _any_ offhand. Interestingly, this non-tree dependency problem does not affect resume. > I also think that you are wrong that the above somehow protects against > non-topological dependencies. If the device you want to keep delay > yourself suspending for is after you in the list, the down_read() on that > may succeed simply because it hasn't even done its down_write() yet and > you got scheduled early. You mean, if A comes before B in the list and A must suspend after B? Then A's down_read() on B _can't_ occur before B's down_write() on itself. The down_write() on B happens before the list_for_each_entry_reverse() iteration reaches A; it even happens before B's async task is launched. > But I guess you could do that by walking the list twice (first to lock > them all, then to actually call the suspend function). That whole > two-phase thing, except the first phase _only_ locks, and doesn't do any > callbacks. Not necessary. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Linus Torvalds on 8 Dec 2009 22:00 On Tue, 8 Dec 2009, Alan Stern wrote: > > You mean, if A comes before B in the list and A must suspend after B? But if they are not topologically ordered, then A wouldn't necessarily be before B on the list in the first place. Of course, if we've mucked with the list by hand and made sure the ordering is ok, then that's a different issue. But your whole point seemed to be that the device could impose its own ordering in its suspend callback, which is not true on its own without external ordering. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Mark Brown on 9 Dec 2009 08:40 On Tue, Dec 08, 2009 at 09:35:59PM -0500, Alan Stern wrote: > On Tue, 8 Dec 2009, Linus Torvalds wrote: > > It's just that I think the "looping over children" is ugly, when I think > > that by doing it the other way around you can make the code simpler and > > only depend on the PM device list and a simple parent pointer access. > I agree that it is uglier. The only advantage is in handling > asynchronous non-tree suspend dependencies, of which we probably won't > have very many. In fact, I don't know of _any_ offhand. There's some potential for this in embedded audio - it wants to bring down the entire embedded audio subsystem at once before the individual devices (and their parents) get suspended since bringing them down out of sync can result in audible artifacts. Depending on the system the suspend may take a noticable amount of time so it'd be nice to be able to run it asynchronously, though we don't currently do so. At the minute we get away with this mostly through not being able to represent the cases that are likely to actually trip up over it. > Interestingly, this non-tree dependency problem does not affect resume. Embedded audio does potentially - the resume needs all the individual devices in the subsystem and can take a substantial proportion of the overall resume time. Currently we get away with a combination of assuming that all the drivers are live when we decide to start resuming them and using the ALSA userspace API to deal with bringing the resume out of line, but it's not ideal. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on 9 Dec 2009 10:30
On Tue, 8 Dec 2009, Linus Torvalds wrote: > On Tue, 8 Dec 2009, Alan Stern wrote: > > > > You mean, if A comes before B in the list and A must suspend after B? > > But if they are not topologically ordered, then A wouldn't necessarily be > before B on the list in the first place. Okay, I see what you're getting at. Yes, this is quite true -- if A doesn't precede B in dpm_list then A can't safely wait for B to suspend. To put it another way, only list-compatible constraints are feasible. This shouldn't be a problem. If it were we'd be seeing it right now, because A would _always_ suspend before B. > Of course, if we've mucked with the list by hand and made sure the > ordering is ok, then that's a different issue. But your whole point seemed > to be that the device could impose its own ordering in its suspend > callback, which is not true on its own without external ordering. No, sorry for not making it clearer. I was assuming all long that the non-tree constraints were compatible with the list ordering. In fact these considerations already affect the USB resume operations, even without asynchronous resume. The code relies on the fact that the PCI layer registers sibling devices on a slot in order of increasing function number. There's no guarantee this will remain true in the future (it may already be wrong AFAIK), so putting in some explicit list manipulation is the prudent thing to do. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |