Re: [PATCH] usb ehci_iaa_watchdog fix

[Alan Stern]

On Mon, 7 Apr 2008, David Brownell wrote:

> Third, if it *does* call end_unlink_async(), it can retriger
> the timer when it needs to do another unlink.  Only when the
> HC is halted (HC_STATE_HALT) is that logic bypassed, scrubbing
> several endpoints at once.  (And a minor fourth point, direct
> calls to end_unlink_async leaves the IAA IRQ machinery active.)
> 
> I think your fix handles the "one pending unlink" case, but
> not the more general "N pending unlinks" ...

Sounds right.  That test at the end of start_unlink_async() should be
changed to !RUNNING instead of HALT.

To help prevent unwanted recursion, I wrote a patch some time ago that
would batch up multiple unlinks.  (The idea was to keep track of which
entries were added to the reclaim queue before the current IAA cycle
got started, and handle them all at one stroke when the current cycle
ends.)  That patch is a bit out-of-date now, but it shouldn't be too
hard to bring it up to speed.

> When the timer would be retriggered, to finish additional unlinks,
> it does matter.  A complete fix for this would (a) disable the IAA
> watchdog timer later, once it can never be retriggered again, and
> (b) make end_unlink_async handle the multiple-unlinks case when the
> HC is suspended, including not retriggering the watchdog.

Agreed.  If the code does (a) after (b) then end_unlink_async doesn't 
need to avoid retriggering the watchdog.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/