On Thu, Mar 7, 2024 at 5:30 PM Jakub Kicinski kuba@kernel.org wrote:

On Mon, 4 Mar 2024 18:01:36 -0800 Mina Almasry wrote:

...

• • void *(*ndo_queue_mem_alloc)(struct net_device *dev, int idx);
• • Allocate memory for an RX queue. The memory returned in the form of
• • a void * can be passed to ndo_queue_mem_free() for freeing or to
• • ndo_queue_start to create an RX queue with this memory.
• • void (*ndo_queue_mem_free)(struct net_device *dev, void *);
• • Free memory from an RX queue.
• • int (*ndo_queue_start)(struct net_device *dev, int idx, void *);
• • Start an RX queue at the specified index.
• • int (*ndo_queue_stop)(struct net_device *dev, int idx, void **);
• • Stop the RX queue at the specified index.
  */

struct net_device_ops { int (*ndo_init)(struct net_device *dev); @@ -1679,6 +1693,16 @@ struct net_device_ops { int (*ndo_hwtstamp_set)(struct net_device *dev, struct kernel_hwtstamp_config *kernel_config, struct netlink_ext_ack *extack);

• void *                  (*ndo_queue_mem_alloc)(struct net_device *dev,
  

•                                                int idx);
  

• void                    (*ndo_queue_mem_free)(struct net_device *dev,
  

•                                               void *queue_mem);
  

• int                     (*ndo_queue_start)(struct net_device *dev,
  

•                                            int idx,
  

•                                            void *queue_mem);
  

• int                     (*ndo_queue_stop)(struct net_device *dev,
  

•                                           int idx,
  

•                                           void **out_queue_mem);
  

The queue configuration object was quite an integral part of the design, I'm slightly worried that it's not here :)

That was a bit of a simplification I'm making since we just want to restart the queue. I thought it was OK to define some minimal version here and extend it later with configuration? Because in this context all we really need is to restart the queue, yes?

If extending with some configuration is a must please let me know what configuration struct you're envisioning. Were you envisioning a stub? Or some real configuration struct that we just don't use at the moment? Or one that we use for this use case somehow?

Also we may want to rename the about-to-be-merged ops from netdev_stat_ops and netdev_queue_ops, and add these there?

https://lore.kernel.org/all/20240306195509.1502746-2-kuba@kernel.org/

Yeah, that sounds reasonable! Thanks! We could also keep the netdev_stat_ops and add new netdev_queue_ops alongside them if you prefer.

Very excited to hear that you made progress on this and ported GVE over!

Actually, we're still discussing but it looks like my GVE queue API implementation I proposed earlier may be a no-go. Likely someone from the GVE team will follow up here with this piece, probably in a separate series.

For now I'm carrying my POC for the GVE implementation out of tree with the rest of the driver changes:

https://github.com/mina/linux/commit/501b734c80186545281e9edb1bf313f5a2d8cbe...

On 2024-03-04 18:01, Mina Almasry wrote:

This API enables the net stack to reset the queues used for devmem.

Signed-off-by: Mina Almasry almasrymina@google.com

---

include/linux/netdevice.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index c41019f34179..3105c586355d 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -1435,6 +1435,20 @@ struct netdev_net_notifier {

• 	   struct kernel_hwtstamp_config *kernel_config,
  

• 	   struct netlink_ext_ack *extack);
  

• Change the hardware timestamping parameters for NIC device.

• • void *(*ndo_queue_mem_alloc)(struct net_device *dev, int idx);
• • Allocate memory for an RX queue. The memory returned in the form of
• • a void * can be passed to ndo_queue_mem_free() for freeing or to
• • ndo_queue_start to create an RX queue with this memory.
• • void (*ndo_queue_mem_free)(struct net_device *dev, void *);
• • Free memory from an RX queue.
• • int (*ndo_queue_start)(struct net_device *dev, int idx, void *);
• • Start an RX queue at the specified index.
• • int (*ndo_queue_stop)(struct net_device *dev, int idx, void **);
• • Stop the RX queue at the specified index.
  */

struct net_device_ops { int (*ndo_init)(struct net_device *dev); @@ -1679,6 +1693,16 @@ struct net_device_ops { int (*ndo_hwtstamp_set)(struct net_device *dev, struct kernel_hwtstamp_config *kernel_config, struct netlink_ext_ack *extack);

• void * (*ndo_queue_mem_alloc)(struct net_device *dev,

• 				       int idx);
  

• void (*ndo_queue_mem_free)(struct net_device *dev,

• 				      void *queue_mem);
  

• int (*ndo_queue_start)(struct net_device *dev,

• 				   int idx,
  

• 				   void *queue_mem);
  

• int (*ndo_queue_stop)(struct net_device *dev,

• 				  int idx,
  

• 				  void **out_queue_mem);
  

};

I'm working to port bnxt over to using this API. What are your thoughts on maybe pulling this out and use bnxt to drive it?

/**

On 3/5/24 22:36, Mina Almasry wrote:

On Tue, Mar 5, 2024 at 1:55 PM David Wei dw@davidwei.uk wrote:

...

On 2024-03-04 18:01, Mina Almasry wrote:

...

+struct memory_provider_ops {

• int (*init)(struct page_pool *pool);
  

• void (*destroy)(struct page_pool *pool);
  

• struct page *(*alloc_pages)(struct page_pool *pool, gfp_t gfp);
  

• bool (*release_page)(struct page_pool *pool, struct page *page);
  

For ZC Rx we added a scrub() function to memory_provider_ops that is called from page_pool_scrub(). Does TCP devmem not custom behaviour waiting for all netmem_refs to return before destroying the page pool? What happens if e.g. application crashes?

(sorry for the long reply, but he refcounting is pretty complicated to explain and I feel like we need to agree on how things currently work)

Yeah, the addition of the page_pool_scrub() function is a bit of a head scratcher for me. Here is how the (complicated) refcounting works for devmem TCP (assuming the driver is not doing its own recycling logic which complicates things further):

1. When a netmem_ref is allocated by the page_pool (from dmabuf or

page), the netmem_get_pp_ref_count_ref()==1 and belongs to the page pool as long as the netmem is waiting in the pool for driver allocation.

2. When a netmem is allocated by the driver, no refcounting is

changed, but the ownership of the netmem_get_pp_ref_count_ref() is implicitly transferred from the page pool to the driver. i.e. the ref now belongs to the driver until an skb is formed.

3. When the driver forms an skb using skb_rx_add_frag_netmem(), no

refcounting is changed, but the ownership of the netmem_get_pp_ref_count_ref() is transferred from the driver to the TCP stack.

4. When the TCP stack hands the skb to the application, the TCP stack

obtains an additional refcount, so netmem_get_pp_ref_count_ref()==2, and frees the skb using skb_frag_unref(), which drops the netmem_get_pp_ref_count_ref()==1.

5. When the user is done with the skb, the user calls the

DEVMEM_DONTNEED setsockopt which calls napi_pp_put_netmem() which recycles the netmem back to the page pool. This doesn't modify any refcounting, but the refcount ownership transfers from the userspace back to the page pool, and we're back at step 1.

So all in all netmem can belong either to (a) the page pool, or (b) the driver, or (c) the TCP stack, or (d) the application depending on where exactly it is in the RX path.

When an application running devmem TCP crashes, the netmem that belong to the page pool or driver are not touched, because the page pool is not tied to the application in our case really. However, the TCP stack notices the devmem socket of the application close, and when it does, the TCP stack will:

1. Free all the skbs in the sockets receive queue. This is not custom

behavior for devmem TCP, it's just standard for TCP to free all skbs waiting to be received by the application. 2. The TCP stack will free references that belong to the application. Since the application crashed, it will not call the DEVMEM_DONTNEED setsockopt, so we need to free those on behalf of the application. This is done in this diff:

@@ -2498,6 +2498,15 @@ static void tcp_md5sig_info_free_rcu(struct rcu_head *head) void tcp_v4_destroy_sock(struct sock *sk) { struct tcp_sock *tp = tcp_sk(sk);

• __maybe_unused unsigned long index;
• __maybe_unused void *netmem;

+#ifdef CONFIG_PAGE_POOL

• xa_for_each(&sk->sk_user_frags, index, netmem)
• WARN_ON_ONCE(!napi_pp_put_page((__force netmem_ref)netmem, false));

+#endif

• xa_destroy(&sk->sk_user_frags);

  trace_tcp_destroy_sock(sk);

To be honest, I think it makes sense for the TCP stack to be responsible for putting the references that belong to it and the application. To me, it does not make much sense for the page pool to be responsible for putting the reference that belongs to the TCP stack or driver via a page_pool_scrub() function, as those references do not belong to the page pool really. I'm not sure why there is a diff between our use cases here because I'm not an io_uring expert. Why do you need to scrub all the references on page pool destruction? Don't these belong to non-page pool components like io_uring stack or TCP stack ol otherwise?

That one is about cleaning buffers that are in b/w 4 and 5, i.e. owned by the user, which devmem does at sock destruction. io_uring could get by without scrub, dropping user refs while unregistering ifq, but then it'd need to wait for all requests to finish so there is no step 4 in the meantime. Might change, can be useful, but it was much easier to hook into the pp release loop.

Another concern is who and when can reset ifq / kill pp outside of io_uring/devmem. I assume it can happen on a whim, which is hard to handle gracefully.

On 3/6/24 17:04, Mina Almasry wrote:

On Wed, Mar 6, 2024 at 6:30 AM Pavel Begunkov asml.silence@gmail.com wrote:

...

On 3/5/24 22:36, Mina Almasry wrote:

...

On Tue, Mar 5, 2024 at 1:55 PM David Wei dw@davidwei.uk wrote:

...

On 2024-03-04 18:01, Mina Almasry wrote:

...

+struct memory_provider_ops {

• int (*init)(struct page_pool *pool);
  

• void (*destroy)(struct page_pool *pool);
  

• struct page *(*alloc_pages)(struct page_pool *pool, gfp_t gfp);
  

• bool (*release_page)(struct page_pool *pool, struct page *page);
  

For ZC Rx we added a scrub() function to memory_provider_ops that is called from page_pool_scrub(). Does TCP devmem not custom behaviour waiting for all netmem_refs to return before destroying the page pool? What happens if e.g. application crashes?

(sorry for the long reply, but he refcounting is pretty complicated to explain and I feel like we need to agree on how things currently work)

Yeah, the addition of the page_pool_scrub() function is a bit of a head scratcher for me. Here is how the (complicated) refcounting works for devmem TCP (assuming the driver is not doing its own recycling logic which complicates things further):

1. When a netmem_ref is allocated by the page_pool (from dmabuf or

page), the netmem_get_pp_ref_count_ref()==1 and belongs to the page pool as long as the netmem is waiting in the pool for driver allocation.

2. When a netmem is allocated by the driver, no refcounting is

changed, but the ownership of the netmem_get_pp_ref_count_ref() is implicitly transferred from the page pool to the driver. i.e. the ref now belongs to the driver until an skb is formed.

3. When the driver forms an skb using skb_rx_add_frag_netmem(), no

refcounting is changed, but the ownership of the netmem_get_pp_ref_count_ref() is transferred from the driver to the TCP stack.

4. When the TCP stack hands the skb to the application, the TCP stack

obtains an additional refcount, so netmem_get_pp_ref_count_ref()==2, and frees the skb using skb_frag_unref(), which drops the netmem_get_pp_ref_count_ref()==1.

5. When the user is done with the skb, the user calls the

DEVMEM_DONTNEED setsockopt which calls napi_pp_put_netmem() which recycles the netmem back to the page pool. This doesn't modify any refcounting, but the refcount ownership transfers from the userspace back to the page pool, and we're back at step 1.

So all in all netmem can belong either to (a) the page pool, or (b) the driver, or (c) the TCP stack, or (d) the application depending on where exactly it is in the RX path.

When an application running devmem TCP crashes, the netmem that belong to the page pool or driver are not touched, because the page pool is not tied to the application in our case really. However, the TCP stack notices the devmem socket of the application close, and when it does, the TCP stack will:

1. Free all the skbs in the sockets receive queue. This is not custom

behavior for devmem TCP, it's just standard for TCP to free all skbs waiting to be received by the application. 2. The TCP stack will free references that belong to the application. Since the application crashed, it will not call the DEVMEM_DONTNEED setsockopt, so we need to free those on behalf of the application. This is done in this diff:

@@ -2498,6 +2498,15 @@ static void tcp_md5sig_info_free_rcu(struct rcu_head *head) void tcp_v4_destroy_sock(struct sock *sk) { struct tcp_sock *tp = tcp_sk(sk);

• __maybe_unused unsigned long index;
• __maybe_unused void *netmem;

+#ifdef CONFIG_PAGE_POOL

• xa_for_each(&sk->sk_user_frags, index, netmem)
• WARN_ON_ONCE(!napi_pp_put_page((__force netmem_ref)netmem, false));

+#endif

• xa_destroy(&sk->sk_user_frags);

  trace_tcp_destroy_sock(sk);

To be honest, I think it makes sense for the TCP stack to be responsible for putting the references that belong to it and the application. To me, it does not make much sense for the page pool to be responsible for putting the reference that belongs to the TCP stack or driver via a page_pool_scrub() function, as those references do not belong to the page pool really. I'm not sure why there is a diff between our use cases here because I'm not an io_uring expert. Why do you need to scrub all the references on page pool destruction? Don't these belong to non-page pool components like io_uring stack or TCP stack ol otherwise?

That one is about cleaning buffers that are in b/w 4 and 5, i.e. owned by the user, which devmem does at sock destruction. io_uring could get by without scrub, dropping user refs while unregistering ifq, but then it'd need to wait for all requests to finish so there is no step 4 in the meantime. Might change, can be useful, but it was much easier to hook into the pp release loop.

Another concern is who and when can reset ifq / kill pp outside of io_uring/devmem. I assume it can happen on a whim, which is hard to handle gracefully.

If this is about dropping application refs in step 4 & step 5, then from devmem TCP perspective it must be done on socket close & skb freeing AFAIU, and not delayed until page_pool destruction.

Right, something in the kernel should take care of it. You temporarily attach it to the socket, which is fine. And you could've also stored it in the netlink socket or some other object. In case of zcrx io_uring impl, it's bound to io_uring, io_uring is responsible for cleaning them up. And we do it before __page_pool_destroy, otherwise there would be a ref dependency.

A side note, attaching to netlink or some other global object sounds conceptually better, as once you return a buffer to the user, the socket should not have any further business with the buffer. FWIW, that better resembles io_uring approach. For example allows to:

recv(sock); close(sock); process_rx_buffers();

or to return (i.e. DEVMEM_DONTNEED) buffers from different sockets in one call. However, I don't think it's important for devmem and perhaps more implementation dictated.

Think about a stupid or malicious user that does something like:

1. Set up dmabuf binding using netlink api.
2. While (100000):
3. create devmem TCP socket.
4. receive some devmem data on TCP socket.
5. close TCP socket without calling DEVMEM_DONTNEED.
6. clean up dmabuf binding using netlink api.

In this case, we need to drop the references in step 5 when the socket is destroyed, so the memory is freed to the page pool and available for the next socket in step 3. We cannot delay the freeing until step 6 when the rx queue is recreated and the page pool is destroyed, otherwise the net_iovs would leak in the loop and eventually the NIC would fail to find available memory. The same bug would be

By "would leak" you probably mean until step 6, right? There are always many ways to shoot yourself in the leg. Even if you clean up in 5, the user can just leak the socket and get the same result with pp starvation. I see it not as a requirement but rather a uapi choice, that's assuming netlink would be cleaned as a normal socket when the task exits.

reproducible with io_uring unless you're creating a new page pool for each new io_uring socket equivalent.

Surely we don't, but it's still the user's responsibility to return buffers back. And in case of io_uring buffers returned to the user are not attached to a socket, so even the scope / lifetime is a bit different.

But even outside of this, I think it's a bit semantically off to ask the page_pool to drop references that belong to the application IMO, because those references are not the page_pool's.

Completely agree with you, which is why it was in a callback, totally controlled by io_uring.

On Thu, Mar 7, 2024 at 8:57 PM David Wei dw@davidwei.uk wrote:

On 2024-03-04 18:01, Mina Almasry wrote:

...

From: Jakub Kicinski kuba@kernel.org

The page providers which try to reuse the same pages will need to hold onto the ref, even if page gets released from the pool - as in releasing the page from the pp just transfers the "ownership" reference from pp to the provider, and provider will wait for other references to be gone before feeding this page back into the pool.

Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Mina Almasry almasrymina@google.com

---

This is implemented by Jakub in his RFC: https://lore.kernel.org/netdev/f8270765-a27b-6ccf-33ea-cda097168d79@redhat.c...

I take no credit for the idea or implementation; I only added minor edits to make this workable with device memory TCP, and removed some hacky test code. This is a critical dependency of device memory TCP and thus I'm pulling it into this series to make it revewable and mergeable.

RFC v3 -> v1

• Removed unusued mem_provider. (Yunsheng).
• Replaced memory_provider & mp_priv with netdev_rx_queue (Jakub).

---

include/net/page_pool/types.h | 12 ++++++++++ net/core/page_pool.c | 43 +++++++++++++++++++++++++++++++---- 2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h index 5e43a08d3231..ffe5f31fb0da 100644 --- a/include/net/page_pool/types.h +++ b/include/net/page_pool/types.h @@ -52,6 +52,7 @@ struct pp_alloc_cache {

• @dev: device, for DMA pre-mapping purposes
• @netdev: netdev this pool will serve (leave as NULL if none or multiple)
• @napi: NAPI which is the sole consumer of pages, otherwise NULL

• • @queue: struct netdev_rx_queue this page_pool is being created for.
  • @dma_dir: DMA mapping direction
  • @max_len: max DMA sync memory size for PP_FLAG_DMA_SYNC_DEV
  • @offset: DMA sync address offset for PP_FLAG_DMA_SYNC_DEV

@@ -64,6 +65,7 @@ struct page_pool_params { int nid; struct device *dev; struct napi_struct *napi;

•         struct netdev_rx_queue *queue;
          enum dma_data_direction dma_dir;
          unsigned int    max_len;
          unsigned int    offset;
  

@@ -126,6 +128,13 @@ struct page_pool_stats { }; #endif

+struct memory_provider_ops {

• int (*init)(struct page_pool *pool);
  

• void (*destroy)(struct page_pool *pool);
  

• struct page *(*alloc_pages)(struct page_pool *pool, gfp_t gfp);
  

• bool (*release_page)(struct page_pool *pool, struct page *page);
  

+};

Separate question as I try to adapt bnxt to this and your queue configuration API.

How does GVE handle the need to allocate kernel pages for headers and dmabuf for payloads?

Reading the code, struct gve_rx_ring is the main per-ring object with a page pool. gve_queue_page_lists are filled with page pool netmem allocations from the page pool in gve_alloc_queue_page_list(). Are these strictly used for payloads only?

You're almost correct. We actually don't use the gve queue page lists for devmem TCP, that's an unrelated GVE feature/code path for low memory VMs. The code in effect is the !qpl code. In that code, for incoming RX packets we allocate a new or recycled netmem from the page pool in gve_alloc_page_dqo(). These buffers are used for payload only in the case where header split is enabled. In the case header split is disabled, these buffers are used for the entire incoming packet.

I found a struct gve_header_buf in both gve_rx_ring and struct gve_per_rx_queue_mem_dpo. This is allocated in gve_rx_queue_mem_alloc() using dma_alloc_coherent(). Is this where GVE stores headers?

Yes, this is where GVE stores headers.

IOW, GVE only uses page pool to allocate memory for QPLs, and QPLs are used by the device for split payloads. Is my understanding correct?

On Tue, Mar 5, 2024 at 4:55 AM Yunsheng Lin linyunsheng@huawei.com wrote:

On 2024/3/5 10:01, Mina Almasry wrote:

...

...

The netdev_dmabuf_binding struct is refcounted, and releases its resources only when all the refs are released.

Signed-off-by: Willem de Bruijn willemb@google.com Signed-off-by: Kaiyuan Zhang kaiyuanz@google.com Signed-off-by: Mina Almasry almasrymina@google.com

---

RFC v6:

• Validate rx queue index
• Refactor new functions into devmem.c (Pavel)

It seems odd that the functions or stucts in a file called devmem.c are named after 'dmabuf' instead of 'devmem'.

So my intention with this naming that devmem.c contains all the functions for all devmem tcp specific support. Currently the only devmem we support is dmabuf. In the future, other devmem may be supported and it can fit nicely in devmem.c. For example, if we want to extend devmem TCP to support NVMe devices, we need to add support for p2pdma, maybe, and we can add that support under the devmem.c umbrella rather than add new files.

But I can rename to dmabuf.c if there is strong objection to the current name.

...

...

...

diff --git a/include/net/netmem.h b/include/net/netmem.h index d8b810245c1d..72e932a1a948 100644 --- a/include/net/netmem.h +++ b/include/net/netmem.h @@ -8,6 +8,16 @@ #ifndef _NET_NETMEM_H #define _NET_NETMEM_H

+#include <net/devmem.h>

+/* net_iov */

+struct net_iov {

• struct dmabuf_genpool_chunk_owner *owner;
  

+};

+/* netmem */

/**

• typedef netmem_ref - a nonexistent type marking a reference to generic
• network memory.

diff --git a/net/core/Makefile b/net/core/Makefile index 821aec06abf1..592f955c1241 100644 --- a/net/core/Makefile +++ b/net/core/Makefile @@ -13,7 +13,7 @@ obj-y += dev.o dev_addr_lists.o dst.o netevent.o \ neighbour.o rtnetlink.o utils.o link_watch.o filter.o \ sock_diag.o dev_ioctl.o tso.o sock_reuseport.o \ fib_notifier.o xdp.o flow_offload.o gro.o \

•                 netdev-genl.o netdev-genl-gen.o gso.o
  

•                 netdev-genl.o netdev-genl-gen.o gso.o devmem.o
  
  

obj-$(CONFIG_NETDEV_ADDR_LIST_TEST) += dev_addr_lists_test.o

diff --git a/net/core/dev.c b/net/core/dev.c index fe054cbd41e9..bbea1b252529 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -155,6 +155,9 @@ #include <net/netdev_rx_queue.h> #include <net/page_pool/types.h> #include <net/page_pool/helpers.h> +#include <linux/genalloc.h> +#include <linux/dma-buf.h> +#include <net/devmem.h>

#include "dev.h" #include "net-sysfs.h" diff --git a/net/core/devmem.c b/net/core/devmem.c new file mode 100644 index 000000000000..779ad990971e --- /dev/null +++ b/net/core/devmem.c @@ -0,0 +1,293 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*

• •  Devmem TCP
    

• •  Authors:     Mina Almasry <almasrymina@google.com>
    

• •               Willem de Bruijn <willemdebruijn.kernel@gmail.com>
    

• •               Kaiyuan Zhang <kaiyuanz@google.com
    

• */

+#include <linux/types.h> +#include <linux/mm.h> +#include <linux/netdevice.h> +#include <trace/events/page_pool.h> +#include <net/netdev_rx_queue.h> +#include <net/page_pool/types.h> +#include <net/page_pool/helpers.h> +#include <linux/genalloc.h> +#include <linux/dma-buf.h> +#include <net/devmem.h>

+/* Device memory support */

+#ifdef CONFIG_DMA_SHARED_BUFFER

I still think it is worth adding its own config for devmem or dma-buf for networking, thinking about the embeded system.

FWIW Willem did weigh on this previously and said he prefers to have it unguarded by a CONFIG, but I will submit to whatever the consensus here. It shouldn't be a huge deal to add a CONFIG technically speaking.

...

+static void netdev_dmabuf_free_chunk_owner(struct gen_pool *genpool,

•                                    struct gen_pool_chunk *chunk,
  

•                                    void *not_used)
  

It seems odd to still keep the netdev_ prefix as it is not really related to netdev, perhaps use 'net_' or something better.

Yes, thanks for catching. I can change to net_devmem_ maybe or net_dmabuf_*.

...

+{

• struct dmabuf_genpool_chunk_owner *owner = chunk->owner;
  

• kvfree(owner->niovs);
  

• kfree(owner);
  

+}

+void __netdev_dmabuf_binding_free(struct netdev_dmabuf_binding *binding) +{

• size_t size, avail;
  

• gen_pool_for_each_chunk(binding->chunk_pool,
  

•                         netdev_dmabuf_free_chunk_owner, NULL);
  

• size = gen_pool_size(binding->chunk_pool);
  

• avail = gen_pool_avail(binding->chunk_pool);
  

• if (!WARN(size != avail, "can't destroy genpool. size=%lu, avail=%lu",
  

•           size, avail))
  

•         gen_pool_destroy(binding->chunk_pool);
  

• dma_buf_unmap_attachment(binding->attachment, binding->sgt,
  

•                          DMA_BIDIRECTIONAL);
  

For now DMA_FROM_DEVICE seems enough as tx is not supported yet.

Yes, good catch. I suspect we want to reuse this code for TX path. But for now, I'll test with DMA_FROM_DEVICE and if I see no issues I'll apply this change.

...

• dma_buf_detach(binding->dmabuf, binding->attachment);
  

• dma_buf_put(binding->dmabuf);
  

• xa_destroy(&binding->bound_rxq_list);
  

• kfree(binding);
  

+}

+static int netdev_restart_rx_queue(struct net_device *dev, int rxq_idx) +{

• void *new_mem;
  

• void *old_mem;
  

• int err;
  

• if (!dev || !dev->netdev_ops)
  

•         return -EINVAL;
  

• if (!dev->netdev_ops->ndo_queue_stop ||
  

•     !dev->netdev_ops->ndo_queue_mem_free ||
  

•     !dev->netdev_ops->ndo_queue_mem_alloc ||
  

•     !dev->netdev_ops->ndo_queue_start)
  

•         return -EOPNOTSUPP;
  

• new_mem = dev->netdev_ops->ndo_queue_mem_alloc(dev, rxq_idx);
  

• if (!new_mem)
  

•         return -ENOMEM;
  

• err = dev->netdev_ops->ndo_queue_stop(dev, rxq_idx, &old_mem);
  

• if (err)
  

•         goto err_free_new_mem;
  

• err = dev->netdev_ops->ndo_queue_start(dev, rxq_idx, new_mem);
  

• if (err)
  

•         goto err_start_queue;
  

• dev->netdev_ops->ndo_queue_mem_free(dev, old_mem);
  

• return 0;
  

+err_start_queue:

• dev->netdev_ops->ndo_queue_start(dev, rxq_idx, old_mem);
  

It might worth mentioning why queue start with old_mem will always success here as the return value seems to be ignored here.

So the old queue, we stopped it, and if we fail to bring up the new queue, then we want to start the old queue back up to get the queue back to a workable state.

I don't see what we can do to recover if restarting the old queue fails. Seems like it should be a requirement that the driver tries as much as possible to keep the old queue restartable.

I can improve this by at least logging or warning if restarting the old queue fails.

...

+err_free_new_mem:

• dev->netdev_ops->ndo_queue_mem_free(dev, new_mem);
  

• return err;
  

+}

+/* Protected by rtnl_lock() */ +static DEFINE_XARRAY_FLAGS(netdev_dmabuf_bindings, XA_FLAGS_ALLOC1);

+void netdev_unbind_dmabuf(struct netdev_dmabuf_binding *binding) +{

• struct netdev_rx_queue *rxq;
  

• unsigned long xa_idx;
  

• unsigned int rxq_idx;
  

• if (!binding)
  

•         return;
  

• if (binding->list.next)
  

•         list_del(&binding->list);
  

The above does not seems to be a good pattern to delete a entry, is there any reason having a checking before the list_del()? seems like defensive programming?

I think I needed to apply this condition to handle the case where netdev_unbind_dmabuf() is called when binding->list is not initialized or is empty.

netdev_nl_bind_rx_doit() will call unbind to free a partially allocated binding in error paths, so, netdev_unbind_dmabuf() may be called with a partially initialized binding. This is why we check for binding->list is initialized here and check that rxq->binding == binding below. The main point is that netdev_unbind_dmabuf() may be asked to unbind a partially bound dmabuf due to error paths.

Maybe a comment here will test this better. I will double confirm the check is needed for the error paths in netdev_nl_bind_rx_doit().

...

• xa_for_each(&binding->bound_rxq_list, xa_idx, rxq) {
  

•         if (rxq->binding == binding) {
  

It seems like defensive programming here too?

...

•                 /* We hold the rtnl_lock while binding/unbinding
  

•                  * dma-buf, so we can't race with another thread that
  

•                  * is also modifying this value. However, the driver
  

•                  * may read this config while it's creating its
  

•                  * rx-queues. WRITE_ONCE() here to match the
  

•                  * READ_ONCE() in the driver.
  

•                  */
  

•                 WRITE_ONCE(rxq->binding, NULL);
  

•                 rxq_idx = get_netdev_rx_queue_index(rxq);
  

•                 netdev_restart_rx_queue(binding->dev, rxq_idx);
  

•         }
  

• }
  

• xa_erase(&netdev_dmabuf_bindings, binding->id);
  

• netdev_dmabuf_binding_put(binding);
  

+}

On Wed, Mar 6, 2024 at 4:38 AM Yunsheng Lin linyunsheng@huawei.com wrote:

On 2024/3/6 5:17, Mina Almasry wrote:

...

On Tue, Mar 5, 2024 at 4:55 AM Yunsheng Lin linyunsheng@huawei.com wrote:

...

On 2024/3/5 10:01, Mina Almasry wrote:

...

...

The netdev_dmabuf_binding struct is refcounted, and releases its resources only when all the refs are released.

Signed-off-by: Willem de Bruijn willemb@google.com Signed-off-by: Kaiyuan Zhang kaiyuanz@google.com Signed-off-by: Mina Almasry almasrymina@google.com

---

RFC v6:

• Validate rx queue index
• Refactor new functions into devmem.c (Pavel)

It seems odd that the functions or stucts in a file called devmem.c are named after 'dmabuf' instead of 'devmem'.

So my intention with this naming that devmem.c contains all the functions for all devmem tcp specific support. Currently the only devmem we support is dmabuf. In the future, other devmem may be supported and it can fit nicely in devmem.c. For example, if we want to extend devmem TCP to support NVMe devices, we need to add support for p2pdma, maybe, and we can add that support under the devmem.c umbrella rather than add new files.

But I can rename to dmabuf.c if there is strong objection to the current name.

Grepping 'dmabuf' seems to show that it may be common rename it to something as *_dmabuf.c.

...

...

...

...

...

diff --git a/include/net/netmem.h b/include/net/netmem.h index d8b810245c1d..72e932a1a948 100644 --- a/include/net/netmem.h +++ b/include/net/netmem.h @@ -8,6 +8,16 @@ #ifndef _NET_NETMEM_H #define _NET_NETMEM_H

+#include <net/devmem.h>

+/* net_iov */

+struct net_iov {

• struct dmabuf_genpool_chunk_owner *owner;
  

+};

+/* netmem */

/**

• typedef netmem_ref - a nonexistent type marking a reference to generic
• network memory.

diff --git a/net/core/Makefile b/net/core/Makefile index 821aec06abf1..592f955c1241 100644 --- a/net/core/Makefile +++ b/net/core/Makefile @@ -13,7 +13,7 @@ obj-y += dev.o dev_addr_lists.o dst.o netevent.o \ neighbour.o rtnetlink.o utils.o link_watch.o filter.o \ sock_diag.o dev_ioctl.o tso.o sock_reuseport.o \ fib_notifier.o xdp.o flow_offload.o gro.o \

•                 netdev-genl.o netdev-genl-gen.o gso.o
  

•                 netdev-genl.o netdev-genl-gen.o gso.o devmem.o
  
  

obj-$(CONFIG_NETDEV_ADDR_LIST_TEST) += dev_addr_lists_test.o

diff --git a/net/core/dev.c b/net/core/dev.c index fe054cbd41e9..bbea1b252529 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -155,6 +155,9 @@ #include <net/netdev_rx_queue.h> #include <net/page_pool/types.h> #include <net/page_pool/helpers.h> +#include <linux/genalloc.h> +#include <linux/dma-buf.h> +#include <net/devmem.h>

#include "dev.h" #include "net-sysfs.h" diff --git a/net/core/devmem.c b/net/core/devmem.c new file mode 100644 index 000000000000..779ad990971e --- /dev/null +++ b/net/core/devmem.c @@ -0,0 +1,293 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*

• •  Devmem TCP
    

• •  Authors:     Mina Almasry <almasrymina@google.com>
    

• •               Willem de Bruijn <willemdebruijn.kernel@gmail.com>
    

• •               Kaiyuan Zhang <kaiyuanz@google.com
    

• */

+#include <linux/types.h> +#include <linux/mm.h> +#include <linux/netdevice.h> +#include <trace/events/page_pool.h> +#include <net/netdev_rx_queue.h> +#include <net/page_pool/types.h> +#include <net/page_pool/helpers.h> +#include <linux/genalloc.h> +#include <linux/dma-buf.h> +#include <net/devmem.h>

+/* Device memory support */

+#ifdef CONFIG_DMA_SHARED_BUFFER

I still think it is worth adding its own config for devmem or dma-buf for networking, thinking about the embeded system.

FWIW Willem did weigh on this previously and said he prefers to have it unguarded by a CONFIG, but I will submit to whatever the consensus here. It shouldn't be a huge deal to add a CONFIG technically speaking.

Grepping 'CONFIG_DMA_SHARED_BUFFER' show that the API user of dmabuf API does not seems to reuse the CONFIG_DMA_SHARED_BUFFER, instead they seem to define its own config, and select CONFIG_DMA_SHARED_BUFFER if necessary, it that any reason it is different here?

...

...

...

+static void netdev_dmabuf_free_chunk_owner(struct gen_pool *genpool,

•                                    struct gen_pool_chunk *chunk,
  

•                                    void *not_used)
  

It seems odd to still keep the netdev_ prefix as it is not really related to netdev, perhaps use 'net_' or something better.

Yes, thanks for catching. I can change to net_devmem_ maybe or net_dmabuf_*.

FWIW, net_dmabuf_* seems like a better name technically.

...

...

...

+{

• struct dmabuf_genpool_chunk_owner *owner = chunk->owner;
  

• kvfree(owner->niovs);
  

• kfree(owner);
  

+}

+void __netdev_dmabuf_binding_free(struct netdev_dmabuf_binding *binding) +{

• size_t size, avail;
  

• gen_pool_for_each_chunk(binding->chunk_pool,
  

•                         netdev_dmabuf_free_chunk_owner, NULL);
  

• size = gen_pool_size(binding->chunk_pool);
  

• avail = gen_pool_avail(binding->chunk_pool);
  

• if (!WARN(size != avail, "can't destroy genpool. size=%lu, avail=%lu",
  

•           size, avail))
  

•         gen_pool_destroy(binding->chunk_pool);
  

• dma_buf_unmap_attachment(binding->attachment, binding->sgt,
  

•                          DMA_BIDIRECTIONAL);
  

For now DMA_FROM_DEVICE seems enough as tx is not supported yet.

Yes, good catch. I suspect we want to reuse this code for TX path. But for now, I'll test with DMA_FROM_DEVICE and if I see no issues I'll apply this change.

...

...

• dma_buf_detach(binding->dmabuf, binding->attachment);
  

• dma_buf_put(binding->dmabuf);
  

• xa_destroy(&binding->bound_rxq_list);
  

• kfree(binding);
  

+}

+static int netdev_restart_rx_queue(struct net_device *dev, int rxq_idx) +{

• void *new_mem;
  

• void *old_mem;
  

• int err;
  

• if (!dev || !dev->netdev_ops)
  

•         return -EINVAL;
  

• if (!dev->netdev_ops->ndo_queue_stop ||
  

•     !dev->netdev_ops->ndo_queue_mem_free ||
  

•     !dev->netdev_ops->ndo_queue_mem_alloc ||
  

•     !dev->netdev_ops->ndo_queue_start)
  

•         return -EOPNOTSUPP;
  

• new_mem = dev->netdev_ops->ndo_queue_mem_alloc(dev, rxq_idx);
  

• if (!new_mem)
  

•         return -ENOMEM;
  

• err = dev->netdev_ops->ndo_queue_stop(dev, rxq_idx, &old_mem);
  

• if (err)
  

•         goto err_free_new_mem;
  

• err = dev->netdev_ops->ndo_queue_start(dev, rxq_idx, new_mem);
  

• if (err)
  

•         goto err_start_queue;
  

• dev->netdev_ops->ndo_queue_mem_free(dev, old_mem);
  

• return 0;
  

+err_start_queue:

• dev->netdev_ops->ndo_queue_start(dev, rxq_idx, old_mem);
  

It might worth mentioning why queue start with old_mem will always success here as the return value seems to be ignored here.

So the old queue, we stopped it, and if we fail to bring up the new queue, then we want to start the old queue back up to get the queue back to a workable state.

I don't see what we can do to recover if restarting the old queue fails. Seems like it should be a requirement that the driver tries as much as possible to keep the old queue restartable.

Is it possible that we may have the 'old_mem' leaking if the driver fails to restart the old queue? how does the driver handle the firmware cmd failure for ndo_queue_start()? it seems a little tricky to implement it.

I'm not sure what we can do to meaningfully recover from failure to restarting the old queue, except log it so the error is visible. In theory because we have not modifying any queue configurations restarting it would be straight forward, but since it's dealing with hardware then any failures are possible.

...

I can improve this by at least logging or warning if restarting the old queue fails.

Also the semantics of the above function seems odd that it is not only restarting rx queue, but also freeing and allocating memory despite the name only suggests 'restart', I am a litte afraid that it may conflict with future usecae when user only need the 'restart' part, perhaps rename it to a more appropriate name.

Oh, what we want here is just the 'restart' part. However, Jakub mandates that if you restart a queue (or a driver), you do it like this, hence the slightly more complicated implementation.

https://patchwork.kernel.org/project/netdevbpf/patch/20231106024413.2801438-... https://lore.kernel.org/netdev/20230815171638.4c057dcd@kernel.org/

...

...

...

+err_free_new_mem:

• dev->netdev_ops->ndo_queue_mem_free(dev, new_mem);
  

• return err;
  

+}

+/* Protected by rtnl_lock() */ +static DEFINE_XARRAY_FLAGS(netdev_dmabuf_bindings, XA_FLAGS_ALLOC1);

+void netdev_unbind_dmabuf(struct netdev_dmabuf_binding *binding) +{

• struct netdev_rx_queue *rxq;
  

• unsigned long xa_idx;
  

• unsigned int rxq_idx;
  

• if (!binding)
  

•         return;
  

• if (binding->list.next)
  

•         list_del(&binding->list);
  

The above does not seems to be a good pattern to delete a entry, is there any reason having a checking before the list_del()? seems like defensive programming?

I think I needed to apply this condition to handle the case where netdev_unbind_dmabuf() is called when binding->list is not initialized or is empty.

netdev_nl_bind_rx_doit() will call unbind to free a partially allocated binding in error paths, so, netdev_unbind_dmabuf() may be called with a partially initialized binding. This is why we check for binding->list is initialized here and check that rxq->binding == binding below. The main point is that netdev_unbind_dmabuf() may be asked to unbind a partially bound dmabuf due to error paths.

Maybe a comment here will test this better. I will double confirm the check is needed for the error paths in netdev_nl_bind_rx_doit().

...

...

• xa_for_each(&binding->bound_rxq_list, xa_idx, rxq) {
  

•         if (rxq->binding == binding) {
  

It seems like defensive programming here too?

On Mon, Mar 4, 2024 at 6:02 PM Mina Almasry almasrymina@google.com wrote:

Abstrace the memory type from the page_pool so we can later add support for new memory types. Convert the page_pool to use the new netmem type abstraction, rather than use struct page directly.

As of this patch the netmem type is a no-op abstraction: it's always a struct page underneath. All the page pool internals are converted to use struct netmem instead of struct page, and the page pool now exports 2 APIs:

1. The existing struct page API.
2. The new struct netmem API.

Keeping the existing API is transitional; we do not want to refactor all the current drivers using the page pool at once.

The netmem abstraction is currently a no-op. The page_pool uses page_to_netmem() to convert allocated pages to netmem, and uses netmem_to_page() to convert the netmem back to pages to pass to mm APIs,

Follow up patches to this series add non-paged netmem support to the page_pool. This change is factored out on its own to limit the code churn to this 1 patch, for ease of code review.

Signed-off-by: Mina Almasry almasrymina@google.com

Per David Howell's request, I'm forwarding these 2 patches from the series to linux-mm & Mathew Wilcox.

https://lore.kernel.org/netdev/950858.1709622997@warthog.procyon.org.uk/

+linux-mm +Mathew Wilcox +David Howells

---

v6:

• Rebased on top of the merged netmem_ref type.

---

include/linux/skbuff.h | 4 +- include/net/netmem.h | 15 ++ include/net/page_pool/helpers.h | 122 +++++++++---- include/net/page_pool/types.h | 17 +- include/trace/events/page_pool.h | 29 +-- net/bpf/test_run.c | 5 +- net/core/page_pool.c | 303 +++++++++++++++++-------------- net/core/skbuff.c | 7 +- 8 files changed, 302 insertions(+), 200 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index d577e0bee18d..ca29d1fd4561 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -3504,7 +3504,7 @@ int skb_pp_cow_data(struct page_pool *pool, struct sk_buff **pskb, unsigned int headroom); int skb_cow_data_for_xdp(struct page_pool *pool, struct sk_buff **pskb, struct bpf_prog *prog); -bool napi_pp_put_page(struct page *page, bool napi_safe); +bool napi_pp_put_page(netmem_ref netmem, bool napi_safe);

static inline void napi_frag_unref(skb_frag_t *frag, bool recycle, bool napi_safe) @@ -3512,7 +3512,7 @@ napi_frag_unref(skb_frag_t *frag, bool recycle, bool napi_safe) struct page *page = skb_frag_page(frag);

#ifdef CONFIG_PAGE_POOL

•   if (recycle && napi_pp_put_page(page, napi_safe))
  

•   if (recycle && napi_pp_put_page(page_to_netmem(page), napi_safe))
            return;
  

#endif put_page(page); diff --git a/include/net/netmem.h b/include/net/netmem.h index ca17ea1d33f8..21f53b29e5fe 100644 --- a/include/net/netmem.h +++ b/include/net/netmem.h @@ -88,4 +88,19 @@ static inline netmem_ref page_to_netmem(struct page *page) return (__force netmem_ref)page; }

+static inline int netmem_ref_count(netmem_ref netmem) +{

•   return page_ref_count(netmem_to_page(netmem));
  

+}

+static inline unsigned long netmem_to_pfn(netmem_ref netmem) +{

•   return page_to_pfn(netmem_to_page(netmem));
  

+}

+static inline netmem_ref netmem_compound_head(netmem_ref netmem) +{

•   return page_to_netmem(compound_head(netmem_to_page(netmem)));
  

+}

#endif /* _NET_NETMEM_H */ diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h index 1d397c1a0043..61814f91a458 100644 --- a/include/net/page_pool/helpers.h +++ b/include/net/page_pool/helpers.h @@ -53,6 +53,8 @@ #define _NET_PAGE_POOL_HELPERS_H

#include <net/page_pool/types.h> +#include <net/net_debug.h> +#include <net/netmem.h>

#ifdef CONFIG_PAGE_POOL_STATS /* Deprecated driver-facing API, use netlink instead */ @@ -101,7 +103,7 @@ static inline struct page *page_pool_dev_alloc_pages(struct page_pool *pool)

• Get a page fragment from the page allocator or page_pool caches.
• Return:

• • Return allocated page fragment, otherwise return NULL.

• • Return allocated page fragment, otherwise return 0.
  */

static inline struct page *page_pool_dev_alloc_frag(struct page_pool *pool, unsigned int *offset, @@ -112,22 +114,22 @@ static inline struct page *page_pool_dev_alloc_frag(struct page_pool *pool, return page_pool_alloc_frag(pool, offset, size, gfp); }

-static inline struct page *page_pool_alloc(struct page_pool *pool,

•                                      unsigned int *offset,
  

•                                      unsigned int *size, gfp_t gfp)
  

+static inline netmem_ref page_pool_alloc(struct page_pool *pool,

•                                    unsigned int *offset,
  

•                                    unsigned int *size, gfp_t gfp)
  

{ unsigned int max_size = PAGE_SIZE << pool->p.order;

•   struct page *page;
  

•   netmem_ref netmem;
  
    if ((*size << 1) > max_size) {
            *size = max_size;
            *offset = 0;
  

•           return page_pool_alloc_pages(pool, gfp);
  

•           return page_pool_alloc_netmem(pool, gfp);
    }
  
  

•   page = page_pool_alloc_frag(pool, offset, *size, gfp);
  

•   if (unlikely(!page))
  

•           return NULL;
  

•   netmem = page_pool_alloc_frag_netmem(pool, offset, *size, gfp);
  

•   if (unlikely(!netmem))
  

•           return 0;
  
    /* There is very likely not enough space for another fragment, so append
     * the remaining size to the current fragment to avoid truesize
  

@@ -138,7 +140,7 @@ static inline struct page *page_pool_alloc(struct page_pool *pool, pool->frag_offset = max_size; }

•   return page;
  

•   return netmem;
  

}

/** @@ -152,7 +154,7 @@ static inline struct page *page_pool_alloc(struct page_pool *pool,

• utilization and performance penalty.
• Return:

• • Return allocated page or page fragment, otherwise return NULL.

• • Return allocated page or page fragment, otherwise return 0.
  */

static inline struct page *page_pool_dev_alloc(struct page_pool *pool, unsigned int *offset, @@ -160,7 +162,7 @@ static inline struct page *page_pool_dev_alloc(struct page_pool *pool, { gfp_t gfp = (GFP_ATOMIC | __GFP_NOWARN);

•   return page_pool_alloc(pool, offset, size, gfp);
  

•   return netmem_to_page(page_pool_alloc(pool, offset, size, gfp));
  

}

static inline void *page_pool_alloc_va(struct page_pool *pool, @@ -170,9 +172,10 @@ static inline void *page_pool_alloc_va(struct page_pool *pool, struct page *page;

    /* Mask off __GFP_HIGHMEM to ensure we can use page_address() */

•   page = page_pool_alloc(pool, &offset, size, gfp & ~__GFP_HIGHMEM);
  

•   page = netmem_to_page(
  

•           page_pool_alloc(pool, &offset, size, gfp & ~__GFP_HIGHMEM));
    if (unlikely(!page))
  

•           return NULL;
  

•           return 0;
  
    return page_address(page) + offset;
  

} @@ -187,7 +190,7 @@ static inline void *page_pool_alloc_va(struct page_pool *pool,

• it returns va of the allocated page or page fragment.
• Return:

• • Return the va for the allocated page or page fragment, otherwise return NULL.

• • Return the va for the allocated page or page fragment, otherwise return 0.
  */

static inline void *page_pool_dev_alloc_va(struct page_pool *pool, unsigned int *size) @@ -210,6 +213,11 @@ inline enum dma_data_direction page_pool_get_dma_dir(struct page_pool *pool) return pool->p.dma_dir; }

+static inline void page_pool_fragment_netmem(netmem_ref netmem, long nr) +{

•   atomic_long_set(&netmem_to_page(netmem)->pp_ref_count, nr);
  

+}

/**

• page_pool_fragment_page() - split a fresh page into fragments
• @page: page to split

@@ -230,11 +238,12 @@ inline enum dma_data_direction page_pool_get_dma_dir(struct page_pool *pool) */ static inline void page_pool_fragment_page(struct page *page, long nr) {

•   atomic_long_set(&page->pp_ref_count, nr);
  

•   page_pool_fragment_netmem(page_to_netmem(page), nr);
  

}

-static inline long page_pool_unref_page(struct page *page, long nr) +static inline long page_pool_unref_netmem(netmem_ref netmem, long nr) {

•   struct page *page = netmem_to_page(netmem);
    long ret;
  
    /* If nr == pp_ref_count then we have cleared all remaining
  

@@ -277,15 +286,41 @@ static inline long page_pool_unref_page(struct page *page, long nr) return ret; }

+static inline long page_pool_unref_page(struct page *page, long nr) +{

•   return page_pool_unref_netmem(page_to_netmem(page), nr);
  

+}

+static inline void page_pool_ref_netmem(netmem_ref netmem) +{

•   atomic_long_inc(&netmem_to_page(netmem)->pp_ref_count);
  

+}

static inline void page_pool_ref_page(struct page *page) {

•   atomic_long_inc(&page->pp_ref_count);
  

•   page_pool_ref_netmem(page_to_netmem(page));
  

}

-static inline bool page_pool_is_last_ref(struct page *page) +static inline bool page_pool_is_last_ref(netmem_ref netmem) { /* If page_pool_unref_page() returns 0, we were the last user */

•   return page_pool_unref_page(page, 1) == 0;
  

•   return page_pool_unref_netmem(netmem, 1) == 0;
  

+}

+static inline void page_pool_put_netmem(struct page_pool *pool,

•                                   netmem_ref netmem,
  

•                                   unsigned int dma_sync_size,
  

•                                   bool allow_direct)
  

+{

•   /* When page_pool isn't compiled-in, net/core/xdp.c doesn't
  

•    * allow registering MEM_TYPE_PAGE_POOL, but shield linker.
  

•    */
  

+#ifdef CONFIG_PAGE_POOL

•   if (!page_pool_is_last_ref(netmem))
  

•           return;
  

•   page_pool_put_unrefed_netmem(pool, netmem, dma_sync_size, allow_direct);
  

+#endif }

/** @@ -306,15 +341,15 @@ static inline void page_pool_put_page(struct page_pool *pool, unsigned int dma_sync_size, bool allow_direct) {

•   /* When page_pool isn't compiled-in, net/core/xdp.c doesn't
  

•    * allow registering MEM_TYPE_PAGE_POOL, but shield linker.
  

•    */
  

-#ifdef CONFIG_PAGE_POOL

•   if (!page_pool_is_last_ref(page))
  

•           return;
  

•   page_pool_put_netmem(pool, page_to_netmem(page), dma_sync_size,
  

•                        allow_direct);
  

+}

•   page_pool_put_unrefed_page(pool, page, dma_sync_size, allow_direct);
  

-#endif +static inline void page_pool_put_full_netmem(struct page_pool *pool,

•                                        netmem_ref netmem,
  

•                                        bool allow_direct)
  

+{

•   page_pool_put_netmem(pool, netmem, -1, allow_direct);
  

}

/** @@ -329,7 +364,7 @@ static inline void page_pool_put_page(struct page_pool *pool, static inline void page_pool_put_full_page(struct page_pool *pool, struct page *page, bool allow_direct) {

•   page_pool_put_page(pool, page, -1, allow_direct);
  

•   page_pool_put_netmem(pool, page_to_netmem(page), -1, allow_direct);
  

}

/** @@ -363,6 +398,18 @@ static inline void page_pool_free_va(struct page_pool *pool, void *va, page_pool_put_page(pool, virt_to_head_page(va), -1, allow_direct); }

+static inline dma_addr_t page_pool_get_dma_addr_netmem(netmem_ref netmem) +{

•   struct page *page = netmem_to_page(netmem);
  

•   dma_addr_t ret = page->dma_addr;
  

•   if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA)
  

•           ret <<= PAGE_SHIFT;
  

•   return ret;
  

+}

/**

• page_pool_get_dma_addr() - Retrieve the stored DMA address.
• @page: page allocated from a page pool

@@ -372,16 +419,14 @@ static inline void page_pool_free_va(struct page_pool *pool, void *va, */ static inline dma_addr_t page_pool_get_dma_addr(struct page *page) {

•   dma_addr_t ret = page->dma_addr;
  

•   if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA)
  

•           ret <<= PAGE_SHIFT;
  

•   return ret;
  

•   return page_pool_get_dma_addr_netmem(page_to_netmem(page));
  

}

-static inline bool page_pool_set_dma_addr(struct page *page, dma_addr_t addr) +static inline bool page_pool_set_dma_addr_netmem(netmem_ref netmem,

•                                            dma_addr_t addr)
  

{

•   struct page *page = netmem_to_page(netmem);
  

•   if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA) {
            page->dma_addr = addr >> PAGE_SHIFT;
  
  

@@ -395,6 +440,11 @@ static inline bool page_pool_set_dma_addr(struct page *page, dma_addr_t addr) return false; }

+static inline bool page_pool_set_dma_addr(struct page *page, dma_addr_t addr) +{

•   return page_pool_set_dma_addr_netmem(page_to_netmem(page), addr);
  

+}

static inline bool page_pool_put(struct page_pool *pool) { return refcount_dec_and_test(&pool->user_cnt); diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h index ffe5f31fb0da..68a24c5ae827 100644 --- a/include/net/page_pool/types.h +++ b/include/net/page_pool/types.h @@ -40,7 +40,7 @@ #define PP_ALLOC_CACHE_REFILL 64 struct pp_alloc_cache { u32 count;

•   struct page *cache[PP_ALLOC_CACHE_SIZE];
  

•   netmem_ref cache[PP_ALLOC_CACHE_SIZE];
  

};

/** @@ -73,7 +73,7 @@ struct page_pool_params { struct_group_tagged(page_pool_params_slow, slow, struct net_device *netdev; /* private: used by test code only */

•           void (*init_callback)(struct page *page, void *arg);
  

•           void (*init_callback)(netmem_ref netmem, void *arg);
            void *init_arg;
    );
  

}; @@ -131,8 +131,8 @@ struct page_pool_stats { struct memory_provider_ops { int (*init)(struct page_pool *pool); void (*destroy)(struct page_pool *pool);

•   struct page *(*alloc_pages)(struct page_pool *pool, gfp_t gfp);
  

•   bool (*release_page)(struct page_pool *pool, struct page *page);
  

•   netmem_ref (*alloc_pages)(struct page_pool *pool, gfp_t gfp);
  

•   bool (*release_page)(struct page_pool *pool, netmem_ref netmem);
  

};

struct page_pool { @@ -142,7 +142,7 @@ struct page_pool { bool has_init_callback;

    long frag_users;

•   struct page *frag_page;
  

•   netmem_ref frag_page;
    unsigned int frag_offset;
    u32 pages_state_hold_cnt;
  
  

@@ -214,8 +214,12 @@ struct page_pool { };

struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp); +netmem_ref page_pool_alloc_netmem(struct page_pool *pool, gfp_t gfp); struct page *page_pool_alloc_frag(struct page_pool *pool, unsigned int *offset, unsigned int size, gfp_t gfp); +netmem_ref page_pool_alloc_frag_netmem(struct page_pool *pool,

•                                  unsigned int *offset, unsigned int size,
  

•                                  gfp_t gfp);
  

struct page_pool *page_pool_create(const struct page_pool_params *params); struct page_pool *page_pool_create_percpu(const struct page_pool_params *params, int cpuid); @@ -245,6 +249,9 @@ static inline void page_pool_put_page_bulk(struct page_pool *pool, void **data, } #endif

+void page_pool_put_unrefed_netmem(struct page_pool *pool, netmem_ref netmem,

•                             unsigned int dma_sync_size,
  

•                             bool allow_direct);
  

void page_pool_put_unrefed_page(struct page_pool *pool, struct page *page, unsigned int dma_sync_size, bool allow_direct); diff --git a/include/trace/events/page_pool.h b/include/trace/events/page_pool.h index 6834356b2d2a..c5b6383ff276 100644 --- a/include/trace/events/page_pool.h +++ b/include/trace/events/page_pool.h @@ -42,51 +42,52 @@ TRACE_EVENT(page_pool_release, TRACE_EVENT(page_pool_state_release,

    TP_PROTO(const struct page_pool *pool,

•            const struct page *page, u32 release),
  

•            netmem_ref netmem, u32 release),
  
  

•   TP_ARGS(pool, page, release),
  

•   TP_ARGS(pool, netmem, release),
  
    TP_STRUCT__entry(
            __field(const struct page_pool *,       pool)
  

•           __field(const struct page *,            page)
  

•           __field(netmem_ref,                     netmem)
            __field(u32,                            release)
            __field(unsigned long,                  pfn)
    ),
  
    TP_fast_assign(
            __entry->pool           = pool;
  

•           __entry->page           = page;
  

•           __entry->netmem         = netmem;
            __entry->release        = release;
  

•           __entry->pfn            = page_to_pfn(page);
  

•           __entry->pfn            = netmem_to_pfn(netmem);
    ),
  
  

•   TP_printk("page_pool=%p page=%p pfn=0x%lx release=%u",
  

•             __entry->pool, __entry->page, __entry->pfn, __entry->release)
  

•   TP_printk("page_pool=%p netmem=%lu pfn=0x%lx release=%u",
  

•             __entry->pool, (__force unsigned long)__entry->netmem,
  

•             __entry->pfn, __entry->release)
  

);

TRACE_EVENT(page_pool_state_hold,

    TP_PROTO(const struct page_pool *pool,

•            const struct page *page, u32 hold),
  

•            netmem_ref netmem, u32 hold),
  
  

•   TP_ARGS(pool, page, hold),
  

•   TP_ARGS(pool, netmem, hold),
  
    TP_STRUCT__entry(
            __field(const struct page_pool *,       pool)
  

•           __field(const struct page *,            page)
  

•           __field(netmem_ref,                     netmem)
            __field(u32,                            hold)
            __field(unsigned long,                  pfn)
    ),
  
    TP_fast_assign(
            __entry->pool   = pool;
  

•           __entry->page   = page;
  

•           __entry->netmem = netmem;
            __entry->hold   = hold;
  

•           __entry->pfn    = page_to_pfn(page);
  

•           __entry->pfn    = netmem_to_pfn(netmem);
    ),
  
  

•   TP_printk("page_pool=%p page=%p pfn=0x%lx hold=%u",
  

•             __entry->pool, __entry->page, __entry->pfn, __entry->hold)
  

•   TP_printk("page_pool=%p netmem=%lu pfn=0x%lx hold=%u",
  

•             __entry->pool, __entry->netmem, __entry->pfn, __entry->hold)
  

);

TRACE_EVENT(page_pool_update_nid, diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index 5535f9adc658..bc8f7ab88f86 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -126,9 +126,10 @@ struct xdp_test_data { #define TEST_XDP_FRAME_SIZE (PAGE_SIZE - sizeof(struct xdp_page_head)) #define TEST_XDP_MAX_BATCH 256

-static void xdp_test_run_init_page(struct page *page, void *arg) +static void xdp_test_run_init_page(netmem_ref netmem, void *arg) {

•   struct xdp_page_head *head = phys_to_virt(page_to_phys(page));
  

•   struct xdp_page_head *head =
  

•           phys_to_virt(page_to_phys(netmem_to_page(netmem)));
    struct xdp_buff *new_ctx, *orig_ctx;
    u32 headroom = XDP_PACKET_HEADROOM;
    struct xdp_test_data *xdp = arg;
  

diff --git a/net/core/page_pool.c b/net/core/page_pool.c index fe9de4ecce94..24d5236b2efc 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c @@ -329,19 +329,18 @@ struct page_pool *page_pool_create(const struct page_pool_params *params) } EXPORT_SYMBOL(page_pool_create);

-static void page_pool_return_page(struct page_pool *pool, struct page *page); +static void page_pool_return_page(struct page_pool *pool, netmem_ref netmem);

-noinline -static struct page *page_pool_refill_alloc_cache(struct page_pool *pool) +static noinline netmem_ref page_pool_refill_alloc_cache(struct page_pool *pool) { struct ptr_ring *r = &pool->ring;

•   struct page *page;
  

•   netmem_ref netmem;
    int pref_nid; /* preferred NUMA node */
  
    /* Quicker fallback, avoid locks when ring is empty */
    if (__ptr_ring_empty(r)) {
            alloc_stat_inc(pool, empty);
  

•           return NULL;
  

•           return 0;
    }
  
    /* Softirq guarantee CPU and thus NUMA node is stable. This,
  

@@ -356,56 +355,56 @@ static struct page *page_pool_refill_alloc_cache(struct page_pool *pool)

    /* Refill alloc array, but only if NUMA match */
    do {

•           page = __ptr_ring_consume(r);
  

•           if (unlikely(!page))
  

•           netmem = (__force netmem_ref)__ptr_ring_consume(r);
  

•           if (unlikely(!netmem))
                    break;
  
  

•           if (likely(page_to_nid(page) == pref_nid)) {
  

•                   pool->alloc.cache[pool->alloc.count++] = page;
  

•           if (likely(page_to_nid(netmem_to_page(netmem)) == pref_nid)) {
  

•                   pool->alloc.cache[pool->alloc.count++] = netmem;
            } else {
                    /* NUMA mismatch;
                     * (1) release 1 page to page-allocator and
                     * (2) break out to fallthrough to alloc_pages_node.
                     * This limit stress on page buddy alloactor.
                     */
  

•                   page_pool_return_page(pool, page);
  

•                   page_pool_return_page(pool, netmem);
                    alloc_stat_inc(pool, waive);
  

•                   page = NULL;
  

•                   netmem = 0;
                    break;
            }
    } while (pool->alloc.count < PP_ALLOC_CACHE_REFILL);
  
    /* Return last page */
    if (likely(pool->alloc.count > 0)) {
  

•           page = pool->alloc.cache[--pool->alloc.count];
  

•           netmem = pool->alloc.cache[--pool->alloc.count];
            alloc_stat_inc(pool, refill);
    }
  
  

•   return page;
  

•   return netmem;
  

}

/* fast path */ -static struct page *__page_pool_get_cached(struct page_pool *pool) +static netmem_ref __page_pool_get_cached(struct page_pool *pool) {

•   struct page *page;
  

•   netmem_ref netmem;
  
    /* Caller MUST guarantee safe non-concurrent access, e.g. softirq */
    if (likely(pool->alloc.count)) {
            /* Fast-path */
  

•           page = pool->alloc.cache[--pool->alloc.count];
  

•           netmem = pool->alloc.cache[--pool->alloc.count];
            alloc_stat_inc(pool, fast);
    } else {
  

•           page = page_pool_refill_alloc_cache(pool);
  

•           netmem = page_pool_refill_alloc_cache(pool);
    }
  
  

•   return page;
  

•   return netmem;
  

}

static void page_pool_dma_sync_for_device(struct page_pool *pool,

•                                     struct page *page,
  

•                                     netmem_ref netmem,
                                      unsigned int dma_sync_size)
  

{

•   dma_addr_t dma_addr = page_pool_get_dma_addr(page);
  

•   dma_addr_t dma_addr = page_pool_get_dma_addr_netmem(netmem);
  
    dma_sync_size = min(dma_sync_size, pool->p.max_len);
    dma_sync_single_range_for_device(pool->p.dev, dma_addr,
  

@@ -413,7 +412,7 @@ static void page_pool_dma_sync_for_device(struct page_pool *pool, pool->p.dma_dir); }

-static bool page_pool_dma_map(struct page_pool *pool, struct page *page) +static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem) { dma_addr_t dma;

@@ -422,18 +421,18 @@ static bool page_pool_dma_map(struct page_pool *pool, struct page *page) * 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, page, 0,
  

•                            (PAGE_SIZE << pool->p.order),
  

•                            pool->p.dma_dir, DMA_ATTR_SKIP_CPU_SYNC |
  

•                                             DMA_ATTR_WEAK_ORDERING);
  

•   dma = dma_map_page_attrs(pool->p.dev, netmem_to_page(netmem), 0,
  

•                            (PAGE_SIZE << pool->p.order), pool->p.dma_dir,
  

•                            DMA_ATTR_SKIP_CPU_SYNC |
  

•                                    DMA_ATTR_WEAK_ORDERING);
    if (dma_mapping_error(pool->p.dev, dma))
            return false;
  
  

•   if (page_pool_set_dma_addr(page, dma))
  

•   if (page_pool_set_dma_addr_netmem(netmem, dma))
            goto unmap_failed;
  
    if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
  

•           page_pool_dma_sync_for_device(pool, page, pool->p.max_len);
  

•           page_pool_dma_sync_for_device(pool, netmem, pool->p.max_len);
  
    return true;
  
  

@@ -445,9 +444,10 @@ static bool page_pool_dma_map(struct page_pool *pool, struct page *page) return false; }

-static void page_pool_set_pp_info(struct page_pool *pool,

•                             struct page *page)
  

+static void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem) {

•   struct page *page = netmem_to_page(netmem);
  

•   page->pp = pool;
    page->pp_magic |= PP_SIGNATURE;
  
  

@@ -457,13 +457,15 @@ static void page_pool_set_pp_info(struct page_pool *pool, * is dirtying the same cache line as the page->pp_magic above, so * the overhead is negligible. */

•   page_pool_fragment_page(page, 1);
  

•   page_pool_fragment_netmem(netmem, 1);
    if (pool->has_init_callback)
  

•           pool->slow.init_callback(page, pool->slow.init_arg);
  

•           pool->slow.init_callback(netmem, pool->slow.init_arg);
  

}

-static void page_pool_clear_pp_info(struct page *page) +static void page_pool_clear_pp_info(netmem_ref netmem) {

•   struct page *page = netmem_to_page(netmem);
  

•   page->pp_magic = 0;
    page->pp = NULL;
  

} @@ -479,34 +481,34 @@ static struct page *__page_pool_alloc_page_order(struct page_pool *pool, return NULL;

    if ((pool->p.flags & PP_FLAG_DMA_MAP) &&

•       unlikely(!page_pool_dma_map(pool, page))) {
  

•       unlikely(!page_pool_dma_map(pool, page_to_netmem(page)))) {
            put_page(page);
            return NULL;
    }
  
    alloc_stat_inc(pool, slow_high_order);
  

•   page_pool_set_pp_info(pool, page);
  

•   page_pool_set_pp_info(pool, page_to_netmem(page));
  
    /* Track how many pages are held 'in-flight' */
    pool->pages_state_hold_cnt++;
  

•   trace_page_pool_state_hold(pool, page, pool->pages_state_hold_cnt);
  

•   trace_page_pool_state_hold(pool, page_to_netmem(page),
  

•                              pool->pages_state_hold_cnt);
    return page;
  

}

/* slow path */ -noinline -static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,

•                                            gfp_t gfp)
  

+static noinline netmem_ref __page_pool_alloc_pages_slow(struct page_pool *pool,

•                                                   gfp_t gfp)
  

{ const int bulk = PP_ALLOC_CACHE_REFILL; unsigned int pp_flags = pool->p.flags; unsigned int pp_order = pool->p.order;

•   struct page *page;
  

•   netmem_ref netmem;
    int i, nr_pages;
  
    /* Don't support bulk alloc for high-order pages */
    if (unlikely(pp_order))
  

•           return __page_pool_alloc_page_order(pool, gfp);
  

•           return page_to_netmem(__page_pool_alloc_page_order(pool, gfp));
  
    /* Unnecessary as alloc cache is empty, but guarantees zero count */
    if (unlikely(pool->alloc.count > 0))
  

@@ -515,60 +517,67 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool, /* Mark empty alloc.cache slots "empty" for alloc_pages_bulk_array */ memset(&pool->alloc.cache, 0, sizeof(void *) * bulk);

•   nr_pages = alloc_pages_bulk_array_node(gfp, pool->p.nid, bulk,
  

•                                          pool->alloc.cache);
  

•   nr_pages = alloc_pages_bulk_array_node(gfp,
  

•                                          pool->p.nid, bulk,
  

•                                          (struct page **)pool->alloc.cache);
    if (unlikely(!nr_pages))
  

•           return NULL;
  

•           return 0;
  
    /* Pages have been filled into alloc.cache array, but count is zero and
     * page element have not been (possibly) DMA mapped.
     */
    for (i = 0; i < nr_pages; i++) {
  

•           page = pool->alloc.cache[i];
  

•           netmem = pool->alloc.cache[i];
            if ((pp_flags & PP_FLAG_DMA_MAP) &&
  

•               unlikely(!page_pool_dma_map(pool, page))) {
  

•                   put_page(page);
  

•               unlikely(!page_pool_dma_map(pool, netmem))) {
  

•                   put_page(netmem_to_page(netmem));
                    continue;
            }
  
  

•           page_pool_set_pp_info(pool, page);
  

•           pool->alloc.cache[pool->alloc.count++] = page;
  

•           page_pool_set_pp_info(pool, netmem);
  

•           pool->alloc.cache[pool->alloc.count++] = netmem;
            /* Track how many pages are held 'in-flight' */
            pool->pages_state_hold_cnt++;
  

•           trace_page_pool_state_hold(pool, page,
  

•           trace_page_pool_state_hold(pool, netmem,
                                       pool->pages_state_hold_cnt);
    }
  
    /* Return last page */
    if (likely(pool->alloc.count > 0)) {
  

•           page = pool->alloc.cache[--pool->alloc.count];
  

•           netmem = pool->alloc.cache[--pool->alloc.count];
            alloc_stat_inc(pool, slow);
    } else {
  

•           page = NULL;
  

•           netmem = 0;
    }
  
    /* When page just alloc'ed is should/must have refcnt 1. */
  

•   return page;
  

•   return netmem;
  

}

/* For using page_pool replace: alloc_pages() API calls, but provide

• synchronization guarantee for allocation side.

*/ -struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp) +netmem_ref page_pool_alloc_netmem(struct page_pool *pool, gfp_t gfp) {

•   struct page *page;
  

•   netmem_ref netmem;
  
    /* Fast-path: Get a page from cache */
  

•   page = __page_pool_get_cached(pool);
  

•   if (page)
  

•           return page;
  

•   netmem = __page_pool_get_cached(pool);
  

•   if (netmem)
  

•           return netmem;
  
    /* Slow-path: cache empty, do real allocation */
    if (static_branch_unlikely(&page_pool_mem_providers) && pool->mp_ops)
  

•           page = pool->mp_ops->alloc_pages(pool, gfp);
  

•           netmem = pool->mp_ops->alloc_pages(pool, gfp);
    else
  

•           page = __page_pool_alloc_pages_slow(pool, gfp);
  

•   return page;
  

•           netmem = __page_pool_alloc_pages_slow(pool, gfp);
  

•   return netmem;
  

+} +EXPORT_SYMBOL(page_pool_alloc_netmem);

+struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp) +{

•   return netmem_to_page(page_pool_alloc_netmem(pool, gfp));
  

} EXPORT_SYMBOL(page_pool_alloc_pages);

@@ -596,8 +605,8 @@ s32 page_pool_inflight(const struct page_pool *pool, bool strict) return inflight; }

-static __always_inline -void __page_pool_release_page_dma(struct page_pool *pool, struct page *page) +static __always_inline void __page_pool_release_page_dma(struct page_pool *pool,

•                                                    netmem_ref netmem)
  

{ dma_addr_t dma;

@@ -607,13 +616,13 @@ void __page_pool_release_page_dma(struct page_pool *pool, struct page *page) */ return;

•   dma = page_pool_get_dma_addr(page);
  

•   dma = page_pool_get_dma_addr_netmem(netmem);
  
    /* When page is unmapped, it cannot be returned to our pool */
    dma_unmap_page_attrs(pool->p.dev, dma,
                         PAGE_SIZE << pool->p.order, pool->p.dma_dir,
                         DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING);
  

•   page_pool_set_dma_addr(page, 0);
  

•   page_pool_set_dma_addr_netmem(netmem, 0);
  

}

/* Disconnects a page (from a page_pool). API users can have a need @@ -621,26 +630,26 @@ void __page_pool_release_page_dma(struct page_pool *pool, struct page *page)

• a regular page (that will eventually be returned to the normal
• page-allocator via put_page).

*/ -void page_pool_return_page(struct page_pool *pool, struct page *page) +void page_pool_return_page(struct page_pool *pool, netmem_ref netmem) { int count; bool put;

    put = true;
    if (static_branch_unlikely(&page_pool_mem_providers) && pool->mp_ops)

•           put = pool->mp_ops->release_page(pool, page);
  

•           put = pool->mp_ops->release_page(pool, netmem);
    else
  

•           __page_pool_release_page_dma(pool, page);
  

•           __page_pool_release_page_dma(pool, netmem);
  
    /* This may be the last page returned, releasing the pool, so
     * it is not safe to reference pool afterwards.
     */
    count = atomic_inc_return_relaxed(&pool->pages_state_release_cnt);
  

•   trace_page_pool_state_release(pool, page, count);
  

•   trace_page_pool_state_release(pool, netmem, count);
  
    if (put) {
  

•           page_pool_clear_pp_info(page);
  

•           put_page(page);
  

•           page_pool_clear_pp_info(netmem);
  

•           put_page(netmem_to_page(netmem));
    }
    /* An optimization would be to call __free_pages(page, pool->p.order)
     * knowing page is not part of page-cache (thus avoiding a
  

@@ -648,14 +657,14 @@ void page_pool_return_page(struct page_pool *pool, struct page *page) */ }

-static bool page_pool_recycle_in_ring(struct page_pool *pool, struct page *page) +static bool page_pool_recycle_in_ring(struct page_pool *pool, netmem_ref netmem) { int ret; /* BH protection not needed if current is softirq */ if (in_softirq())

•           ret = ptr_ring_produce(&pool->ring, page);
  

•           ret = ptr_ring_produce(&pool->ring, (__force void *)netmem);
    else
  

•           ret = ptr_ring_produce_bh(&pool->ring, page);
  

•           ret = ptr_ring_produce_bh(&pool->ring, (__force void *)netmem);
  
    if (!ret) {
            recycle_stat_inc(pool, ring);
  

@@ -670,7 +679,7 @@ static bool page_pool_recycle_in_ring(struct page_pool *pool, struct page *page)

• Caller must provide appropriate safe context.

*/ -static bool page_pool_recycle_in_cache(struct page *page, +static bool page_pool_recycle_in_cache(netmem_ref netmem, struct page_pool *pool) { if (unlikely(pool->alloc.count == PP_ALLOC_CACHE_SIZE)) { @@ -679,14 +688,15 @@ static bool page_pool_recycle_in_cache(struct page *page, }

    /* Caller MUST have verified/know (page_ref_count(page) == 1) */

•   pool->alloc.cache[pool->alloc.count++] = page;
  

•   pool->alloc.cache[pool->alloc.count++] = netmem;
    recycle_stat_inc(pool, cached);
    return true;
  

}

-static bool __page_pool_page_can_be_recycled(const struct page *page) +static bool __page_pool_page_can_be_recycled(netmem_ref netmem) {

•   return page_ref_count(page) == 1 && !page_is_pfmemalloc(page);
  

•   return page_ref_count(netmem_to_page(netmem)) == 1 &&
  

•          !page_is_pfmemalloc(netmem_to_page(netmem));
  

}

/* If the page refcnt == 1, this will try to recycle the page. @@ -695,8 +705,8 @@ static bool __page_pool_page_can_be_recycled(const struct page *page)

• If the page refcnt != 1, then the page will be returned to memory
• subsystem.

*/ -static __always_inline struct page * -__page_pool_put_page(struct page_pool *pool, struct page *page, +static __always_inline netmem_ref +__page_pool_put_page(struct page_pool *pool, netmem_ref netmem, unsigned int dma_sync_size, bool allow_direct) { lockdep_assert_no_hardirq(); @@ -710,19 +720,19 @@ __page_pool_put_page(struct page_pool *pool, struct page *page, * page is NOT reusable when allocated when system is under * some pressure. (page_is_pfmemalloc) */

•   if (likely(__page_pool_page_can_be_recycled(page))) {
  

•   if (likely(__page_pool_page_can_be_recycled(netmem))) {
            /* Read barrier done in page_ref_count / READ_ONCE */
  
            if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
  

•                   page_pool_dma_sync_for_device(pool, page,
  

•                   page_pool_dma_sync_for_device(pool, netmem,
                                                  dma_sync_size);
  
            if (allow_direct && in_softirq() &&
  

•               page_pool_recycle_in_cache(page, pool))
  

•                   return NULL;
  

•               page_pool_recycle_in_cache(netmem, pool))
  

•                   return 0;
  
            /* Page found as candidate for recycling */
  

•           return page;
  

•           return netmem;
    }
    /* Fallback/non-XDP mode: API user have elevated refcnt.
     *
  

@@ -738,21 +748,30 @@ __page_pool_put_page(struct page_pool *pool, struct page *page, * will be invoking put_page. */ recycle_stat_inc(pool, released_refcnt);

•   page_pool_return_page(pool, page);
  

•   page_pool_return_page(pool, netmem);
  
  

•   return NULL;
  

•   return 0;
  

}

-void page_pool_put_unrefed_page(struct page_pool *pool, struct page *page,

•                           unsigned int dma_sync_size, bool allow_direct)
  

+void page_pool_put_unrefed_netmem(struct page_pool *pool, netmem_ref netmem,

•                             unsigned int dma_sync_size, bool allow_direct)
  

{

•   page = __page_pool_put_page(pool, page, dma_sync_size, allow_direct);
  

•   if (page && !page_pool_recycle_in_ring(pool, page)) {
  

•   netmem =
  

•           __page_pool_put_page(pool, netmem, dma_sync_size, allow_direct);
  

•   if (netmem && !page_pool_recycle_in_ring(pool, netmem)) {
            /* Cache full, fallback to free pages */
            recycle_stat_inc(pool, ring_full);
  

•           page_pool_return_page(pool, page);
  

•           page_pool_return_page(pool, netmem);
    }
  

} +EXPORT_SYMBOL(page_pool_put_unrefed_netmem);

+void page_pool_put_unrefed_page(struct page_pool *pool, struct page *page,

•                           unsigned int dma_sync_size, bool allow_direct)
  

+{

•   page_pool_put_unrefed_netmem(pool, page_to_netmem(page), dma_sync_size,
  

•                                allow_direct);
  

+} EXPORT_SYMBOL(page_pool_put_unrefed_page);

/** @@ -777,16 +796,16 @@ void page_pool_put_page_bulk(struct page_pool *pool, void **data, bool in_softirq;

    for (i = 0; i < count; i++) {

•           struct page *page = virt_to_head_page(data[i]);
  

•           netmem_ref netmem = page_to_netmem(virt_to_head_page(data[i]));
  
            /* It is not the last user for the page frag case */
  

•           if (!page_pool_is_last_ref(page))
  

•           if (!page_pool_is_last_ref(netmem))
                    continue;
  
  

•           page = __page_pool_put_page(pool, page, -1, false);
  

•           netmem = __page_pool_put_page(pool, netmem, -1, false);
            /* Approved for bulk recycling in ptr_ring cache */
  

•           if (page)
  

•                   data[bulk_len++] = page;
  

•           if (netmem)
  

•                   data[bulk_len++] = (__force void *)netmem;
    }
  
    if (unlikely(!bulk_len))
  

@@ -812,100 +831,108 @@ void page_pool_put_page_bulk(struct page_pool *pool, void **data, * since put_page() with refcnt == 1 can be an expensive operation */ for (; i < bulk_len; i++)

•           page_pool_return_page(pool, data[i]);
  

•           page_pool_return_page(pool, (__force netmem_ref)data[i]);
  

} EXPORT_SYMBOL(page_pool_put_page_bulk);

-static struct page *page_pool_drain_frag(struct page_pool *pool,

•                                    struct page *page)
  

+static netmem_ref page_pool_drain_frag(struct page_pool *pool,

•                                  netmem_ref netmem)
  

{ long drain_count = BIAS_MAX - pool->frag_users;

    /* Some user is still using the page frag */

•   if (likely(page_pool_unref_page(page, drain_count)))
  

•           return NULL;
  

•   if (likely(page_pool_unref_netmem(netmem, drain_count)))
  

•           return 0;
  
  

•   if (__page_pool_page_can_be_recycled(page)) {
  

•   if (__page_pool_page_can_be_recycled(netmem)) {
            if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
  

•                   page_pool_dma_sync_for_device(pool, page, -1);
  

•                   page_pool_dma_sync_for_device(pool, netmem, -1);
  
  

•           return page;
  

•           return netmem;
    }
  
  

•   page_pool_return_page(pool, page);
  

•   return NULL;
  

•   page_pool_return_page(pool, netmem);
  

•   return 0;
  

}

static void page_pool_free_frag(struct page_pool *pool) { long drain_count = BIAS_MAX - pool->frag_users;

•   struct page *page = pool->frag_page;
  

•   netmem_ref netmem = pool->frag_page;
  
  

•   pool->frag_page = NULL;
  

•   pool->frag_page = 0;
  
  

•   if (!page || page_pool_unref_page(page, drain_count))
  

•   if (!netmem || page_pool_unref_netmem(netmem, drain_count))
            return;
  
  

•   page_pool_return_page(pool, page);
  

•   page_pool_return_page(pool, netmem);
  

}

-struct page *page_pool_alloc_frag(struct page_pool *pool,

•                             unsigned int *offset,
  

•                             unsigned int size, gfp_t gfp)
  

+netmem_ref page_pool_alloc_frag_netmem(struct page_pool *pool,

•                                  unsigned int *offset, unsigned int size,
  

•                                  gfp_t gfp)
  

{ unsigned int max_size = PAGE_SIZE << pool->p.order;

•   struct page *page = pool->frag_page;
  

•   netmem_ref netmem = pool->frag_page;
  
    if (WARN_ON(size > max_size))
  

•           return NULL;
  

•           return 0;
  
    size = ALIGN(size, dma_get_cache_alignment());
    *offset = pool->frag_offset;
  
  

•   if (page && *offset + size > max_size) {
  

•           page = page_pool_drain_frag(pool, page);
  

•           if (page) {
  

•   if (netmem && *offset + size > max_size) {
  

•           netmem = page_pool_drain_frag(pool, netmem);
  

•           if (netmem) {
                    alloc_stat_inc(pool, fast);
                    goto frag_reset;
            }
    }
  
  

•   if (!page) {
  

•           page = page_pool_alloc_pages(pool, gfp);
  

•           if (unlikely(!page)) {
  

•                   pool->frag_page = NULL;
  

•                   return NULL;
  

•   if (!netmem) {
  

•           netmem = page_pool_alloc_netmem(pool, gfp);
  

•           if (unlikely(!netmem)) {
  

•                   pool->frag_page = 0;
  

•                   return 0;
            }
  
  

•           pool->frag_page = page;
  

•           pool->frag_page = netmem;
  
  

frag_reset: pool->frag_users = 1; *offset = 0; pool->frag_offset = size;

•           page_pool_fragment_page(page, BIAS_MAX);
  

•           return page;
  

•           page_pool_fragment_netmem(netmem, BIAS_MAX);
  

•           return netmem;
    }
  
    pool->frag_users++;
    pool->frag_offset = *offset + size;
    alloc_stat_inc(pool, fast);
  

•   return page;
  

•   return netmem;
  

+} +EXPORT_SYMBOL(page_pool_alloc_frag_netmem);

+struct page *page_pool_alloc_frag(struct page_pool *pool, unsigned int *offset,

•                             unsigned int size, gfp_t gfp)
  

+{

•   return netmem_to_page(page_pool_alloc_frag_netmem(pool, offset, size,
  

•                                                     gfp));
  

} EXPORT_SYMBOL(page_pool_alloc_frag);

static void page_pool_empty_ring(struct page_pool *pool) {

•   struct page *page;
  

•   netmem_ref netmem;
  
    /* Empty recycle ring */
  

•   while ((page = ptr_ring_consume_bh(&pool->ring))) {
  

•   while ((netmem = (__force netmem_ref)ptr_ring_consume_bh(&pool->ring))) {
            /* Verify the refcnt invariant of cached pages */
  

•           if (!(page_ref_count(page) == 1))
  

•           if (!(page_ref_count(netmem_to_page(netmem)) == 1))
                    pr_crit("%s() page_pool refcnt %d violation\n",
  

•                           __func__, page_ref_count(page));
  

•                           __func__, netmem_ref_count(netmem));
  
  

•           page_pool_return_page(pool, page);
  

•           page_pool_return_page(pool, netmem);
    }
  

}

@@ -927,7 +954,7 @@ static void __page_pool_destroy(struct page_pool *pool)

static void page_pool_empty_alloc_cache_once(struct page_pool *pool) {

•   struct page *page;
  

•   netmem_ref netmem;
  
    if (pool->destroy_cnt)
            return;
  

@@ -937,8 +964,8 @@ static void page_pool_empty_alloc_cache_once(struct page_pool *pool) * call concurrently. */ while (pool->alloc.count) {

•           page = pool->alloc.cache[--pool->alloc.count];
  

•           page_pool_return_page(pool, page);
  

•           netmem = pool->alloc.cache[--pool->alloc.count];
  

•           page_pool_return_page(pool, netmem);
    }
  

}

@@ -1044,15 +1071,15 @@ EXPORT_SYMBOL(page_pool_destroy); /* Caller must provide appropriate safe context, e.g. NAPI. */ void page_pool_update_nid(struct page_pool *pool, int new_nid) {

•   struct page *page;
  

•   netmem_ref netmem;
  
    trace_page_pool_update_nid(pool, new_nid);
    pool->p.nid = new_nid;
  
    /* Flush pool alloc cache, as refill will check NUMA node */
    while (pool->alloc.count) {
  

•           page = pool->alloc.cache[--pool->alloc.count];
  

•           page_pool_return_page(pool, page);
  

•           netmem = pool->alloc.cache[--pool->alloc.count];
  

•           page_pool_return_page(pool, netmem);
    }
  

} EXPORT_SYMBOL(page_pool_update_nid); diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 1f918e602bc4..e1118b637085 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -1006,8 +1006,9 @@ int skb_cow_data_for_xdp(struct page_pool *pool, struct sk_buff **pskb, EXPORT_SYMBOL(skb_cow_data_for_xdp);

#if IS_ENABLED(CONFIG_PAGE_POOL) -bool napi_pp_put_page(struct page *page, bool napi_safe) +bool napi_pp_put_page(netmem_ref netmem, bool napi_safe) {

•   struct page *page = netmem_to_page(netmem);
    bool allow_direct = false;
    struct page_pool *pp;
  
  

@@ -1044,7 +1045,7 @@ bool napi_pp_put_page(struct page *page, bool napi_safe) * The page will be returned to the pool here regardless of the * 'flipped' fragment being in use or not. */

•   page_pool_put_full_page(pp, page, allow_direct);
  

•   page_pool_put_full_netmem(pp, page_to_netmem(page), allow_direct);
  
    return true;
  

} @@ -1055,7 +1056,7 @@ static bool skb_pp_recycle(struct sk_buff *skb, void *data, bool napi_safe) { if (!IS_ENABLED(CONFIG_PAGE_POOL) || !skb->pp_recycle) return false;

•   return napi_pp_put_page(virt_to_page(data), napi_safe);
  

•   return napi_pp_put_page(page_to_netmem(virt_to_page(data)), napi_safe);
  

}

/**

2.44.0.rc1.240.g4c46232300-goog

On Mon, Mar 4, 2024 at 6:02 PM Mina Almasry almasrymina@google.com wrote:

Convert netmem to be a union of struct page and struct netmem. Overload the LSB of struct netmem* to indicate that it's a net_iov, otherwise it's a page.

Currently these entries in struct page are rented by the page_pool and used exclusively by the net stack:

struct { unsigned long pp_magic; struct page_pool *pp; unsigned long _pp_mapping_pad; unsigned long dma_addr; atomic_long_t pp_ref_count; };

Mirror these (and only these) entries into struct net_iov and implement netmem helpers that can access these common fields regardless of whether the underlying type is page or net_iov.

Implement checks for net_iov in netmem helpers which delegate to mm APIs, to ensure net_iov are never passed to the mm stack.

Signed-off-by: Mina Almasry almasrymina@google.com

Per David Howell's request, I'm forwarding these 2 patches from the series to linux-mm & Mathew Wilcox.

https://lore.kernel.org/netdev/950858.1709622997@warthog.procyon.org.uk/

+linux-mm +Mathew Wilcox +David Howells

Some background on how we arrived at the current design may be useful. This was largely discussed across 2 (long) email threads, largely with Jason Gunthorpe and Christian Konig:

https://lore.kernel.org/lkml/CAHS8izPm6XRS54LdCDZVd0C75tA1zHSu6jLVO8nzTLXCc=...

https://lore.kernel.org/bpf/CAHS8izMdKYyjE9bdcFDWWPWECwVZL7XQjtjOFoTq5_bEEJv...

The approach is based on supporting dma-buf scatterlist entries in the networking stack, to support receiving data directly into GPU memory typically represented by dmabuf. I added the abstract netmem_ref type in a recent series that can multiplex between struct pages and non-struct pages (called net_iovs in this series):

https://lore.kernel.org/netdev/20240214223405.1972973-1-almasrymina@google.c...

---

v6:

• Rebased on top of the merged netmem_ref type.
• Rebased on top of the merged skb_pp_frag_ref() changes.

v5:

• Use netmem instead of page* with LSB set.
• Use pp_ref_count for refcounting net_iov.
• Removed many of the custom checks for netmem.

v1:

• Disable fragmentation support for iov properly.
• fix napi_pp_put_page() path (Yunsheng).
• Use pp_frag_count for devmem refcounting.

---

include/net/netmem.h | 142 ++++++++++++++++++++++++++++++-- include/net/page_pool/helpers.h | 25 +++--- include/net/page_pool/types.h | 1 + net/core/page_pool.c | 26 +++--- net/core/skbuff.c | 23 +++--- 5 files changed, 171 insertions(+), 46 deletions(-)

diff --git a/include/net/netmem.h b/include/net/netmem.h index 21f53b29e5fe..8699788d587d 100644 --- a/include/net/netmem.h +++ b/include/net/netmem.h @@ -9,14 +9,51 @@ #define _NET_NETMEM_H

#include <net/devmem.h> +#include <net/net_debug.h>

/* net_iov */

+DECLARE_STATIC_KEY_FALSE(page_pool_mem_providers);

+/* We overload the LSB of the struct page pointer to indicate whether it's

• • a page or net_iov.
• */

+#define NET_IOV 0x01UL

struct net_iov {

•   unsigned long __unused_padding;
  

•   unsigned long pp_magic;
  

•   struct page_pool *pp;
    struct dmabuf_genpool_chunk_owner *owner;
    unsigned long dma_addr;
  

•   atomic_long_t pp_ref_count;
  

};

+/* These fields in struct page are used by the page_pool and net stack:

• • struct {
    

• •         unsigned long pp_magic;
    

• •         struct page_pool *pp;
    

• •         unsigned long _pp_mapping_pad;
    

• •         unsigned long dma_addr;
    

• •         atomic_long_t pp_ref_count;
    

• • };
    

• • We mirror the page_pool fields here so the page_pool can access these fields
• • without worrying whether the underlying fields belong to a page or net_iov.
• • The non-net stack fields of struct page are private to the mm stack and must
• • never be mirrored to net_iov.
• */

+#define NET_IOV_ASSERT_OFFSET(pg, iov) \

•   static_assert(offsetof(struct page, pg) == \
  

•                 offsetof(struct net_iov, iov))
  

+NET_IOV_ASSERT_OFFSET(pp_magic, pp_magic); +NET_IOV_ASSERT_OFFSET(pp, pp); +NET_IOV_ASSERT_OFFSET(dma_addr, dma_addr); +NET_IOV_ASSERT_OFFSET(pp_ref_count, pp_ref_count); +#undef NET_IOV_ASSERT_OFFSET

static inline struct dmabuf_genpool_chunk_owner * net_iov_owner(const struct net_iov *niov) { @@ -69,20 +106,27 @@ net_iov_binding(const struct net_iov *niov) */ typedef unsigned long __bitwise netmem_ref;

+static inline bool netmem_is_net_iov(const netmem_ref netmem) +{ +#ifdef CONFIG_PAGE_POOL

•   return static_branch_unlikely(&page_pool_mem_providers) &&
  

•          (__force unsigned long)netmem & NET_IOV;
  

+#else

•   return false;
  

+#endif +}

/* This conversion fails (returns NULL) if the netmem_ref is not struct page

• backed.

• • Currently struct page is the only possible netmem, and this helper never
• • fails.
  */

static inline struct page *netmem_to_page(netmem_ref netmem) {

•   if (WARN_ON_ONCE(netmem_is_net_iov(netmem)))
  

•           return NULL;
  

•   return (__force struct page *)netmem;
  

}

-/* Converting from page to netmem is always safe, because a page can always be

• • a netmem.
• */

static inline netmem_ref page_to_netmem(struct page *page) { return (__force netmem_ref)page; @@ -90,17 +134,103 @@ static inline netmem_ref page_to_netmem(struct page *page)

static inline int netmem_ref_count(netmem_ref netmem) {

•   /* The non-pp refcount of net_iov is always 1. On net_iov, we only
  

•    * support pp refcounting which uses the pp_ref_count field.
  

•    */
  

•   if (netmem_is_net_iov(netmem))
  

•           return 1;
  

•   return page_ref_count(netmem_to_page(netmem));
  

}

static inline unsigned long netmem_to_pfn(netmem_ref netmem) {

•   if (netmem_is_net_iov(netmem))
  

•           return 0;
  

•   return page_to_pfn(netmem_to_page(netmem));
  

}

+static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem) +{

•   return (struct net_iov *)((__force unsigned long)netmem & ~NET_IOV);
  

+}

+static inline unsigned long netmem_get_pp_magic(netmem_ref netmem) +{

•   return __netmem_clear_lsb(netmem)->pp_magic;
  

+}

+static inline void netmem_or_pp_magic(netmem_ref netmem, unsigned long pp_magic) +{

•   __netmem_clear_lsb(netmem)->pp_magic |= pp_magic;
  

+}

+static inline void netmem_clear_pp_magic(netmem_ref netmem) +{

•   __netmem_clear_lsb(netmem)->pp_magic = 0;
  

+}

+static inline struct page_pool *netmem_get_pp(netmem_ref netmem) +{

•   return __netmem_clear_lsb(netmem)->pp;
  

+}

+static inline void netmem_set_pp(netmem_ref netmem, struct page_pool *pool) +{

•   __netmem_clear_lsb(netmem)->pp = pool;
  

+}

+static inline unsigned long netmem_get_dma_addr(netmem_ref netmem) +{

•   return __netmem_clear_lsb(netmem)->dma_addr;
  

+}

+static inline void netmem_set_dma_addr(netmem_ref netmem,

•                                  unsigned long dma_addr)
  

+{

•   __netmem_clear_lsb(netmem)->dma_addr = dma_addr;
  

+}

+static inline atomic_long_t *netmem_get_pp_ref_count_ref(netmem_ref netmem) +{

•   return &__netmem_clear_lsb(netmem)->pp_ref_count;
  

+}

+static inline bool netmem_is_pref_nid(netmem_ref netmem, int pref_nid) +{

•   /* Assume net_iov are on the preferred node without actually
  

•    * checking...
  

•    *
  

•    * This check is only used to check for recycling memory in the page
  

•    * pool's fast paths. Currently the only implementation of net_iov
  

•    * is dmabuf device memory. It's a deliberate decision by the user to
  

•    * bind a certain dmabuf to a certain netdev, and the netdev rx queue
  

•    * would not be able to reallocate memory from another dmabuf that
  

•    * exists on the preferred node, so, this check doesn't make much sense
  

•    * in this case. Assume all net_iovs can be recycled for now.
  

•    */
  

•   if (netmem_is_net_iov(netmem))
  

•           return true;
  

•   return page_to_nid(netmem_to_page(netmem)) == pref_nid;
  

+}

static inline netmem_ref netmem_compound_head(netmem_ref netmem) {

•   /* niov are never compounded */
  

•   if (netmem_is_net_iov(netmem))
  

•           return netmem;
  

•   return page_to_netmem(compound_head(netmem_to_page(netmem)));
  

}

+static inline void *netmem_address(netmem_ref netmem) +{

•   if (netmem_is_net_iov(netmem))
  

•           return NULL;
  

•   return page_address(netmem_to_page(netmem));
  

+}

#endif /* _NET_NETMEM_H */ diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h index 61814f91a458..c6a55eddefae 100644 --- a/include/net/page_pool/helpers.h +++ b/include/net/page_pool/helpers.h @@ -215,7 +215,7 @@ inline enum dma_data_direction page_pool_get_dma_dir(struct page_pool *pool)

static inline void page_pool_fragment_netmem(netmem_ref netmem, long nr) {

•   atomic_long_set(&netmem_to_page(netmem)->pp_ref_count, nr);
  

•   atomic_long_set(netmem_get_pp_ref_count_ref(netmem), nr);
  

}

/** @@ -243,7 +243,7 @@ static inline void page_pool_fragment_page(struct page *page, long nr)

static inline long page_pool_unref_netmem(netmem_ref netmem, long nr) {

•   struct page *page = netmem_to_page(netmem);
  

•   atomic_long_t *pp_ref_count = netmem_get_pp_ref_count_ref(netmem);
    long ret;
  
    /* If nr == pp_ref_count then we have cleared all remaining
  

@@ -260,19 +260,19 @@ static inline long page_pool_unref_netmem(netmem_ref netmem, long nr) * initially, and only overwrite it when the page is partitioned into * more than one piece. */

•   if (atomic_long_read(&page->pp_ref_count) == nr) {
  

•   if (atomic_long_read(pp_ref_count) == nr) {
            /* As we have ensured nr is always one for constant case using
             * the BUILD_BUG_ON(), only need to handle the non-constant case
             * here for pp_ref_count draining, which is a rare case.
             */
            BUILD_BUG_ON(__builtin_constant_p(nr) && nr != 1);
            if (!__builtin_constant_p(nr))
  

•                   atomic_long_set(&page->pp_ref_count, 1);
  

•                   atomic_long_set(pp_ref_count, 1);
  
            return 0;
    }
  
  

•   ret = atomic_long_sub_return(nr, &page->pp_ref_count);
  

•   ret = atomic_long_sub_return(nr, pp_ref_count);
    WARN_ON(ret < 0);
  
    /* We are the last user here too, reset pp_ref_count back to 1 to
  

@@ -281,7 +281,7 @@ static inline long page_pool_unref_netmem(netmem_ref netmem, long nr) * page_pool_unref_page() currently. */ if (unlikely(!ret))

•           atomic_long_set(&page->pp_ref_count, 1);
  

•           atomic_long_set(pp_ref_count, 1);
  
    return ret;
  

} @@ -400,9 +400,7 @@ static inline void page_pool_free_va(struct page_pool *pool, void *va,

static inline dma_addr_t page_pool_get_dma_addr_netmem(netmem_ref netmem) {

•   struct page *page = netmem_to_page(netmem);
  

•   dma_addr_t ret = page->dma_addr;
  

•   dma_addr_t ret = netmem_get_dma_addr(netmem);
  
    if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA)
            ret <<= PAGE_SHIFT;
  

@@ -425,18 +423,17 @@ static inline dma_addr_t page_pool_get_dma_addr(struct page *page) static inline bool page_pool_set_dma_addr_netmem(netmem_ref netmem, dma_addr_t addr) {

•   struct page *page = netmem_to_page(netmem);
  

•   if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA) {
  

•           page->dma_addr = addr >> PAGE_SHIFT;
  

•           netmem_set_dma_addr(netmem, addr >> PAGE_SHIFT);
  
            /* We assume page alignment to shave off bottom bits,
             * if this "compression" doesn't work we need to drop.
             */
  

•           return addr != (dma_addr_t)page->dma_addr << PAGE_SHIFT;
  

•           return addr != (dma_addr_t)netmem_get_dma_addr(netmem)
  

•                                  << PAGE_SHIFT;
    }
  
  

•   page->dma_addr = addr;
  

•   netmem_set_dma_addr(netmem, addr);
    return false;
  

}

diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h index 68a24c5ae827..e29e77f7934e 100644 --- a/include/net/page_pool/types.h +++ b/include/net/page_pool/types.h @@ -6,6 +6,7 @@ #include <linux/dma-direction.h> #include <linux/ptr_ring.h> #include <linux/types.h> +#include <net/netmem.h>

#define PP_FLAG_DMA_MAP BIT(0) /* Should page_pool do the DMA * map/unmap diff --git a/net/core/page_pool.c b/net/core/page_pool.c index 24d5236b2efc..22e3d439da18 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c @@ -25,7 +25,7 @@

#include "page_pool_priv.h"

-static DEFINE_STATIC_KEY_FALSE(page_pool_mem_providers); +DEFINE_STATIC_KEY_FALSE(page_pool_mem_providers);

#define DEFER_TIME (msecs_to_jiffies(1000)) #define DEFER_WARN_INTERVAL (60 * HZ) @@ -359,7 +359,7 @@ static noinline netmem_ref page_pool_refill_alloc_cache(struct page_pool *pool) if (unlikely(!netmem)) break;

•           if (likely(page_to_nid(netmem_to_page(netmem)) == pref_nid)) {
  

•           if (likely(netmem_is_pref_nid(netmem, pref_nid))) {
                    pool->alloc.cache[pool->alloc.count++] = netmem;
            } else {
                    /* NUMA mismatch;
  

@@ -446,10 +446,8 @@ static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem)

static void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem) {

•   struct page *page = netmem_to_page(netmem);
  

•   page->pp = pool;
  

•   page->pp_magic |= PP_SIGNATURE;
  

•   netmem_set_pp(netmem, pool);
  

•   netmem_or_pp_magic(netmem, PP_SIGNATURE);
  
    /* Ensuring all pages have been split into one fragment initially:
     * page_pool_set_pp_info() is only called once for every page when it
  

@@ -464,10 +462,8 @@ static void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem)

static void page_pool_clear_pp_info(netmem_ref netmem) {

•   struct page *page = netmem_to_page(netmem);
  

•   page->pp_magic = 0;
  

•   page->pp = NULL;
  

•   netmem_clear_pp_magic(netmem);
  

•   netmem_set_pp(netmem, NULL);
  

}

static struct page *__page_pool_alloc_page_order(struct page_pool *pool, @@ -695,8 +691,9 @@ static bool page_pool_recycle_in_cache(netmem_ref netmem,

static bool __page_pool_page_can_be_recycled(netmem_ref netmem) {

•   return page_ref_count(netmem_to_page(netmem)) == 1 &&
  

•          !page_is_pfmemalloc(netmem_to_page(netmem));
  

•   return netmem_is_net_iov(netmem) ||
  

•          (page_ref_count(netmem_to_page(netmem)) == 1 &&
  

•           !page_is_pfmemalloc(netmem_to_page(netmem)));
  

}

/* If the page refcnt == 1, this will try to recycle the page. @@ -718,7 +715,7 @@ __page_pool_put_page(struct page_pool *pool, netmem_ref netmem, * refcnt == 1 means page_pool owns page, and can recycle it. * * page is NOT reusable when allocated when system is under

•    * some pressure. (page_is_pfmemalloc)
  

•    * some pressure. (page_pool_page_is_pfmemalloc)
     */
    if (likely(__page_pool_page_can_be_recycled(netmem))) {
            /* Read barrier done in page_ref_count / READ_ONCE */
  

@@ -734,6 +731,7 @@ __page_pool_put_page(struct page_pool *pool, netmem_ref netmem, /* Page found as candidate for recycling */ return netmem; }

•   /* Fallback/non-XDP mode: API user have elevated refcnt.
     *
     * Many drivers split up the page into fragments, and some
  

@@ -928,7 +926,7 @@ static void page_pool_empty_ring(struct page_pool *pool) /* Empty recycle ring */ while ((netmem = (__force netmem_ref)ptr_ring_consume_bh(&pool->ring))) { /* Verify the refcnt invariant of cached pages */

•           if (!(page_ref_count(netmem_to_page(netmem)) == 1))
  

•           if (!(netmem_ref_count(netmem) == 1))
                    pr_crit("%s() page_pool refcnt %d violation\n",
                            __func__, netmem_ref_count(netmem));
  
  

diff --git a/net/core/skbuff.c b/net/core/skbuff.c index e1118b637085..cf23392e97f5 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -908,9 +908,9 @@ static void skb_clone_fraglist(struct sk_buff *skb) skb_get(list); }

-static bool is_pp_page(struct page *page) +static bool is_pp_netmem(netmem_ref netmem) {

•   return (page->pp_magic & ~0x3UL) == PP_SIGNATURE;
  

•   return (netmem_get_pp_magic(netmem) & ~0x3UL) == PP_SIGNATURE;
  

}

int skb_pp_cow_data(struct page_pool *pool, struct sk_buff **pskb, @@ -1008,11 +1008,10 @@ EXPORT_SYMBOL(skb_cow_data_for_xdp); #if IS_ENABLED(CONFIG_PAGE_POOL) bool napi_pp_put_page(netmem_ref netmem, bool napi_safe) {

•   struct page *page = netmem_to_page(netmem);
    bool allow_direct = false;
    struct page_pool *pp;
  
  

•   page = compound_head(page);
  

•   netmem = netmem_compound_head(netmem);
  
    /* page->pp_magic is OR'ed with PP_SIGNATURE after the allocation
     * in order to preserve any existing bits, such as bit 0 for the
  

@@ -1021,10 +1020,10 @@ bool napi_pp_put_page(netmem_ref netmem, bool napi_safe) * and page_is_pfmemalloc() is checked in __page_pool_put_page() * to avoid recycling the pfmemalloc page. */

•   if (unlikely(!is_pp_page(page)))
  

•   if (unlikely(!is_pp_netmem(netmem)))
            return false;
  
  

•   pp = page->pp;
  

•   pp = netmem_get_pp(netmem);
  
    /* Allow direct recycle if we have reasons to believe that we are
     * in the same context as the consumer would run, so there's
  

@@ -1045,7 +1044,7 @@ bool napi_pp_put_page(netmem_ref netmem, bool napi_safe) * The page will be returned to the pool here regardless of the * 'flipped' fragment being in use or not. */

•   page_pool_put_full_netmem(pp, page_to_netmem(page), allow_direct);
  

•   page_pool_put_full_netmem(pp, netmem, allow_direct);
  
    return true;
  

} @@ -1072,7 +1071,7 @@ static bool skb_pp_recycle(struct sk_buff *skb, void *data, bool napi_safe) static int skb_pp_frag_ref(struct sk_buff *skb) { struct skb_shared_info *shinfo;

•   struct page *head_page;
  

•   netmem_ref head_netmem;
    int i;
  
    if (!skb->pp_recycle)
  

@@ -1081,11 +1080,11 @@ static int skb_pp_frag_ref(struct sk_buff *skb) shinfo = skb_shinfo(skb);

    for (i = 0; i < shinfo->nr_frags; i++) {

•           head_page = compound_head(skb_frag_page(&shinfo->frags[i]));
  

•           if (likely(is_pp_page(head_page)))
  

•                   page_pool_ref_page(head_page);
  

•           head_netmem = netmem_compound_head(shinfo->frags[i].netmem);
  

•           if (likely(is_pp_netmem(head_netmem)))
  

•                   page_pool_ref_netmem(head_netmem);
            else
  

•                   page_ref_inc(head_page);
  

•                   page_ref_inc(netmem_to_page(head_netmem));
    }
    return 0;
  

}

2.44.0.rc1.240.g4c46232300-goog