This is a revised patch implementing usb_kill_urb().  Major points:

        -EPERM added to Documentation/usb/error-codes.txt.

        Failure to use URB_ASYNC_UNLINK with usb_unlink_urb() is
        deprecated in the documentation.

        New ->reject field added to struct urb.

        Single wait_queue used for all processes waiting inside 
        usb_kill_urb().

        usb_rh_status_dequeue() changed to return int.  It looks like
        this function should be declared static; it's not used outside
        the hcd.c file.

        URB idle status determined by checking for the reference
        count being <= 1.  No separate counter is used.

        Prototype for unlink_urb() in struct usb_operations is changed
        to include a status code argument.  This is necessary so that
        the different unlink paths can return -ENOENT and -ECONNRESET
        as appropriate.

        Support for synchronous usb_unlink_urb() has been removed,
        such calls are passed to usb_kill_urb().

        Kerneldoc for usb_unlink_urb() is updated.

        hc_simple() host driver is partially updated -- it should
        compile but it won't really work right.

That last change isn't complete, but the hc_sl811 driver doesn't look like 
it really works properly under 2.6 anyway.

The earlier patch as278, updating the hub driver to use usb_kill_urb(), is 
still compatible with this patch.

Alan Stern



===== Documentation/usb/error-codes.txt 1.9 vs edited =====
--- 1.9/Documentation/usb/error-codes.txt       Wed Mar 17 14:16:42 2004
+++ edited/Documentation/usb/error-codes.txt    Mon May 17 11:08:41 2004
@@ -47,6 +47,8 @@
 -ESHUTDOWN     The host controller has been disabled due to some
                problem that could not be worked around.
 
+-EPERM         Submission failed because urb->reject was set.
+
 
 **************************************************************************
 *                   Error codes returned by in urb->status               *
===== include/linux/usb.h 1.185 vs edited =====
--- 1.185/include/linux/usb.h   Mon May  3 02:42:51 2004
+++ edited/include/linux/usb.h  Mon May 17 11:03:10 2004
@@ -667,7 +667,7 @@
  * calling usb_alloc_urb() and freed with a call to usb_free_urb().
  * Initialization may be done using various usb_fill_*_urb() functions.  URBs
  * are submitted using usb_submit_urb(), and pending requests may be canceled
- * using usb_unlink_urb().
+ * using usb_unlink_urb() or usb_kill_urb().
  *
  * Data Transfer Buffers:
  *
@@ -694,7 +694,9 @@
  * All URBs submitted must initialize dev, pipe,
  * transfer_flags (may be zero), complete, timeout (may be zero).
  * The URB_ASYNC_UNLINK transfer flag affects later invocations of
- * the usb_unlink_urb() routine.
+ * the usb_unlink_urb() routine.  Note: Failure to set URB_ASYNC_UNLINK
+ * with usb_unlink_urb() is deprecated.  For synchronous unlinks use
+ * usb_kill_urb() instead.
  *
  * All URBs must also initialize 
  * transfer_buffer and transfer_buffer_length.  They may provide the
@@ -772,6 +774,7 @@
        void *hcpriv;                   /* private data for host controller */
        struct list_head urb_list;      /* list pointer to all active urbs */
        int bandwidth;                  /* bandwidth for INT/ISO request */
+       int reject;                     /* submissions will fail */
 
        /* public, documented fields in the urb that can be used by drivers */
        struct usb_device *dev;         /* (in) pointer to associated device */
@@ -907,6 +910,7 @@
 extern struct urb *usb_get_urb(struct urb *urb);
 extern int usb_submit_urb(struct urb *urb, int mem_flags);
 extern int usb_unlink_urb(struct urb *urb);
