Currently net_iovs support only pp ref counts, and do not support a
page ref equivalent.
This is fine for the RX path as net_iovs are used exclusively with the
pp and only pp refcounting is needed there. The TX path however does not
use pp ref counts, thus, support for get_page/put_page equivalent is
needed for netmem.
Support get_netmem/put_netmem. Check the type of the netmem before
passing it to page or net_iov specific code to obtain a page ref
equivalent.
For dmabuf net_iovs, we obtain a ref on the underlying binding. This
ensures the entire binding doesn't disappear until all the net_iovs have
been put_netmem'ed. We do not need to track the refcount of individual
dmabuf net_iovs as we don't allocate/free them from a pool similar to
what the buddy allocator does for pages.
This code is written to be extensible by other net_iov implementers.
get_netmem/put_netmem will check the type of the netmem and route it to
the correct helper:
pages -> [get|put]_page()
dmabuf net_iovs -> net_devmem_[get|put]_net_iov()
new net_iovs -> new helpers
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
include/linux/skbuff_ref.h | 4 ++--
include/net/netmem.h | 3 +++
net/core/devmem.c | 10 ++++++++++
net/core/devmem.h | 11 +++++++++++
net/core/skbuff.c | 30 ++++++++++++++++++++++++++++++
5 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/include/linux/skbuff_ref.h b/include/linux/skbuff_ref.h
index 0f3c58007488..9e49372ef1a0 100644
--- a/include/linux/skbuff_ref.h
+++ b/include/linux/skbuff_ref.h
@@ -17,7 +17,7 @@
*/
static inline void __skb_frag_ref(skb_frag_t *frag)
{
- get_page(skb_frag_page(frag));
+ get_netmem(skb_frag_netmem(frag));
}
/**
@@ -40,7 +40,7 @@ static inline void skb_page_unref(netmem_ref netmem, bool recycle)
if (recycle && napi_pp_put_page(netmem))
return;
#endif
- put_page(netmem_to_page(netmem));
+ put_netmem(netmem);
}
/**
diff --git a/include/net/netmem.h b/include/net/netmem.h
index 1b58faa4f20f..d30f31878a09 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -245,4 +245,7 @@ static inline unsigned long netmem_get_dma_addr(netmem_ref netmem)
return __netmem_clear_lsb(netmem)->dma_addr;
}
+void get_netmem(netmem_ref netmem);
+void put_netmem(netmem_ref netmem);
+
#endif /* _NET_NETMEM_H */
diff --git a/net/core/devmem.c b/net/core/devmem.c
index 0b6ed7525b22..f7e06a8cba01 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -322,6 +322,16 @@ void dev_dmabuf_uninstall(struct net_device *dev)
}
}
+void net_devmem_get_net_iov(struct net_iov *niov)
+{
+ net_devmem_dmabuf_binding_get(niov->owner->binding);
+}
+
+void net_devmem_put_net_iov(struct net_iov *niov)
+{
+ net_devmem_dmabuf_binding_put(niov->owner->binding);
+}
+
/*** "Dmabuf devmem memory provider" ***/
int mp_dmabuf_devmem_init(struct page_pool *pool)
diff --git a/net/core/devmem.h b/net/core/devmem.h
index 76099ef9c482..54e30fea80b3 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -119,6 +119,9 @@ net_devmem_dmabuf_binding_put(struct net_devmem_dmabuf_binding *binding)
__net_devmem_dmabuf_binding_free(binding);
}
+void net_devmem_get_net_iov(struct net_iov *niov);
+void net_devmem_put_net_iov(struct net_iov *niov);
+
struct net_iov *
net_devmem_alloc_dmabuf(struct net_devmem_dmabuf_binding *binding);
void net_devmem_free_dmabuf(struct net_iov *ppiov);
@@ -126,6 +129,14 @@ void net_devmem_free_dmabuf(struct net_iov *ppiov);
#else
struct net_devmem_dmabuf_binding;
+static inline void net_devmem_get_net_iov(struct net_iov *niov)
+{
+}
+
+static inline void net_devmem_put_net_iov(struct net_iov *niov)
+{
+}
+
static inline void
__net_devmem_dmabuf_binding_free(struct net_devmem_dmabuf_binding *binding)
{
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index a441613a1e6c..815245d5c36b 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -88,6 +88,7 @@
#include <linux/textsearch.h>
#include "dev.h"
+#include "devmem.h"
#include "netmem_priv.h"
#include "sock_destructor.h"
@@ -7290,3 +7291,32 @@ bool csum_and_copy_from_iter_full(void *addr, size_t bytes,
return false;
}
EXPORT_SYMBOL(csum_and_copy_from_iter_full);
+
+void get_netmem(netmem_ref netmem)
+{
+ if (netmem_is_net_iov(netmem)) {
+ /* Assume any net_iov is devmem and route it to
+ * net_devmem_get_net_iov. As new net_iov types are added they
+ * need to be checked here.
+ */
+ net_devmem_get_net_iov(netmem_to_net_iov(netmem));
+ return;
+ }
+ get_page(netmem_to_page(netmem));
+}
+EXPORT_SYMBOL(get_netmem);
+
+void put_netmem(netmem_ref netmem)
+{
+ if (netmem_is_net_iov(netmem)) {
+ /* Assume any net_iov is devmem and route it to
+ * net_devmem_put_net_iov. As new net_iov types are added they
+ * need to be checked here.
+ */
+ net_devmem_put_net_iov(netmem_to_net_iov(netmem));
+ return;
+ }
+
+ put_page(netmem_to_page(netmem));
+}
+EXPORT_SYMBOL(put_netmem);
--
2.47.1.613.gc27f4b7a9f-googOn 12/21, Mina Almasry wrote:
> Currently net_iovs support only pp ref counts, and do not support a
> page ref equivalent.
>
> This is fine for the RX path as net_iovs are used exclusively with the
> pp and only pp refcounting is needed there. The TX path however does not
> use pp ref counts, thus, support for get_page/put_page equivalent is
> needed for netmem.
>
> Support get_netmem/put_netmem. Check the type of the netmem before
> passing it to page or net_iov specific code to obtain a page ref
> equivalent.
>
> For dmabuf net_iovs, we obtain a ref on the underlying binding. This
> ensures the entire binding doesn't disappear until all the net_iovs have
> been put_netmem'ed. We do not need to track the refcount of individual
> dmabuf net_iovs as we don't allocate/free them from a pool similar to
> what the buddy allocator does for pages.
>
> This code is written to be extensible by other net_iov implementers.
> get_netmem/put_netmem will check the type of the netmem and route it to
> the correct helper:
>
> pages -> [get|put]_page()
> dmabuf net_iovs -> net_devmem_[get|put]_net_iov()
> new net_iovs -> new helpers
>
> Signed-off-by: Mina Almasry <almasrymina@google.com>
>
> ---
> include/linux/skbuff_ref.h | 4 ++--
> include/net/netmem.h | 3 +++
> net/core/devmem.c | 10 ++++++++++
> net/core/devmem.h | 11 +++++++++++
> net/core/skbuff.c | 30 ++++++++++++++++++++++++++++++
> 5 files changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/skbuff_ref.h b/include/linux/skbuff_ref.h
> index 0f3c58007488..9e49372ef1a0 100644
> --- a/include/linux/skbuff_ref.h
> +++ b/include/linux/skbuff_ref.h
> @@ -17,7 +17,7 @@
> */
> static inline void __skb_frag_ref(skb_frag_t *frag)
> {
> - get_page(skb_frag_page(frag));
> + get_netmem(skb_frag_netmem(frag));
> }
>
> /**
> @@ -40,7 +40,7 @@ static inline void skb_page_unref(netmem_ref netmem, bool recycle)
> if (recycle && napi_pp_put_page(netmem))
> return;
> #endif
[..]
> - put_page(netmem_to_page(netmem));
> + put_netmem(netmem);
I moved the release operation onto a workqueue in my series [1] to avoid
calling dmabuf detach (which can sleep) from the socket close path
(which is called with bh disabled). You should probably do something similar,
see the trace attached below.
1: https://github.com/fomichev/linux/commit/3b3ad4f36771a376c204727e5a167c4993d4c65a#diff-3c58b866674b2f9beb5ac7349f81566e4df595c25c647710203549589d450f2dR436
(the condition to trigger that is to have an skb in the write queue
and call close from the userspace)
[ 1.548495] BUG: sleeping function called from invalid context at drivers/dma-buf/dma-buf.c:1255
[ 1.548741] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 149, name: ncdevmem
[ 1.548926] preempt_count: 201, expected: 0
[ 1.549026] RCU nest depth: 0, expected: 0
[ 1.549197]
[ 1.549237] =============================
[ 1.549331] [ BUG: Invalid wait context ]
[ 1.549425] 6.13.0-rc3-00770-gbc9ef9606dc9-dirty #15 Tainted: G W
[ 1.549609] -----------------------------
[ 1.549704] ncdevmem/149 is trying to lock:
[ 1.549801] ffff8880066701c0 (reservation_ww_class_mutex){+.+.}-{4:4}, at: dma_buf_unmap_attachment_unlocked+0x4b/0x90
[ 1.550051] other info that might help us debug this:
[ 1.550167] context-{5:5}
[ 1.550229] 3 locks held by ncdevmem/149:
[ 1.550322] #0: ffff888005730208 (&sb->s_type->i_mutex_key#11){+.+.}-{4:4}, at: sock_close+0x40/0xf0
[ 1.550530] #1: ffff88800b148f98 (sk_lock-AF_INET6){+.+.}-{0:0}, at: tcp_close+0x19/0x80
[ 1.550731] #2: ffff88800b148f18 (slock-AF_INET6){+.-.}-{3:3}, at: __tcp_close+0x185/0x4b0
[ 1.550921] stack backtrace:
[ 1.550990] CPU: 0 UID: 0 PID: 149 Comm: ncdevmem Tainted: G W 6.13.0-rc3-00770-gbc9ef9606dc9-dirty #15
[ 1.551233] Tainted: [W]=WARN
[ 1.551304] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.16.3-1-1 04/01/2014
[ 1.551518] Call Trace:
[ 1.551584] <TASK>
[ 1.551636] dump_stack_lvl+0x86/0xc0
[ 1.551723] __lock_acquire+0xb0f/0xc30
[ 1.551814] ? dma_buf_unmap_attachment_unlocked+0x4b/0x90
[ 1.551941] lock_acquire+0xf1/0x2a0
[ 1.552026] ? dma_buf_unmap_attachment_unlocked+0x4b/0x90
[ 1.552152] ? dma_buf_unmap_attachment_unlocked+0x4b/0x90
[ 1.552281] ? dma_buf_unmap_attachment_unlocked+0x4b/0x90
[ 1.552408] __ww_mutex_lock+0x121/0x1060
[ 1.552503] ? dma_buf_unmap_attachment_unlocked+0x4b/0x90
[ 1.552648] ww_mutex_lock+0x3d/0xa0
[ 1.552733] dma_buf_unmap_attachment_unlocked+0x4b/0x90
[ 1.552857] __net_devmem_dmabuf_binding_free+0x56/0xb0
[ 1.552979] skb_release_data+0x120/0x1f0
[ 1.553074] __kfree_skb+0x29/0xa0
[ 1.553156] tcp_write_queue_purge+0x41/0x310
[ 1.553259] tcp_v4_destroy_sock+0x127/0x320
[ 1.553363] ? __tcp_close+0x169/0x4b0
[ 1.553452] inet_csk_destroy_sock+0x53/0x130
[ 1.553560] __tcp_close+0x421/0x4b0
[ 1.553646] tcp_close+0x24/0x80
[ 1.553724] inet_release+0x5d/0x90
[ 1.553806] sock_close+0x4a/0xf0
[ 1.553886] __fput+0x9c/0x2b0
[ 1.553960] task_work_run+0x89/0xc0
[ 1.554046] do_exit+0x27f/0x980
[ 1.554125] do_group_exit+0xa4/0xb0
[ 1.554211] __x64_sys_exit_group+0x17/0x20
[ 1.554309] x64_sys_call+0x21a0/0x21a0
[ 1.554400] do_syscall_64+0xec/0x1d0
[ 1.554487] ? exc_page_fault+0x8a/0xf0
[ 1.554585] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 1.554703] RIP: 0033:0x7f2f8a27abcd
On Thu, Dec 26, 2024 at 11:07 AM Stanislav Fomichev
<stfomichev@gmail.com> wrote:
>
> On 12/21, Mina Almasry wrote:
> > Currently net_iovs support only pp ref counts, and do not support a
> > page ref equivalent.
> >
> > This is fine for the RX path as net_iovs are used exclusively with the
> > pp and only pp refcounting is needed there. The TX path however does not
> > use pp ref counts, thus, support for get_page/put_page equivalent is
> > needed for netmem.
> >
> > Support get_netmem/put_netmem. Check the type of the netmem before
> > passing it to page or net_iov specific code to obtain a page ref
> > equivalent.
> >
> > For dmabuf net_iovs, we obtain a ref on the underlying binding. This
> > ensures the entire binding doesn't disappear until all the net_iovs have
> > been put_netmem'ed. We do not need to track the refcount of individual
> > dmabuf net_iovs as we don't allocate/free them from a pool similar to
> > what the buddy allocator does for pages.
> >
> > This code is written to be extensible by other net_iov implementers.
> > get_netmem/put_netmem will check the type of the netmem and route it to
> > the correct helper:
> >
> > pages -> [get|put]_page()
> > dmabuf net_iovs -> net_devmem_[get|put]_net_iov()
> > new net_iovs -> new helpers
> >
> > Signed-off-by: Mina Almasry <almasrymina@google.com>
> >
> > ---
> > include/linux/skbuff_ref.h | 4 ++--
> > include/net/netmem.h | 3 +++
> > net/core/devmem.c | 10 ++++++++++
> > net/core/devmem.h | 11 +++++++++++
> > net/core/skbuff.c | 30 ++++++++++++++++++++++++++++++
> > 5 files changed, 56 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/skbuff_ref.h b/include/linux/skbuff_ref.h
> > index 0f3c58007488..9e49372ef1a0 100644
> > --- a/include/linux/skbuff_ref.h
> > +++ b/include/linux/skbuff_ref.h
> > @@ -17,7 +17,7 @@
> > */
> > static inline void __skb_frag_ref(skb_frag_t *frag)
> > {
> > - get_page(skb_frag_page(frag));
> > + get_netmem(skb_frag_netmem(frag));
> > }
> >
> > /**
> > @@ -40,7 +40,7 @@ static inline void skb_page_unref(netmem_ref netmem, bool recycle)
> > if (recycle && napi_pp_put_page(netmem))
> > return;
> > #endif
>
> [..]
>
> > - put_page(netmem_to_page(netmem));
> > + put_netmem(netmem);
>
> I moved the release operation onto a workqueue in my series [1] to avoid
> calling dmabuf detach (which can sleep) from the socket close path
> (which is called with bh disabled). You should probably do something similar,
> see the trace attached below.
>
> 1: https://github.com/fomichev/linux/commit/3b3ad4f36771a376c204727e5a167c4993d4c65a#diff-3c58b866674b2f9beb5ac7349f81566e4df595c25c647710203549589d450f2dR436
>
> (the condition to trigger that is to have an skb in the write queue
> and call close from the userspace)
>
Thanks for catching this indeed. I've also changed the unbinding to
scheduled_work, although I arrived at a slightly different
implementation that is simpler to my eye. I'll upload what I have for
review shortly as RFC.
--
Thanks,
Mina
© 2016 - 2026 Red Hat, Inc.