[alsa-devel] [PATCH 1/2] ALSA: snd-usb: tighten EP_FLAG_RUNNING checks
zonque at gmail.com
Thu Jul 12 17:27:02 CEST 2012
On 12.07.2012 17:20, Clemens Ladisch wrote:
> Daniel Mack wrote:
>> On 12.07.2012 16:29, Clemens Ladisch wrote:
>>> Daniel Mack wrote:
>>>> In endpoint.c, bail out earlier in case the stream is stopped.
>>>> @@ -350,7 +350,8 @@ static void snd_complete_urb(struct urb *urb)
>>>> urb->status == -ENODEV || /* device removed */
>>>> urb->status == -ECONNRESET || /* unlinked */
>>>> urb->status == -ESHUTDOWN || /* device disabled */
>>>> - ep->chip->shutdown)) /* device disconnected */
>>>> + ep->chip->shutdown) || /* device disconnected */
>>>> + !test_bit(EP_FLAG_RUNNING, &ep->flags))
>>>> goto exit_clear;
>>> Is this really needed?
>>> The URBs will be unlinked at the same time.
>> This just brings the code in sync with what we had before. If URBs are
>> just unlinked but not killed, they will return with data payload, and in
>> case of implicit feedback streams, the retire code could issue new
>> output packets.
> And is that bad?
> The EP_FLAG_RUNNING bit is cleared immediately before the URBs are
> unlinked, and some URB could have been completed regularly immediately
> before that. So if there is any case where the additional test_bit()
> call prevents submitting a new playback packet, there is another similar
> case where you've just got such a playback packet anyway.
> In other words: if that test_bit() call were necessary to protect
> against something bad, it wouldn't be sufficient.
> The driver ensures that (implicit feedback) playback URBs are unlinked
> only after the corresponding capture URBs have been completely killed,
> doesn't it?
You're right. Especially because sending out packets in implicit
feedback mode happens from queue_pending_output_urbs() which has an
So we can drop the whole patch then. Thanks for the review!
More information about the Alsa-devel