From patchwork Wed Mar 5 08:28:38 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Claudiu Manoil X-Patchwork-Id: 326633 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id D3F992C00CE for ; Wed, 5 Mar 2014 19:30:27 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751919AbaCEIaO (ORCPT ); Wed, 5 Mar 2014 03:30:14 -0500 Received: from tx2ehsobe004.messaging.microsoft.com ([65.55.88.14]:36780 "EHLO tx2outboundpool.messaging.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750747AbaCEIaM (ORCPT ); Wed, 5 Mar 2014 03:30:12 -0500 Received: from mail14-tx2-R.bigfish.com (10.9.14.253) by TX2EHSOBE010.bigfish.com (10.9.40.30) with Microsoft SMTP Server id 14.1.225.22; Wed, 5 Mar 2014 08:30:11 +0000 Received: from mail14-tx2 (localhost [127.0.0.1]) by mail14-tx2-R.bigfish.com (Postfix) with ESMTP id 5B7353401C3; Wed, 5 Mar 2014 08:30:11 +0000 (UTC) X-Forefront-Antispam-Report: CIP:70.37.183.190; KIP:(null); UIP:(null); IPV:NLI; H:mail.freescale.net; RD:none; EFVD:NLI X-SpamScore: 0 X-BigFish: VS0(zzzz1f42h2148h208ch1ee6h1de0h1fdah2073h2146h1202h1e76h2189h1d1ah1d2ah21bch1fc6hzz1de098h8275bh1de097hz2dh2a8h839hd24he5bhf0ah1288h12a5h12a9h12bdh12e5h137ah139eh13b6h1441h1504h1537h162dh1631h1758h1898h18e1h1946h19b5h1ad9h1b0ah1b2fh2222h224fh1fb3h1d0ch1d2eh1d3fh1dfeh1dffh1e23h1fe8h1ff5h2218h2216h226dh22d0h24afh2327h2336h2438h2461h2487h24d7h2516h2545h255eh25cch25f6h1155h) Received: from mail14-tx2 (localhost.localdomain [127.0.0.1]) by mail14-tx2 (MessageSwitch) id 1394008209104956_3820; Wed, 5 Mar 2014 08:30:09 +0000 (UTC) Received: from TX2EHSMHS038.bigfish.com (unknown [10.9.14.226]) by mail14-tx2.bigfish.com (Postfix) with ESMTP id 09BD2A00B6; Wed, 5 Mar 2014 08:30:09 +0000 (UTC) Received: from mail.freescale.net (70.37.183.190) by TX2EHSMHS038.bigfish.com (10.9.99.138) with Microsoft SMTP Server (TLS) id 14.16.227.3; Wed, 5 Mar 2014 08:30:08 +0000 Received: from tx30smr01.am.freescale.net (10.81.153.31) by 039-SN1MMR1-004.039d.mgd.msft.net (10.84.1.14) with Microsoft SMTP Server (TLS) id 14.3.158.2; Wed, 5 Mar 2014 08:30:08 +0000 Received: from fsr-fed1764-016.ea.freescale.net (fsr-fed1764-016.ea.freescale.net [10.171.81.161]) by tx30smr01.am.freescale.net (8.14.3/8.14.0) with ESMTP id s258U0CB016412; Wed, 5 Mar 2014 01:30:07 -0700 From: Claudiu Manoil To: CC: "David S. Miller" Subject: [PATCH net-next 1/2] gianfar: Separate out the Tx interrupt handling (Tx NAPI) Date: Wed, 5 Mar 2014 10:28:38 +0200 Message-ID: <1394008119-27899-2-git-send-email-claudiu.manoil@freescale.com> X-Mailer: git-send-email 1.7.11.7 In-Reply-To: <1394008119-27899-1-git-send-email-claudiu.manoil@freescale.com> References: <1394008119-27899-1-git-send-email-claudiu.manoil@freescale.com> MIME-Version: 1.0 X-OriginatorOrg: freescale.com X-FOPE-CONNECTOR: Id%0$Dn%*$RO%0$TLS%0$FQDN%$TlsDn% Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org There are some concurrency issues on devices w/ 2 CPUs related to the handling of Rx and Tx interrupts. eTSEC has separate interrupt lines for Rx and Tx but a single imask register to mask these interrupts and a single NAPI instance to handle both Rx and Tx work. As a result, the Rx and Tx ISRs are identical, both are invoking gfar_schedule_cleanup(), however both handlers can be entered at the same time when the Rx and Tx interrupts are taken by different CPUs. In this case spurrious interrupts (SPU) show up (in /proc/interrupts) indicating a concurrency issue. Also, Tx overruns followed by Tx timeout have been observed under heavy Tx traffic load. To address these issues, the schedule cleanup ISR part has been changed to handle the Rx and Tx interrupts independently. The patch adds a separate NAPI poll routine for Tx cleanup to be triggerred independently by the Tx confirmation interrupts only. Existing poll functions are modified to handle only the Rx path processing. The Tx poll routine does not need a budget, since Tx processing doesn't consume NAPI budget, and hence it is registered with minimum NAPI weight. NAPI scheduling does not require locking since there are different NAPI instances between the Rx and Tx confirmation paths now. So, the patch fixes the occurence of spurrious Rx/Tx interrupts. Tx overruns also occur less frequently now. Signed-off-by: Claudiu Manoil --- drivers/net/ethernet/freescale/gianfar.c | 218 ++++++++++++++++++++++--------- drivers/net/ethernet/freescale/gianfar.h | 11 +- 2 files changed, 160 insertions(+), 69 deletions(-) diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c index c5b9320..1aa2d55 100644 --- a/drivers/net/ethernet/freescale/gianfar.c +++ b/drivers/net/ethernet/freescale/gianfar.c @@ -128,8 +128,10 @@ static void free_skb_resources(struct gfar_private *priv); static void gfar_set_multi(struct net_device *dev); static void gfar_set_hash_for_addr(struct net_device *dev, u8 *addr); static void gfar_configure_serdes(struct net_device *dev); -static int gfar_poll(struct napi_struct *napi, int budget); -static int gfar_poll_sq(struct napi_struct *napi, int budget); +static int gfar_poll_rx(struct napi_struct *napi, int budget); +static int gfar_poll_tx(struct napi_struct *napi, int budget); +static int gfar_poll_rx_sq(struct napi_struct *napi, int budget); +static int gfar_poll_tx_sq(struct napi_struct *napi, int budget); #ifdef CONFIG_NET_POLL_CONTROLLER static void gfar_netpoll(struct net_device *dev); #endif @@ -614,16 +616,20 @@ static void disable_napi(struct gfar_private *priv) { int i; - for (i = 0; i < priv->num_grps; i++) - napi_disable(&priv->gfargrp[i].napi); + for (i = 0; i < priv->num_grps; i++) { + napi_disable(&priv->gfargrp[i].napi_rx); + napi_disable(&priv->gfargrp[i].napi_tx); + } } static void enable_napi(struct gfar_private *priv) { int i; - for (i = 0; i < priv->num_grps; i++) - napi_enable(&priv->gfargrp[i].napi); + for (i = 0; i < priv->num_grps; i++) { + napi_enable(&priv->gfargrp[i].napi_rx); + napi_enable(&priv->gfargrp[i].napi_tx); + } } static int gfar_parse_group(struct device_node *np, @@ -1257,13 +1263,19 @@ static int gfar_probe(struct platform_device *ofdev) dev->ethtool_ops = &gfar_ethtool_ops; /* Register for napi ...We are registering NAPI for each grp */ - if (priv->mode == SQ_SG_MODE) - netif_napi_add(dev, &priv->gfargrp[0].napi, gfar_poll_sq, + if (priv->mode == SQ_SG_MODE) { + netif_napi_add(dev, &priv->gfargrp[0].napi_rx, gfar_poll_rx_sq, GFAR_DEV_WEIGHT); - else - for (i = 0; i < priv->num_grps; i++) - netif_napi_add(dev, &priv->gfargrp[i].napi, gfar_poll, - GFAR_DEV_WEIGHT); + netif_napi_add(dev, &priv->gfargrp[0].napi_tx, gfar_poll_tx_sq, + 2); + } else { + for (i = 0; i < priv->num_grps; i++) { + netif_napi_add(dev, &priv->gfargrp[i].napi_rx, + gfar_poll_rx, GFAR_DEV_WEIGHT); + netif_napi_add(dev, &priv->gfargrp[i].napi_tx, + gfar_poll_tx, 2); + } + } if (priv->device_flags & FSL_GIANFAR_DEV_HAS_CSUM) { dev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG | @@ -2538,31 +2550,6 @@ static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue) netdev_tx_completed_queue(txq, howmany, bytes_sent); } -static void gfar_schedule_cleanup(struct gfar_priv_grp *gfargrp) -{ - unsigned long flags; - - spin_lock_irqsave(&gfargrp->grplock, flags); - if (napi_schedule_prep(&gfargrp->napi)) { - gfar_write(&gfargrp->regs->imask, IMASK_RTX_DISABLED); - __napi_schedule(&gfargrp->napi); - } else { - /* Clear IEVENT, so interrupts aren't called again - * because of the packets that have already arrived. - */ - gfar_write(&gfargrp->regs->ievent, IEVENT_RTX_MASK); - } - spin_unlock_irqrestore(&gfargrp->grplock, flags); - -} - -/* Interrupt Handler for Transmit complete */ -static irqreturn_t gfar_transmit(int irq, void *grp_id) -{ - gfar_schedule_cleanup((struct gfar_priv_grp *)grp_id); - return IRQ_HANDLED; -} - static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp, struct sk_buff *skb) { @@ -2633,7 +2620,48 @@ static inline void count_errors(unsigned short status, struct net_device *dev) irqreturn_t gfar_receive(int irq, void *grp_id) { - gfar_schedule_cleanup((struct gfar_priv_grp *)grp_id); + struct gfar_priv_grp *grp = (struct gfar_priv_grp *)grp_id; + unsigned long flags; + u32 imask; + + if (likely(napi_schedule_prep(&grp->napi_rx))) { + spin_lock_irqsave(&grp->grplock, flags); + imask = gfar_read(&grp->regs->imask); + imask &= IMASK_RX_DISABLED; + gfar_write(&grp->regs->imask, imask); + spin_unlock_irqrestore(&grp->grplock, flags); + __napi_schedule(&grp->napi_rx); + } else { + /* Clear IEVENT, so interrupts aren't called again + * because of the packets that have already arrived. + */ + gfar_write(&grp->regs->ievent, IEVENT_RX_MASK); + } + + return IRQ_HANDLED; +} + +/* Interrupt Handler for Transmit complete */ +static irqreturn_t gfar_transmit(int irq, void *grp_id) +{ + struct gfar_priv_grp *grp = (struct gfar_priv_grp *)grp_id; + unsigned long flags; + u32 imask; + + if (likely(napi_schedule_prep(&grp->napi_tx))) { + spin_lock_irqsave(&grp->grplock, flags); + imask = gfar_read(&grp->regs->imask); + imask &= IMASK_TX_DISABLED; + gfar_write(&grp->regs->imask, imask); + spin_unlock_irqrestore(&grp->grplock, flags); + __napi_schedule(&grp->napi_tx); + } else { + /* Clear IEVENT, so interrupts aren't called again + * because of the packets that have already arrived. + */ + gfar_write(&grp->regs->ievent, IEVENT_TX_MASK); + } + return IRQ_HANDLED; } @@ -2757,7 +2785,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit) rx_queue->stats.rx_bytes += pkt_len; skb_record_rx_queue(skb, rx_queue->qindex); gfar_process_frame(dev, skb, amount_pull, - &rx_queue->grp->napi); + &rx_queue->grp->napi_rx); } else { netif_warn(priv, rx_err, dev, "Missing skb!\n"); @@ -2786,55 +2814,81 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit) return howmany; } -static int gfar_poll_sq(struct napi_struct *napi, int budget) +static int gfar_poll_rx_sq(struct napi_struct *napi, int budget) { struct gfar_priv_grp *gfargrp = - container_of(napi, struct gfar_priv_grp, napi); + container_of(napi, struct gfar_priv_grp, napi_rx); struct gfar __iomem *regs = gfargrp->regs; - struct gfar_priv_tx_q *tx_queue = gfargrp->priv->tx_queue[0]; struct gfar_priv_rx_q *rx_queue = gfargrp->priv->rx_queue[0]; int work_done = 0; /* Clear IEVENT, so interrupts aren't called again * because of the packets that have already arrived */ - gfar_write(®s->ievent, IEVENT_RTX_MASK); - - /* run Tx cleanup to completion */ - if (tx_queue->tx_skbuff[tx_queue->skb_dirtytx]) - gfar_clean_tx_ring(tx_queue); + gfar_write(®s->ievent, IEVENT_RX_MASK); work_done = gfar_clean_rx_ring(rx_queue, budget); if (work_done < budget) { + u32 imask; napi_complete(napi); /* Clear the halt bit in RSTAT */ gfar_write(®s->rstat, gfargrp->rstat); - gfar_write(®s->imask, IMASK_DEFAULT); + spin_lock_irq(&gfargrp->grplock); + imask = gfar_read(®s->imask); + imask |= IMASK_RX_DEFAULT; + gfar_write(®s->imask, imask); + spin_unlock_irq(&gfargrp->grplock); } return work_done; } -static int gfar_poll(struct napi_struct *napi, int budget) +static int gfar_poll_tx_sq(struct napi_struct *napi, int budget) { struct gfar_priv_grp *gfargrp = - container_of(napi, struct gfar_priv_grp, napi); + container_of(napi, struct gfar_priv_grp, napi_tx); + struct gfar __iomem *regs = gfargrp->regs; + struct gfar_priv_tx_q *tx_queue = gfargrp->priv->tx_queue[0]; + u32 imask; + + /* Clear IEVENT, so interrupts aren't called again + * because of the packets that have already arrived + */ + gfar_write(®s->ievent, IEVENT_TX_MASK); + + /* run Tx cleanup to completion */ + if (tx_queue->tx_skbuff[tx_queue->skb_dirtytx]) + gfar_clean_tx_ring(tx_queue); + + napi_complete(napi); + + spin_lock_irq(&gfargrp->grplock); + imask = gfar_read(®s->imask); + imask |= IMASK_TX_DEFAULT; + gfar_write(®s->imask, imask); + spin_unlock_irq(&gfargrp->grplock); + + return 0; +} + +static int gfar_poll_rx(struct napi_struct *napi, int budget) +{ + struct gfar_priv_grp *gfargrp = + container_of(napi, struct gfar_priv_grp, napi_rx); struct gfar_private *priv = gfargrp->priv; struct gfar __iomem *regs = gfargrp->regs; - struct gfar_priv_tx_q *tx_queue = NULL; struct gfar_priv_rx_q *rx_queue = NULL; int work_done = 0, work_done_per_q = 0; int i, budget_per_q = 0; - int has_tx_work = 0; unsigned long rstat_rxf; int num_act_queues; /* Clear IEVENT, so interrupts aren't called again * because of the packets that have already arrived */ - gfar_write(®s->ievent, IEVENT_RTX_MASK); + gfar_write(®s->ievent, IEVENT_RX_MASK); rstat_rxf = gfar_read(®s->rstat) & RSTAT_RXF_MASK; @@ -2842,15 +2896,6 @@ static int gfar_poll(struct napi_struct *napi, int budget) if (num_act_queues) budget_per_q = budget/num_act_queues; - for_each_set_bit(i, &gfargrp->tx_bit_map, priv->num_tx_queues) { - tx_queue = priv->tx_queue[i]; - /* run Tx cleanup to completion */ - if (tx_queue->tx_skbuff[tx_queue->skb_dirtytx]) { - gfar_clean_tx_ring(tx_queue); - has_tx_work = 1; - } - } - for_each_set_bit(i, &gfargrp->rx_bit_map, priv->num_rx_queues) { /* skip queue if not active */ if (!(rstat_rxf & (RSTAT_CLEAR_RXF0 >> i))) @@ -2873,19 +2918,62 @@ static int gfar_poll(struct napi_struct *napi, int budget) } } - if (!num_act_queues && !has_tx_work) { - + if (!num_act_queues) { + u32 imask; napi_complete(napi); /* Clear the halt bit in RSTAT */ gfar_write(®s->rstat, gfargrp->rstat); - gfar_write(®s->imask, IMASK_DEFAULT); + spin_lock_irq(&gfargrp->grplock); + imask = gfar_read(®s->imask); + imask |= IMASK_RX_DEFAULT; + gfar_write(®s->imask, imask); + spin_unlock_irq(&gfargrp->grplock); } return work_done; } +static int gfar_poll_tx(struct napi_struct *napi, int budget) +{ + struct gfar_priv_grp *gfargrp = + container_of(napi, struct gfar_priv_grp, napi_tx); + struct gfar_private *priv = gfargrp->priv; + struct gfar __iomem *regs = gfargrp->regs; + struct gfar_priv_tx_q *tx_queue = NULL; + int has_tx_work = 0; + int i; + + /* Clear IEVENT, so interrupts aren't called again + * because of the packets that have already arrived + */ + gfar_write(®s->ievent, IEVENT_TX_MASK); + + for_each_set_bit(i, &gfargrp->tx_bit_map, priv->num_tx_queues) { + tx_queue = priv->tx_queue[i]; + /* run Tx cleanup to completion */ + if (tx_queue->tx_skbuff[tx_queue->skb_dirtytx]) { + gfar_clean_tx_ring(tx_queue); + has_tx_work = 1; + } + } + + if (!has_tx_work) { + u32 imask; + napi_complete(napi); + + spin_lock_irq(&gfargrp->grplock); + imask = gfar_read(®s->imask); + imask |= IMASK_TX_DEFAULT; + gfar_write(®s->imask, imask); + spin_unlock_irq(&gfargrp->grplock); + } + + return 0; +} + + #ifdef CONFIG_NET_POLL_CONTROLLER /* Polling 'interrupt' - used by things like netconsole to send skbs * without having to re-enable interrupts. It's not called while diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h index 1e16216..1aeb34e 100644 --- a/drivers/net/ethernet/freescale/gianfar.h +++ b/drivers/net/ethernet/freescale/gianfar.h @@ -377,8 +377,11 @@ extern const char gfar_driver_version[]; IMASK_RXFEN0 | IMASK_BSY | IMASK_EBERR | IMASK_BABR | \ IMASK_XFUN | IMASK_RXC | IMASK_BABT | IMASK_DPE \ | IMASK_PERR) -#define IMASK_RTX_DISABLED ((~(IMASK_RXFEN0 | IMASK_TXFEN | IMASK_BSY)) \ - & IMASK_DEFAULT) +#define IMASK_RX_DEFAULT (IMASK_RXFEN0 | IMASK_BSY) +#define IMASK_TX_DEFAULT (IMASK_TXFEN | IMASK_TXBEN) + +#define IMASK_RX_DISABLED ((~(IMASK_RX_DEFAULT)) & IMASK_DEFAULT) +#define IMASK_TX_DISABLED ((~(IMASK_TX_DEFAULT)) & IMASK_DEFAULT) /* Fifo management */ #define FIFO_TX_THR_MASK 0x01ff @@ -1014,13 +1017,13 @@ struct gfar_irqinfo { struct gfar_priv_grp { spinlock_t grplock __attribute__ ((aligned (SMP_CACHE_BYTES))); - struct napi_struct napi; + struct napi_struct napi_rx; + struct napi_struct napi_tx; struct gfar_private *priv; struct gfar __iomem *regs; unsigned int rstat; unsigned long num_rx_queues; unsigned long rx_bit_map; - /* cacheline 3 */ unsigned int tstat; unsigned long num_tx_queues; unsigned long tx_bit_map;