On Tue, Feb 25, 2025 at 10:03 AM David Ahern [off-list ref] wrote:

On 2/25/25 10:41 AM, Mina Almasry wrote:

quoted

On Tue, Feb 25, 2025 at 5:04 AM Paolo Abeni [off-list ref] wrote:

quoted

On 2/22/25 8:15 PM, Mina Almasry wrote:
[...]

quoted

@@ -119,6 +122,13 @@ void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
      unsigned long xa_idx;
      unsigned int rxq_idx;

+     xa_erase(&net_devmem_dmabuf_bindings, binding->id);
+
+     /* Ensure no tx net_devmem_lookup_dmabuf() are in flight after the
+      * erase.
+      */
+     synchronize_net();

Is the above statement always true? can the dmabuf being stuck in some
qdisc? or even some local socket due to redirect?

Yes, we could have have netmems in the TX path in the qdisc or waiting
for retransmits that still have references to the dmabuf, and that's
fine here I think.

skbs with references to netmem from a dmabuf can get stuck in retransmit
queues for a long time. The comment should at least acknowledge that.

Will do, although I think maybe I'll add that comment above the
refcount_t definition in net_devmem_dmabuf_binding, if that's OK with
you. That was meant to explain how the refcounting on the binding
works.

Maybe worthy of note, this is not that new, in fact something similar
is happening in the RX path. In the RX path each skb waiting in the
receive queue to be recvmsg()'d will hold a reference on the
underlying page_pool, which in-turn holds a reference on the
underlying dmabuf. It's just that a similar thing is happening in the
TX path.

-- 
Thanks,
Mina