On Thu, 13 May 2004, Oliver Neukum wrote:
> Am Donnerstag, 13. Mai 2004 20:02 schrieb Alan Stern:
> > +void usb_kill_urb(struct urb *urb)
> > +{
> > +�������urb->transfer_flags |= (URB_ASYNC_UNLINK | URB_REJECT);
> > +�������usb_unlink_urb(urb);
> > +�������wait_event(urb->handler_queue, atomic_read(&urb->use_count) == 0);
> > +�������urb->transfer_flags &= ~(URB_ASYNC_UNLINK | URB_REJECT);
> > +}
> > +
>
> Here I see a little problem.
> 1. URB_REJECT
> You cannot simply define a new transfer_flag, because the completion
> handler may clear transfer_flags. Either add a further field, or use maskes
> in in the fill_urb functions
That's a good objection.
Right now, none of the completion handlers under drivers/usb clears the
transfer flags. But that's no guarantee about the future.
FWIW, the usb_fill_xxx_urb() functions don't touch the transfer flags.
Maybe we should change these flags to single-bit fields. At any rate,
I'll add a note that usb_kill_urb() won't work if a completion handler
clears the USB_REJECT flag.
> 2. URB_ASYNC_UNLINK
> The completion handler may certainly reset that flag, so it has to work
> without it. Therefore it makes little sense to set it.
It doesn't matter if a completion handler clears the flag. If a
completion handler runs then the URB will no longer be linked anyway, so
our call to usb_unlink_urb() will simply fail regardless of the flag.
There may be a recursive call to usb_kill_urb() and usb_unlink_urb() again
-- it'll still work okay.
> 3. use of an atomic_t
> You could use a short and use the spinlock already in the URB to protect
> it. It saves two bytes on x86.
No it doesn't, because of field alignment. All of the other fields in
struct urb are aligned on 4-byte boundaries on x86.
Personally, I would have been just as happy to use the polling loop. It
would avoid the overhead of calling wake_up() every time an URB completes,
and I don't think a 5 millisecond penalty is very much during a
synchronous unlink. But if people want to use a wait_queue...
Alan Stern
-------------------------------------------------------
This SF.Net email is sponsored by: SourceForge.net Broadband
Sign-up now for SourceForge Broadband and get the fastest
6.0/768 connection for only $19.95/mo for the first 3 months!
http://ads.osdn.com/?ad_id=2562&alloc_id=6184&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel