On Fri, 18 Apr 2025 12:35:32 +0200, Hillf Danton wrote:
On Wed, 29 Sep 2021 10:08:37 +0200 Takashi Iwai wrote:
USB-audio driver tries to sync with the clear of all pending URBs in wait_clear_urbs(), and it waits for all bits in active_mask getting cleared. This works fine for the normal operations, but when a stream is managed in the implicit feedback mode, there is still a very thin race window: namely, in snd_complete_usb(), the active_mask bit for the current URB is once cleared before re-submitted in queue_pending_output_urbs(). If wait_clear_urbs() is called during that period, it may pass the test and go forward even though there may be a still pending URB.
For covering it, this patch adds a new counter to each endpoint to keep the number of in-flight URBs, and changes wait_clear_urbs() checking this number instead. The counter is decremented at the end of URB complete, hence the reference is kept as long as the URB complete is in process.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/usb/card.h | 1 + sound/usb/endpoint.c | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/sound/usb/card.h b/sound/usb/card.h index 3329ce710cb9..746a765b2437 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -97,6 +97,7 @@ struct snd_usb_endpoint { unsigned int nominal_queue_size; /* total buffer sizes in URBs */ unsigned long active_mask; /* bitmask of active urbs */ unsigned long unlink_mask; /* bitmask of unlinked urbs */
- atomic_t submitted_urbs; /* currently submitted urbs */ char *syncbuf; /* sync buffer for all sync URBs */ dma_addr_t sync_dma; /* DMA address of syncbuf */
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 29c4865966f5..06241568abf7 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -451,6 +451,7 @@ static void queue_pending_output_urbs(struct snd_usb_endpoint *ep) }
set_bit(ctx->index, &ep->active_mask);
atomic_inc(&ep->submitted_urbs);}
@@ -488,6 +489,7 @@ static void snd_complete_urb(struct urb *urb) clear_bit(ctx->index, &ep->active_mask); spin_unlock_irqrestore(&ep->lock, flags); queue_pending_output_urbs(ep);
smp_mb();
atomic_dec(&ep->submitted_urbs); /* decrement at last */Does it match the comment to add a mb?
How...? I don't understand your intention.
Takashi