From mboxrd@z Thu Jan 1 00:00:00 1970 From: grant.likely@secretlab.ca (Grant Likely) Date: Fri, 14 Oct 2011 12:25:03 -0600 Subject: [PATCH 2/5] drivercore: Add driver probe deferral mechanism In-Reply-To: References: Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, Oct 14, 2011 at 11:33 AM, Alan Stern wrote: > On Fri, 14 Oct 2011, Grant Likely wrote: > >> > I don't think the second part needs to be quite so invasive. >> > Certainly drivers that never defer probes shouldn't require anything to >> > be moved. >> >> Think about that a minute. ?Consider a dpm_list of devices: >> ? ? ? ? abcDefGh >> >> Now assume that D has an implicit dependency on G. ?If D gets probed >> first, then it will be deferred until G gets probed and then D will >> get retried and moved to the end of the list resulting in: >> ? ? ? ? abcefGhD >> Everything is good now for the order that things need to be suspended in. >> >> Now lets assume that the drivers get linked into the kernel in a >> different order (or the modules get loaded in a different order) and G >> gets probed first, followed by D. ?No deferral occurred and so no >> reordering occurs, resulting in: >> ? ? ? ? abcDefGh (unchanged) >> But now suspend is broken because D depends on G, but G will be >> suspended before D. > > However D sometimes does defer probes. ?Therefore the device always > needs to be moved, even though this particular probe wasn't deferred. Yes, that's part of my point. >> ?This looks and smells like a bug to me. ?In fact, >> even without using probe deferral it looks like a bug because the >> dap_list isn't taking into account the fact that there are very likely >> to be implicit dependencies between devices that are not represented >> in the device hierarchy (maybe not common in PCs, but certainly is the >> case for embedded). ?But, it is also easy to solve by ensuring the >> dap_list is also probe-order sorted. >> >> > A deferred probe _should_ move the device to the end of the list. ?But >> > this needs to happen in the probe routine itself (after it has verified >> > that all the other required devices are present and before it has >> > registered any children) to prevent races. ?It can't be done in a >> > central location. >> >> I'm really concerned about drivers having to implement this and not >> getting it correct; particularly when moving a device to the end of >> the list is cheap, and it shouldn't be a problem to do the move >> unconditionally after a driver has been matches, but before probe() is >> called. > > But that's too early. ?What if D gets moved to the end of the list, > then G gets added to the list and probed, and then D's probe succeeds? So the argument is that if really_probe() called dpm_move_last() immediately before the dev->bus->probe()/drv->probe() call then there is a race condition that G could get both registered and probed before D's probe() starts using G's resources. And so, the list would still have G after D which is in the wrong order. Do I understand correctly? > And after the probe returns is too late, because by then there may well > be child devices. Agreed, after probe returns is definitely too late. >> ?We may be able to keep watch to make sure that drivers using >> probe deferral are also reordering themselves, but that does nothing >> for the cases described above where the link order of the drivers >> determines probe order, not the dap_list order. > > Devices need to be moved whenever they have any external dependencies, > regardless of the particular order they get probed in. I suspect this gets messy in a hurry, but perhaps it is worth trying. So, any device that makes use of a PHY, GPIO line, codec, etc will need to call dpm_move_last() before adding child devices, correct? I'd be much happier to find a way to do this in core code though. And there is still a potential race condition here. For example, if G is in the middle of it's probe routine, and D gets probed between G registering GPIOs and calling dpm_move_last(), then we're in the same boat again. I think the window for this race can be considered to be of the same magnitude as the moved to early race described above. I need to think about this more... g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.