diff options
author | Joshua Hoke <Joshua.Hoke@sixnet.com> | 2010-10-24 21:44:22 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2010-10-25 15:14:11 -0400 |
commit | b336369c1e1ad88495895260a9068eb18bc48b6c (patch) | |
tree | 3ae8bbf24949584c521ac8bf0755c2eb1b970b97 /drivers/net/macb.c | |
parent | c6ce2f4b270cb1d4d6b6f4f692a12ca2fea13f3f (diff) |
macb: Don't re-enable interrupts while in polling mode
On a busy network, the macb driver could get stuck in the interrupt
handler, quickly triggering the watchdog, due to a confluence of
factors:
1. macb_poll re-enables interrupts unconditionally, even when it will
be called again because it exhausted its rx budget
2. macb_interrupt only disables interrupts after scheduling
macb_poll, but scheduling fails when macb_poll is already scheduled
because it didn't call napi_complete
3. macb_interrupt loops until the interrupt status register is clear,
which will never happen in this case if the driver doesn't disable
the RX interrupt
Since macb_interrupt runs in interrupt context, this effectively locks
up the machine, triggering the hardware watchdog.
This issue was readily reproducible on a flooded network with a
modified 2.6.27.48 kernel. The same problem appears to still be in the
2.6.36-rc8 driver code, so I am submitting this patch against that
version. I have not tested this version of the patch except to make
sure the kernel compiles.
Signed-off-by: Joshua Hoke <joshua.hoke@sixnet.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers/net/macb.c')
-rw-r--r-- | drivers/net/macb.c | 27 |
1 files changed, 16 insertions, 11 deletions
diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 4297f6e8c4bc..f69e73e2191e 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c | |||
@@ -515,14 +515,15 @@ static int macb_poll(struct napi_struct *napi, int budget) | |||
515 | (unsigned long)status, budget); | 515 | (unsigned long)status, budget); |
516 | 516 | ||
517 | work_done = macb_rx(bp, budget); | 517 | work_done = macb_rx(bp, budget); |
518 | if (work_done < budget) | 518 | if (work_done < budget) { |
519 | napi_complete(napi); | 519 | napi_complete(napi); |
520 | 520 | ||
521 | /* | 521 | /* |
522 | * We've done what we can to clean the buffers. Make sure we | 522 | * We've done what we can to clean the buffers. Make sure we |
523 | * get notified when new packets arrive. | 523 | * get notified when new packets arrive. |
524 | */ | 524 | */ |
525 | macb_writel(bp, IER, MACB_RX_INT_FLAGS); | 525 | macb_writel(bp, IER, MACB_RX_INT_FLAGS); |
526 | } | ||
526 | 527 | ||
527 | /* TODO: Handle errors */ | 528 | /* TODO: Handle errors */ |
528 | 529 | ||
@@ -550,12 +551,16 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id) | |||
550 | } | 551 | } |
551 | 552 | ||
552 | if (status & MACB_RX_INT_FLAGS) { | 553 | if (status & MACB_RX_INT_FLAGS) { |
554 | /* | ||
555 | * There's no point taking any more interrupts | ||
556 | * until we have processed the buffers. The | ||
557 | * scheduling call may fail if the poll routine | ||
558 | * is already scheduled, so disable interrupts | ||
559 | * now. | ||
560 | */ | ||
561 | macb_writel(bp, IDR, MACB_RX_INT_FLAGS); | ||
562 | |||
553 | if (napi_schedule_prep(&bp->napi)) { | 563 | if (napi_schedule_prep(&bp->napi)) { |
554 | /* | ||
555 | * There's no point taking any more interrupts | ||
556 | * until we have processed the buffers | ||
557 | */ | ||
558 | macb_writel(bp, IDR, MACB_RX_INT_FLAGS); | ||
559 | dev_dbg(&bp->pdev->dev, | 564 | dev_dbg(&bp->pdev->dev, |
560 | "scheduling RX softirq\n"); | 565 | "scheduling RX softirq\n"); |
561 | __napi_schedule(&bp->napi); | 566 | __napi_schedule(&bp->napi); |