On Wed, 13 Jun 2018, Mikulas Patocka wrote:
[Skipping lots of material...]
> BTW I found this piece of code in sound/usb/usx2y/usbusx2yaudio.c:
> urb->transfer_buffer_length = subs->maxpacksize * nr_of_packs();
> if ((err = usb_submit_urb(urb, GFP_ATOMIC)) < 0) {
> snd_printk (KERN_ERR "cannot submit datapipe for urb %d, err = %d\n", i, err);
> err = -EPIPE;
> goto cleanup;
> } else
> if (i == 0)
> usX2Y->wait_iso_frame = urb->start_frame;
> urb->transfer_flags = 0;
> It seems like a bug to modify transfer_flags after the urb is submitted.
Yes, it is definitely a bug.
> I have a single-core machine with usb2 soundcard. When I increase mplayer
> priority (to real-time or high non-realtime priority), the sound is
> stuttering. The reason for the stuttering is that mplayer at high priority
> preempts the softirq thread, preventing URBs from being completed. It was
> caused by the patch 428aac8a81058 that offloads URB completion to softirq.
> --- linux-4.17.orig/drivers/usb/host/ehci-q.c 2018-06-12 22:35:21.000000000 +0200
> +++ linux-4.17/drivers/usb/host/ehci-q.c 2018-06-12 22:44:09.000000000 +0200
> @@ -238,6 +238,8 @@ static int qtd_copy_status (
>
> static void
> ehci_urb_done(struct ehci_hcd *ehci, struct urb *urb, int status)
> +__releases(ehci->lock)
> +__acquires(ehci->lock)
> {
> if (usb_pipetype(urb->pipe) == PIPE_INTERRUPT) {
> /* ... update hc-wide periodic stats */
> @@ -264,7 +266,17 @@ ehci_urb_done(struct ehci_hcd *ehci, str
> #endif
>
> usb_hcd_unlink_urb_from_ep(ehci_to_hcd(ehci), urb);
> - usb_hcd_giveback_urb(ehci_to_hcd(ehci), urb, status);
> + if (urb->transfer_flags & URB_FAST_COMPLETION) {
> + /*
> + * USB audio experiences skipping of we offload completion
> + * to ksoftirq.
> + */
This comment seems unnecessary.
> + spin_unlock(&ehci->lock);
> + usb_hcd_giveback_urb(ehci_to_hcd(ehci), urb, status);
> + spin_lock(&ehci->lock);
> + } else {
> + usb_hcd_giveback_urb(ehci_to_hcd(ehci), urb, status);
> + }
> }
I'm not at all sure about this. Have you audited all of ehci-hcd to
make sure the driver doesn't assume that ehci->lock remains held while
an URB is given back? It's been so long since I worked on this area
that I don't remember the answer offhand.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
