RE: [PATCH] usb: gadget: composite: dequeue cdev->req before free it in composite_dev_cleanup

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

 




 
> 
> > 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




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

  Powered by Linux