Re: [PATCH 2/5] drivercore: Add driver probe deferral mechanism

Alan Stern
2011-10-14T17:33:27Z

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF-8, Size: 2829 bytes --]

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.

> 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?

And after the probe returns is too late, because by then there may well
be child devices.

> 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.

Alan Stern