Here is another version of my patch to provide proper synchronous unlinks.
This one uses a struct completion rather than a spinlock, and it also
eliminates all the awkwardness of the "splice" mechanism currently in use.
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 Tue Feb 4 11:55:28 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;
@@ -726,6 +727,7 @@
struct urb
{
spinlock_t lock; /* lock for the URB */
+ struct completion *wakeup; /* synchronous unlink waiting queue */
atomic_t count; /* reference count of the URB */
void *hcpriv; /* private data for host controller */
struct list_head urb_list; /* list pointer to all active urbs */
===== 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 Tue Feb 4 12:18:51 2003
@@ -192,9 +192,15 @@
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) {
+ err ("%s: attempted double submission of urb", __FUNCTION__);
+ // This would be a good place for a stack trace
+ return -EINPROGRESS;
+ }
if (!(dev = urb->dev) || !dev->bus || dev->devnum <= 0)
return -ENODEV;
if (!(op = dev->bus->op) || !op->submit_urb)
@@ -346,7 +352,12 @@
urb->interval = temp;
}
- return op->submit_urb (urb, mem_flags);
+ status = op->submit_urb (urb, mem_flags);
+ if (!status) {
+ urb->transfer_flags |= URB_ALIVE;
+ urb->wakeup = NULL;
+ }
+ return status;
}
/*-------------------------------------------------------------------*/
===== 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 Tue Feb 4 12:11:26 2003
@@ -1067,30 +1067,6 @@
/*-------------------------------------------------------------------------*/
-struct completion_splice { // modified urb context:
- /* did we complete? */
- struct completion done;
-
- /* original urb data */
- usb_complete_t complete;
- void *context;
-};
-
-static void unlink_complete (struct urb *urb, struct pt_regs *regs)
-{
- struct completion_splice *splice;
-
- splice = (struct completion_splice *) urb->context;
-
- /* issue original completion call */
- urb->complete = splice->complete;
- urb->context = splice->context;
- urb->complete (urb, regs);
-
- /* then let the synchronous unlink call complete */
- complete (&splice->done);
-}
-
/*
* called in any context; note ASYNC_UNLINK restrictions
*
@@ -1103,8 +1079,9 @@
struct usb_hcd *hcd = 0;
struct device *sys = 0;
unsigned long flags;
- struct completion_splice splice;
+ struct completion wakeup_list;
int retval;
+ int just_waiting = 0;
if (!urb)
return -EINVAL;
@@ -1136,24 +1113,37 @@
goto done;
}
- if (!urb->hcpriv) {
+ /* If the URB is not ALIVE, then either the completion routine is
+ * running or else the completion routine has exited and the URB
+ * is now idle. We can distinguish the second case by seeing if
+ * urb->wakeup is NULL; it is always set while the completion routine
+ * is running, unless the completion routine resubmits the URB (in
+ * which case the URB would now be ALIVE).
+ */
+ if (!(urb->transfer_flags & URB_ALIVE) && !urb->wakeup) {
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
+ * request is async there's no more work to do. Otherwise we will
+ * just wait for the completion routine to return.
*
* FIXME use better explicit urb state
*/
- if (urb->status != -EINPROGRESS) {
- retval = -EBUSY;
- goto done;
+ if (urb->status != -EINPROGRESS || !urb->hcpriv) {
+ if (urb->transfer_flags & URB_ASYNC_UNLINK) {
+ retval = -EBUSY;
+ goto done;
+ }
+ just_waiting = 1;
}
/* maybe set up to block until the urb's completion fires. the
* lower level hcd code is always async, locking on urb->status
- * updates; an intercepted completion unblocks us.
+ * updates; return from the completion routine unblocks us.
*/
if (!(urb->transfer_flags & URB_ASYNC_UNLINK)) {
if (in_interrupt ()) {
@@ -1161,13 +1151,16 @@
retval = -EWOULDBLOCK;
goto done;
}
- /* synchronous unlink: block till we see the completion */
- init_completion (&splice.done);
- splice.complete = urb->complete;
- splice.context = urb->context;
- urb->complete = unlink_complete;
- urb->context = &splice;
- urb->status = -ENOENT;
+ /* synchronous unlink: block till after the completion */
+ if (!urb->wakeup) {
+ urb->wakeup = &wakeup_list;
+ init_completion (&wakeup_list);
+ }
+ /* 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;
@@ -1175,21 +1168,17 @@
spin_unlock (&hcd_data_lock);
spin_unlock_irqrestore (&urb->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 {
+ else {
retval = hcd->driver->urb_dequeue (hcd, urb);
/* hcds shouldn't really fail these calls, but... */
if (retval) {
dev_dbg (sys, "dequeue %p --> %d\n", urb, retval);
- if (!(urb->transfer_flags & URB_ASYNC_UNLINK)) {
- spin_lock_irqsave (&urb->lock, flags);
- urb->complete = splice.complete;
- urb->context = splice.context;
- spin_unlock_irqrestore (&urb->lock, flags);
- }
goto bye;
}
}
@@ -1198,7 +1187,7 @@
if (urb->transfer_flags & URB_ASYNC_UNLINK)
return -EINPROGRESS;
- wait_for_completion (&splice.done);
+ wait_for_completion (urb->wakeup);
return 0;
done:
@@ -1289,6 +1278,27 @@
*/
void usb_hcd_giveback_urb (struct usb_hcd *hcd, struct urb *urb, struct pt_regs *regs)
{
+ struct completion wakeup_list;
+ struct completion *wakeup_save;
+
+ /*
+ * Mark the URB as no longer ALIVE.
+ *
+ * If there aren't already any processes waiting for the completion
+ * handler, start a new urb->wakeup queue. Synchronous unlink requests
+ * arriving while the completion handler runs will wait on this queue.
+ * If the handler resubmits the URB then a new queue will be created,
+ * and we will only wake up the processes waiting on _this_ queue.
+ */
+ spin_lock (&urb->lock);
+ urb->transfer_flags &= ~URB_ALIVE;
+ if (!urb->wakeup) {
+ urb->wakeup = &wakeup_list;
+ init_completion (&wakeup_list);
+ }
+ wakeup_save = urb->wakeup;
+ spin_unlock (&urb->lock);
+
urb_unlink (urb);
// NOTE: a generic device/urb monitoring hook would go here.
@@ -1311,6 +1321,18 @@
/* pass ownership to the completion handler */
urb->complete (urb, regs);
+
+ /*
+ * If the urb->wakeup pointer hasn't changed then the URB wasn't
+ * resubmitted, so we must clear urb->wakeup to indicate that the
+ * URB is now idle.
+ */
+ spin_lock (&urb->lock);
+ if (urb->wakeup == wakeup_save)
+ urb->wakeup = NULL;
+ spin_unlock (&urb->lock);
+
+ complete_all (wakeup_save);
usb_put_urb (urb);
}
EXPORT_SYMBOL (usb_hcd_giveback_urb);
-------------------------------------------------------
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
