- uses "struct completion" for simplicity
- the wait_event_timeout() logic is now like the other macros which can return before the event happens: returns a value
- clarity for MAX_SCHEDULE_TIMEOUT scenarios, by using the standard "timeouts are long" calling convention internally
- uses only the async primitives
- restores a warning when the request times out.
- can use dev_dbg() calls in drivers/usb/core/message.c
Still needs more testing (notably the timeout paths!) and cleanup, but I thought it'd be good to send this one around.
- Dave
--- 1.22/drivers/usb/core/message.c Wed Mar 5 07:24:34 2003 +++ edited/drivers/usb/core/message.c Wed Mar 19 23:18:46 2003 @@ -2,6 +2,14 @@ * message.c - synchronous message handling */ +#include <linux/config.h> + +#ifdef CONFIG_USB_DEBUG + #define DEBUG +#else + #undef DEBUG +#endif + #include <linux/pci.h> /* for scatterlist macros */ #include <linux/usb.h> #include <linux/module.h> @@ -11,78 +19,57 @@ #include "hcd.h" /* for usbcore internals */ -struct usb_api_data { - wait_queue_head_t wqh; - int done; -}; - -static void usb_api_blocking_completion(struct urb *urb, struct pt_regs *regs) +static void done (struct urb *urb, struct pt_regs *regs) { - struct usb_api_data *awd = (struct usb_api_data *)urb->context; - - awd->done = 1; - wmb(); - wake_up(&awd->wqh); + dev_dbg (&urb->dev->dev, "urb %p done\n", urb); + complete ((struct completion *)urb->context); } // Starts urb and waits for completion or timeout -static int usb_start_wait_urb(struct urb *urb, int timeout, int* actual_length) +static int usb_start_wait_urb(struct urb *urb, long timeout, int *actual_length) { - DECLARE_WAITQUEUE(wait, current); - struct usb_api_data awd; + DECLARE_COMPLETION (wait); int status; - init_waitqueue_head(&awd.wqh); - awd.done = 0; + might_sleep(); - set_current_state(TASK_UNINTERRUPTIBLE); - add_wait_queue(&awd.wqh, &wait); - - urb->context = &awd; - status = usb_submit_urb(urb, GFP_ATOMIC); + urb->context = &wait; + urb->complete = done; + urb->transfer_flags |= URB_ASYNC_UNLINK; + status = usb_submit_urb (urb, GFP_NOIO); if (status) { // something went wrong - usb_free_urb(urb); - set_current_state(TASK_RUNNING); - remove_wait_queue(&awd.wqh, &wait); return status; } - while (timeout && !awd.done) - { - timeout = schedule_timeout(timeout); - set_current_state(TASK_UNINTERRUPTIBLE); - rmb(); - } - - set_current_state(TASK_RUNNING); - remove_wait_queue(&awd.wqh, &wait); - - if (!timeout && !awd.done) { - if (urb->status != -EINPROGRESS) { /* No callback?!! */ - printk(KERN_ERR "usb: raced timeout, " - "pipe 0x%x status %d time left %d\n", - urb->pipe, urb->status, timeout); - status = urb->status; - } else { - warn("usb_control/bulk_msg: timeout"); - usb_unlink_urb(urb); // remove urb safely - status = -ETIMEDOUT; + /* cancel request if we time out */ + if (wait_event_timeout (wait.wait, wait.done, timeout) == 0) { + if ((status = usb_unlink_urb (urb)) == -EINPROGRESS) + wait_event (wait.wait, wait.done); + /* patch status unless we raced a "real" completion */ + if (urb->status == -ECONNRESET) { + dev_warn (&urb->dev->dev, + "timeout urb for ep%d%s (len %d/%d)\n", + usb_pipeendpoint (urb->pipe), + usb_pipein (urb->pipe) ? "in" : "out", + urb->actual_length, + urb->transfer_buffer_length); + urb->status = -ETIME; } - } else - status = urb->status; + } + status = urb->status; if (actual_length) *actual_length = urb->actual_length; - - usb_free_urb(urb); - return status; + + return status; } /*-------------------------------------------------------------------*/ // returns status (negative) or length (positive) int usb_internal_control_msg(struct usb_device *usb_dev, unsigned int pipe, - struct usb_ctrlrequest *cmd, void *data, int len, int timeout) + struct usb_ctrlrequest *cmd, void *data, int len, + int timeout /* 0 means MAX_SCHEDULE_TIMEOUT */ ) { struct urb *urb; int retv; @@ -93,9 +80,12 @@ return -ENOMEM; usb_fill_control_urb(urb, usb_dev, pipe, (unsigned char*)cmd, data, len, - usb_api_blocking_completion, 0); + 0, 0); - retv = usb_start_wait_urb(urb, timeout, &length); + retv = usb_start_wait_urb(urb, + timeout ? (long)timeout : MAX_SCHEDULE_TIMEOUT, + &length); + usb_free_urb(urb); if (retv < 0) return retv; else @@ -182,6 +172,7 @@ void *data, int len, int *actual_length, int timeout) { struct urb *urb; + int r; if (len < 0) return -EINVAL; @@ -190,10 +181,13 @@ if (!urb) return -ENOMEM; - usb_fill_bulk_urb(urb, usb_dev, pipe, data, len, - usb_api_blocking_completion, 0); + usb_fill_bulk_urb(urb, usb_dev, pipe, data, len, 0, 0); - return usb_start_wait_urb(urb,timeout,actual_length); + r = usb_start_wait_urb(urb, + timeout ? (long)timeout : MAX_SCHEDULE_TIMEOUT, + actual_length); + usb_free_urb(urb); + return r; } /*-------------------------------------------------------------------*/ --- 1.12/include/linux/wait.h Fri Nov 15 07:36:32 2002 +++ edited/include/linux/wait.h Wed Mar 19 21:25:21 2003 @@ -173,6 +173,34 @@ __ret; \ }) +#define __wait_event_timeout(wq, condition, ret) \ +do { \ + wait_queue_t __wait; \ + init_waitqueue_entry(&__wait, current); \ + \ + add_wait_queue(&wq, &__wait); \ + for (;;) { \ + set_current_state(TASK_UNINTERRUPTIBLE); \ + if (condition) \ + break; \ + ret = schedule_timeout(ret); \ + if (!ret) \ + break; \ + } \ + current->state = TASK_RUNNING; \ + remove_wait_queue(&wq, &__wait); \ +} while (0) + +#define wait_event_timeout(wq, condition, timeout) \ +({ \ + long __ret = timeout; \ + if (!(condition)) \ + __wait_event_timeout(wq, condition, __ret); \ + __ret; \ +}) + + + #define __wait_event_interruptible_timeout(wq, condition, ret) \ do { \ wait_queue_t __wait; \