panic: uhci_abort_xfer: not in process context

[Ian Dowse]

In message <20050402103624.GS2072 at cicely12.cicely.de>, Bernd Walter writes:
>In ohci.c it does:
>        if (xfer->device->bus->intr_context || !curproc)
>                panic("ohci_abort_xfer: not in process context");
>OK - this one is from ohci, but they are all the same.
>intr_context is raised on interrupt handler start and decreased later.
>It seems that there is an artifical limitation that we are not allowed
>to sleep when processing USB interrupts.
>If one really goes to sleep a userland call might come in and has
>this variable raised.
>I don't know why it enforces not to be in interrupt context, maybe
>because of the long delays required to take an abort.
>But in this case it triggers wrong.
>In the meantime it should be safe not to check the intr_context part.

Just a few general comments on this:

Event processing systems can be either synchronous (blocking) or
asynchronous (callback based). It is easy to make efficient use of
an asynchronous system from synchronous code, but to do the reverse
without holding up other requests you need to perform each blocking
operation in a separate thread. That's why underlying event systems
tend to be asynchronous.

Mixing the two models improperly, such as allowing blocking from
callbacks, results in something much worse than either model on its
own: events may be delayed waiting for other unrelated events, and
you open up a new class of race conditions by losing the normal
atomicity of asynchronous event processing.

We should give driver writers the option of using either event
model, as for many drivers it is much simpler to write them using
the synchronous model. For example, most USB drivers could set up
one thread per USB pipe (and maybe one per-device thread) and then
just do everything synchronously. A typical USB network driver would
have one pipe's thread always blocking on an input pipe. There might
be some extra things that the taskqueue code could do to make it
easier to implement such threads.

It would be possible to implement an asynchronous version of
usbd_abort_pipe() that invokes a callback on completion. That seems
like a good idea, as it would allow us to handle the cases where
we need to initiate the abort from a callback.

We can add an #ifdef DIAGNOSTIC check to usb_transfer_complete()
to catch callbacks that sleep, by doing something along the lines
of the `dont_sleep_in_callout' approach in kern_timeout.c.

In summary:
  non-blocking: good!
  blocking: good!
  blocking from non-blocking contexts: bad!

Ian