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

Reply via email to