[PATCH 01/15] ARM minor irq handler cleanups - Kernel

This is a discussion on [PATCH 01/15] ARM minor irq handler cleanups - Kernel ; On Tue, Apr 22, 2008 at 06:05:40PM +1000, Benjamin Herrenschmidt wrote: > > On Sat, 2008-04-19 at 08:00 +0200, Rogier Wolff wrote: > > > > You added a "XXX Using free_irq in the interrupt is not wise!". When I ...

+ Reply to Thread
Page 2 of 2 FirstFirst 1 2
Results 21 to 23 of 23

Thread: [PATCH 01/15] ARM minor irq handler cleanups

  1. Re: [PATCH 05/15] drivers/char: minor irq handler cleanups

    On Tue, Apr 22, 2008 at 06:05:40PM +1000, Benjamin Herrenschmidt wrote:
    >
    > On Sat, 2008-04-19 at 08:00 +0200, Rogier Wolff wrote:
    > >
    > > You added a "XXX Using free_irq in the interrupt is not wise!". When I
    > > wrote that code, I didn't know about this. These lines triggered when
    > > the level-triggered PCI interrupt stuck "active" this would mean that
    > > NO userspace code would get executed anymore: Hard lock up. Difficult
    > > to debug. This happend a few times during development when the code
    > > behind the "if (!polled)": "tell the hardware we've seen the
    > > interrupt" didn't work. On the other hand, some failures in the field
    > > have triggered this. So I think it's wise to keep it in. Disabling the
    > > interrupt on the card is not an option, because that's exactly what
    > > this is supposed to catch: We're unable to make the card stop
    > > interrupting the CPU.
    > >
    > > Note that it also doesn't work (i.e. hard lock of the machine) if some
    > > other driver is using the same interupt.

    >
    > You should let the kernel generic code deal with the runaway interrupt,
    > it should be capable of doing so nowadays pretty reliably.
    >
    > free_irq() is definitely not going to be happy when it start messing
    > with /proc from an interrupt... It will at least give you a WARN_ON.


    The situation is NOT normal operation. It is an emergency measure in
    an attempt to prevent a full hang. It is great that other parts of the
    kernel also "shout" that something is wrong.

    Consider it similar to a "kernel null pointer dereference". Once that
    happens, all bets are off. In practise you've probably seen one, and
    you were able to continue to work. It is advisable to save everything
    you can, and reboot. This is similar.

    The "generic code for runaway interrupts" didn't exist when this was
    written. If it exists, and works for the case that this was written
    for, then all is fine, and we can remove my code. As you can see, I
    copied over the code from one driver to the next after I got bitten
    again with the second driver. So having something generic is of course
    preferable. :-)

    Roger.

    --
    ** R.E.Wolff@BitWizard.nl ** http://www.BitWizard.nl/ ** +31-15-2600998 **
    ** Delftechpark 26 2628 XH Delft, The Netherlands. KVK: 27239233 **
    *-- BitWizard writes Linux device drivers for any device you may have! --*
    Q: It doesn't work. A: Look buddy, doesn't work is an ambiguous statement.
    Does it sit on the couch all day? Is it unemployed? Please be specific!
    Define 'it' and what it isn't doing. --------- Adapted from lxrbot FAQ
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH 05/15] drivers/char: minor irq handler cleanups


    On Tue, 2008-04-22 at 12:13 +0200, Rogier Wolff wrote:
    > > You should let the kernel generic code deal with the runaway

    > interrupt,
    > > it should be capable of doing so nowadays pretty reliably.
    > >
    > > free_irq() is definitely not going to be happy when it start messing
    > > with /proc from an interrupt... It will at least give you a WARN_ON.

    >
    > The situation is NOT normal operation. It is an emergency measure in
    > an attempt to prevent a full hang. It is great that other parts of the
    > kernel also "shout" that something is wrong.


    But free_irq at interrupt time has great chances of crashing the machine
    or currupting data structures in subtle ways. You should -not- do that.

    Either let the kernel handle the runaway interrupt, or eventually if you
    want to be brutal, call disable_irq_nosync yourself, but not free_irq.

    > Consider it similar to a "kernel null pointer dereference". Once that
    > happens, all bets are off. In practise you've probably seen one, and
    > you were able to continue to work. It is advisable to save everything
    > you can, and reboot. This is similar.


    It would be if you used something like disable_irq_nosync. Not free_irq.
    The later is just bogus.

    > The "generic code for runaway interrupts" didn't exist when this was
    > written. If it exists, and works for the case that this was written
    > for, then all is fine, and we can remove my code. As you can see, I
    > copied over the code from one driver to the next after I got bitten
    > again with the second driver. So having something generic is of course
    > preferable. :-)


    Well, if you know how to trigger the bug, it would be useful to verify
    that the kernel generic code properly detects and masks the runaway
    interrupt. If that works, then remove your code completely. If not, it
    would be useful to figure out why :-) But either way, just replace it
    with disable_irq_nosync.

    Cheers,
    Ben.


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

  3. Re: [PATCH 12/15] [MIPS] pmc-sierra/msp71xx/msp_hwbutton.c: minor irq handler cleanups

    On Fri, Apr 18, 2008 at 07:23:01PM -0400, Jeff Garzik wrote:

    > - remove always-true test
    >
    > - neaten request_irq() indentation
    >
    > This change's main purpose is to prepare for the patchset in
    > jgarzik/misc-2.6.git#irq-remove, that explores removal of the
    > never-used 'irq' argument in each interrupt handler.
    >
    > Signed-off-by: Jeff Garzik


    Applied, thanks Jeff.

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

+ Reply to Thread
Page 2 of 2 FirstFirst 1 2