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