> It's never a NOP unless the unlink somehow completed already. > As I said: using async unlinks because I want to see all the > synchronization right here, clearly visible, no hidden magic.
OK, maybe I am dense, but this doesn't make sense. Let's have another closer look: --- 1.33/drivers/usb/core/message.c Fri Aug 1 05:03:12 2003 +++ edited/drivers/usb/core/message.c Fri Aug 8 16:39:14 2003 @@ -16,75 +16,60 @@ #include <linux/slab.h> #include <linux/init.h> #include <linux/mm.h> +#include <linux/completion.h> #include <asm/byteorder.h> #include "hcd.h" /* for usbcore internals */ #include "usb.h" -struct usb_api_data { - wait_queue_head_t wqh; - int done; -}; - static void usb_api_blocking_completion(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); + complete ((struct completion *)urb->context); Here, you get woken up. } // 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; + DEFINE_WAIT(wait); + DECLARE_COMPLETION(done); int status; - init_waitqueue_head(&awd.wqh); - awd.done = 0; - - set_current_state(TASK_UNINTERRUPTIBLE); - add_wait_queue(&awd.wqh, &wait); + urb->transfer_flags |= URB_ASYNC_UNLINK; + urb->context = &done; - urb->context = &awd; + might_sleep(); unnecessary, wait_for_completion() has the test + prepare_to_wait(&done.wait, &wait, TASK_UNINTERRUPTIBLE); You prepare to sleep using a conventional wait queue 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); - return status; - } - - while (timeout && !awd.done) - { - timeout = schedule_timeout(timeout); - set_current_state(TASK_UNINTERRUPTIBLE); - rmb(); - } + if (status != 0) { + urb->actual_length = 0; - set_current_state(TASK_RUNNING); - remove_wait_queue(&awd.wqh, &wait); + /* normal case: we woke without a timeout */ + } else if (schedule_timeout((timeout == 0) And here you sleep, doing the wait on the waitqueue of the struct completion + ? MAX_SCHEDULE_TIMEOUT + : timeout) != 0) { + status = urb->status; - 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; + /* abnormal: timed out, so force completion via unlink */ + } else { + status = usb_unlink_urb(urb); + switch (status) { + case -EINPROGRESS: /* normal */ + case -EBUSY: /* already completing */ + dev_err(&urb->dev->dev, "control/bulk timeout\n"); + break; + default: + /* shouldn't happen */ + dev_err(&urb->dev->dev, "c/b unlink err %d\n", status); + break; } - } else - status = urb->status; + wait_for_completion(&done); What for? You already waited with schedule_timeout. + /* NOTE: HCDs return this status too */ + status = -ETIMEDOUT; + } + finish_wait(&done.wait, &wait); if (actual_length) *actual_length = urb->actual_length; - usb_free_urb(urb); return status; } ------------------------------------------------------- This SF.Net email sponsored by: Free pre-built ASP.NET sites including Data Reports, E-commerce, Portals, and Forums are available now. Download today and enter to win an XBOX or Visual Studio .NET. http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01 _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel