Re: [PATCH] ASoC: rt5677: Avoid recursive locking in the DEADLOCK detector

[Lars-Peter Clausen <lars@metafoo.de>]

On 06/05/2015 12:31 PM, Mark Brown wrote:
> On Fri, Jun 05, 2015 at 08:53:54AM +0800, Nicolas Boichat wrote:
>
>>> Could you be more specific about the deadlock you're seeing?  This isn't
>>> really enough to understand either what the problem is or why this fixes
>>> it.  Converting all the per-regmap mutexes into a single global mutex
>>> isn't an immediately obvious step.
>
>> We originally reported the issue to Realtek:
>
> Any analysis needs to be in the changelog for the commit and...
>
>> I suggested reworking the register read/write calls in rt5677.c to
>> direct them to the correct regmap earlier on (rt5677->regmap or
>> rt5677->regmap_physical), before locks are acquired. But the patch
>> above also fixes the issue (that is, it removes the warning).
>
> ...the above sounds like there's a bug in the locking anyway which this
> is just a bodge for?

It's the same issue Antti reported a while ago. The issue is that lockdep 
for performance reasons does not look at each lock separately, but only at 
lock classes. By default all locks initialized by the same mutex_init() call 
end up in the same lock class. When one lock of a lock class is locked while 
already holding a lock from the same lock class lockdep complains about a 
potential deadlock because to lockdep those look like the same lock. For 
most locks this is OK since they do not recursively lock locks from the same 
lock class.

Now the issue here is that we have nested regmap instances, meaning one 
regmap instances uses another instance in its read/write implementation. 
This will lead to nested locking of the mutex of the regmap struct. Since 
both mutexes are in the same lock class lockdep generates a warning. Rather 
than silencing the warning with some per driver hacks the correct way to fix 
this is to allow to properly annotate the locks as being different locks.

I think Antti submitted some patches to attend to fix this, but there were 
still issues with the patches and they never got merged.

- Lars