Re: [PATCH 2/2] ASoC: max98090: fix lockdep warning

[Marek Szyprowski]

Hi

On 09.01.2020 06:36, Tzung-Bi Shih wrote:
> On Wed, Jan 8, 2020 at 7:50 PM Marek Szyprowski
> <m.szyprowski@xxxxxxxxxxx> wrote:
>> Fix this by introducing a separate mutex only for serializing the SHDN
>> hardware register related operations.
> This fix makes less sense to me.  We used dapm_mutex intentionally
> because: both DAPM and userspace mixer control would change SHDN bit
> at the same time.
>
>> This fixes the following lockdep warning observed on Exynos4412-based
>> Odroid U3 board:
>> ======================================================
>> WARNING: possible circular locking dependency detected
>> 5.5.0-rc5-next-20200107 #166 Not tainted
>> ------------------------------------------------------
>> alsactl/1104 is trying to acquire lock:
>> ed0d50f4 (&card->dapm_mutex){+.+.}, at: max98090_shdn_save+0x1c/0x28
>>
>> but task is already holding lock:
>> edb4b49c (&card->controls_rwsem){++++}, at: snd_ctl_ioctl+0xcc/0xbb8
>>
>> which lock already depends on the new lock.
>>
>> the existing dependency chain (in reverse order) is:
>>
>> -> #1 (&card->controls_rwsem){++++}:
>>         snd_ctl_add_replace+0x3c/0x84
>>         dapm_create_or_share_kcontrol+0x24c/0x2e0
>>         snd_soc_dapm_new_widgets+0x308/0x594
>>         snd_soc_bind_card+0x80c/0xad4
>>         devm_snd_soc_register_card+0x34/0x6c
>>         odroid_audio_probe+0x288/0x34c
>>         platform_drv_probe+0x6c/0xa4
>>         really_probe+0x200/0x490
>>         driver_probe_device+0x78/0x1f8
>>         bus_for_each_drv+0x74/0xb8
>>         __device_attach+0xd4/0x16c
>>         bus_probe_device+0x88/0x90
>>         deferred_probe_work_func+0x3c/0xd0
>>         process_one_work+0x22c/0x7c4
>>         worker_thread+0x44/0x524
>>         kthread+0x130/0x164
>>         ret_from_fork+0x14/0x20
>>         0x0
>>
>> -> #0 (&card->dapm_mutex){+.+.}:
>>         lock_acquire+0xe8/0x270
>>         __mutex_lock+0x9c/0xb18
>>         mutex_lock_nested+0x1c/0x24
>>         max98090_shdn_save+0x1c/0x28
>>         max98090_put_enum_double+0x20/0x40
>>         snd_ctl_ioctl+0x190/0xbb8
>>         ksys_ioctl+0x470/0xaf8
>>         ret_fast_syscall+0x0/0x28
>>         0xbefaa564
> As the stack trace suggested: when the card was still registering,
> alsactl can see the mixer control and try to change it.

Nope. This is just a lockdep warning about possible hypothetical 
situation on the test system during the normal boot. It doesn't mean 
that the circular dependency actually happens (if so, it would end in 
deadlock). It also doesn't mean that such circular dependency can be 
really triggered, because some other dependencies that not known to 
lockdep engine might protect against it. However the easiest way to fix 
it is to use fine-grained locking instead of reusing some framework 
locks for other purposes. Such approach is also usually a good practice.

> We have a discussion on an older thread
> (https://mailman.alsa-project.org/pipermail/alsa-devel/2019-December/160454.html).
> It is weird: userspace should not see things (e.g. no controlC0) until
> snd_card_register( ).
>
> I would like to spend some time to find the root cause.  It would be a
> little challenging though (I have no real runtime to test...).


Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland