diff mbox series

[net-next,v8,06/17] net: page pool: add helper creating area from pages

Message ID 20241204172204.4180482-7-dw@davidwei.uk (mailing list archive)
State New
Headers show
Series io_uring zero copy rx | expand

Commit Message

David Wei Dec. 4, 2024, 5:21 p.m. UTC
From: Pavel Begunkov <asml.silence@gmail.com>

Add a helper that takes an array of pages and initialises passed in
memory provider's area with them, where each net_iov takes one page.
It's also responsible for setting up dma mappings.

We keep it in page_pool.c not to leak netmem details to outside
providers like io_uring, which don't have access to netmem_priv.h
and other private helpers.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: David Wei <dw@davidwei.uk>
---
 include/net/page_pool/memory_provider.h | 10 ++++
 net/core/page_pool.c                    | 63 ++++++++++++++++++++++++-
 2 files changed, 71 insertions(+), 2 deletions(-)
 create mode 100644 include/net/page_pool/memory_provider.h

Comments

Jakub Kicinski Dec. 10, 2024, 3:29 a.m. UTC | #1
On Wed,  4 Dec 2024 09:21:45 -0800 David Wei wrote:
> From: Pavel Begunkov <asml.silence@gmail.com>
> 
> Add a helper that takes an array of pages and initialises passed in
> memory provider's area with them, where each net_iov takes one page.
> It's also responsible for setting up dma mappings.
> 
> We keep it in page_pool.c not to leak netmem details to outside
> providers like io_uring, which don't have access to netmem_priv.h
> and other private helpers.

User space will likely give us hugepages. Feels a bit wasteful to map
and manage them 4k at a time. But okay, we can optimize this later.

> diff --git a/include/net/page_pool/memory_provider.h b/include/net/page_pool/memory_provider.h
> new file mode 100644
> index 000000000000..83d7eec0058d
> --- /dev/null
> +++ b/include/net/page_pool/memory_provider.h
> @@ -0,0 +1,10 @@

nit: missing SPDX

> +#ifndef _NET_PAGE_POOL_MEMORY_PROVIDER_H
> +#define _NET_PAGE_POOL_MEMORY_PROVIDER_H
> +
> +int page_pool_mp_init_paged_area(struct page_pool *pool,
> +				struct net_iov_area *area,
> +				struct page **pages);
> +void page_pool_mp_release_area(struct page_pool *pool,
> +				struct net_iov_area *area);
> +
> +#endif

> +static void page_pool_release_page_dma(struct page_pool *pool,
> +				       netmem_ref netmem)
> +{
> +	__page_pool_release_page_dma(pool, netmem);

I'm guessing this is to save text? Because
__page_pool_release_page_dma() is always_inline? 
Maybe add a comment?

> +}
> +
> +int page_pool_mp_init_paged_area(struct page_pool *pool,
> +				 struct net_iov_area *area,
> +				 struct page **pages)
> +{
> +	struct net_iov *niov;
> +	netmem_ref netmem;
> +	int i, ret = 0;
> +
> +	if (!pool->dma_map)
> +		return -EOPNOTSUPP;
> +
> +	for (i = 0; i < area->num_niovs; i++) {
> +		niov = &area->niovs[i];
> +		netmem = net_iov_to_netmem(niov);
> +
> +		page_pool_set_pp_info(pool, netmem);

Maybe move setting pp down, after we successfully mapped. Technically
it's not a bug to leave it set on netmem, but it would be on a page
struct.

> +		if (!page_pool_dma_map_page(pool, netmem, pages[i])) {
> +			ret = -EINVAL;
> +			goto err_unmap_dma;
> +		}
> +	}
> +	return 0;
> +
> +err_unmap_dma:
> +	while (i--) {
> +		netmem = net_iov_to_netmem(&area->niovs[i]);
> +		page_pool_release_page_dma(pool, netmem);
> +	}
> +	return ret;
> +}
> +
> +void page_pool_mp_release_area(struct page_pool *pool,
> +			       struct net_iov_area *area)
> +{
> +	int i;
> +
> +	if (!pool->dma_map)
> +		return;
> +
> +	for (i = 0; i < area->num_niovs; i++) {
> +		struct net_iov *niov = &area->niovs[i];
> +
> +		page_pool_release_page_dma(pool, net_iov_to_netmem(niov));
> +	}
> +}
Pavel Begunkov Dec. 10, 2024, 3:58 a.m. UTC | #2
On 12/10/24 03:29, Jakub Kicinski wrote:
> On Wed,  4 Dec 2024 09:21:45 -0800 David Wei wrote:
>> From: Pavel Begunkov <asml.silence@gmail.com>
>>
>> Add a helper that takes an array of pages and initialises passed in
>> memory provider's area with them, where each net_iov takes one page.
>> It's also responsible for setting up dma mappings.
>>
>> We keep it in page_pool.c not to leak netmem details to outside
>> providers like io_uring, which don't have access to netmem_priv.h
>> and other private helpers.
> 
> User space will likely give us hugepages. Feels a bit wasteful to map
> and manage them 4k at a time. But okay, we can optimize this later.

