From: Russell King <rmk+kernel@arm•linux.org.uk>
To: "David S. Miller" <davem@davemloft•net>
Cc: netdev@vger•kernel.org, linux-arm-kernel@lists•infradead.org
Subject: [PATCH A 03/12] net: fec: fix interrupt handling races
Date: Tue, 08 Jul 2014 00:22:44 +0100 [thread overview]
Message-ID: <E1X4IFA-0004wp-6x@rmk-PC.arm.linux.org.uk> (raw)
In-Reply-To: <20140707232148.GG21766@n2100.arm.linux.org.uk>
While running: while :; do iperf -c <HOST> -P 4; done, transmit timeouts
are regularly reported. With the tx ring dumping in place, we can see
that all entries are in use, and the hardware has finished transmitting
these packets. However, the driver has not reclaimed these ring
entries.
This can occur if the interrupt handler is invoked at the wrong moment -
eg:
CPU0 CPU1
fec_enet_tx()
interrupt, IEVENT = FEC_ENET_TXF
FEC_ENET_TXF cleared
napi_schedule_prep()
napi_complete()
The result is that we clear the transmit interrupt, but we don't trigger
any cleaning of the transmit ring. Instead, use a different strategy:
- When receiving a transmit or receive interrupt, disable both tx and rx
interrupts, but do not acknowledge them. Schedule a napi poll. Don't
loop.
- When we are polled, read IEVENT, acknowledging the pending transmit
and receive interrupts, before then going on to process the
appropriate rings.
This allows us to avoid the race, and has a number of other advantages:
- we cut down on the number of transmit interrupts we have to process.
- we only look at the rings which have pending events.
- we gain additional throughput: the iperf total bandwidth increases
from about 180Mbps to 240Mbps:
[ 3] 0.0-10.0 sec 68.1 MBytes 57.0 Mbits/sec
[ 5] 0.0-10.0 sec 72.4 MBytes 60.5 Mbits/sec
[ 4] 0.0-10.1 sec 76.1 MBytes 63.5 Mbits/sec
[ 6] 0.0-10.1 sec 71.9 MBytes 59.9 Mbits/sec
[SUM] 0.0-10.1 sec 288 MBytes 241 Mbits/sec
Acked-by: Fugang Duan <B38611@freescale•com>
Signed-off-by: Russell King <rmk+kernel@arm•linux.org.uk>
---
drivers/net/ethernet/freescale/fec_main.c | 40 +++++++++++++++++--------------
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 045ea71f2b59..4e695b742030 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1369,29 +1369,25 @@ fec_enet_interrupt(int irq, void *dev_id)
{
struct net_device *ndev = dev_id;
struct fec_enet_private *fep = netdev_priv(ndev);
+ const unsigned napi_mask = FEC_ENET_RXF | FEC_ENET_TXF;
uint int_events;
irqreturn_t ret = IRQ_NONE;
- do {
- int_events = readl(fep->hwp + FEC_IEVENT);
- writel(int_events, fep->hwp + FEC_IEVENT);
+ int_events = readl(fep->hwp + FEC_IEVENT);
+ writel(int_events & ~napi_mask, fep->hwp + FEC_IEVENT);
- if (int_events & (FEC_ENET_RXF | FEC_ENET_TXF)) {
- ret = IRQ_HANDLED;
+ if (int_events & napi_mask) {
+ ret = IRQ_HANDLED;
- /* Disable the RX interrupt */
- if (napi_schedule_prep(&fep->napi)) {
- writel(FEC_RX_DISABLED_IMASK,
- fep->hwp + FEC_IMASK);
- __napi_schedule(&fep->napi);
- }
- }
+ /* Disable the NAPI interrupts */
+ writel(FEC_ENET_MII, fep->hwp + FEC_IMASK);
+ napi_schedule(&fep->napi);
+ }
- if (int_events & FEC_ENET_MII) {
- ret = IRQ_HANDLED;
- complete(&fep->mdio_done);
- }
- } while (int_events);
+ if (int_events & FEC_ENET_MII) {
+ ret = IRQ_HANDLED;
+ complete(&fep->mdio_done);
+ }
return ret;
}
@@ -1399,8 +1395,16 @@ fec_enet_interrupt(int irq, void *dev_id)
static int fec_enet_rx_napi(struct napi_struct *napi, int budget)
{
struct net_device *ndev = napi->dev;
- int pkts = fec_enet_rx(ndev, budget);
struct fec_enet_private *fep = netdev_priv(ndev);
+ int pkts;
+
+ /*
+ * Clear any pending transmit or receive interrupts before
+ * processing the rings to avoid racing with the hardware.
+ */
+ writel(FEC_ENET_RXF | FEC_ENET_TXF, fep->hwp + FEC_IEVENT);
+
+ pkts = fec_enet_rx(ndev, budget);
fec_enet_tx(ndev);
--
1.8.3.1
next prev parent reply other threads:[~2014-07-07 23:22 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-07 23:21 [PATCH A 00/12] Freescale ethernet driver updates Russell King - ARM Linux
2014-07-07 23:22 ` [PATCH A 01/12] net: fec: iMX6 FEC does not support half-duplex gigabit Russell King
2014-07-08 16:52 ` Sergei Shtylyov
2014-07-07 23:22 ` [PATCH A 02/12] net: fec: fix ethtool set_pauseparam duplex bug Russell King
2014-07-07 23:22 ` Russell King [this message]
2014-07-07 23:22 ` [PATCH A 04/12] net: fec: use netif_tx_disable() rather than netif_stop_queue() Russell King
2014-07-07 23:22 ` [PATCH A 05/12] net: fec: remove checking for NULL phy_dev in fec_enet_close() Russell King
2014-07-07 23:22 ` [PATCH A 06/12] net: fec: ensure that a disconnected phy isn't configured Russell King
2014-07-07 23:23 ` [PATCH A 07/12] net: fec: stop the phy before shutting down the MAC Russell King
2014-07-07 23:23 ` [PATCH A 08/12] net: fec: remove useless fep->opened Russell King
2014-07-07 23:23 ` [PATCH A 09/12] net: fec: make rx skb handling more robust Russell King
2014-07-07 23:23 ` [PATCH A 10/12] net: fec: clean up transmit descriptor setup Russell King
2014-07-07 23:23 ` [PATCH A 11/12] net: fec: ensure fec_enet_free_buffers() properly cleans the rings Russell King
2014-07-07 23:23 ` [PATCH A 12/12] net: fec: fix missing kmalloc() failure check in fec_enet_alloc_buffers() Russell King
2014-07-08 4:22 ` [PATCH A 00/12] Freescale ethernet driver updates David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E1X4IFA-0004wp-6x@rmk-PC.arm.linux.org.uk \
--to=rmk+kernel@arm$(echo .)linux.org.uk \
--cc=davem@davemloft$(echo .)net \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox