On Thu, 21 Aug 2014, Peter Chen wrote: > On Wed, Aug 20, 2014 at 03:18:47PM -0400, Alan Stern wrote: > > On Wed, 20 Aug 2014, Felipe Balbi wrote: > > > > > > > > --- a/drivers/usb/gadget/composite.c > > > > > > +++ b/drivers/usb/gadget/composite.c > > > > > > @@ -1956,6 +1956,7 @@ void composite_dev_cleanup(struct usb_composite_dev *cdev) > > > > > > } > > > > > > if (cdev->req) { > > > > > > kfree(cdev->req->buf); > > > > > > + usb_ep_dequeue(cdev->gadget->ep0, cdev->req); > > > > > > > > > > it's best to dequeue the request before freeing its buffer. > > > > > > > > In fact, it's best to wait for the request's completion routine to be > > > > called before freeing the buffer. The hardware can access the buffer's > > > > memory at any time before the completion routine runs. > > > > > > dequeue should cause the transfer to be cancelled and given back. > > > There's a slight possible race window because we release the controller > > > lock in order to call gadget driver's ->complete(). > > > > > The dp has already been pulled down and the flush pending FIFO buffer > has finished, so the controller should not try to access memory buffer > after that. In that case, why does the patch add a call to usb_ep_dequeue()? Shouldn't the request be unlinked already? > > > Other than that, it should be fine. No ? > > > > Dequeue causes the transfer to be cancelled and given back, yes. But > > usb_ep_dequeue() is allowed to return before those things happen. > > > > If usb_ep_dequeue is returned before doing any flush and cancel > transfer, it means there is no request on the queue, we don't need > to cancel any requests. It can be different for different UDCs. The documentation doesn't say that usb_ep_dequeue has to wait until the transfer has been cancelled and flushed. 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