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@redhat.com Reported-by: Marek Szyprowski m.szyprowski@samsung.com Link: https://lore.kernel.org/r/73018f3c-9769-72ea-0325-b3f8e2381e30@redhat.com Link: https://lore.kernel.org/alsa-devel/9a0abddd-49e9-872d-2f00-a1697340f786@sams... Signed-off-by: Takashi Iwai tiwai@suse.de
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.
Takashi
--- --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1268,6 +1268,7 @@ static void dpcm_be_reparent(struct snd_soc_pcm_runtime *fe, void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream) { struct snd_soc_dpcm *dpcm, *d; + LIST_HEAD(deleted_dpcms);
snd_soc_dpcm_mutex_assert_held(fe);
@@ -1287,13 +1288,18 @@ void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream) /* BEs still alive need new FE */ dpcm_be_reparent(fe, dpcm->be, stream);
- dpcm_remove_debugfs_state(dpcm); - list_del(&dpcm->list_be); + list_move(&dpcm->list_fe, &deleted_dpcms); + } + snd_soc_dpcm_stream_unlock_irq(fe, stream); + + while (!list_empty(&deleted_dpcms)) { + dpcm = list_first_entry(&deleted_dpcms, struct snd_soc_dpcm, + list_fe); list_del(&dpcm->list_fe); + dpcm_remove_debugfs_state(dpcm); kfree(dpcm); } - snd_soc_dpcm_stream_unlock_irq(fe, stream); }
/* get BE for DAI widget and stream */