Re: [PATCH] usb/g_zero: don't access private data in complete callback on error

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

 



On Mon, Jan 23, 2012 at 07:36:27PM +0100, Sebastian Andrzej Siewior wrote:
> On 01/23/2012 06:22 PM, Alan Stern wrote:
> >On Mon, 23 Jan 2012, Sebastian Andrzej Siewior wrote:
> >
> >>The following scenario:
> >>- use a third party tool
> >>- issue SetConfiguration, start a transfer
> >>- g_zero enqueues 4k reqs. Be rude and stop after a multiple of maxpacketsize
> >>   but before the 4k
> >>- Assume the UDC can handle 4k transfer in one go and will interrupt after the
> >>   4k arrived or a short transfer. That the transfer is still "busy".
> >>- now soft the disconnect the device.
> >>- the UDC gets a SetConfiguration 0 and will disable all endpoints
> >>- disabling an endpoint means cancles all pending transfers.
> >>
> >>On DWC3 we issue a cancel transfer command and wait for the command/transfer to
> >>complete.
> >
> >Do you mean you _don't_ wait for the transfer to complete?
> 
> yes. Lets say 2k of 4k have been transferred. I don't get an interrupt
> which says transfer complete. So I am interrupting the transfer "in
> progress."

if 2k is all you need to get, of course you will get Transfer Complete.
Host will append a ZLP to finish the transfer.

> >>  That means we return immediately from ->disable(). The _gadget_ has to
> >>wait for the ->complete() callback. g_zero does not and the result is:
> >
> >This patch is wrong; DWC3 must be fixed instead.  From
> >include/linux/usb/gadget.h:
> >
> >/**
> >  * usb_ep_disable - endpoint is no longer usable
> >  *
> >  * no other task may be using this endpoint when this is called.
> >  * any pending and uncompleted requests will complete with status
> >  * indicating disconnect (-ESHUTDOWN) before this call returns.
> >  */
> 
> Fixing this not easy. We are atomic here and I can't sleep. A busy loop
> to wait until the command is not simple (but ugly) because other
> commands may complete in between. So the only way to fix this behavior

actually, I don't think you need to wait for the interrupt at all. Just
poll CMDAct bit on DEPCMD (like we already do), after that's cleared the
command has completed and all other bitfields you might need are valid.

The real problem, though, is that there's always a request pending on
request_list which doesn't get givenback early enough.

-- 
balbi

Attachment: signature.asc
Description: Digital signature


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

  Powered by Linux