P.S. to David: I want to look into writing a replacement for the usb_bulk_msg() family of routines along the lines you described (probably following the usb_sg_xxx calls as a model). It seems clear that a prerequisite is a version of wait_for_completion() that also takes a timeout. I vaguely recall seeing something you did once along those lines. Do you still have it lying around?

Here's a slightly updated version against 2.5.64 ... basically it just replaces the odd "roll your own struct completion" logic with stuff that's cribbed from kernel/sched.c (and someday should live there, but I shouldn't have posted that to LKML when Linus was out).

One end-user visible change of note: the text of the dread "usb_control/bulk_msg timeout" message. It now identifies the
device and endpoint, using dev_warn().


So io_wait_completion() is like wait_for_completion() except:

 - flags the process as in i/o wait state, if it can
 - has a timeout, in jiffies
 - so it returns (nonzero) sometimes if the completion didn't fire

Greg, you seemed willing to merge the first version of this when it
came around (2.5.50 or so), though you seemed to agree that it'd be
worth trying to have that primitive outside of usbcore.  At this
point I'd rather merge it sooner, then maybe move it later.

- Dave



--- 1.34/drivers/usb/core/message.c     Mon Feb 24 12:22:23 2003
+++ edited/drivers/usb/core/message.c   Wed Mar  5 11:47:23 2003
@@ -11,72 +11,79 @@
 
 #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)
+/*-------------------------------------------------------------------*/
+
+#ifdef CONFIG_USB_MODULE
+/* this function is not exported ... intentional? */
+#define        io_schedule_timeout     schedule_timeout
+#endif
+
+/* this is a version of wait_for_completion(), and should eventually
+ * live in kernel/sched.c for better re-use and upkeep
+ */
+int io_wait_timeout(struct completion *x, signed long timeout)
 {
-       struct usb_api_data *awd = (struct usb_api_data *)urb->context;
+       might_sleep();
+       spin_lock_irq(&x->wait.lock);
+       if (!x->done) {
+               DECLARE_WAITQUEUE(wait, current);
+
+               wait.flags |= WQ_FLAG_EXCLUSIVE;
+               __add_wait_queue_tail(&x->wait, &wait);
+               do {
+                       __set_current_state(TASK_UNINTERRUPTIBLE);
+                       spin_unlock_irq(&x->wait.lock);
+                       timeout = io_schedule_timeout(timeout);
+                       spin_lock_irq(&x->wait.lock);
+               } while (!x->done && timeout != 0);
+               __remove_wait_queue(&x->wait, &wait);
+       }
+       if (x->done) {
+               timeout = 1;
+               x->done--;
+       } /* else timeout == 0 */
+       spin_unlock_irq(&x->wait.lock);
+       /* nonzero return means we timed out */
+       return timeout == 0;
+}
+EXPORT_SYMBOL(io_wait_timeout);
 
-       awd->done = 1;
-       wmb();
-       wake_up(&awd->wqh);
+/*-------------------------------------------------------------------*/
+
+static void usb_api_blocking_completion(struct urb *urb, struct pt_regs *regs)
+{
+       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)
-{ 
-       DECLARE_WAITQUEUE(wait, current);
-       struct usb_api_data awd;
-       int status;
-
-       init_waitqueue_head(&awd.wqh);  
-       awd.done = 0;
-
-       set_current_state(TASK_UNINTERRUPTIBLE);
-       add_wait_queue(&awd.wqh, &wait);
-
-       urb->context = &awd;
-       status = usb_submit_urb(urb, GFP_ATOMIC);
-       if (status) {
-               // something went wrong
-               usb_free_urb(urb);
-               set_current_state(TASK_RUNNING);
-               remove_wait_queue(&awd.wqh, &wait);
+{
+       struct completion       complete;
+       int                     status;
+
+       init_completion (&complete);
+       urb->context = &complete;
+       urb->complete = usb_api_blocking_completion;
+       urb->transfer_flags |= URB_ASYNC_UNLINK;
+
+       // NOTE:  watchdogs that unlink would race this submit ...
+       // else this routine could usefully be exported.
+       status = usb_submit_urb (urb, SLAB_KERNEL);
+       if (status < 0)
                return status;
-       }
 
-       while (timeout && !awd.done)
-       {
-               timeout = schedule_timeout(timeout);
-               set_current_state(TASK_UNINTERRUPTIBLE);
-               rmb();
+       if (io_wait_timeout (&complete, timeout)) {
+               dev_warn (&urb->dev->dev, "timeout urb for ep%d%s\n",
+                       usb_pipeendpoint (urb->pipe),
+                       usb_pipein (urb->pipe) ? "in" : "out");
+               usb_unlink_urb (urb);
+               wait_for_completion (&complete);
+               urb->status = -ETIMEDOUT;
        }
-
-       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;
-               }
-       } else
-               status = urb->status;
-
        if (actual_length)
                *actual_length = urb->actual_length;
-
-       usb_free_urb(urb);
-       return status;
+       return urb->status;
 }
 
 /*-------------------------------------------------------------------*/

Reply via email to