From patchwork Tue Feb 17 00:02:53 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vlastimil Setka X-Patchwork-Id: 440417 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 B0AC714028F for ; Tue, 17 Feb 2015 11:03:29 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751810AbbBQADZ (ORCPT ); Mon, 16 Feb 2015 19:03:25 -0500 Received: from mail.spsostrov.cz ([217.117.209.230]:57119 "EHLO mail.spsostrov.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751694AbbBQADB (ORCPT ); Mon, 16 Feb 2015 19:03:01 -0500 Received: from kolej-mk-90.zcu.cz ([147.228.209.158] helo=[10.10.90.17]) by mail.spsostrov.cz with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.72) (envelope-from ) id 1YNVcv-0002xE-21; Tue, 17 Feb 2015 01:02:57 +0100 Message-ID: <54E2852D.2080702@vsis.cz> Date: Tue, 17 Feb 2015 01:02:53 +0100 From: Vlastimil Setka User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: vbridger@opensource.altera.com, netdev@vger.kernel.org, rfi , rpisl@kky.zcu.cz Subject: Re: Altera TSE (altera_tse): fix NAPI polling (was: net_rx_action WARNING) References: <54C63585.1010908@vsis.cz> In-Reply-To: <54C63585.1010908@vsis.cz> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hello, Here is a patch which fixes incorrect NAPI polling in altera_tse driver. It caused warnings I reported here some time before (attached below). I extensively tested this patch and it also solved stability issues I had with the driver under high load at socfpga platform. I am not so familiar with NAPI infrastructure, so I got inspiration in other drivers using NAPI. Comments are welcome. Subject: [PATCH 1/1] Altera TSE: fix NAPI polling Incorrect NAPI polling caused WARNING at net/core/dev.c net_rx_action. Some stability issues were also seen at high throughput and system load before this patch. This patch contains several changes in altera_tse_main.c: - tse_rx() is fixed to not process more than `limit` frames - tse_poll() is refactored to match NAPI logic - only received frames are counted for return value - removed bogus condition `(rxcomplete >= budget || txcomplete > 0)` - replace by: if (rxcomplete < budget) -> call __napi_complete and enable irq - altera_isr() - replace spin_lock_irqsave() by spin_lock() - we are in isr - use spinlocks just over irq manipulation, not over __napi_schedule - reset IRQ first, then disable and schedule napi Signed-off-by: Vlastimil Setka Signed-off-by: Roman Pisl --- drivers/net/ethernet/altera/altera_tse_main.c | 50 ++++++++++++++------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c index f3d784a..088a43f 100644 --- a/drivers/net/ethernet/altera/altera_tse_main.c +++ b/drivers/net/ethernet/altera/altera_tse_main.c @@ -376,7 +376,8 @@ static int tse_rx(struct altera_tse_private *priv, int limit) u16 pktlength; u16 pktstatus; - while ((rxstatus = priv->dmaops->get_rx_status(priv)) != 0) { + while (((rxstatus = priv->dmaops->get_rx_status(priv)) != 0) && + (count < limit)) { pktstatus = rxstatus >> 16; pktlength = rxstatus & 0xffff; @@ -491,28 +492,29 @@ static int tse_poll(struct napi_struct *napi, int budget) struct altera_tse_private *priv = container_of(napi, struct altera_tse_private, napi); int rxcomplete = 0; - int txcomplete = 0; unsigned long int flags; - txcomplete = tse_tx_complete(priv); + tse_tx_complete(priv); rxcomplete = tse_rx(priv, budget); - if (rxcomplete >= budget || txcomplete > 0) - return rxcomplete; + /* if we did not reach work limit, then we're done with polling */ + if (rxcomplete < budget) { - napi_gro_flush(napi, false); - __napi_complete(napi); + napi_gro_flush(napi, false); + __napi_complete(napi); - netdev_dbg(priv->dev, - "NAPI Complete, did %d packets with budget %d\n", - txcomplete+rxcomplete, budget); + netdev_dbg(priv->dev, + "NAPI Complete, did %d packets with budget %d\n", + rxcomplete, budget); - spin_lock_irqsave(&priv->rxdma_irq_lock, flags); - priv->dmaops->enable_rxirq(priv); - priv->dmaops->enable_txirq(priv); - spin_unlock_irqrestore(&priv->rxdma_irq_lock, flags); - return rxcomplete + txcomplete; + spin_lock_irqsave(&priv->rxdma_irq_lock, flags); + priv->dmaops->enable_rxirq(priv); + priv->dmaops->enable_txirq(priv); + spin_unlock_irqrestore(&priv->rxdma_irq_lock, flags); + } + + return rxcomplete; } /* DMA TX & RX FIFO interrupt routing @@ -521,7 +523,6 @@ static irqreturn_t altera_isr(int irq, void *dev_id) { struct net_device *dev = dev_id; struct altera_tse_private *priv; - unsigned long int flags; if (unlikely(!dev)) { pr_err("%s: invalid dev pointer\n", __func__); @@ -529,21 +530,22 @@ static irqreturn_t altera_isr(int irq, void *dev_id) } priv = netdev_priv(dev); - /* turn off desc irqs and enable napi rx */ - spin_lock_irqsave(&priv->rxdma_irq_lock, flags); + /* reset IRQs */ + spin_lock(&priv->rxdma_irq_lock); + priv->dmaops->clear_rxirq(priv); + priv->dmaops->clear_txirq(priv); + spin_unlock(&priv->rxdma_irq_lock); if (likely(napi_schedule_prep(&priv->napi))) { + /* turn off desc irqs and enable napi rx */ + spin_lock(&priv->rxdma_irq_lock); priv->dmaops->disable_rxirq(priv); priv->dmaops->disable_txirq(priv); + spin_unlock(&priv->rxdma_irq_lock); + __napi_schedule(&priv->napi); } - /* reset IRQs */ - priv->dmaops->clear_rxirq(priv); - priv->dmaops->clear_txirq(priv); - - spin_unlock_irqrestore(&priv->rxdma_irq_lock, flags); - return IRQ_HANDLED; }