ChangeSet 1.1757.66.2, 2004/07/14 14:28:52-07:00, stern@rowland.harvard.edu [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 Signed-off-by: Greg Kroah-Hartman Documentation/usb/error-codes.txt | 2 drivers/usb/core/hcd.c | 139 +++++++++++--------------------------- drivers/usb/core/hcd.h | 5 - drivers/usb/core/urb.c | 74 +++++++++++++++----- drivers/usb/host/hc_simple.c | 44 ++---------- include/linux/usb.h | 9 +- 6 files changed, 121 insertions(+), 152 deletions(-) diff -Nru a/Documentation/usb/error-codes.txt b/Documentation/usb/error-codes.txt --- a/Documentation/usb/error-codes.txt 2004-07-14 16:46:14 -07:00 +++ b/Documentation/usb/error-codes.txt 2004-07-14 16:46:14 -07:00 @@ -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 * diff -Nru a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c --- a/drivers/usb/core/hcd.c 2004-07-14 16:46:14 -07:00 +++ b/drivers/usb/core/hcd.c 2004-07-14 16:46:14 -07:00 @@ -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 = NULL; usb_hcd_giveback_urb (hcd, urb, NULL); local_irq_restore (flags); + return 0; } /*-------------------------------------------------------------------------*/ @@ -1029,7 +1033,6 @@ 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 @@ 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 @@ // 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 @@ 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 @@ * 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 @@ /* 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 @@ 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; @@ -1536,6 +1480,9 @@ /* 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); diff -Nru a/drivers/usb/core/hcd.h b/drivers/usb/core/hcd.h --- a/drivers/usb/core/hcd.h 2004-07-14 16:46:14 -07:00 +++ b/drivers/usb/core/hcd.h 2004-07-14 16:46:14 -07:00 @@ -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; @@ -359,6 +359,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); diff -Nru a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c --- a/drivers/usb/core/urb.c 2004-07-14 16:46:14 -07:00 +++ b/drivers/usb/core/urb.c 2004-07-14 16:46:14 -07:00 @@ -407,26 +407,25 @@ * 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 @@ * 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_get_urb); EXPORT_SYMBOL(usb_submit_urb); EXPORT_SYMBOL(usb_unlink_urb); +EXPORT_SYMBOL(usb_kill_urb); diff -Nru a/drivers/usb/host/hc_simple.c b/drivers/usb/host/hc_simple.c --- a/drivers/usb/host/hc_simple.c 2004-07-14 16:46:14 -07:00 +++ b/drivers/usb/host/hc_simple.c 2004-07-14 16:46:14 -07:00 @@ -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 (urb->reject) + wake_up (&usb_kill_urb_queue); } } diff -Nru a/include/linux/usb.h b/include/linux/usb.h --- a/include/linux/usb.h 2004-07-14 16:46:14 -07:00 +++ b/include/linux/usb.h 2004-07-14 16:46:14 -07:00 @@ -657,7 +657,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: * @@ -684,7 +684,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 @@ -762,6 +764,8 @@ 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 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,