Am Freitag, 30. April 2004 20:01 schrieb Alan Stern:
> Well, in your patch you've got it changing urb->phase. And that's at a
> time when usbcore doesn't own the URB.
It is a private field and we still own a reference count.
> Furthermore, the issue isn't how giveback_urb knows when the handler has
> run -- it's how usb_kill_urb() knows.
By being woken up.
> > > Also, whatever we use, it has to take into account the possibility of
> > > resubmission (and even the theoretical possibility of nested
> > > invocations of giveback_urb!). The only thing that will handle this
> > > properly is a refcount of some sort. Yes, we could introduce another
> > > one that would track just the active submissions -- but why go to all
> > > that trouble (and introduce yet another field into the already-bloated
> > > struct urb) when we have a ready-made reference count sitting right
> > > there?
> >
> > I don't think we need a refcount. Could you have a look at this code:
>
> I looked at it quickly. In effect, your urb->phase _is_ a refcount,
> although it only takes on 2-1/2 values (DOING and DONE are approximately
> equivalent). You have ignored the possibility that giveback_urb might be
> called reentrantly for the same URB. You have ignored the possibility
How? giveback_urb is an irq handler. You would have to resubmit
to a device on another bus.
> that more than one thread might call usb_kill_urb() for the same URB
> simultaneously. Rather than preventing resubmission, you are allowing it
> and then awkwardly trying to unlink the URB again.
You cannot reliably prevent resubmission. There's a race between
setting the flag and checking it. It would need further locking.
Your patch falls short in that regard, too. It's a hard problem.
> Also, you have added a bunch of extra stuff to struct urb, which is
> already too big, and a lot of extra code to usbcore. If you object to
> using the existing refcount, wouldn't it be a lot simpler to adopt my
> scheme but using an additional submission-count field? That would
> increase the size of struct urb by a smaller amount.
Why does the size of struct urb matter? It is not a numerous struct
like an inode or a dentry. Making URBs more efficient for 2.7 requires
us to grow struct urb, not shrink it.
As for the code, little of it is in hot code path and the part which is,
allows removal of code from properly written handlers like hub_irq()
Regards
Oliver
-------------------------------------------------------
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
