Am Samstag, 18. Januar 2003 02:59 schrieb David Brownell:
> Oliver Neukum wrote:
> > Am Samstag, 18. Januar 2003 02:00 schrieb David Brownell:
> >>Oliver Neukum wrote:
> >>>Hi,
> >>>
> >>>regarding this piece of code:
> >>> spin_lock_irqsave (&ehci->lock, flags);
> >>> ...
> >>>
> >>>Does this mean that any call to usb_unlink_urb() for an URB on EHCI
> >>>with a spinlock held will fail?
> >>
> >>No, but a _synchronous_ one might need to wait_ms(1) for the relevant
> >>hardware resource to become available, and that wouldn't be a good
> >>thing with a spinlock held. (That was the line after your quote.)
> >
> > But you don't check for asynchronousity, you check for in_interrupt().
>
> Got patch? :)
Not the complicated patch you prefer. I don't know enough about ehci yet.
I'd likely kill the driver.
> In that case I suspect that since hcd.c already checked for the bogus
> in_interrupt()/synchronous case, this code only needs to check for
The check itself is bogus.
> the ASYNC flag ... which need not be in_interrupt(). I wonder if that
> explains any interesting behaviors.
Possible, but I have no idea.
> > This is not the same thing. In fact it means that usb_unlink_urb()
> > may schedule with a spinlock held, even if the asynchronous attribute
> > is set.
> >
> > As far as I can tell, the safe code would be:
> >
> > if (urb->transfer_flags | URB_ASYNC_UNLINK) {
> > udelay(...);
>
> That'd do nasty things for latency, and the implication
> seems to be that the interrupt handler would recurse...
Why? I thought udelay() spins?
> No, likely a simpler solution would be to flag that qh
> as needing to unlink, and make end_unlink_async() do
> the extra work if some qh is flagged that way.
>
> In that case the logic there would be pretty much just
>
> qh->state = ... some UNLINKING state, maybe,
> but only after auditing all
> the state transitions
> if (!ehci->reclaim || controller dead)
> start_unlink_async (ehci, qh);
> /* else end_unlink_async() will do it soon */
>
> and the end_unlink_async() logic would know (by counter
> or list) that it should start another unlink.
>
> That'd abolish the whole need to block on that path, too.
Good.
Regards
Oliver
-------------------------------------------------------
This SF.NET email is sponsored by: Thawte.com - A 128-bit supercerts will
allow you to extend the highest allowed 128 bit encryption to all your
clients even if they use browsers that are limited to 40 bit encryption.
Get a guide here:http://ads.sourceforge.net/cgi-bin/redirect.pl?thaw0030en
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