+extern void usb_kill_urb(struct urb *urb);
 
 #define HAVE_USB_BUFFERS
 void *usb_buffer_alloc (struct usb_device *dev, size_t size,
===== drivers/usb/core/hcd.c 1.137 vs edited =====
--- 1.137/drivers/usb/core/hcd.c        Wed Apr 21 06:46:29 2004
+++ edited/drivers/usb/core/hcd.c       Mon May 17 11:31:34 2004
@@ -102,6 +102,9 @@
 /* used when updating hcd data */
 static spinlock_t hcd_data_lock = SPIN_LOCK_UNLOCKED;
 
+/* wait queue for synchronous unlinks */
+DECLARE_WAIT_QUEUE_HEAD(usb_kill_urb_queue);
+
 /*-------------------------------------------------------------------------*/
 
 /*
@@ -569,7 +572,7 @@
 
 /*-------------------------------------------------------------------------*/
 
-void usb_rh_status_dequeue (struct usb_hcd *hcd, struct urb *urb)
+int usb_rh_status_dequeue (struct usb_hcd *hcd, struct urb *urb)
 {
        unsigned long   flags;
 
@@ -581,6 +584,7 @@
        urb->hcpriv = 0;
        usb_hcd_giveback_urb (hcd, urb, NULL);
        local_irq_restore (flags);
+       return 0;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -1074,23 +1078,27 @@
        // FIXME:  verify that quiescing hc works right (RH cleans up)
 
        spin_lock_irqsave (&hcd_data_lock, flags);
-       if (HCD_IS_RUNNING (hcd->state) && hcd->state != USB_STATE_QUIESCING) {
+       if (unlikely(urb->reject))
+               status = -EPERM;
+       else 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);
                status = 0;
-       } else {
-               INIT_LIST_HEAD (&urb->urb_list);
+       } else
                status = -ESHUTDOWN;
-       }
        spin_unlock_irqrestore (&hcd_data_lock, flags);
-       if (status)
+       if (status) {
+               INIT_LIST_HEAD (&urb->urb_list);
                return status;
+       }
 
        /* increment urb's reference count as part of giving it to the HCD
         * (which now controls it).  HCD guarantees that it either returns
         * an error or calls giveback(), but not both.
         */
        urb = usb_get_urb (urb);
+
        if (urb->dev == hcd->self.root_hub) {
                /* NOTE:  requirement on hub callers (usbfs and the hub
                 * driver, for now) that URBs' urb->transfer_buffer be
@@ -1128,8 +1136,12 @@
        status = hcd->driver->urb_enqueue (hcd, urb, mem_flags);
 done:
        if (status) {
-               usb_put_urb (urb);
+               int rej = urb->reject;
+
                urb_unlink (urb);
+               usb_put_urb (urb);
+               if (unlikely (rej))
+                       wake_up (&usb_kill_urb_queue);
        }
        return status;
 }
@@ -1152,60 +1164,39 @@
  * soon as practical.  we've already set up the urb's return status,
  * but we can't know if the callback completed already.
  */
-static void
+static int
 unlink1 (struct usb_hcd *hcd, struct urb *urb)
 {
+       int             value;
+
        if (urb == (struct urb *) hcd->rh_timer.data)
-               usb_rh_status_dequeue (hcd, urb);
+               value = usb_rh_status_dequeue (hcd, urb);
        else {
-               int             value;
 
-               /* failures "should" be harmless */
+               /* The only reason an HCD might fail this call is if
+                * it has not yet fully queued the urb to begin with.
+                * Such failures should be harmless. */
                value = hcd->driver->urb_dequeue (hcd, urb);
-               if (value != 0)
-                       dev_dbg (hcd->self.controller,
-                               "dequeue %p --> %d\n",
-                               urb, value);
        }
-}
-
-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);
+       if (value != 0)
+               dev_dbg (hcd->self.controller, "dequeue %p --> %d\n",
+                               urb, value);
+       return value;
 }
 
 /*
- * called in any context; note ASYNC_UNLINK restrictions
+ * called in any context
  *
  * caller guarantees urb won't be recycled till both unlink()
  * and the urb's completion function return
  */
-static int hcd_unlink_urb (struct urb *urb)
+static int hcd_unlink_urb (struct urb *urb, int status)
 {
        struct hcd_dev                  *dev;
        struct usb_hcd                  *hcd = 0;
        struct device                   *sys = 0;
        unsigned long                   flags;
-       struct completion_splice        splice;
        struct list_head                *tmp;
        int                             retval;
 
@@ -1257,8 +1248,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;
@@ -1276,62 +1265,19 @@
                hcd->saw_irq = 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.
-        */
-       if (!(urb->transfer_flags & URB_ASYNC_UNLINK)) {
-               if (in_interrupt ()) {
-                       dev_dbg (hcd->self.controller, 
-                               "non-async unlink in_interrupt");
-                       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;
-       } else {
-               /* asynchronous unlink */
-               urb->status = -ECONNRESET;
-       }
+       urb->status = status;
+
        spin_unlock (&hcd_data_lock);
        spin_unlock_irqrestore (&urb->lock, flags);
 
-       // FIXME remove splicing, so this becomes unlink1 (hcd, urb);
-       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;
-               }
-       }
-
-       /* block till giveback, if needed */
-       if (urb->transfer_flags & URB_ASYNC_UNLINK)
-               return -EINPROGRESS;
-
-       wait_for_completion (&splice.done);
-       return 0;
+       retval = unlink1 (hcd, urb);
+       if (retval == 0)
+               retval = -EINPROGRESS;
+       return retval;
 
 done:
        spin_unlock (&hcd_data_lock);
        spin_unlock_irqrestore (&urb->lock, flags);
-bye:
        if (retval != -EIDRM && sys && sys->driver)
                dev_dbg (sys, "hcd_unlink_urb %p fail %d\n", urb, retval);
        return retval;
@@ -1374,6 +1320,7 @@
        spin_lock (&hcd_data_lock);
        list_for_each_entry (urb, &dev->urb_list, urb_list) {
                int     tmp = urb->pipe;
+               int     rej = 0;
 
                /* ignore urbs for other endpoints */
                if (usb_pipeendpoint (tmp) != epnum)
@@ -1400,6 +1347,7 @@
 
                /* kick hcd unless it's already returning this */
                if (tmp == -EINPROGRESS) {
+                       rej = urb->reject;
                        tmp = urb->pipe;
                        unlink1 (hcd, urb);
                        dev_dbg (hcd->self.controller,
@@ -1415,6 +1363,8 @@
                                }; s;}));
                }
                usb_put_urb (urb);
+               if (unlikely (rej))
+                       wake_up (&usb_kill_urb_queue);
 
                /* list contents may have changed */
                goto rescan;
@@ -1506,6 +1456,8 @@
  */
 void usb_hcd_giveback_urb (struct usb_hcd *hcd, struct urb *urb, struct pt_regs *regs)
 {
+       int rej = urb->reject;
+
        urb_unlink (urb);
 
        // NOTE:  a generic device/urb monitoring hook would go here.
@@ -1532,6 +1484,8 @@
        /* pass ownership to the completion handler */
        urb->complete (urb, regs);
        usb_put_urb (urb);
+       if (unlikely (rej))
+               wake_up (&usb_kill_urb_queue);
 }
 EXPORT_SYMBOL (usb_hcd_giveback_urb);
 
===== drivers/usb/core/hcd.h 1.70 vs edited =====
--- 1.70/drivers/usb/core/hcd.h Wed Apr 28 10:01:03 2004
+++ edited/drivers/usb/core/hcd.h       Mon May 17 11:30:41 2004
@@ -142,7 +142,7 @@
        int (*deallocate)(struct usb_device *);
        int (*get_frame_number) (struct usb_device *usb_dev);
        int (*submit_urb) (struct urb *urb, int mem_flags);
