Commit 8c721c53 authored by Takashi Iwai's avatar Takashi Iwai

ALSA: usb-audio: Fix recursive locking at XRUN during syncing

The recent support of low latency playback in USB-audio driver made
the snd_usb_queue_pending_output_urbs() function to be called via PCM
ack ops.  In the new code path, the function is performed already in
the PCM stream lock.  The problem is that, when an XRUN is detected,
the function calls snd_pcm_xrun() to notify, but snd_pcm_xrun() is
supposed to be called only outside the stream lock.  As a result, it
leads to a deadlock of PCM stream locking.

For avoiding such a recursive locking, this patch adds an additional
check to the code paths in PCM core that call the ack callback; now it
checks the error code from the callback, and if it's -EPIPE, the XRUN
is handled in the PCM core side gracefully.  Along with it, the
USB-audio driver code is changed to follow that, i.e. -EPIPE is
returned instead of the explicit snd_pcm_xrun() call when the function
is performed already in the stream lock.

Fixes: d5f871f8 ("ALSA: usb-audio: Improved lowlatency playback support")
Reported-and-tested-by: default avatarJohn Keeping <john@metanate.com>
Link: https://lore.kernel.org/r/20230317195128.3911155-1-john@metanate.comReviewed-by: default avatarJaroslav Kysela <perex@perex.cz>
Reviewed-by; Takashi Sakamoto <o-takashi@sakamocchi.jp>
Link: https://lore.kernel.org/r/20230320142838.494-1-tiwai@suse.deSigned-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent b871cb97
...@@ -2155,6 +2155,8 @@ int pcm_lib_apply_appl_ptr(struct snd_pcm_substream *substream, ...@@ -2155,6 +2155,8 @@ int pcm_lib_apply_appl_ptr(struct snd_pcm_substream *substream,
ret = substream->ops->ack(substream); ret = substream->ops->ack(substream);
if (ret < 0) { if (ret < 0) {
runtime->control->appl_ptr = old_appl_ptr; runtime->control->appl_ptr = old_appl_ptr;
if (ret == -EPIPE)
__snd_pcm_xrun(substream);
return ret; return ret;
} }
} }
......
...@@ -455,8 +455,8 @@ static void push_back_to_ready_list(struct snd_usb_endpoint *ep, ...@@ -455,8 +455,8 @@ static void push_back_to_ready_list(struct snd_usb_endpoint *ep,
* This function is used both for implicit feedback endpoints and in low- * This function is used both for implicit feedback endpoints and in low-
* latency playback mode. * latency playback mode.
*/ */
void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep, int snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep,
bool in_stream_lock) bool in_stream_lock)
{ {
bool implicit_fb = snd_usb_endpoint_implicit_feedback_sink(ep); bool implicit_fb = snd_usb_endpoint_implicit_feedback_sink(ep);
...@@ -480,7 +480,7 @@ void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep, ...@@ -480,7 +480,7 @@ void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep,
spin_unlock_irqrestore(&ep->lock, flags); spin_unlock_irqrestore(&ep->lock, flags);
if (ctx == NULL) if (ctx == NULL)
return; break;
/* copy over the length information */ /* copy over the length information */
if (implicit_fb) { if (implicit_fb) {
...@@ -495,11 +495,14 @@ void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep, ...@@ -495,11 +495,14 @@ void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep,
break; break;
if (err < 0) { if (err < 0) {
/* push back to ready list again for -EAGAIN */ /* push back to ready list again for -EAGAIN */
if (err == -EAGAIN) if (err == -EAGAIN) {
push_back_to_ready_list(ep, ctx); push_back_to_ready_list(ep, ctx);
else break;
}
if (!in_stream_lock)
notify_xrun(ep); notify_xrun(ep);
return; return -EPIPE;
} }
err = usb_submit_urb(ctx->urb, GFP_ATOMIC); err = usb_submit_urb(ctx->urb, GFP_ATOMIC);
...@@ -507,13 +510,16 @@ void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep, ...@@ -507,13 +510,16 @@ void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep,
usb_audio_err(ep->chip, usb_audio_err(ep->chip,
"Unable to submit urb #%d: %d at %s\n", "Unable to submit urb #%d: %d at %s\n",
ctx->index, err, __func__); ctx->index, err, __func__);
notify_xrun(ep); if (!in_stream_lock)
return; notify_xrun(ep);
return -EPIPE;
} }
set_bit(ctx->index, &ep->active_mask); set_bit(ctx->index, &ep->active_mask);
atomic_inc(&ep->submitted_urbs); atomic_inc(&ep->submitted_urbs);
} }
return 0;
} }
/* /*
......
...@@ -52,7 +52,7 @@ int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep); ...@@ -52,7 +52,7 @@ int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep);
int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep, int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep,
struct snd_urb_ctx *ctx, int idx, struct snd_urb_ctx *ctx, int idx,
unsigned int avail); unsigned int avail);
void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep, int snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep,
bool in_stream_lock); bool in_stream_lock);
#endif /* __USBAUDIO_ENDPOINT_H */ #endif /* __USBAUDIO_ENDPOINT_H */
...@@ -1639,7 +1639,7 @@ static int snd_usb_pcm_playback_ack(struct snd_pcm_substream *substream) ...@@ -1639,7 +1639,7 @@ static int snd_usb_pcm_playback_ack(struct snd_pcm_substream *substream)
* outputs here * outputs here
*/ */
if (!ep->active_mask) if (!ep->active_mask)
snd_usb_queue_pending_output_urbs(ep, true); return snd_usb_queue_pending_output_urbs(ep, true);
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment