Hi,
On Fri, Jul 02, 2021 at 10:19:46PM -0700, Linus Torvalds wrote:
> On Fri, Jul 2, 2021 at 9:37 PM Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > But I thought I'd report this as a likely candidate.
>
> Confirmed. The watchdog hang bisects right down to commit 9ce650a75a3b
> ("ALSA: usb-audio: Reduce latency at playback start").
>
> And reverting it on top of my tree also fixes the hang, so it's not
> some bisection fluke.
>
> I have no idea what is actually wrong with that commit, but it most
> definitely is the problem, and I have reverted it in my tree so that I
> can continue merging stuff tomorrow.
The cause seems to be the attempt to lock PCM substream recursively
introduced by the issued commit.
Would I ask you to test with below patch? I apologize that the patch is
still untested in my side since at present I have no preparation to debug
USB stuffs instantly (I'm just a maintainer for ALSA firewire stack...),
so I'm glad if getting your cooperation for the issue.
======== 8< --------
>From f7ab449f10152635ad7083aa73d80e3fb1adabb4 Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx>
Date: Sat, 3 Jul 2021 15:23:25 +0900
Subject: [PATCH] ALSA: usb-audio: fix recursive lock of PCM substream when
starting playback PCM substream
A commit 9ce650a75a3b ("ALSA: usb-audio: Reduce latency at playback start")
unfortunately introduced the call of snd_pcm_period_elapsed() under acquired
lock of PCM substream. This causes recursive lock and results in dead-lock.
->ioctl(2)
(sound/core/pcm_native.c)
->snd_pcm_stream_lock_irqsave() <-
...
->struct snd_pcm_ops.trigger()
(sound/usb/pcm.c)
= snd_usb_substream_playback_trigger()
->start_endpoints()
(sound/usb/endpoint.c)
->snd_usb_endpoint_start()
->prepare_outbound_urb()
->struct snd_usb_endpoint.prepare_data_urb()
(sound/usb/pcm.c)
= prepare_playback_urb()
(sound/core/pcm_lib.c)
->snd_pcm_period_elapsed()
(sound/core/pcm_native.c)
->snd_pcm_stream_lock_irqsave() <-
This commit fixes the issue to use newly added function;
snd_pcm_period_elapsed_under_stream_lock() with condition to check running
context.
Reported-by: Hector Martin <marcan@xxxxxxxxx>
Fixes: 9ce650a75a3b ("ALSA: usb-audio: Reduce latency at playback start")
Signed-off-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx>
---
sound/usb/pcm.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index c66831ee15f9..235070f0236a 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -1395,8 +1395,16 @@ static void prepare_playback_urb(struct snd_usb_substream *subs,
spin_unlock_irqrestore(&subs->lock, flags);
urb->transfer_buffer_length = bytes;
- if (period_elapsed)
- snd_pcm_period_elapsed(subs->pcm_substream);
+ if (period_elapsed) {
+ // The callback of struct snd_pcm_ops.trigger with SNDRV_PCM_TRIGGER_START command
+ // can reach here, under acquired lock of PCM substream. To avoid dead-lock, check
+ // current context and call corresponding function.
+ if (in_softirq()) {
+ snd_pcm_period_elapsed(subs->pcm_substream);
+ } else {
+ snd_pcm_period_elapsed_under_stream_lock(subs->pcm_substream);
+ }
+ }
}
/*
--
2.30.2
======== 8< --------
Thanks
Takashi (not subsystem maintainer) Sakamoto
