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.
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 */
