Re: [PATCH 4/4] USB: add urb->unlinked field

On Tue, 21 Aug 2007, David Brownell wrote:

> On Tuesday 21 August 2007, Alan Stern wrote:
> > This patch (as970) adds a new urb->unlinked field, which is used to
> > store the status of unlinked URBs since we can't use urb->status for
> > that purpose any more.  To help simplify the HCDs, usbcore will check
> > urb->unlinked before calling the completion handler; if the value is
> > set it will automatically override the status reported by the HCD.
> 
> That changes behavior, right?  Previously, if e.g. an IO error
> was reported in urb->status and the HCD was somewhere in the
> process of cleaning up after that error before reporting that
> completion, an unlink would fail but that status would be made
> available as the urb's completion status.
> 
> Now, in that same scenario, the unlink would complete and that
> hardware status would be discarded.  The documentation for the
> unlink would seem to need to be changed too.

No, it wouldn't work like that, because of changes made earlier in this 
series.

After this patch, suppose an I/O error occurs and the HCD is
somewhere in the process of cleaning it up before reporting the URB
completion.  When usb_unlink_urb() runs, before deciding that the
unlink has succeeded usbcore will call the HCD's urb_dequeue method,
which will block waiting for the HCD's private spinlock.  That spinlock 
isn't released until the URB is given back (with the error status, not 
the unlink status -- the unlink status hasn't been stored in 
urb->unlinked yet).

When it can proceed, the urb_dequeue method will turn around and call 
usb_hcd_check_unlink_urb(), which will see that the URB is no longer 
queued and so will return an error.  This error is then sent back 
to usb_unlink_urb(), which will report that the unlink failed.

It's all a result of that business we discussed earlier, whereby races
between unlink and completion are solved by doing everything under the
protection of the HCD's private spinlock.

I suppose there's a possible behavior change in situations where the 
HCD decides the URB has failed and has stored an error code in 
urb->status but doesn't give the URB back right away.  Right now the 
only place this happens is in ohci-hcd, and there's another patch lined 
up in my queue which will change ohci-hcd to prevent that sort of 
thing.

Alan Stern


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel