[PATCH] USB: Add usb_kill_urb()

This patch is a slightly revised version of as277c, updated to match the
current source.  The only difference from the older version is that this
makes urb->use_count into an atomic_t, to avoid the overhead of an extra
locking step each time an URB is submitted and given back.  The important
features of this patch are:

	-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 and ->use_count fields added to struct urb.
	The reject field is protected by urb->lock, and locking is
	required only in usb_kill_urb() which doesn't have to be fast.

	Single wait_queue used for all processes waiting inside
	usb_kill_urb().  The wait queue is woken up only when an URB
	is given back with ->reject set.

	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.

	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.

	usb_kill_urb() added to urb.c.

	hc_simple() host driver is partially updated -- it should
	compile but it won't really work right.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>

Documentation/usb/error-codes.txt

@@ -47,6 +47,8 @@ USB-specific:
 -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               *

drivers/usb/core/hcd.c

@@ -102,6 +102,9 @@ EXPORT_SYMBOL_GPL (usb_bus_list_lock);
 /* 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 @@ static int rh_urb_enqueue (struct usb_hcd *hcd, struct urb *urb)
 
 /*-------------------------------------------------------------------------*/
 
-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 @@ void usb_rh_status_dequeue (struct usb_hcd *hcd, struct urb *urb)
 	urb->hcpriv = NULL;
 	usb_hcd_giveback_urb (hcd, urb, NULL);
 	local_irq_restore (flags);
+	return 0;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -1029,7 +1033,6 @@ static int hcd_alloc_dev (struct usb_device *udev)
 static void urb_unlink (struct urb *urb)
 {
 	unsigned long		flags;
-	struct usb_device	*dev;
 
 	/* Release any periodic transfer bandwidth */
 	if (urb->bandwidth)
@@ -1040,9 +1043,8 @@ static void urb_unlink (struct urb *urb)
 
 	spin_lock_irqsave (&hcd_data_lock, flags);
 	list_del_init (&urb->urb_list);
-	dev = urb->dev;
 	spin_unlock_irqrestore (&hcd_data_lock, flags);
-	usb_put_dev (dev);
+	usb_put_dev (urb->dev);
 }
 
 
@@ -1079,23 +1081,28 @@ static int hcd_submit_urb (struct urb *urb, int mem_flags)
 	// 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);
+	atomic_inc (&urb->use_count);
+
 	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
@@ -1132,9 +1139,12 @@ static int hcd_submit_urb (struct urb *urb, int mem_flags)
 
 	status = hcd->driver->urb_enqueue (hcd, urb, mem_flags);
 done:
-	if (status) {
-		usb_put_urb (urb);
+	if (unlikely (status)) {
 		urb_unlink (urb);
+		atomic_dec (&urb->use_count);
+		if (urb->reject)
+			wake_up (&usb_kill_urb_queue);
+		usb_put_urb (urb);
 	}
 	return status;
 }
@@ -1157,60 +1167,39 @@ static int hcd_get_frame_number (struct usb_device *udev)
  * 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 = NULL;
 	struct device			*sys = NULL;
 	unsigned long			flags;
-	struct completion_splice	splice;
 	struct list_head		*tmp;
 	int				retval;
 
@@ -1262,8 +1251,6 @@ static int hcd_unlink_urb (struct urb *urb)
 
 	/* 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;
@@ -1281,62 +1268,19 @@ static int hcd_unlink_urb (struct urb *urb)
 		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;
-	}
-	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);
+	urb->status = status;
 
-		/* 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 (&hcd_data_lock);
 	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;
@@ -1536,6 +1480,9 @@ void usb_hcd_giveback_urb (struct usb_hcd *hcd, struct urb *urb, struct pt_regs
 
 	/* pass ownership to the completion handler */
 	urb->complete (urb, regs);
+	atomic_dec (&urb->use_count);
+	if (unlikely (urb->reject))
+		wake_up (&usb_kill_urb_queue);
 	usb_put_urb (urb);
 }
 EXPORT_SYMBOL (usb_hcd_giveback_urb);

drivers/usb/core/hcd.h

@@ -142,7 +142,7 @@ struct usb_operations {
 	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 @@ struct hc_driver {
 
 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;
@@ -359,6 +359,7 @@ static inline int hcd_register_root (struct usb_device *usb_dev,
 
 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

@@ -407,26 +407,25 @@ int usb_submit_urb(struct urb *urb, int mem_flags)
  * canceled (rather than any other code) and 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
@@ -449,16 +448,54 @@ int usb_submit_urb(struct urb *urb, int mem_flags)
  * An unlinked URB may leave a gap in the stream of packets.  It is undefined
  * whether such gaps can be filled in.
  *
- * When control URBs terminates with an error, it is likely that the
+ * When a control URB terminates with an error, it is likely that the
  * status stage of the transfer will not take place, even if it is merely
  * a soft error resulting from a short-packet with URB_SHORT_NOT_OK set.
  */
 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.
+ *
+ * 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;
+	spin_lock_irq(&urb->lock);
+	++urb->reject;
+	spin_unlock_irq(&urb->lock);
+
+	urb->dev->bus->op->unlink_urb(urb, -ENOENT);
+	wait_event(usb_kill_urb_queue, atomic_read(&urb->use_count) == 0);
+
+	spin_lock_irq(&urb->lock);
+	--urb->reject;
+	spin_unlock_irq(&urb->lock);
 }
 
 EXPORT_SYMBOL(usb_init_urb);
@@ -467,4 +504,5 @@ EXPORT_SYMBOL(usb_free_urb);
 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

@@ -189,7 +189,7 @@ static int hci_submit_urb (struct urb * urb, int mem_flags)
  *
  * 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 @@ static int hci_unlink_urb (struct urb * urb)
 	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);
+		urb->status = status;
 		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);
-		}
 	} 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 (urb->reject)
+				wake_up (&usb_kill_urb_queue);
 		}
 	}
 

include/linux/usb.h

@@ -657,7 +657,7 @@ typedef void (*usb_complete_t)(struct urb *, struct pt_regs *);
  * 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:
  *
@@ -684,7 +684,9 @@ typedef void (*usb_complete_t)(struct urb *, struct pt_regs *);
  * 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
@@ -762,6 +764,8 @@ struct urb
 	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 */
+	atomic_t use_count;		/* concurrent submissions counter */
+	u8 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 */
@@ -897,6 +901,7 @@ extern void usb_free_urb(struct urb *urb);
 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,