Skip to content

Commit

Permalink
macb: Don't re-enable interrupts while in polling mode
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Joshua Hoke authored and David S. Miller committed Oct 25, 2010
1 parent c6ce2f4 commit b336369
Showing 1 changed file with 16 additions and 11 deletions.
27 changes: 16 additions & 11 deletions drivers/net/macb.c
Original file line number Diff line number Diff line change
Expand Up @@ -515,14 +515,15 @@ static int macb_poll(struct napi_struct *napi, int budget)
(unsigned long)status, budget);

work_done = macb_rx(bp, budget);
if (work_done < budget)
if (work_done < budget) {
napi_complete(napi);

/*
* We've done what we can to clean the buffers. Make sure we
* get notified when new packets arrive.
*/
macb_writel(bp, IER, MACB_RX_INT_FLAGS);
/*
* We've done what we can to clean the buffers. Make sure we
* get notified when new packets arrive.
*/
macb_writel(bp, IER, MACB_RX_INT_FLAGS);
}

/* TODO: Handle errors */

Expand Down Expand Up @@ -550,12 +551,16 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
}

if (status & MACB_RX_INT_FLAGS) {
/*
* There's no point taking any more interrupts
* until we have processed the buffers. The
* scheduling call may fail if the poll routine
* is already scheduled, so disable interrupts
* now.
*/
macb_writel(bp, IDR, MACB_RX_INT_FLAGS);

if (napi_schedule_prep(&bp->napi)) {
/*
* There's no point taking any more interrupts
* until we have processed the buffers
*/
macb_writel(bp, IDR, MACB_RX_INT_FLAGS);
dev_dbg(&bp->pdev->dev,
"scheduling RX softirq\n");
__napi_schedule(&bp->napi);
Expand Down

0 comments on commit b336369

Please sign in to comment.