| From stable-bounces@linux.kernel.org Wed Nov 21 14:10:14 2007 |
| From: Herbert Xu <herbert@gondor.apana.org.au> |
| From: Chuck Ebbert <cebbert@redhat.com> |
| Date: Wed, 21 Nov 2007 17:09:39 -0500 |
| Subject: Fix synchronize_irq races with IRQ handler |
| To: linux-stable <stable@kernel.org> |
| Cc: Thomas Gleixner <tglx@linutronix.de> |
| Message-ID: <4744ACA3.8090408@redhat.com> |
| |
| From: Herbert Xu <herbert@gondor.apana.org.au> |
| |
| patch a98ce5c6feead6bfedefabd46cb3d7f5be148d9a in mainline. |
| |
| Fix synchronize_irq races with IRQ handler |
| |
| As it is some callers of synchronize_irq rely on memory barriers |
| to provide synchronisation against the IRQ handlers. For example, |
| the tg3 driver does |
| |
| tp->irq_sync = 1; |
| smp_mb(); |
| synchronize_irq(); |
| |
| and then in the IRQ handler: |
| |
| if (!tp->irq_sync) |
| netif_rx_schedule(dev, &tp->napi); |
| |
| Unfortunately memory barriers only work well when they come in |
| pairs. Because we don't actually have memory barriers on the |
| IRQ path, the memory barrier before the synchronize_irq() doesn't |
| actually protect us. |
| |
| In particular, synchronize_irq() may return followed by the |
| result of netif_rx_schedule being made visible. |
| |
| This patch (mostly written by Linus) fixes this by using spin |
| locks instead of memory barries on the synchronize_irq() path. |
| |
| Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> |
| Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> |
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
| Cc: Chuck Ebbert <cebbert@redhat.com> |
| Cc: Thomas Gleixner <tglx@linutronix.de> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> |
| |
| --- |
| kernel/irq/manage.c | 20 ++++++++++++++++++-- |
| 1 file changed, 18 insertions(+), 2 deletions(-) |
| |
| --- a/kernel/irq/manage.c |
| +++ b/kernel/irq/manage.c |
| @@ -29,12 +29,28 @@ |
| void synchronize_irq(unsigned int irq) |
| { |
| struct irq_desc *desc = irq_desc + irq; |
| + unsigned int status; |
| |
| if (irq >= NR_IRQS) |
| return; |
| |
| - while (desc->status & IRQ_INPROGRESS) |
| - cpu_relax(); |
| + do { |
| + unsigned long flags; |
| + |
| + /* |
| + * Wait until we're out of the critical section. This might |
| + * give the wrong answer due to the lack of memory barriers. |
| + */ |
| + while (desc->status & IRQ_INPROGRESS) |
| + cpu_relax(); |
| + |
| + /* Ok, that indicated we're done: double-check carefully. */ |
| + spin_lock_irqsave(&desc->lock, flags); |
| + status = desc->status; |
| + spin_unlock_irqrestore(&desc->lock, flags); |
| + |
| + /* Oops, that failed? */ |
| + } while (status & IRQ_INPROGRESS); |
| } |
| EXPORT_SYMBOL(synchronize_irq); |
| |