Re: [PATCH net] net/packet: defer vmalloc TX_RING free until skbs finish

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Eric Dumazet wrote:
> On Tue, Jul 21, 2026 at 3:58 AM Kyle Zeng <kylebot@openai.com> wrote:
> >
> > AF_PACKET TX_RING skbs keep a raw pointer to their ring frame. The skb
> > page references preserve page-backed ring blocks after pg_vec is freed,
> > but they do not preserve a vmalloc mapping.
> >
> > tpacket_destruct_skb() currently drops the pending reference before
> > writing the timestamp and TP_STATUS_AVAILABLE to the frame. Move the
> > decrement after those stores. The smp_wmb() in __packet_set_status()
> > orders the frame stores before the decrement.
> >
> > On socket close, scan every pg_vec entry because allocation can produce
> > a mixture of page-backed and vmalloc-backed blocks. If any block is
> > vmalloc-backed and TX skbs remain pending, defer the whole vector to
> > system_long_wq.
> >
> > After pg_vec is detached, a late destructor can skip the pending
> > decrement. Use socket write-memory accounting as the deferred lifetime
> > gate instead: an skb remains charged through its final sock_wfree(),
> > after all ring-frame accesses. The delayed work retains a socket
> > reference and reschedules itself until no TX skbs remain. Fall back to
> > a synchronous wait if the work allocation fails.
> >
> > Move pending_refcnt release to packet_sock_destruct() so late skb
> > destructors and deferred cleanup can safely use it after
> > packet_release(). Page-backed teardown remains synchronous, and no lock
> > is added to the TX completion hot path.
> >
> > Fixes: b013840810c2 ("packet: use percpu mmap tx frame pending refcount")
> > Cc: stable@vger.kernel.org
> > Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > Assisted-by: Codex:gpt-5.6
> > Signed-off-by: Kyle Zeng <kylebot@openai.com>
> > ---
> 
> Hi Kyle,
> 
> Rather than doing kmalloc() inside free_pg_vec() during socket teardown/close,
> would it be better to pre-allocate the deferred work storage at ring setup
> time (in alloc_pg_vec / packet_set_ring)?

Probably superfluous, but: only if vmalloc was used.

Neat secondary feature will be that the non-NULL status of that
pointer can be used to detect use of vmalloc at teardown, without
having to iterate over all the pgvec entries.
 
> Pre-allocating at ring setup has a few advantages:
> 
> 1) If allocation fails, setsockopt(PACKET_TX_RING) fails early with -ENOMEM,
> avoiding allocation failures during socket close/teardown.
> 
> 2) Teardown becomes deterministic: free_pg_vec() is guaranteed to have
> the storage available to queue work.
> 
> 3) We can drop the extra fallback logic (packet_wait_for_tx_skbs(),
> po->skb_completion, and the 1-jiffy fallback polling loop).
> 
> You could embed a struct delayed_work (or a header struct) into the allocation
> returned by alloc_pg_vec().
> 
> Thanks,