From: John Keeping <john@metanate.com>
To: Takashi Iwai <tiwai@suse.com>
Cc: John Keeping <john@metanate.com>,
Jaroslav Kysela <perex@perex.cz>,
alsa-devel@alsa-project.org (moderated list:SOUND),
linux-kernel@vger.kernel.org (open list)
Subject: [PATCH] ALSA: usb-audio: Fix recursive locking on XRUN
Date: Fri, 17 Mar 2023 19:51:27 +0000 [thread overview]
Message-ID: <20230317195128.3911155-1-john@metanate.com> (raw)
snd_usb_queue_pending_output_urbs() may be called from
snd_pcm_ops::ack() which means the PCM stream is locked.
For the normal case where the call back into the PCM core is via
prepare_output_urb() the "_under_stream_lock" variant of
snd_pcm_period_elapsed() is called, but when an error occurs and the
stream is stopped as XRUN then snd_pcm_xrun() tries to recursively lock
the stream which results in deadlock.
Follow the example of snd_pcm_period_elapsed() by adding
snd_pcm_xrun_under_stream_lock() and use this when the PCM substream
lock is already held.
Signed-off-by: John Keeping <john@metanate.com>
---
include/sound/pcm.h | 1 +
sound/core/pcm_native.c | 28 ++++++++++++++++++++++++----
sound/usb/endpoint.c | 18 +++++++++++-------
3 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 27040b472a4f..98551907453a 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -571,6 +571,7 @@ int snd_pcm_status64(struct snd_pcm_substream *substream,
int snd_pcm_start(struct snd_pcm_substream *substream);
int snd_pcm_stop(struct snd_pcm_substream *substream, snd_pcm_state_t status);
int snd_pcm_drain_done(struct snd_pcm_substream *substream);
+int snd_pcm_stop_xrun_under_stream_lock(struct snd_pcm_substream *substream);
int snd_pcm_stop_xrun(struct snd_pcm_substream *substream);
#ifdef CONFIG_PM
int snd_pcm_suspend_all(struct snd_pcm *pcm);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 331380c2438b..617f5dc74df0 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1559,24 +1559,44 @@ int snd_pcm_drain_done(struct snd_pcm_substream *substream)
SNDRV_PCM_STATE_SETUP);
}
+/**
+ * snd_pcm_stop_xrun_under_stream_lock - stop the running stream as XRUN under the lock of
+ * the PCM substream.
+ * @substream: the PCM substream instance
+ *
+ * This stops the given running substream (and all linked substreams) as XRUN.
+ * This function assumes that the substream lock is already held.
+ *
+ * Return: Zero if successful, or a negative error core.
+ */
+int snd_pcm_stop_xrun_under_stream_lock(struct snd_pcm_substream *substream)
+{
+ if (substream->runtime && snd_pcm_running(substream))
+ __snd_pcm_xrun(substream);
+
+ return 0;
+}
+
/**
* snd_pcm_stop_xrun - stop the running streams as XRUN
* @substream: the PCM substream instance
*
+ * This function is similar to ``snd_pcm_stop_xrun_under_stream_lock()`` except that it
+ * acquires the substream lock itself.
+ *
* This stops the given running substream (and all linked substreams) as XRUN.
- * Unlike snd_pcm_stop(), this function takes the substream lock by itself.
*
* Return: Zero if successful, or a negative error code.
*/
int snd_pcm_stop_xrun(struct snd_pcm_substream *substream)
{
unsigned long flags;
+ int ret;
snd_pcm_stream_lock_irqsave(substream, flags);
- if (substream->runtime && snd_pcm_running(substream))
- __snd_pcm_xrun(substream);
+ ret = snd_pcm_stop_xrun_under_stream_lock(substream);
snd_pcm_stream_unlock_irqrestore(substream, flags);
- return 0;
+ return ret;
}
EXPORT_SYMBOL_GPL(snd_pcm_stop_xrun);
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 1e0af1179ca8..83a6b6d41374 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -400,13 +400,17 @@ static int prepare_inbound_urb(struct snd_usb_endpoint *ep,
}
/* notify an error as XRUN to the assigned PCM data substream */
-static void notify_xrun(struct snd_usb_endpoint *ep)
+static void notify_xrun(struct snd_usb_endpoint *ep, bool in_stream_lock)
{
struct snd_usb_substream *data_subs;
data_subs = READ_ONCE(ep->data_subs);
- if (data_subs && data_subs->pcm_substream)
- snd_pcm_stop_xrun(data_subs->pcm_substream);
+ if (data_subs && data_subs->pcm_substream) {
+ if (in_stream_lock)
+ snd_pcm_stop_xrun_under_stream_lock(data_subs->pcm_substream);
+ else
+ snd_pcm_stop_xrun(data_subs->pcm_substream);
+ }
}
static struct snd_usb_packet_info *
@@ -498,7 +502,7 @@ void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep,
if (err == -EAGAIN)
push_back_to_ready_list(ep, ctx);
else
- notify_xrun(ep);
+ notify_xrun(ep, in_stream_lock);
return;
}
@@ -507,7 +511,7 @@ void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep,
usb_audio_err(ep->chip,
"Unable to submit urb #%d: %d at %s\n",
ctx->index, err, __func__);
- notify_xrun(ep);
+ notify_xrun(ep, in_stream_lock);
return;
}
@@ -574,7 +578,7 @@ static void snd_complete_urb(struct urb *urb)
return;
usb_audio_err(ep->chip, "cannot submit urb (err = %d)\n", err);
- notify_xrun(ep);
+ notify_xrun(ep, false);
exit_clear:
clear_bit(ctx->index, &ep->active_mask);
@@ -1762,7 +1766,7 @@ static void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep,
usb_audio_err(ep->chip,
"next package FIFO overflow EP 0x%x\n",
ep->ep_num);
- notify_xrun(ep);
+ notify_xrun(ep, false);
return;
}
--
2.40.0
next reply other threads:[~2023-03-17 19:52 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-17 19:51 John Keeping [this message]
2023-03-18 0:20 ` Takashi Sakamoto
2023-03-18 10:59 ` Takashi Sakamoto
2023-03-19 3:28 ` Takashi Sakamoto
2023-03-19 7:57 ` Takashi Iwai
2023-03-19 9:15 ` Takashi Iwai
2023-03-20 12:04 ` John Keeping
2023-03-20 14:28 ` Takashi Iwai
2023-03-18 2:29 ` kernel test robot
2023-03-18 5:46 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230317195128.3911155-1-john@metanate.com \
--to=john@metanate.com \
--cc=alsa-devel@alsa-project.org \
--cc=linux-kernel@vger.kernel.org \
--cc=perex@perex.cz \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®