-       int (*unlink_urb) (struct urb *urb);
+       int (*unlink_urb) (struct urb *urb, int status);
 
        /* allocate dma-consistent buffer for URB_DMA_NOMAPPING */
        void *(*buffer_alloc)(struct usb_bus *bus, size_t size,
@@ -207,7 +207,7 @@
 
 extern void usb_hcd_giveback_urb (struct usb_hcd *hcd, struct urb *urb, struct 
pt_regs *regs);
 extern void usb_bus_init (struct usb_bus *bus);
-extern void usb_rh_status_dequeue (struct usb_hcd *hcd, struct urb *urb);
+extern int usb_rh_status_dequeue (struct usb_hcd *hcd, struct urb *urb);
 
 #ifdef CONFIG_PCI
 struct pci_dev;
@@ -366,6 +366,7 @@
 
 extern struct list_head usb_bus_list;
 extern struct semaphore usb_bus_list_lock;
+extern wait_queue_head_t usb_kill_urb_queue;
 
 extern struct usb_bus *usb_bus_get (struct usb_bus *bus);
 extern void usb_bus_put (struct usb_bus *bus);
===== drivers/usb/core/urb.c 1.41 vs edited =====
--- 1.41/drivers/usb/core/urb.c Wed Apr 28 10:00:02 2004
+++ edited/drivers/usb/core/urb.c       Mon May 17 11:12:05 2004
@@ -404,29 +404,28 @@
  * once per submission, and may be canceled only once per submission.
  * Successful cancelation means the requests's completion handler will
  * be called with a status code indicating that the request has been
- * canceled (rather than any other code) and will quickly be removed
- * from host controller data structures.
+ * canceled (rather than any other code) and the request will quickly
+ * be removed from host controller data structures.
  *
- * 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 its completion
- * handler will have been called with urb->status == -ENOENT.  Failure is
- * indicated by any other return value.
- *
- * The synchronous cancelation mode may not be used
- * when unlinking an urb from an interrupt context, such as a bottom
- * half or a completion handler; or when holding a spinlock; or in
- * other cases when the caller can't schedule().
+ * In the past, clearing the URB_ASYNC_UNLINK transfer flag for the
+ * URB indicated that the request was synchronous.  This usage is now
+ * deprecated; if the flag is clear the call will be forwarded to
+ * usb_kill_urb() and the return value will be 0.  In the future, drivers
+ * should call usb_kill_urb() directly for synchronous unlinking.
  *
  * 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.
- * The completion function will see urb->status == -ECONNRESET.  Failure
- * is indicated by any other return value.
+ * at which time the URB will normally have been unlinked but not yet
+ * given back to the device driver.  When it is called, the completion
+ * function will see urb->status == -ECONNRESET.  Failure is indicated
+ * by any other return value.  Unlinking will fail when the URB is not
+ * currently "linked" (i.e., it was never submitted, or it was unlinked
+ * before, or the hardware is already finished with it), even if the
+ * completion handler has not yet run.
  *
  * Unlinking and Endpoint Queues:
  *
- * Host Controller Driver (HCDs) place all the URBs for a particular
+ * Host Controller Drivers (HCDs) place all the URBs for a particular
  * endpoint in a queue.  Normally the queue advances as the controller
  * hardware processes each request.  But when an URB terminates with any
  * fault (such as an error, or being unlinked) its queue stops, at least
@@ -455,10 +454,46 @@
  */
 int usb_unlink_urb(struct urb *urb)
 {
-       if (urb && urb->dev && urb->dev->bus && urb->dev->bus->op)
-               return urb->dev->bus->op->unlink_urb(urb);
-       else
+       if (!(urb->transfer_flags & URB_ASYNC_UNLINK)) {
+               usb_kill_urb(urb);
+               return 0;
+       }
+       if (!(urb->dev && urb->dev->bus && urb->dev->bus->op))
                return -ENODEV;
+       return urb->dev->bus->op->unlink_urb(urb, -ECONNRESET);
+}
+
+/**
+ * usb_kill_urb - cancel a transfer request and wait for it to finish
+ * @urb: pointer to URB describing a previously submitted request
+ *
+ * This routine cancels an in-progress request.  It is guaranteed that
+ * upon return all completion handlers will have finished and the URB
+ * will be totally idle and available for reuse.  These features make
+ * this an ideal way to stop I/O in a disconnect() callback or close()
+ * function.  If the request has not already finished or been unlinked
+ * the completion handler will see urb->status == -ENOENT.
+ *
+ * While the routine is running, attempts to resubmit the URB will fail
+ * with error -EPERM.  Thus even if the URB's completion handler always
+ * tries to resubmit, it will not succeed and the URB will become idle.
+ *
+ * The URB is determined to be idle when its reference count is 1.  As a
+ * result, this routine won't work if the driver has used usb_get_urb()
+ * to increase the URB's reference count.
+ *
+ * This routine may not be used in an interrupt context, such as a bottom
+ * half or a completion handler; or when holding a spinlock; or in other
+ * situations where the caller can't schedule().
+ */
+void usb_kill_urb(struct urb *urb)
+{
+       if (!(urb->dev && urb->dev->bus && urb->dev->bus->op))
+               return;
+       urb->reject = 1;
+       urb->dev->bus->op->unlink_urb(urb, -ENOENT);
+       wait_event(usb_kill_urb_queue, atomic_read(&urb->kref.refcount) <= 1);
+       urb->reject = 0;
 }
 
 EXPORT_SYMBOL(usb_init_urb);
@@ -467,4 +502,5 @@
 EXPORT_SYMBOL(usb_get_urb);
 EXPORT_SYMBOL(usb_submit_urb);
 EXPORT_SYMBOL(usb_unlink_urb);
+EXPORT_SYMBOL(usb_kill_urb);
 
===== drivers/usb/host/hc_simple.c 1.12 vs edited =====
--- 1.12/drivers/usb/host/hc_simple.c   Mon Mar 17 19:32:26 2003
+++ edited/drivers/usb/host/hc_simple.c Mon May 17 10:07:56 2004
@@ -189,7 +189,7 @@
  *
  * Return: 0 if success or error code 
  **************************************************************************/
-static int hci_unlink_urb (struct urb * urb)
+static int hci_unlink_urb (struct urb * urb, int status)
 {
        unsigned long flags;
        hci_t *hci;
@@ -219,45 +219,21 @@
        if (!list_empty (&urb->urb_list) && urb->status == -EINPROGRESS) {
                /* URB active? */
 
-               if (urb->transfer_flags & URB_ASYNC_UNLINK) {
-                       /* asynchronous with callback */
-                       /* relink the urb to the del list */
-                       list_move (&urb->urb_list, &hci->del_list);
-                       spin_unlock_irqrestore (&usb_urb_lock, flags);
-               } else {
-                       /* synchronous without callback */
-
-                       add_wait_queue (&hci->waitq, &wait);
-
-                       set_current_state (TASK_UNINTERRUPTIBLE);
-                       comp = urb->complete;
-                       urb->complete = NULL;
-
-                       /* relink the urb to the del list */
-                       list_move(&urb->urb_list, &hci->del_list);
-
-                       spin_unlock_irqrestore (&usb_urb_lock, flags);
-
-                       schedule_timeout (HZ / 50);
-
-                       if (!list_empty (&urb->urb_list))
-                               list_del (&urb->urb_list);
-
-                       urb->complete = comp;
-                       urb->hcpriv = NULL;
-                       remove_wait_queue (&hci->waitq, &wait);
-               }
+               /* asynchronous with callback */
+               /* relink the urb to the del list */
+               list_move (&urb->urb_list, &hci->del_list);
+               urb->status = status;
+               spin_unlock_irqrestore (&usb_urb_lock, flags);
        } else {
                /* hcd does not own URB but we keep the driver happy anyway */
                spin_unlock_irqrestore (&usb_urb_lock, flags);
 
-               if (urb->complete && (urb->transfer_flags & URB_ASYNC_UNLINK)) {
-                       urb->status = -ENOENT;
+               if (urb->complete) {
+                       urb->status = status;
                        urb->actual_length = 0;
                        urb->complete (urb, NULL);
-                       urb->status = 0;
-               } else {
-                       urb->status = -ENOENT;
+               if (unlikely (tf & URB_REJECT))
+                       wake_up (&usb_kill_urb_queue);
                }
        }
 



-------------------------------------------------------
This SF.Net email is sponsored by: SourceForge.net Broadband
Sign-up now for SourceForge Broadband and get the fastest
6.0/768 connection for only $19.95/mo for the first 3 months!
http://ads.osdn.com/?ad_id=2562&alloc_id=6184&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to