Am Freitag, 14. Mai 2004 21:25 schrieb Alan Stern:
> 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.
OK.
> > 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.
OK.
> > 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.
Put it next to last. Just kidding ;-)
> 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,
Make it conditional on URB_REJECT and use unlikely(). That should do
the trick.
Regards
Oliver
-------------------------------------------------------
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%62&alloc_ida84&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
