Oliver Neukum wrote:

wait_event_timeout still seems ideally suited to the task.

Yet the folk maintaining <linux/wait.h> haven't bothered to merge it, and I'm tired of asking for it. Those macros do have the downside of expanding to much more code than it looks like they should; the prepare_to_wait() style doesn't duplicate as much code.


It's a net code shrink and simplification.
...
+       urb->transfer_flags |= URB_ASYNC_UNLINK;

Why async?

Why not? Having all the blocking in one place simplifies code review and minimizes dependencies. And I've never been a fan of synchronous unlink anyway; it's not necessary.


+       might_sleep();
+       prepare_to_wait(&done.wait, &wait, TASK_UNINTERRUPTIBLE);
        status = usb_submit_urb(urb, GFP_ATOMIC);

If you are doing it that way, why do you use a struct completion?
A simple wait queue is sufficient. And doesn't export details of how
struct completion looks internally.

They're already exported. No need to re-invent struct completion, since it's doing what needs to be done: wrapping a flag and a waitqueue head.


+ wait_for_completion(&done);

This is a nop unless you timed out. Why not do the unlink synchronously?

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.

- Dave





-------------------------------------------------------
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

Reply via email to