Here's another revision for usb_kill_urb(). This one adds new fields for
both the reject flag and the usage counter, so there's no possibility of
usb_get_urb() breaking things.
Although Oliver claimed the usage counter could be a single bit, that's
not correct. It needs to be able to count at least up to 2, because while
an URB's completion handler is running the count is 1, and if the handler
resubmits the count has to go up to 2. Thus at least a 2-bit counter is
necessary, and an 8-bit counter should be sufficient.
Likewise, to account for the faint possibility that two different threads
might call usb_kill_urb() for the same URB simultaneously, I decided to
make urb->reject an 8-bit counter as well. Both fields are protected by
urb->lock, so the fact that updates aren't atomic doesn't matter. And it
still leaves 16 bits free and available for other uses! :-)
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 Tue May 18 11:34:53 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,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 */
+ u8 reject; /* submissions will fail */
+ u8 use_count; /* concurrent submissions counter */
/* public, documented fields in the urb that can be used by drivers */
struct usb_device *dev; /* (in) pointer to associated device */
@@ -907,6 +911,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 Tue May 18 11:52:49 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;
}
/*-------------------------------------------------------------------------*/
@@ -1073,24 +1077,32 @@
// 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) {
+ spin_lock_irqsave (&urb->lock, flags);
+ if (unlikely (urb->reject))
+ status = -EPERM;
+ else if (HCD_IS_RUNNING (hcd->state) &&
+ hcd->state != USB_STATE_QUIESCING) {
+ ++urb->use_count;
usb_get_dev (urb->dev);
+ spin_lock (&hcd_data_lock);
list_add_tail (&urb->urb_list, &dev->urb_list);
+ spin_unlock (&hcd_data_lock);
status = 0;
- } else {
- INIT_LIST_HEAD (&urb->urb_list);
+ } else
status = -ESHUTDOWN;
- }
- spin_unlock_irqrestore (&hcd_data_lock, flags);
- if (status)
+ spin_unlock_irqrestore (&urb->lock, flags);
+
+ 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
@@ -1127,9 +1139,14 @@
status = hcd->driver->urb_enqueue (hcd, urb, mem_flags);
done:
- if (status) {
- usb_put_urb (urb);
+ if (unlikely (status)) {
urb_unlink (urb);
+ spin_lock_irqsave (&urb->lock, flags);
+ --urb->use_count;
+ spin_unlock_irqrestore (&urb->lock, flags);
+ if (urb->reject)
+ wake_up (&usb_kill_urb_queue);
+ usb_put_urb (urb);
}
return status;
}
@@ -1152,60 +1169,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 +1253,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 +1270,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;
@@ -1531,6 +1482,11 @@
/* pass ownership to the completion handler */
urb->complete (urb, regs);
+ spin_lock (&urb->lock);
+ --urb->use_count;
+ spin_unlock (&urb->lock);
+ 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 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 Tue May 18 11:34:20 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,48 @@
*/
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, 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);
===== 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
