On 13.01.2022 17:12, Takashi Iwai wrote:
> On Thu, 13 Jan 2022 15:59:31 +0100, Marek Szyprowski wrote:
>> On 13.01.2022 15:18, Takashi Iwai wrote:
>>> The recent change for DPCM locking caused spurious lockdep warnings.
>>> Actually the warnings are false-positive, as those are triggered due
>>> to the nested stream locks for FE and BE. Since both locks belong to
>>> the same lock class, lockdep sees it as if a deadlock.
>>>
>>> For fixing this, we need take PCM stream locks for BE with the nested
>>> lock primitives. Since currently snd_pcm_stream_lock*() helper
>>> assumes only the top-level single locking, a new helper function
>>> snd_pcm_stream_lock_irqsave_nested() is defined for a single-depth
>>> nested lock, which is now used in the BE DAI trigger that is always
>>> performed inside a FE stream lock.
>>>
>>> Fixes: b7898396f4bb ("ASoC: soc-pcm: Fix and cleanup DPCM locking")
>>> Reported-and-tested-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>>> Reported-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
>>> Link: https://lore.kernel.org/r/73018f3c-9769-72ea-0325-b3f8e2381e30@xxxxxxxxxx
>>> Link: https://lore.kernel.org/alsa-devel/9a0abddd-49e9-872d-2f00-a1697340f786@xxxxxxxxxxx
>>> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
>> Thanks for the fix! It helps a bit, but I still get a warning (a
>> different one now):
> Thanks for the quick testing. Actually we do have multiple issues.
>
>> root@target:~# speaker-test -l1
>>
>> speaker-test 1.1.8
>>
>> Playback device is default
>> Stream parameters are 48000Hz, S16_LE, 1 channels
>> Using 16 octaves of pink noise
>> Rate set to 48000Hz (requested 48000Hz)
>> Buffer size range from 128 to 131072
>> Period size range from 64 to 65536
>> Using max buffer size 131072
>> Periods = 4
>> was set period_size = 32768
>> was set buffer_size = 131072
>> 0 - Front Left
>> Time per period = 0.022199
>> max98090 1-0010: PLL unlocked
>>
>> =====================================================
>> WARNING: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected
>> 5.16.0-next-20220113-00001-g3967460dbcf4 #11212 Not tainted
>> -----------------------------------------------------
>> speaker-test/1319 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
>> c1296410 (pin_fs_lock){+.+.}-{2:2}, at: simple_pin_fs+0x1c/0xac
>>
>> and this task is already holding:
>> c2fe6ea4 (&group->lock){..-.}-{2:2}, at: dpcm_be_disconnect+0x3c/0x348
>> which would create a new lock dependency:
>> (&group->lock){..-.}-{2:2} -> (pin_fs_lock){+.+.}-{2:2}
> So that's the problem: we call debugfs_remove_recursive() in the
> spinlocked context.
>
> Could you try the patch below? It's a bit hackish, but let's check
> whether that's the real cause or not.
Right, this fixes the issue. It was trivial to find, but I did a test
just a few minutes before finishing my work time, so I only copied the
log to the mail without analyzing it.
Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
