mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Alexander Lobakin <aleksander.lobakin@intel.com>
To: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Cc: Alexander Lobakin <aleksander.lobakin@intel.com>,
	Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
	Larysa Zaremba <larysa.zaremba@intel.com>,
	Yunsheng Lin <linyunsheng@huawei.com>,
	Alexander Duyck <alexanderduyck@fb.com>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH RFC net-next v2 5/7] net: page_pool: avoid calling no-op externals when possible
Date: Fri, 14 Jul 2023 19:08:50 +0200	[thread overview]
Message-ID: <20230714170853.866018-8-aleksander.lobakin@intel.com> (raw)
In-Reply-To: <20230714170853.866018-1-aleksander.lobakin@intel.com>

Turned out page_pool_put{,_full}_page() can burn quite a bunch of cycles
even when on DMA-coherent platforms (like x86) with no active IOMMU or
swiotlb, just for the call ladder.
Indeed, it's

page_pool_put_page()
  page_pool_put_defragged_page()                  <- external
    __page_pool_put_page()
      page_pool_dma_sync_for_device()             <- non-inline
        dma_sync_single_range_for_device()
          dma_sync_single_for_device()            <- external
            dma_direct_sync_single_for_device()
              dev_is_dma_coherent()               <- exit

For the inline functions, no guarantees the compiler won't uninline them
(they're clearly not one-liners and sometimes compilers uninline even
2 + 2). The first external call is necessary, but the rest 2+ are done
for nothing each time, plus a bunch of checks here and there.
Since Page Pool mappings are long-term and for one "device + addr" pair
dma_need_sync() will always return the same value (basically, whether it
belongs to an swiotlb pool), addresses can be tested once right after
they're obtained and the result can be reused until the page is unmapped.
Define the new PP DMA sync operation type, which will mean "do DMA syncs
for the device, but only when needed" and turn it on by default when the
driver asks to sync pages. When a page is mapped, check whether it needs
syncs and if so, replace that "sync when needed" back to "always do
syncs" globally for the whole pool (better safe than sorry). As long as
the pool has no pages requiring DMA syncs, this cuts off a good piece
of calls and checks. When at least one page required it, the pool
conservatively falls back to "always call sync functions", no per-page
verdicts. It's a fairly rare case anyway that only a few pages would
require syncing.
On my x86_64, this gives from 2% to 5% performance benefit with no
negative impact for cases when IOMMU is on and the shortcut can't be
used.

Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
---
 include/net/page_pool.h | 1 +
 net/core/page_pool.c    | 7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/net/page_pool.h b/include/net/page_pool.h
index aefcb76baf0e..3a2e1b29ff3e 100644
--- a/include/net/page_pool.h
+++ b/include/net/page_pool.h
@@ -133,6 +133,7 @@ struct page_pool {
 	bool dma_map:1;				/* Perform DMA mapping */
 	enum {
 		PP_DMA_SYNC_ACT_DISABLED = 0,	/* Driver didn't ask to sync */
+		PP_DMA_SYNC_ACT_SKIP,		/* Syncs can be skipped */
 		PP_DMA_SYNC_ACT_DO,		/* Perform DMA sync ops */
 	} dma_sync_act:2;
 
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 67957e74e1f5..ee6ce6786e29 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -196,7 +196,8 @@ static int page_pool_init(struct page_pool *pool,
 		if (!pool->p.max_len)
 			return -EINVAL;
 
-		pool->dma_sync_act = PP_DMA_SYNC_ACT_DO;
+		/* Try to avoid calling no-op syncs */
+		pool->dma_sync_act = PP_DMA_SYNC_ACT_SKIP;
 
 		/* pool->p.offset has to be set according to the address
 		 * offset used by the DMA engine to start copying rx data
@@ -345,6 +346,10 @@ static bool page_pool_dma_map(struct page_pool *pool, struct page *page)
 
 	page_pool_set_dma_addr(page, dma);
 
+	if (pool->dma_sync_act == PP_DMA_SYNC_ACT_SKIP &&
+	    dma_need_sync(pool->p.dev, dma))
+		pool->dma_sync_act = PP_DMA_SYNC_ACT_DO;
+
 	if (pool->dma_sync_act == PP_DMA_SYNC_ACT_DO)
 		page_pool_dma_sync_for_device(pool, page, pool->p.max_len);
 
-- 
2.41.0


  parent reply	other threads:[~2023-07-14 17:11 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-14 17:08 [PATCH RFC net-next v2 0/7] net: page_pool: a couple of assorted optimizations Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 1/7] net: skbuff: don't include <net/page_pool.h> to <linux/skbuff.h> Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 2/7] net: page_pool: place frag_* fields in one cacheline Alexander Lobakin
2023-07-14 17:14   ` Alexander Lobakin
2023-07-14 18:37   ` Jesper Dangaard Brouer
2023-07-18 13:50     ` Alexander Lobakin
2023-07-26  8:13       ` Ilias Apalodimas
2023-07-26 10:39         ` Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 2/7] net: page_pool: shrink &page_pool_params a tiny bit Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 3/7] net: page_pool: place frag_* fields in one cacheline Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 3/7] net: page_pool: shrink &page_pool_params a tiny bit Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 4/7] net: page_pool: don't use driver-set flags field directly Alexander Lobakin
2023-07-14 17:08 ` Alexander Lobakin [this message]
2023-07-14 17:08 ` [PATCH RFC net-next v2 6/7] net: skbuff: avoid accessing page_pool if !napi_safe when returning page Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 7/7] net: skbuff: always try to recycle PP pages directly when in softirq Alexander Lobakin
2023-07-19  0:40   ` Jakub Kicinski
2023-07-19 16:34     ` Alexander Lobakin
2023-07-19 20:51       ` Jakub Kicinski
2023-07-20 16:46         ` Alexander Lobakin
2023-07-20 17:12           ` Jakub Kicinski
2023-07-20 17:48             ` Alexander Lobakin
2023-07-20 18:00               ` Jakub Kicinski
2023-07-20 18:01                 ` Alexander Lobakin
2023-07-20 18:13                   ` Alexander Lobakin
2023-07-20 19:20                     ` Jakub Kicinski
2023-07-20 19:33                       ` Alexander Lobakin
2023-07-20 19:46                         ` Jakub Kicinski
2023-07-21 11:53                           ` Yunsheng Lin
2023-07-21 15:37                             ` Alexander Lobakin
2023-07-21 16:01                               ` Jakub Kicinski

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=20230714170853.866018-8-aleksander.lobakin@intel.com \
    --to=aleksander.lobakin@intel.com \
    --cc=alexanderduyck@fb.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hawk@kernel.org \
    --cc=ilias.apalodimas@linaro.org \
    --cc=kuba@kernel.org \
    --cc=larysa.zaremba@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linyunsheng@huawei.com \
    --cc=maciej.fijalkowski@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /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

all inboxes | Powered by JetHome®