Commit 3f9a1484 authored by Ingo Molnar's avatar Ingo Molnar Committed by Russell King

[PATCH] fix synchronize_irq() bug

This fixes a synchronize_irq() bug: if the interrupt is freed while an
IRQ handler is running (irq state is IRQ_INPROGRESS) then
synchronize_irq() will return early, which is incorrect.

there was another do_IRQ() bug that in fact necessiated the bad code that
caused the synchronize_irq() bug - we kept the IRQ_INPROGRESS bit set for
not active interrupt sources - after they happen for the first time. Now
the only effect this has is on i8259A irq handling - we used to keep these
irqs disabled after the first 'spurious' interrupt happened.  Now what the
i8259A code really wants to do IMO is to keep the interrupt disabled if
there is no handler defined for that interrupt source. The patch adds
exactly this. I dont remember why this was needed in the first place (irq
probing? avoidance of interrupt storms?), but with the patch the behavior
should be equivalent.
parent c005bcd1
...@@ -38,7 +38,8 @@ spinlock_t i8259A_lock = SPIN_LOCK_UNLOCKED; ...@@ -38,7 +38,8 @@ spinlock_t i8259A_lock = SPIN_LOCK_UNLOCKED;
static void end_8259A_irq (unsigned int irq) static void end_8259A_irq (unsigned int irq)
{ {
if (!(irq_desc[irq].status & (IRQ_DISABLED|IRQ_INPROGRESS))) if (!(irq_desc[irq].status & (IRQ_DISABLED|IRQ_INPROGRESS)) &&
irq_desc[irq].action)
enable_8259A_irq(irq); enable_8259A_irq(irq);
} }
......
...@@ -187,10 +187,6 @@ int show_interrupts(struct seq_file *p, void *v) ...@@ -187,10 +187,6 @@ int show_interrupts(struct seq_file *p, void *v)
#if CONFIG_SMP #if CONFIG_SMP
inline void synchronize_irq(unsigned int irq) inline void synchronize_irq(unsigned int irq)
{ {
/* is there anything to synchronize with? */
if (!irq_desc[irq].action)
return;
while (irq_desc[irq].status & IRQ_INPROGRESS) while (irq_desc[irq].status & IRQ_INPROGRESS)
cpu_relax(); cpu_relax();
} }
...@@ -350,7 +346,7 @@ asmlinkage unsigned int do_IRQ(struct pt_regs regs) ...@@ -350,7 +346,7 @@ asmlinkage unsigned int do_IRQ(struct pt_regs regs)
* use the action we have. * use the action we have.
*/ */
action = NULL; action = NULL;
if (!(status & (IRQ_DISABLED | IRQ_INPROGRESS))) { if (likely(!(status & (IRQ_DISABLED | IRQ_INPROGRESS)))) {
action = desc->action; action = desc->action;
status &= ~IRQ_PENDING; /* we commit to handling */ status &= ~IRQ_PENDING; /* we commit to handling */
status |= IRQ_INPROGRESS; /* we are handling it */ status |= IRQ_INPROGRESS; /* we are handling it */
...@@ -363,7 +359,7 @@ asmlinkage unsigned int do_IRQ(struct pt_regs regs) ...@@ -363,7 +359,7 @@ asmlinkage unsigned int do_IRQ(struct pt_regs regs)
a different instance of this same irq, the other processor a different instance of this same irq, the other processor
will take care of it. will take care of it.
*/ */
if (!action) if (unlikely(!action))
goto out; goto out;
/* /*
...@@ -381,12 +377,12 @@ asmlinkage unsigned int do_IRQ(struct pt_regs regs) ...@@ -381,12 +377,12 @@ asmlinkage unsigned int do_IRQ(struct pt_regs regs)
handle_IRQ_event(irq, &regs, action); handle_IRQ_event(irq, &regs, action);
spin_lock(&desc->lock); spin_lock(&desc->lock);
if (!(desc->status & IRQ_PENDING)) if (likely(!(desc->status & IRQ_PENDING)))
break; break;
desc->status &= ~IRQ_PENDING; desc->status &= ~IRQ_PENDING;
} }
desc->status &= ~IRQ_INPROGRESS;
out: out:
desc->status &= ~IRQ_INPROGRESS;
/* /*
* The ->end() handler has to deal with interrupts which got * The ->end() handler has to deal with interrupts which got
* disabled while the handler was running. * disabled while the handler was running.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment