Re: [cdi-devel] Interrupts

[Kevin Wolf <kevin@xxxxxxxxxx>]

On Mon, Jan 18, 2010 at 07:28:45AM +1000, Matthew Iselin wrote:
> On Sun, Jan 17, 2010 at 9:03 PM, Kevin Wolf <kevin@xxxxxxxxxx> wrote:
> > Hi all,
> >
> > I'm currently implementing CDI for a small monolithic kernel. Besides
> > getting drivers, my goal in doing so is to find pontential problems with
> > CDI. I think with interrupt handling I've found a first thing that needs
> > clarification on what drivers can expect. In particular, I remember
> > Matt's change to pcnet to use cdi_wait_irq instead of doing it manually.
> >
> > So the question is: Has pcnet ever been right in doing it manually? Was
> > it just ugly or was it wrong?
> 
> I would say that pcnet's implementation of its own solution when
> cdi_wait_irq already exists was fundamentally wrong. There is no point
> reinventing the wheel in each driver when you already have the
> functionality available in the interface.

Reinventing the wheel is just ugliness, it doesn't lead to fundamentally
broken code. But at least you don't seem to be opposed to forbid such
things in a better specification.

> > The point is that currently in my kernel CDI drivers generally run with
> > IRQs turned off as the kernel lacks proper locking, so I'm not sure what
> > would happen if I would allow IRQs in any place (or even let it be
> > preempted). So pcnet would wait forever for an IRQ to happen.
> 
> Perhaps cdi_wait_irq needs a timeout parameter. This way we can also
> enforce a decent timeframe in which an IRQ should occur, and avoid the
> case where a driver hangs because an IRQ never arrives.

It already has this parameter.

> > What do you think? Do all other OSes implementing CDI so far allow IRQs
> > at any time or does it just happen to work? (I think meanwhile all
> > drivers are well-behaved and don't do it manually)
> 
> Drivers that use interrupts should be able to assume interrupts are
> enabled and working properly.

I fully agree that driver should be safe to assume that interrupts work.
The question is what "working properly" means. Basically this is about
whether a driver function can be interrupted for a IRQ handler (like for
a Unix signal handler) or if the IRQs are queued. I strongly suggest the
latter - in the example of pcnet this would mean that the code was
completely wrong and could not even have worked (which shows that tyndur
doesn't implement this semantics yet).

I've though a bit more about it, and in fact I think what we need to
clearly define is semantics with respect to concurrency and/or
signal-like interruptions. IDC message handling is another thing of this
type, should it ever be implemented.

If we want to allow concurrency/interruption, we need to define things
like locks, signal blocking etc. I really don't want to get into that
business with CDI because I fear trouble with the different types of
OSes here. So maybe the most reasonable approach is to defined that at
the same time at most one driver function per device is running and that
it may not be interrupted as in signals. The library can then define the
necessary message queues, threading with locks or whatever model it
likes. The drivers themselves can be kept simple then.

Kevin