From 283e38db65e77be9bbe2d455452584ee7834e4e9 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Thu, 22 Jan 2015 12:44:08 +0000 Subject: [PATCH] sh_eth: Fix serialisation of interrupt disable with interrupt & NAPI handlers In order to stop the RX path accessing the RX ring while it's being stopped or resized, we clear the interrupt mask (EESIPR) and then call free_irq() or synchronise_irq(). This is insufficient because the interrupt handler or NAPI poller may set EESIPR again after we clear it. Also, in sh_eth_set_ringparam() we currently don't disable NAPI polling at all. I could easily trigger a crash by running the loop: while ethtool -G eth0 rx 128 && ethtool -G eth0 rx 64; do echo -n .; done and 'ping -f' toward the sh_eth port from another machine. To fix this: - Add a software flag (irq_enabled) to signal whether interrupts should be enabled - In the interrupt handler, if the flag is clear then clear EESIPR and return - In the NAPI poller, if the flag is clear then don't set EESIPR - Set the flag before enabling interrupts in sh_eth_dev_init() and sh_eth_set_ringparam() - Clear the flag and serialise with the interrupt and NAPI handlers before clearing EESIPR in sh_eth_close() and sh_eth_set_ringparam() After this, I could run the loop for 100,000 iterations successfully. Signed-off-by: Ben Hutchings Signed-off-by: David S. Miller --- drivers/net/ethernet/renesas/sh_eth.c | 39 ++++++++++++++++++++------- drivers/net/ethernet/renesas/sh_eth.h | 1 + 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c index 7facda1328f8..99a838db032e 100644 --- a/drivers/net/ethernet/renesas/sh_eth.c +++ b/drivers/net/ethernet/renesas/sh_eth.c @@ -1316,8 +1316,10 @@ static int sh_eth_dev_init(struct net_device *ndev, bool start) RFLR); sh_eth_write(ndev, sh_eth_read(ndev, EESR), EESR); - if (start) + if (start) { + mdp->irq_enabled = true; sh_eth_write(ndev, mdp->cd->eesipr_value, EESIPR); + } /* PAUSE Prohibition */ val = (sh_eth_read(ndev, ECMR) & ECMR_DM) | @@ -1653,7 +1655,12 @@ static irqreturn_t sh_eth_interrupt(int irq, void *netdev) if (intr_status & (EESR_RX_CHECK | cd->tx_check | cd->eesr_err_check)) ret = IRQ_HANDLED; else - goto other_irq; + goto out; + + if (!likely(mdp->irq_enabled)) { + sh_eth_write(ndev, 0, EESIPR); + goto out; + } if (intr_status & EESR_RX_CHECK) { if (napi_schedule_prep(&mdp->napi)) { @@ -1684,7 +1691,7 @@ static irqreturn_t sh_eth_interrupt(int irq, void *netdev) sh_eth_error(ndev, intr_status); } -other_irq: +out: spin_unlock(&mdp->lock); return ret; @@ -1712,7 +1719,8 @@ static int sh_eth_poll(struct napi_struct *napi, int budget) napi_complete(napi); /* Reenable Rx interrupts */ - sh_eth_write(ndev, mdp->cd->eesipr_value, EESIPR); + if (mdp->irq_enabled) + sh_eth_write(ndev, mdp->cd->eesipr_value, EESIPR); out: return budget - quota; } @@ -1970,12 +1978,20 @@ static int sh_eth_set_ringparam(struct net_device *ndev, if (netif_running(ndev)) { netif_device_detach(ndev); netif_tx_disable(ndev); - /* Disable interrupts by clearing the interrupt mask. */ + + /* Serialise with the interrupt handler and NAPI, then + * disable interrupts. We have to clear the + * irq_enabled flag first to ensure that interrupts + * won't be re-enabled. + */ + mdp->irq_enabled = false; + synchronize_irq(ndev->irq); + napi_synchronize(&mdp->napi); sh_eth_write(ndev, 0x0000, EESIPR); + /* Stop the chip's Tx and Rx processes. */ sh_eth_write(ndev, 0, EDTRR); sh_eth_write(ndev, 0, EDRRR); - synchronize_irq(ndev->irq); /* Free all the skbuffs in the Rx queue. */ sh_eth_ring_free(ndev); @@ -2001,6 +2017,7 @@ static int sh_eth_set_ringparam(struct net_device *ndev, return ret; } + mdp->irq_enabled = true; sh_eth_write(ndev, mdp->cd->eesipr_value, EESIPR); /* Setting the Rx mode will start the Rx process. */ sh_eth_write(ndev, EDRRR_R, EDRRR); @@ -2184,7 +2201,13 @@ static int sh_eth_close(struct net_device *ndev) netif_stop_queue(ndev); - /* Disable interrupts by clearing the interrupt mask. */ + /* Serialise with the interrupt handler and NAPI, then disable + * interrupts. We have to clear the irq_enabled flag first to + * ensure that interrupts won't be re-enabled. + */ + mdp->irq_enabled = false; + synchronize_irq(ndev->irq); + napi_disable(&mdp->napi); sh_eth_write(ndev, 0x0000, EESIPR); /* Stop the chip's Tx and Rx processes. */ @@ -2201,8 +2224,6 @@ static int sh_eth_close(struct net_device *ndev) free_irq(ndev->irq, ndev); - napi_disable(&mdp->napi); - /* Free all the skbuffs in the Rx queue. */ sh_eth_ring_free(ndev); diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h index 71f5de1171bd..332d3c16d483 100644 --- a/drivers/net/ethernet/renesas/sh_eth.h +++ b/drivers/net/ethernet/renesas/sh_eth.h @@ -513,6 +513,7 @@ struct sh_eth_private { u32 rx_buf_sz; /* Based on MTU+slack. */ int edmac_endian; struct napi_struct napi; + bool irq_enabled; /* MII transceiver section. */ u32 phy_id; /* PHY ID */ struct mii_bus *mii_bus; /* MDIO bus control */ -- 2.30.2