This is an updated version of a patch that's originally from Alan Stern. What it does:
- Adds a "struct completion *" to struct urb, for use if that urb is being synchronously unlinked.
- Uses that to remove the somewhat dubious "splice" mechanism from the hcd.c synchronous unlink logic.
- Defines a new internal helper, unlink1(), that is needed for certain pending bugfixes. (The usbcore code doesn't clean up pending requests in a lot of cases it should.)
- Incorporates some doc feedback.
Net change in behavior should be zero, which is one of the key differences from Alan's version.
- Dave
--- 1.92/drivers/usb/core/hcd.c Tue Mar 25 07:10:33 2003
+++ edited/drivers/usb/core/hcd.c Wed Apr 2 17:34:25 2003
@@ -959,6 +959,7 @@
/* clear all state linking urb to this dev (and hcd) */
spin_lock_irqsave (&hcd_data_lock, flags);
+ urb->wait = 0;
list_del_init (&urb->urb_list);
dev = urb->dev;
usb_put_dev (dev);
@@ -996,9 +997,9 @@
* i/o completion (normal or fault) or unlinking.
*/
- // FIXME: verify that quiescing hc works right (RH cleans up)
-
spin_lock_irqsave (&hcd_data_lock, flags);
+ urb->status = -EINPROGRESS;
+ urb->wait = 0;
if (HCD_IS_RUNNING (hcd->state) && hcd->state != USB_STATE_QUIESCING) {
usb_get_dev (urb->dev);
list_add_tail (&urb->urb_list, &dev->urb_list);
@@ -1068,28 +1069,29 @@
/*-------------------------------------------------------------------------*/
-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)
+/* this makes the hcd giveback() the urb more quickly, by kicking it
+ * off hardware endpoint queues (which may take a while) and returning
+ * it as soon as practical. we already set up the urb's return status,
+ * but we can't know if the callback completed already.
+ *
+ * hcds may maintain state about the endpoint (qh) until they're told
+ * to get rid of it because of config change or disconnect events.
+ */
+static void
+unlink1 (struct usb_hcd *hcd, struct urb *urb)
{
- 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);
+ if (urb == (struct urb *) hcd->rh_timer.data)
+ usb_rh_status_dequeue (hcd, urb);
+ else {
+ int value;
- /* then let the synchronous unlink call complete */
- complete (&splice->done);
+ /* failures "should" be harmless */
+ value = hcd->driver->urb_dequeue (hcd, urb);
+ if (value != 0)
+ dev_dbg (hcd->controller,
+ "dequeue %p --> %d\n",
+ urb, value);
+ }
}
/*
@@ -1104,7 +1106,7 @@
struct usb_hcd *hcd = 0;
struct device *sys = 0;
unsigned long flags;
- struct completion_splice splice;
+ struct completion done;
int retval;
if (!urb)
@@ -1137,6 +1139,13 @@
goto done;
}
+ /* submissions can only be canceled once! */
+ if (urb->wait) {
+ retval = -EINVAL;
+ goto done;
+ }
+
+ /* hcd must still be using the urb */
if (!urb->hcpriv) {
retval = -EINVAL;
goto done;
@@ -1144,8 +1153,6 @@
/* Any status except -EINPROGRESS means something already started to
* unlink this URB from the hardware. So there's no more work to do.
- *
- * FIXME use better explicit urb state
*/
if (urb->status != -EINPROGRESS) {
retval = -EBUSY;
@@ -1158,54 +1165,38 @@
*/
if (!(urb->transfer_flags & URB_ASYNC_UNLINK)) {
if (in_interrupt ()) {
- dev_dbg (hcd->controller, "non-async unlink in_interrupt");
+ /* this is a bug in the device driver */
+ dev_err (&urb->dev->dev, "driver bug: "
+ "non-async unlink in_interrupt");
+ dump_stack ();
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;
+ /* synchronous unlink: block till we see
+ * giveback() signal this completion
+ */
+ init_completion (&done);
+ urb->wait = &done;
urb->status = -ENOENT;
+ retval = 0;
} else {
/* asynchronous unlink */
urb->status = -ECONNRESET;
+ retval = -EINPROGRESS;
}
spin_unlock (&hcd_data_lock);
spin_unlock_irqrestore (&urb->lock, flags);
- if (urb == (struct urb *) hcd->rh_timer.data) {
- usb_rh_status_dequeue (hcd, urb);
- retval = 0;
- } 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;
- }
- }
+ unlink1 (hcd, urb);
/* block till giveback, if needed */
- if (urb->transfer_flags & URB_ASYNC_UNLINK)
- return -EINPROGRESS;
-
- wait_for_completion (&splice.done);
- return 0;
+ if (retval != -EINPROGRESS)
+ wait_for_completion (&done);
+ return retval;
done:
spin_unlock (&hcd_data_lock);
spin_unlock_irqrestore (&urb->lock, flags);
-bye:
if (retval && sys)
dev_dbg (sys, "hcd_unlink_urb %p fail %d\n", urb, retval);
return retval;
@@ -1290,6 +1281,8 @@
*/
void usb_hcd_giveback_urb (struct usb_hcd *hcd, struct urb *urb, struct pt_regs *regs)
{
+ struct completion *wait = urb->wait;
+
urb_unlink (urb);
// NOTE: a generic device/urb monitoring hook would go here.
@@ -1313,6 +1306,9 @@
/* pass ownership to the completion handler */
urb->complete (urb, regs);
usb_put_urb (urb);
+
+ if (wait)
+ complete (wait);
}
EXPORT_SYMBOL (usb_hcd_giveback_urb);
--- 1.26/drivers/usb/core/urb.c Mon Mar 17 16:32:26 2003
+++ edited/drivers/usb/core/urb.c Thu Apr 3 08:32:09 2003
@@ -360,24 +360,25 @@
* usb_unlink_urb - abort/cancel a transfer request for an endpoint
* @urb: pointer to urb describing a previously submitted request
*
- * This routine cancels an in-progress request. The requests's
- * completion handler will be called with a status code indicating
- * that the request has been canceled, and that control of the URB
- * has been returned to that device driver.
+ * This routine cancels an in-progress request. Successful cancelation
+ * means the requests's completion handler will be called with a status
+ * code indicating that the request has been canceled, and usually means
+ * that the requested I/O won't yet have finished. URBs complete only
+ * once per submission, and so may be canceled only once per submission.
*
* When the URB_ASYNC_UNLINK transfer flag for the URB is clear, this
- * request is synchronous. Success is indicated by returning zero,
- * at which time the urb will have been unlinked,
- * and the completion function will see status -ENOENT. Failure is
- * indicated by any other return value. This mode may not be used
- * when unlinking an urb from an interrupt context, such as a bottom
- * half or a completion handler,
+ * request is synchronous. Success is indicated by returning zero, at
+ * which time the urb will have been unlinked and its completion handler
+ * will have reported urb->status -ENOENT. Other return values indicate
+ * failure. This mode may not be used when unlinking an urb from an
+ * interrupt context, or in other cases when the caller can't schedule().
*
* When the URB_ASYNC_UNLINK transfer flag for the URB is set, this
- * request is asynchronous. Success is indicated by returning -EINPROGRESS,
- * at which time the urb will normally not have been unlinked,
- * and the completion function will see status -ECONNRESET. Failure is
- * indicated by any other return value.
+ * request is asynchronous. Success is indicated by returning -EINPROGRESS.
+ * Failure is indicated by any other return value. When invoked because of
+ * a successful asynchronous unlink, the urb's completion handler reports
+ * urb->status -ECONNRESET. Such completions will usually not be reported
+ * until after this call returns.
*/
int usb_unlink_urb(struct urb *urb)
{
--- 1.133/include/linux/usb.h Mon Mar 17 16:32:33 2003
+++ edited/include/linux/usb.h Wed Apr 2 17:17:58 2003
@@ -637,6 +637,7 @@
spinlock_t lock; /* lock for the URB */
atomic_t count; /* reference count of the URB */
void *hcpriv; /* private data for host controller */
+ struct completion *wait; /* for usbcore synchronous unlinking */
struct list_head urb_list; /* list pointer to all active urbs */
struct usb_device *dev; /* (in) pointer to associated device */
unsigned int pipe; /* (in) pipe information */
