On Tuesday 08 March 2005 9:05 am, Alan Stern wrote: > On Mon, 7 Mar 2005, Nishanth Aravamudan wrote: > > > - int maxretry = 50; > > + unsigned long timeout = 500; > > _Please_ don't set timeout to 500 jiffies! Use msecs_to_jiffies(500) > instead.
Hmm, somewhere in the queue are patches changing all timeouts to milliseconds ... but not for the primitive used in this patch. > Note that schedule_timeout can return before the timeout has elapsed, even > though the condition you're waiting for (URB completed) hasn't happened > yet. People use a loop to catch these things, and you definitely need a > way to test for URB completed. But seeing if ep->urb->status is equal to > -EINPROGRESS is not the correct way to do it. I see that the completion > handler, usb_write_callback, doesn't do anything but wake up the > waitqueue. That's not good enough; it needs to set a flag in the ep > structure for you to test. All true. Seems like if there were an interruptible version of usb_bulk_msg(), that's what this driver should use ... and lacking that, it's pretty much re-inventing that logic. Wouldn't a better fix just be a new usb_bulk_msg_interruptible(), which uses one of the interruptible variants of wait_for_completion() ? After all, one of the longstanding API glitches in usb_bulk_msg() has been exactly that it's not interruptible ... hence inappropriate for use from syscalls including read() and write(). - Dave ------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
