Re: [PATCH v1 1/5] drivercore: Revert "deferral race condition fix"

[Peter Ujfalusi <peter.ujfalusi@ti.com>]

Andy,

On 2018-11-14 10:45, Andy Shevchenko wrote:
> On Wed, Nov 14, 2018 at 2:34 AM Mark Brown <broonie@kernel.org> wrote:
>>
>> On Mon, Nov 12, 2018 at 06:11:26PM +0200, Peter Ujfalusi wrote:
>>
>>> if we revert the commit then the original issue will re-surfaces. afaik
>>> it was not only audio which hit the 'last driver to be probed from the
>>> deferred list would never probe, unless we provoke the kernel to load
>>> additional module, or remove/reload the module' issue.
>>
>> Right, aren't we just going to be swapping one bug for another?
> 
> Have anyone in possession of Davinchi tested most recent kernel with
> this revert?

I don't think it is related to daVinci, in fact we have seen the very
same issue with OMAP4.
Fwiw this was my initial patch
http://lkml.iu.edu/hypermail/linux/kernel/1404.0/01603.html
Grant based his change on this.

Note the part in the commit message:
"The issue has been observed on embedded systems with CONFIG_PREEMPT
enabled, audio support built as modules and using nfsroot for root
filesystem."

as I recall I was only able to reproduce the stall with nfsroot and
buildroot fs. The timings were different compared to MMC rootfs.

Anyways the patch fixes a real race condition which is generic:
We have driverA and driverB as modules. driverB needs driverA to be
registered to a framework (no direct, hard dependency).

1. driverA starts to probe
2. driverB starts to probe
3. driverB can not continue as driverA is not registered itself to the
framework -> driverB will defer
4. driverA registers to the framework
5. driverA probes successfully
6. driver core will prepare the deferred probe list (note: driverB is
still in it's probe, not in the deferred list)
7. driverB returns from it's probe with -EPROBE_DEFER

and here we are, driverB is alone in the deferred list and the list is
not going to be tried as driverA and driverB were the last ones to
probe.

So, driverB is in the deferred list and stays there until other driver
probes successfully as the deferred driver list only tried when a driver
loads successfully (to see if that satisfy any of the deferred driver).

We have evidence that this has happened, it is a generic issue given
that now days we have more frameworks drivers are relying on.

>>> Do I understand correctly that in your case you have two modules
>>> (dwc3-pci and extcon-intel-mrfld) in a deferred probe loop, iow both of
>>> the drivers returns -EPROBE_DEFER and they just spin?
>>
>>> If both is deferring, how this supposed to work?
>>
>> I'm struggling to follow the original explanation too :(
> 
> Sorry, guys, I confused a nit myself. The bug is there, but
> exxplanation is not fully corrent, indeed. I'll come back with more
> details later.
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki