When David mentioned that unlink_urb tests urb->hcpriv to see whether the
low-level driver has finished processing the urb, I realized that my patch
needed to be fixed. Here's the new version.
Alan Stern
===== include/linux/usb.h 1.124 vs edited =====
--- 1.124/include/linux/usb.h Wed Jan 22 12:38:08 2003
+++ edited/include/linux/usb.h Thu Jan 30 15:37:47 2003
@@ -554,6 +554,7 @@
#define URB_NO_FSBR 0x0020 /* UHCI-specific */
#define URB_ZERO_PACKET 0x0040 /* Finish bulk OUTs with short packet
*/
#define URB_NO_INTERRUPT 0x0080 /* HINT: no non-error interrupt needed */
+#define URB_ALIVE 0x2000 /* Completion handler not yet called */
struct usb_iso_packet_descriptor {
unsigned int offset;
@@ -727,6 +728,7 @@
{
spinlock_t lock; /* lock for the URB */
atomic_t count; /* reference count of the URB */
+ spinlock_t handler_lock; /* lock during completion routine */
void *hcpriv; /* private data for host controller */
struct list_head urb_list; /* list pointer to all active urbs */
struct usb_device *dev; /* (in) pointer to associated device */
===== drivers/usb/core/hcd.c 1.90 vs edited =====
--- 1.90/drivers/usb/core/hcd.c Wed Jan 22 13:02:11 2003
+++ edited/drivers/usb/core/hcd.c Thu Jan 30 15:36:53 2003
@@ -1105,6 +1105,7 @@
unsigned long flags;
struct completion_splice splice;
int retval;
+ int just_waiting = 0;
if (!urb)
return -EINVAL;
@@ -1119,8 +1120,13 @@
* lock sequence needed for the usb_hcd_giveback_urb() code paths
* (urb lock, then hcd_data_lock) in case some other CPU is now
* unlinking it.
+ *
+ * In case the completion handler is already running, we first
+ * acquire urb->handler_lock; this will cause us to wait until
+ * the handler has returned.
*/
- spin_lock_irqsave (&urb->lock, flags);
+ spin_lock_irqsave (&urb->handler_lock, flags);
+ spin_lock (&urb->lock);
spin_lock (&hcd_data_lock);
if (!urb->dev || !urb->dev->bus) {
@@ -1136,19 +1142,24 @@
goto done;
}
- if (!urb->hcpriv) {
- retval = -EINVAL;
- goto done;
- }
-
/* Any status except -EINPROGRESS means something already started to
- * unlink this URB from the hardware. So there's no more work to do.
+ * unlink this URB from the hardware. Likewise, if hcpriv is not
+ * set then the low-level driver is done with this URB.
+ * If the completion handler has run or the request is async then
+ * there's no more work to do. Otherwise we must wait for the
+ * handler.
*
* FIXME use better explicit urb state
*/
- if (urb->status != -EINPROGRESS) {
+ if (urb->status != -EINPROGRESS || !urb->hcpriv) {
+ if (!(urb->transfer_flags & URB_ALIVE)) {
+ retval = -EINVAL;
+ goto done;
+ }
retval = -EBUSY;
- goto done;
+ if (urb->transfer_flags & URB_ASYNC_UNLINK)
+ goto done;
+ just_waiting = 1;
}
/* maybe set up to block until the urb's completion fires. the
@@ -1167,15 +1178,23 @@
splice.context = urb->context;
urb->complete = unlink_complete;
urb->context = &splice;
- urb->status = -ENOENT;
+ /* Only indicate that the URB was unlinked if the request
+ * arrived before the URB had finished anyway.
+ */
+ if (!just_waiting)
+ urb->status = -ENOENT;
} else {
/* asynchronous unlink */
urb->status = -ECONNRESET;
}
spin_unlock (&hcd_data_lock);
- spin_unlock_irqrestore (&urb->lock, flags);
+ spin_unlock (&urb->lock);
+ spin_unlock_irqrestore (&urb->handler_lock, flags);
- if (urb == (struct urb *) hcd->rh_timer.data) {
+ if (just_waiting)
+ /* no need to unlink; the urb is already finishing up */
+ ;
+ else if (urb == (struct urb *) hcd->rh_timer.data) {
usb_rh_status_dequeue (hcd, urb);
retval = 0;
} else {
@@ -1199,11 +1218,12 @@
return -EINPROGRESS;
wait_for_completion (&splice.done);
- return 0;
+ return retval;
done:
spin_unlock (&hcd_data_lock);
- spin_unlock_irqrestore (&urb->lock, flags);
+ spin_unlock (&urb->lock);
+ spin_unlock_irqrestore (&urb->handler_lock, flags);
bye:
if (retval && sys)
dev_dbg (sys, "hcd_unlink_urb %p fail %d\n", urb, retval);
@@ -1310,7 +1330,10 @@
}
/* pass ownership to the completion handler */
+ spin_lock (&urb->handler_lock);
+ urb->transfer_flags &= ~URB_ALIVE;
urb->complete (urb, regs);
+ spin_unlock (&urb->handler_lock);
usb_put_urb (urb);
}
EXPORT_SYMBOL (usb_hcd_giveback_urb);
===== drivers/usb/core/urb.c 1.19 vs edited =====
--- 1.19/drivers/usb/core/urb.c Wed Oct 30 22:55:08 2002
+++ edited/drivers/usb/core/urb.c Thu Jan 30 15:37:03 2003
@@ -44,6 +44,7 @@
memset(urb, 0, sizeof(*urb));
urb->count = (atomic_t)ATOMIC_INIT(1);
spin_lock_init(&urb->lock);
+ spin_lock_init(&urb->handler_lock);
return urb;
}
@@ -192,9 +193,14 @@
struct usb_device *dev;
struct usb_operations *op;
int is_out;
+ int status;
if (!urb || urb->hcpriv || !urb->complete)
return -EINVAL;
+ if (urb->transfer_flags & URB_ALIVE) {
+ BUG();
+ return -EINPROGRESS;
+ }
if (!(dev = urb->dev) || !dev->bus || dev->devnum <= 0)
return -ENODEV;
if (!(op = dev->bus->op) || !op->submit_urb)
@@ -346,7 +352,10 @@
urb->interval = temp;
}
- return op->submit_urb (urb, mem_flags);
+ status = op->submit_urb (urb, mem_flags);
+ if (!status)
+ urb->transfer_flags |= URB_ALIVE;
+ return status;
}
/*-------------------------------------------------------------------*/
-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