I killed the api and moved bits of it out of net/. If multiple pools
are expected, dma mapping and pp assigning need to happen at different
points in time, and having such api doesn't make sense.
diff mbox series

Patch

diff --git a/include/net/page_pool/memory_provider.h b/include/net/page_pool/memory_provider.h
new file mode 100644
index 000000000000..83d7eec0058d
--- /dev/null
+++ b/include/net/page_pool/memory_provider.h
@@ -0,0 +1,10 @@ 
+#ifndef _NET_PAGE_POOL_MEMORY_PROVIDER_H
+#define _NET_PAGE_POOL_MEMORY_PROVIDER_H
+
+int page_pool_mp_init_paged_area(struct page_pool *pool,
+				struct net_iov_area *area,
+				struct page **pages);
+void page_pool_mp_release_area(struct page_pool *pool,
+				struct net_iov_area *area);
+
+#endif
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 13f1a4a63760..d17e536ba8b8 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -13,6 +13,7 @@ 
 
 #include <net/netdev_rx_queue.h>
 #include <net/page_pool/helpers.h>
+#include <net/page_pool/memory_provider.h>
 #include <net/xdp.h>
 
 #include <linux/dma-direction.h>
@@ -459,7 +460,8 @@  page_pool_dma_sync_for_device(const struct page_pool *pool,
 		__page_pool_dma_sync_for_device(pool, netmem, dma_sync_size);
 }
 
-static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem)
+static bool page_pool_dma_map_page(struct page_pool *pool, netmem_ref netmem,
+				   struct page *page)
 {
 	dma_addr_t dma;
 
@@ -468,7 +470,7 @@  static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem)
 	 * into page private data (i.e 32bit cpu with 64bit DMA caps)
 	 * This mapping is kept for lifetime of page, until leaving pool.
 	 */
-	dma = dma_map_page_attrs(pool->p.dev, netmem_to_page(netmem), 0,
+	dma = dma_map_page_attrs(pool->p.dev, page, 0,
 				 (PAGE_SIZE << pool->p.order), pool->p.dma_dir,
 				 DMA_ATTR_SKIP_CPU_SYNC |
 					 DMA_ATTR_WEAK_ORDERING);
@@ -490,6 +492,11 @@  static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem)
 	return false;
 }
 
+static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem)
+{
+	return page_pool_dma_map_page(pool, netmem, netmem_to_page(netmem));
+}
+
 static struct page *__page_pool_alloc_page_order(struct page_pool *pool,
 						 gfp_t gfp)
 {
@@ -1154,3 +1161,55 @@  void page_pool_update_nid(struct page_pool *pool, int new_nid)
 	}
 }
 EXPORT_SYMBOL(page_pool_update_nid);
+
+static void page_pool_release_page_dma(struct page_pool *pool,
+				       netmem_ref netmem)
+{
+	__page_pool_release_page_dma(pool, netmem);
+}
+
+int page_pool_mp_init_paged_area(struct page_pool *pool,
+				 struct net_iov_area *area,
+				 struct page **pages)
+{
+	struct net_iov *niov;
+	netmem_ref netmem;
+	int i, ret = 0;
+
+	if (!pool->dma_map)
+		return -EOPNOTSUPP;
+
+	for (i = 0; i < area->num_niovs; i++) {
+		niov = &area->niovs[i];
+		netmem = net_iov_to_netmem(niov);
+
+		page_pool_set_pp_info(pool, netmem);
+		if (!page_pool_dma_map_page(pool, netmem, pages[i])) {
+			ret = -EINVAL;
+			goto err_unmap_dma;
+		}
+	}
+	return 0;
+
+err_unmap_dma:
+	while (i--) {
+		netmem = net_iov_to_netmem(&area->niovs[i]);
+		page_pool_release_page_dma(pool, netmem);
+	}
+	return ret;
+}
+
+void page_pool_mp_release_area(struct page_pool *pool,
+			       struct net_iov_area *area)
+{
+	int i;
+
+	if (!pool->dma_map)
+		return;
+
+	for (i = 0; i < area->num_niovs; i++) {
+		struct net_iov *niov = &area->niovs[i];
+
+		page_pool_release_page_dma(pool, net_iov_to_netmem(niov));
+	}
+}