> > > 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? > I mean the controller should not try to access memory buffer after flush pending buffer which should be done at usb_ep_dequeue, so it is safe we free the request buffer after usb_ep_dequeue > > > > 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. I think it should add this content, it is strange that we still need to Wait the transfer has finished/cancelled after calling dequeue, we de-queue the request, of cos we hope the request has finished, and its resource can be re-used again. Peter -- 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