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