linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jon Hunter <jonathanh@nvidia.com>
To: Jose Abreu <Jose.Abreu@synopsys.com>,
	<linux-kernel@vger.kernel.org>, <netdev@vger.kernel.org>,
	<linux-stm32@st-md-mailman.stormreply.com>,
	<linux-arm-kernel@lists.infradead.org>
Cc: Joao Pinto <Joao.Pinto@synopsys.com>,
	"David S . Miller" <davem@davemloft.net>,
	Giuseppe Cavallaro <peppe.cavallaro@st.com>,
	Alexandre Torgue <alexandre.torgue@st.com>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	Chen-Yu Tsai <wens@csie.org>, Robin Murphy <robin.murphy@arm.com>,
	"wens@csie.org" <wens@csie.org>,
	linux-tegra <linux-tegra@vger.kernel.org>,
	"peppe.cavallaro@st.com" <peppe.cavallaro@st.com>
Subject: Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
Date: Thu, 25 Jul 2019 14:20:07 +0100	[thread overview]
Message-ID: <7a79be5d-7ba2-c457-36d3-1ccef6572181@nvidia.com> (raw)
In-Reply-To: <1b254bb7fc6044c5e6e2fdd9e00088d1d13a808b.1562149883.git.joabreu@synopsys.com>


On 03/07/2019 11:37, Jose Abreu wrote:
> Mapping and unmapping DMA region is an high bottleneck in stmmac driver,
> specially in the RX path.
> 
> This commit introduces support for Page Pool API and uses it in all RX
> queues. With this change, we get more stable troughput and some increase
> of banwidth with iperf:
> 	- MAC1000 - 950 Mbps
> 	- XGMAC: 9.22 Gbps
> 
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> ---
>  drivers/net/ethernet/stmicro/stmmac/Kconfig       |   1 +
>  drivers/net/ethernet/stmicro/stmmac/stmmac.h      |  10 +-
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 196 ++++++----------------
>  3 files changed, 63 insertions(+), 144 deletions(-)

...

> @@ -3306,49 +3295,22 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
>  		else
>  			p = rx_q->dma_rx + entry;
>  
> -		if (likely(!rx_q->rx_skbuff[entry])) {
> -			struct sk_buff *skb;
> -
> -			skb = netdev_alloc_skb_ip_align(priv->dev, bfsize);
> -			if (unlikely(!skb)) {
> -				/* so for a while no zero-copy! */
> -				rx_q->rx_zeroc_thresh = STMMAC_RX_THRESH;
> -				if (unlikely(net_ratelimit()))
> -					dev_err(priv->device,
> -						"fail to alloc skb entry %d\n",
> -						entry);
> -				break;
> -			}
> -
> -			rx_q->rx_skbuff[entry] = skb;
> -			rx_q->rx_skbuff_dma[entry] =
> -			    dma_map_single(priv->device, skb->data, bfsize,
> -					   DMA_FROM_DEVICE);
> -			if (dma_mapping_error(priv->device,
> -					      rx_q->rx_skbuff_dma[entry])) {
> -				netdev_err(priv->dev, "Rx DMA map failed\n");
> -				dev_kfree_skb(skb);
> +		if (!buf->page) {
> +			buf->page = page_pool_dev_alloc_pages(rx_q->page_pool);
> +			if (!buf->page)
>  				break;
> -			}
> -
> -			stmmac_set_desc_addr(priv, p, rx_q->rx_skbuff_dma[entry]);
> -			stmmac_refill_desc3(priv, rx_q, p);
> -
> -			if (rx_q->rx_zeroc_thresh > 0)
> -				rx_q->rx_zeroc_thresh--;
> -
> -			netif_dbg(priv, rx_status, priv->dev,
> -				  "refill entry #%d\n", entry);
>  		}
> -		dma_wmb();
> +
> +		buf->addr = buf->page->dma_addr;
> +		stmmac_set_desc_addr(priv, p, buf->addr);
> +		stmmac_refill_desc3(priv, rx_q, p);
>  
>  		rx_q->rx_count_frames++;
>  		rx_q->rx_count_frames %= priv->rx_coal_frames;
>  		use_rx_wd = priv->use_riwt && rx_q->rx_count_frames;
>  
> -		stmmac_set_rx_owner(priv, p, use_rx_wd);
> -
>  		dma_wmb();
> +		stmmac_set_rx_owner(priv, p, use_rx_wd);
>  
>  		entry = STMMAC_GET_ENTRY(entry, DMA_RX_SIZE);
>  	}

I was looking at this change in a bit closer detail and one thing that
stuck out to me was the above where the barrier had been moved from
after the stmmac_set_rx_owner() call to before. 

So I moved this back and I no longer saw the crash. However, then I
recalled I had the patch to enable the debug prints for the buffer
address applied but after reverting that, the crash occurred again. 

In other words, what works for me is moving the above barrier and adding
the debug print, which appears to suggest that there is some
timing/coherency issue here. Anyway, maybe this is clue to what is going
on?

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index a7486c2f3221..2f016397231b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3303,8 +3303,8 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
                rx_q->rx_count_frames %= priv->rx_coal_frames;
                use_rx_wd = priv->use_riwt && rx_q->rx_count_frames;
 
-               dma_wmb();
                stmmac_set_rx_owner(priv, p, use_rx_wd);
+               dma_wmb();
 
                entry = STMMAC_GET_ENTRY(entry, DMA_RX_SIZE);
        }
@@ -3438,6 +3438,10 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
                        dma_sync_single_for_device(priv->device, buf->addr,
                                                   frame_len, DMA_FROM_DEVICE);
 
+                       pr_info("%s: paddr=0x%llx, vaddr=0x%llx, len=%d", __func__,
+                                       buf->addr, page_address(buf->page),
+                                       frame_len);
+
                        if (netif_msg_pktdata(priv)) {
                                netdev_dbg(priv->dev, "frame received (%dbytes)",
                                           frame_len);

Cheers
Jon

-- 
nvpublic

  parent reply	other threads:[~2019-07-25 13:20 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-03 10:37 [PATCH net-next 0/3] net: stmmac: Some performance improvements and a fix Jose Abreu
2019-07-03 10:37 ` [PATCH net-next 1/3] net: stmmac: Implement RX Coalesce Frames setting Jose Abreu
2019-07-03 20:41   ` Jakub Kicinski
2019-07-03 10:37 ` [PATCH net-next 2/3] net: stmmac: Fix descriptors address being in > 32 bits address space Jose Abreu
2019-07-03 10:37 ` [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool Jose Abreu
2019-07-03 10:40   ` Jose Abreu
2019-07-04  9:39   ` Jesper Dangaard Brouer
2019-07-04 14:45     ` Jose Abreu
2019-07-04 15:09       ` Jesper Dangaard Brouer
2019-07-04 15:18         ` Jose Abreu
2019-07-04 15:33           ` Jesper Dangaard Brouer
2019-07-04  9:48   ` Jesper Dangaard Brouer
2019-07-04 10:00   ` Jesper Dangaard Brouer
2019-07-04 10:13     ` Jose Abreu
2019-07-04 11:11       ` Ilias Apalodimas
2019-07-04 11:54       ` Jesper Dangaard Brouer
2019-07-04 12:04         ` Ilias Apalodimas
2019-07-04 12:59           ` Jose Abreu
2019-07-04 13:06             ` Ilias Apalodimas
2019-07-04 10:30     ` Ilias Apalodimas
2019-07-04 12:14       ` Arnd Bergmann
2019-07-04 12:49         ` Ilias Apalodimas
2019-07-17 18:58   ` Jon Hunter
2019-07-18  7:29     ` Jose Abreu
2019-07-18  7:48     ` Jose Abreu
2019-07-18  9:16       ` Jon Hunter
2019-07-19  7:51         ` Jose Abreu
2019-07-19  8:37           ` Jon Hunter
2019-07-19  8:44             ` Jose Abreu
2019-07-19  8:49               ` Jon Hunter
2019-07-19 10:25                 ` Jose Abreu
2019-07-19 12:28                   ` Jose Abreu
2019-07-19 13:33                     ` Jon Hunter
2019-07-19 12:30                   ` Jon Hunter
2019-07-19 12:32                     ` Jose Abreu
2019-07-19 13:35                       ` Jon Hunter
2019-07-22  7:23                         ` Jose Abreu
2019-07-22  9:37                           ` Jon Hunter
2019-07-22  9:47                             ` Jose Abreu
2019-07-22  9:57                               ` Jose Abreu
2019-07-22 10:27                                 ` Jon Hunter
2019-07-22 10:18       ` Ilias Apalodimas
2019-07-22 11:11         ` Lars Persson
2019-07-22 11:39           ` Jose Abreu
2019-07-22 12:05             ` Jon Hunter
2019-07-22 14:04               ` Jose Abreu
2019-07-23  8:14                 ` Jose Abreu
2019-07-23 10:01                   ` Jon Hunter
2019-07-23 10:07                     ` Jose Abreu
2019-07-23 10:29                       ` Robin Murphy
2019-07-23 11:22                         ` Jose Abreu
2019-07-23 12:09                         ` Jon Hunter
2019-07-23 13:19                           ` Robin Murphy
2019-07-23 21:39                             ` Jon Hunter
2019-07-24 10:03                               ` Robin Murphy
2019-07-23 18:51                           ` David Miller
2019-07-24  8:54                             ` Ilias Apalodimas
2019-07-24  9:43                               ` Jose Abreu
2019-07-24  9:53                                 ` Ilias Apalodimas
2019-07-24 10:04                                   ` Jose Abreu
2019-07-24 11:10                                     ` Jon Hunter
2019-07-24 11:34                                       ` Jose Abreu
2019-07-24 11:58                                         ` Jon Hunter
2019-07-25  7:44                                           ` Jose Abreu
2019-07-25  9:45                                             ` Jon Hunter
2019-07-25 11:39                                               ` Ilias Apalodimas
2019-07-23 10:38                       ` Jon Hunter
2019-07-23 10:49                         ` Jose Abreu
2019-07-23 11:58                           ` Jon Hunter
2019-07-23 12:51                             ` Jose Abreu
2019-07-23 13:34                               ` Jon Hunter
2019-07-29  9:45                                 ` Mikko Perttunen
2019-07-25 13:20   ` Jon Hunter [this message]
2019-07-25 13:26     ` Jose Abreu
2019-07-25 14:25       ` Jon Hunter
2019-07-25 15:12         ` Jose Abreu
2019-07-26 14:11           ` Jon Hunter
2019-07-27 15:56             ` Jose Abreu
2019-07-29  8:16               ` Jose Abreu
2019-07-29 10:55                 ` Jon Hunter
2019-07-29 11:29                   ` Jose Abreu
2019-07-29 11:52                     ` Robin Murphy
2019-07-29 14:08                       ` Jose Abreu
2019-07-29 21:33                         ` Jon Hunter
2019-07-30  9:39                           ` Jose Abreu
2019-07-30 13:36                             ` Jon Hunter
2019-07-30 13:58                               ` Jose Abreu

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=7a79be5d-7ba2-c457-36d3-1ccef6572181@nvidia.com \
    --to=jonathanh@nvidia.com \
    --cc=Joao.Pinto@synopsys.com \
    --cc=Jose.Abreu@synopsys.com \
    --cc=alexandre.torgue@st.com \
    --cc=davem@davemloft.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=linux-tegra@vger.kernel.org \
    --cc=maxime.ripard@bootlin.com \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=peppe.cavallaro@st.com \
    --cc=robin.murphy@arm.com \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).