Am Freitag, 30. März 2007 07:07 schrieb Greg KH:
> On Thu, Mar 29, 2007 at 02:25:34PM +0200, Oliver Neukum wrote:
> > @@ -332,12 +330,12 @@
> >
> > dbg("%s - port %d", __FUNCTION__, port->number);
> >
> > - port->write_urb_busy = 0;
> > if (urb->status) {
> > dbg("%s - nonzero write bulk status received: %d",
> > __FUNCTION__, urb->status);
> > return;
> > }
> > -
> > + smp_mb(); /* we are busy until we have read our status */
>
> I don't understand why this is necessary, we shouldn't have to put this
> all over our drivers, if so, something is really wrong.
OK, this is subtle. The completion handlers are called without locks held,
eg. this from ehci-q.c:
/* complete() can reenter this HCD */
spin_unlock (&ehci->lock);
usb_hcd_giveback_urb (ehci_to_hcd(ehci), urb);
spin_lock (&ehci->lock);
The comment gives you the reason and is right. The locks must be dropped.
Therefore, if the urb is resubmitted while the completion handler is running,
usbcore will fully process this urb. This means that the status will be set
to -EINPROGRESS.
So the status is valid only until the urb is resubmitted. The driver
in question may resubmit the urb in the write path if !port->write_urb_busy.
We must consider clearing a busy flag equivalent to resubmitting the
urb ourselves. In a completion handler that resubmits the urb you do that
resubmission last in the handler, don't you?
As an added complication, the resubmission would happen on another cpu.
Therefore the order on the bus must be correct, hence the "smp_mb()".
Regards
Oliver
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
