lkml.org 
[lkml]   [2010]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [RFC][PATCH] IRQ: Fix oneshot irq race between irq_finalize_oneshot and handle_level_irq
    On Wed, Mar 10, 2010 at 12:22:12AM +0100, Thomas Gleixner wrote:
    > B1;2005;0cOn Tue, 9 Mar 2010, Lars-Peter Clausen wrote:
    > >
    > > - desc->status |= IRQ_INPROGRESS;
    > > + desc->status |= IRQ_INPROGRESS | IRQ_ONESHOT_INPROGRESS;
    > > raw_spin_unlock(&desc->lock);
    >
    > That keeps the IRQ_ONESHOT_INPROGRESS dangling for non ONESHOT
    > interrupts. Not a big deal, but not pretty either.
    >
    > The race between the thread and the irq handler exists indeed on SMP,
    > but I think there are more fundamental issues about the state which
    > need to be addressed.
    >
    > The first thing is that we do not mark the status MASKED when we
    > actually mask the interrupt in mask_ack_irq().
    >
    > That conditional MASKED after running the primary handler is really
    > horrible - I already ranted in private at the moron who committed that
    > crime :)
    >
    > So the following patch fixes that and the SMP race scenario:

    Hi Thomas,

    How about the following patch(maybe a little ugly). I think it will
    resolve your concerns.

    diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
    index d70394f..23b79c6 100644
    --- a/kernel/irq/chip.c
    +++ b/kernel/irq/chip.c
    @@ -461,9 +461,24 @@ handle_level_irq(unsigned int irq, struct irq_desc *desc)
    raw_spin_lock(&desc->lock);
    mask_ack_irq(desc, irq);

    - if (unlikely(desc->status & IRQ_INPROGRESS))
    - goto out_unlock;
    + /*
    + * if we are in oneshot mode and the irq thread is running on
    + * another cpu, just return because the irq thread will unmask
    + * the irq
    + */
    + if (unlikely(desc->status & IRQ_ONESHOT)) {
    + if (unlikely(desc->status & (IRQ_INPROGRESS | IRQ_MASKED)
    + == IRQ_INPROGRESS | IRQ_MASKED))
    + goto out_unlock;
    + }
    + else {
    + if (unlikely(desc->status & IRQ_INPROGRESS))
    + goto out_unlock;
    + }
    +
    desc->status &= ~(IRQ_REPLAY | IRQ_WAITING);
    + if (unlikely(desc->status & IRQ_ONESHOT))
    + desc->status |= IRQ_MASKED;
    kstat_incr_irqs_this_cpu(irq, desc);

    /*
    @@ -484,9 +499,7 @@ handle_level_irq(unsigned int irq, struct irq_desc *desc)
    raw_spin_lock(&desc->lock);
    desc->status &= ~IRQ_INPROGRESS;

    - if (unlikely(desc->status & IRQ_ONESHOT))
    - desc->status |= IRQ_MASKED;
    - else if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
    + if (!(desc->status & (IRQ_DISABLED | IRQ_MASKED) && desc->chip->unmask)
    desc->chip->unmask(irq);
    out_unlock:
    raw_spin_unlock(&desc->lock);

    \
     
     \ /
      Last update: 2010-03-10 04:23    [W:4.445 / U:0.404 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site