[lkml]   [2009]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: lockdep and threaded IRQs (was: ...)

    On Wed, 4 Mar 2009, David Brownell wrote:

    > On Tuesday 03 March 2009, Thomas Gleixner wrote:
    > > >
    > > > > [patch 0/4] genirq: add infrastructure for threaded interrupt handlers V2
    > > >
    > > > I did check them out, as noted earlier in this thread.
    > > >
    > > > The significant omission is lack of support for chaining
    > > > such threads.  Example, an I2C device that exposes
    > > > several dozen IRQs with mask/ack/... operations that
    > > > require I2C access.
    > >
    > > Well, the significant omission is on your side.
    > The facts don't quite match up with that story though ... for
    > starters, as I've already pointed out in this thread, I didn't
    > write that code (or even "create a private form of abuse" as
    > you put it). My name isn't even on the copyright.
    > I did however clean it up a lot, in hope that such cleanup
    > would make later updates easier. Anyone could tell such
    > updates would be needed. In fact ...

    Sorry, did not realize that it was not your design in the first place.

    > Your IRQ threading patches appeared well after this driver went
    > to mainline. So I did talk to "us" about those problems, earlier,
    > but it doesn't seem to have gotten your attention until now.

    Fair enough. I did not realize the horror of this chip until now. From
    what you told me at KS I figured it would be a halfways straight
    forward thing.

    > You're referring to the second issue. The code in
    > question doesn't actually have any dependency on
    > hardirq context though.

    Err. handle_IRQ_event was never meant to run in thread context,
    neither the handle_TYPE functions.

    > Assuming all IRQ configuring and dispatching runs with IRQs
    > disabled. Your threaded IRQ patches kick in only *after*
    > dispatching has been done. So it affects just one of the
    > three main unusual bits of behavior involved here.
    > Which mess were you thinking of? :)

    None, there is no mess in the irq code.

    > > The problem you described is straight forward and as I said before
    > > it's not rocket science to provide support for that in the genirq
    > > code. Your use case does not need to use the chained handler setup at
    > > all, it just needs to request the main IRQ as a simple type and handle
    > > the ack/mask/unmask from the two handler parts.
    > When there is a "main IRQ" that calls the handlers, that's
    > exactly what chaining involves ...

    And how does this rabulistic nit picking help us here ? :)

    Again: the chained_handler functionality was never designed to run in
    a thread.

    > > The only change in the generic code which is needed is a new handler
    > > function for the chained irqs "handle_irq_simple_threaded()" which is
    > > designed to handle the calls from thread context.
    > I'm not 100% sure that's right; the dispatching is a bit quirky.
    > That is however where I'll start.
    > The top level handler (for the PIH module) could easily use a
    > "handle_irq_simple_threaded()", yes ... but the secondary (SIH)
    > handlers have a few oddball behaviors including mixes of edge
    > and level trigger modes.

    I took a closer look at this code and the more I look the more it
    confuses me.

    You told me that the demux handler runs the secondary handlers in its
    thread context, but looking at the code there is a work queue as well.

    The mask/unmask functions which are called for the secondary handlers
    are just queueing work. I really do not understand the logic here:

    primary interrupt happens
    ->primary thread is woken up

    primary thread runs
    -> primary thread raises secondary irq via
    generic_handle_irq(irq), which results in:
    desc->handle_irq(irq, desc);

    The secondary handler has is set to: handle_edge_irq and
    handle_edge_irq() does: desc->chip->ack(irq);

    But the irqchip, which is set for those secondary irqs has a NULL ack
    function. How can this work at all ?

    I'm probably missing something and I would appreciate if you could
    shed some light on this. An abstract description of the requirements
    of the hardware w/o any reference to the current implementation would
    be definitely helpful.


     \ /
      Last update: 2009-03-06 15:51    [W:0.026 / U:10.716 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site