Re: [PATCH] of: fix race when matching drivers

From: Grant Likely
Date: Wed May 18 2011 - 11:47:36 EST


[cc'ing Linus: This is fix for a 2.6.39 regression, so just a heads up
that I'll be sending you a pull req ASAP (later this morning)]

On Wed, May 18, 2011 at 9:41 AM, Grant Likely <grant.likely@xxxxxxxxxxxx> wrote:
> On Wed, May 18, 2011 at 9:27 AM, Milton Miller <miltonm@xxxxxxx> wrote:
>> If two drivers are probing devices at the same time, both will write
>> their match table result to the dev->of_match cache at the same time.
>>
>> Only write the result if the device matches.
>>
>> In a thread titled "SBus devices sometimes detected, sometimes not",
>> Meelis reported his SBus hme was not detected about 50% of the time.
>> From the debug suggested by Grant it was obvious another driver matched
>> some devices between the call to match the hme and the hme discovery
>> failling.
>>
>> Reported-by: Meelis Roos <mroos@xxxxxxxx>
>> Signed-off-by: Milton Miller <miltonm@xxxxxxx>
>> ---
>>
>> Grant, I really think this of_match cache in the device node is bad a
>> bad tradeoff, and am willing to submit patches to remove it for 2.6.40.
>> It is only used by about 26 drivers and all use it once during probe
>> to fill out their driver data.  It comes at the cost of a long for
>> every struct device in every system.
>
> Ah, bugger.  I had /thought/ that matching and probing were kept
> together with a mutex.  So, yes, this is bad and the of_match needs to
> be removed.  Thanks for volunteering to submit the patch.  It should
> be backported to 2.6.39 too.
>
>> I'll even offer to throw in a patch to cache the parsing of the
>> compatible property to speed up of_device_is_compatible if needed.
>
> That would be useful.  :-)
>
> I'll pick up your patch right now and fire it off to Linus.

... it's not perfect since it will break some theoretical drivers
unbind/rebind use-cases, but I cannot think of any actual examples,
and it is far safer than the current code. Regardless, the removal of
of_match will definitely need to go into stable when it is ready.

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/