Re: [PATCH] USB: fix a bug in the scatter-gather library

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Please use my linux.intel.com address; that doesn't go through a nasty
M$ server that changes tabs into spaces in emails.

On Tue, Oct 27, 2009 at 03:26:50PM -0400, Alan Stern wrote:
> This patch (as1298) fixes a bug in the new scatter-gather URB
> facility.  If an URB uses a scatterlist then it should not have the
> URB_NO_INTERRUPT flag set; otherwise the system won't be notified when
> the transfer completes.
> 
> Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> CC: David Vrabel <david.vrabel@xxxxxxx>
> CC: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
> 
> ---
> 
> Unless David or Sarah thinks otherwise, this patch can wait until
> 2.6.33.  It affects only HCDs with scatter-gather support, which
> currently means only WHCI and xHCI.
> 
> I'm a little surprised the bug hasn't shown up in testing on those
> controllers, however.  Is it hidden by an I/O watchdog timer?  Or do
> they simply not support URB_NO_INTERRUPT?

I guess I've never understood what this flag was supposed to be for.

Right now, the xHCI driver gets an interrupt when the last part of an
URB buffer completes, or an error happens.  It doesn't scan the list of
submitted transfers like EHCI or have a watchdog (aside from the
cancellation watchdog).  If the xHCI driver doesn't get an interrupt and
an event on the event ring for a transfer, the URB will never be given
back, and I think xHCI's ring handling will cease to function properly.
So I'm not sure how this flag could be used.

Sarah

> Index: usb-2.6/drivers/usb/core/message.c
> ===================================================================
> --- usb-2.6.orig/drivers/usb/core/message.c
> +++ usb-2.6/drivers/usb/core/message.c
> @@ -403,7 +403,7 @@ int usb_sg_init(struct usb_sg_request *i
>  	if (!io->urbs)
>  		goto nomem;
>  
> -	urb_flags = URB_NO_INTERRUPT;
> +	urb_flags = 0;
>  	if (dma)
>  		urb_flags |= URB_NO_TRANSFER_DMA_MAP;
>  	if (usb_pipein(pipe))
> @@ -435,6 +435,7 @@ int usb_sg_init(struct usb_sg_request *i
>  		io->urbs[0]->num_sgs = io->entries;
>  		io->entries = 1;
>  	} else {
> +		urb_flags |= URB_NO_INTERRUPT;
>  		for_each_sg(sg, sg, io->entries, i) {
>  			unsigned len;
>  
> 
> --
> 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
--
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

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux