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: Paul Menzel <pmenzel@molgen.mpg.de>, Jesper Dangaard Brouer <hawk@kernel.org>, Larysa Zaremba <larysa.zaremba@intel.com>, netdev@vger.kernel.org, Ilias Apalodimas <ilias.apalodimas@linaro.org>, linux-kernel@vger.kernel.org, Michal Kubiak <michal.kubiak@intel.com>, intel-wired-lan@lists.osuosl.org, Christoph Hellwig <hch@lst.de>, Magnus Karlsson <magnus.karlsson@intel.com> Subject: [Intel-wired-lan] [PATCH net-next v2 07/12] net: page_pool: avoid calling no-op externals when possible Date: Thu, 25 May 2023 14:57:41 +0200 [thread overview] Message-ID: <20230525125746.553874-8-aleksander.lobakin@intel.com> (raw) In-Reply-To: <20230525125746.553874-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 new PP flag, which will mean "do DMA syncs for 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 a pool has no pages requiring DMA syncs, this cuts off a good piece of calls and checks. 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 | 3 +++ net/core/page_pool.c | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/include/net/page_pool.h b/include/net/page_pool.h index 821c75bba125..08e9571d2545 100644 --- a/include/net/page_pool.h +++ b/include/net/page_pool.h @@ -46,6 +46,9 @@ * device driver responsibility */ #define PP_FLAG_PAGE_FRAG BIT(2) /* for page frag feature */ +#define PP_FLAG_DMA_MAYBE_SYNC BIT(3) /* Internal, should not be used in + * drivers + */ #define PP_FLAG_ALL (PP_FLAG_DMA_MAP |\ PP_FLAG_DMA_SYNC_DEV |\ PP_FLAG_PAGE_FRAG) diff --git a/net/core/page_pool.c b/net/core/page_pool.c index e212e9d7edcb..57f323dee6c4 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c @@ -175,6 +175,10 @@ static int page_pool_init(struct page_pool *pool, /* pool->p.offset has to be set according to the address * offset used by the DMA engine to start copying rx data */ + + /* Try to avoid calling no-op syncs */ + pool->p.flags |= PP_FLAG_DMA_MAYBE_SYNC; + pool->p.flags &= ~PP_FLAG_DMA_SYNC_DEV; } if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT && @@ -323,6 +327,12 @@ static bool page_pool_dma_map(struct page_pool *pool, struct page *page) page_pool_set_dma_addr(page, dma); + if ((pool->p.flags & PP_FLAG_DMA_MAYBE_SYNC) && + dma_need_sync(pool->p.dev, dma)) { + pool->p.flags |= PP_FLAG_DMA_SYNC_DEV; + pool->p.flags &= ~PP_FLAG_DMA_MAYBE_SYNC; + } + if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV) page_pool_dma_sync_for_device(pool, page, pool->p.max_len); -- 2.40.1 _______________________________________________ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
WARNING: multiple messages have this Message-ID (diff)
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>, Magnus Karlsson <magnus.karlsson@intel.com>, Michal Kubiak <michal.kubiak@intel.com>, Larysa Zaremba <larysa.zaremba@intel.com>, Jesper Dangaard Brouer <hawk@kernel.org>, Ilias Apalodimas <ilias.apalodimas@linaro.org>, Christoph Hellwig <hch@lst.de>, Paul Menzel <pmenzel@molgen.mpg.de>, netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org, linux-kernel@vger.kernel.org Subject: [PATCH net-next v2 07/12] net: page_pool: avoid calling no-op externals when possible Date: Thu, 25 May 2023 14:57:41 +0200 [thread overview] Message-ID: <20230525125746.553874-8-aleksander.lobakin@intel.com> (raw) In-Reply-To: <20230525125746.553874-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 new PP flag, which will mean "do DMA syncs for 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 a pool has no pages requiring DMA syncs, this cuts off a good piece of calls and checks. 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 | 3 +++ net/core/page_pool.c | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/include/net/page_pool.h b/include/net/page_pool.h index 821c75bba125..08e9571d2545 100644 --- a/include/net/page_pool.h +++ b/include/net/page_pool.h @@ -46,6 +46,9 @@ * device driver responsibility */ #define PP_FLAG_PAGE_FRAG BIT(2) /* for page frag feature */ +#define PP_FLAG_DMA_MAYBE_SYNC BIT(3) /* Internal, should not be used in + * drivers + */ #define PP_FLAG_ALL (PP_FLAG_DMA_MAP |\ PP_FLAG_DMA_SYNC_DEV |\ PP_FLAG_PAGE_FRAG) diff --git a/net/core/page_pool.c b/net/core/page_pool.c index e212e9d7edcb..57f323dee6c4 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c @@ -175,6 +175,10 @@ static int page_pool_init(struct page_pool *pool, /* pool->p.offset has to be set according to the address * offset used by the DMA engine to start copying rx data */ + + /* Try to avoid calling no-op syncs */ + pool->p.flags |= PP_FLAG_DMA_MAYBE_SYNC; + pool->p.flags &= ~PP_FLAG_DMA_SYNC_DEV; } if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT && @@ -323,6 +327,12 @@ static bool page_pool_dma_map(struct page_pool *pool, struct page *page) page_pool_set_dma_addr(page, dma); + if ((pool->p.flags & PP_FLAG_DMA_MAYBE_SYNC) && + dma_need_sync(pool->p.dev, dma)) { + pool->p.flags |= PP_FLAG_DMA_SYNC_DEV; + pool->p.flags &= ~PP_FLAG_DMA_MAYBE_SYNC; + } + if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV) page_pool_dma_sync_for_device(pool, page, pool->p.max_len); -- 2.40.1
next prev parent reply other threads:[~2023-05-25 12:59 UTC|newest] Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-05-25 12:57 [Intel-wired-lan] [PATCH net-next v2 00/12] net: intel: start The Great Code Dedup + Page Pool for iavf Alexander Lobakin 2023-05-25 12:57 ` Alexander Lobakin 2023-05-25 12:57 ` [Intel-wired-lan] [PATCH net-next v2 01/12] net: intel: introduce Intel Ethernet common library Alexander Lobakin 2023-05-25 12:57 ` Alexander Lobakin 2023-05-25 12:57 ` [Intel-wired-lan] [PATCH net-next v2 02/12] iavf: kill "legacy-rx" for good Alexander Lobakin 2023-05-25 12:57 ` Alexander Lobakin 2023-05-30 15:29 ` Alexander H Duyck 2023-05-30 15:29 ` [Intel-wired-lan] " Alexander H Duyck 2023-05-30 16:22 ` Alexander Lobakin 2023-05-30 16:22 ` [Intel-wired-lan] " Alexander Lobakin 2023-05-25 12:57 ` [Intel-wired-lan] [PATCH net-next v2 03/12] iavf: optimize Rx buffer allocation a bunch Alexander Lobakin 2023-05-25 12:57 ` Alexander Lobakin 2023-05-30 16:18 ` Alexander H Duyck 2023-05-30 16:18 ` [Intel-wired-lan] " Alexander H Duyck 2023-05-31 11:14 ` Maciej Fijalkowski 2023-05-31 11:14 ` [Intel-wired-lan] " Maciej Fijalkowski 2023-05-31 15:22 ` Alexander Lobakin 2023-05-31 15:22 ` [Intel-wired-lan] " Alexander Lobakin 2023-05-31 15:13 ` Alexander Lobakin 2023-05-31 15:13 ` Alexander Lobakin 2023-05-31 17:22 ` Alexander Duyck 2023-06-02 13:58 ` Alexander Lobakin 2023-06-02 13:58 ` Alexander Lobakin 2023-06-02 15:04 ` Alexander Duyck 2023-06-02 15:04 ` Alexander Duyck 2023-06-02 16:15 ` Alexander Lobakin 2023-06-02 16:15 ` Alexander Lobakin 2023-06-02 17:50 ` Alexander Duyck 2023-06-02 17:50 ` Alexander Duyck 2023-06-06 12:47 ` Alexander Lobakin 2023-06-06 12:47 ` Alexander Lobakin 2023-06-06 14:24 ` Alexander Duyck 2023-06-06 14:24 ` Alexander Duyck 2023-05-25 12:57 ` [Intel-wired-lan] [PATCH net-next v2 04/12] iavf: remove page splitting/recycling Alexander Lobakin 2023-05-25 12:57 ` Alexander Lobakin 2023-05-25 12:57 ` [Intel-wired-lan] [PATCH net-next v2 05/12] iavf: always use a full order-0 page Alexander Lobakin 2023-05-25 12:57 ` Alexander Lobakin 2023-05-26 8:57 ` David Laight 2023-05-26 8:57 ` [Intel-wired-lan] " David Laight 2023-05-26 12:52 ` Alexander Lobakin 2023-05-26 12:52 ` [Intel-wired-lan] " Alexander Lobakin 2023-05-25 12:57 ` [Intel-wired-lan] [PATCH net-next v2 06/12] net: skbuff: don't include <net/page_pool.h> into <linux/skbuff.h> Alexander Lobakin 2023-05-25 12:57 ` Alexander Lobakin 2023-05-27 3:54 ` Jakub Kicinski 2023-05-27 3:54 ` [Intel-wired-lan] " Jakub Kicinski 2023-05-30 13:12 ` Alexander Lobakin 2023-05-30 13:12 ` [Intel-wired-lan] " Alexander Lobakin 2023-05-25 12:57 ` Alexander Lobakin [this message] 2023-05-25 12:57 ` [PATCH net-next v2 07/12] net: page_pool: avoid calling no-op externals when possible Alexander Lobakin 2023-05-25 12:57 ` [Intel-wired-lan] [PATCH net-next v2 08/12] net: page_pool: add DMA-sync-for-CPU inline helpers Alexander Lobakin 2023-05-25 12:57 ` Alexander Lobakin 2023-05-25 12:57 ` [Intel-wired-lan] [PATCH net-next v2 09/12] iavf: switch to Page Pool Alexander Lobakin 2023-05-25 12:57 ` Alexander Lobakin 2023-05-25 12:57 ` [Intel-wired-lan] [PATCH net-next v2 10/12] libie: add common queue stats Alexander Lobakin 2023-05-25 12:57 ` Alexander Lobakin 2023-05-25 12:57 ` [Intel-wired-lan] [PATCH net-next v2 11/12] libie: add per-queue Page Pool stats Alexander Lobakin 2023-05-25 12:57 ` Alexander Lobakin 2023-05-25 12:57 ` [Intel-wired-lan] [PATCH net-next v2 12/12] iavf: switch queue stats to libie Alexander Lobakin 2023-05-25 12:57 ` Alexander Lobakin
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=20230525125746.553874-8-aleksander.lobakin@intel.com \ --to=aleksander.lobakin@intel.com \ --cc=davem@davemloft.net \ --cc=edumazet@google.com \ --cc=hawk@kernel.org \ --cc=hch@lst.de \ --cc=ilias.apalodimas@linaro.org \ --cc=intel-wired-lan@lists.osuosl.org \ --cc=kuba@kernel.org \ --cc=larysa.zaremba@intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=magnus.karlsson@intel.com \ --cc=michal.kubiak@intel.com \ --cc=netdev@vger.kernel.org \ --cc=pabeni@redhat.com \ --cc=pmenzel@molgen.mpg.de \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.