On Thu, 29 Apr 2004, Oliver Neukum wrote:
> Am Donnerstag, 29. April 2004 23:04 schrieb Alan Stern:
> > Probably not just network drivers. But this is really a separate issue.
>
> No, think about it. The problem exists only because we don't wait
> for the completion handler. It doesn't look the same, but it is.
>
> > It would better be solved by having a way to prevent the resubmission from
> > succeeding in the first place. Maybe some way to ask usbcore to fail all
> > URBs for a particular endpoint (although the URB might be resubmitted for
> > a differend endpoint!).
>
> No, as you yourself have pointed out, the problem is not confined to
> disconnect(). Reusing the URB has the same problem. If we stay with
> the example, the bug is not only in disconnect() but also in close().
> We cannot solve it with disabling, because we may need to reenable
> in arbitrarily short periods of time.
You're right that doing something to the endpoint is a bad idea. It's
much better to do something to the URB instead. Here's something I just
thought of today; it seems like a pretty good approach.
Let's define a new field in struct urb. It could even be one of the bits
in transfer_flags; call it URB_REJECT. The idea is that if the URB_REJECT
flag is set then usb_submit_urb() will fail. What we want can be
accomplished as simply as this:
urb->transfer_flags |= (URB_REJECT | URB_ASYNC_UNLINK);
usb_unlink_urb(urb);
while (atomic_read(&urb->kref.refcount) > 1) {
current->state = TASK_UNINTERRUPTIBLE;
schedule_timeout((5 * HZ + 999) / 1000);
}
urb->transfer_flags &= ~(URB_REJECT | URB_ASYNC_UNLINK);
The only change needed to existing code would be to add the test for
URB_REJECT at the proper place. This is extremely simple and low-profile;
you would be hard-pressed to do any better. And provided the completion
handler doesn't clear the URB_REJECT flag (there's no reason why it
should), this does exactly what we've been talking about. Resubmission is
prevented and the routine won't return until the completion handler is
finished. The polling loop isn't as bad as it looks -- over 99% of the
time it will succeed after no more than one iteration.
> I would _like_ to call it usb_kill_urb()
> But we are in a stable series. IMHO using the same function for synchronous
> and asynchronous unlink was an error in the first place.
(I agree completely with that last sentence -- this should be changed in
2.7.)
usb_kill_urb() is not a bad name. I kind of like usb_terminate_urb() --
it reminds me of Arnold Schwartzenegger. :-) Either way, almost all
existing calls to synchronous usb_unlink_urb() could be changed to call
this function. Once the few remaining places are dealt with, we could
eliminate synchronous unlink entirely.
Alan Stern
-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g.
Take an Oracle 10g class now, and we'll give you the exam FREE.
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
