On Tue, Feb 25, 2025 at 10:03 AM David Ahern <dsahern@xxxxxxxxxx> wrote: > > On 2/25/25 10:41 AM, Mina Almasry wrote: > > On Tue, Feb 25, 2025 at 5:04 AM Paolo Abeni <pabeni@xxxxxxxxxx> wrote: > >> > >> On 2/22/25 8:15 PM, Mina Almasry wrote: > >> [...] > >>> @@ -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
