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

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

[Kyle Zeng]

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>
---
 net/packet/af_packet.c | 90 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 82 insertions(+), 8 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 8e6f3a734ba0..1a4c548c59ac 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -88,6 +88,7 @@
 #include <linux/errqueue.h>
 #include <linux/net_tstamp.h>
 #include <linux/percpu.h>
+#include <linux/workqueue.h>
 #ifdef CONFIG_INET
 #include <net/inet_common.h>
 #endif
@@ -1329,6 +1330,8 @@ static void packet_sock_destruct(struct sock *sk)
 	WARN_ON(atomic_read(&sk->sk_rmem_alloc));
 	WARN_ON(refcount_read(&sk->sk_wmem_alloc));
 
+	packet_free_pending(pkt_sk(sk));
+
 	if (!sock_flag(sk, SOCK_DEAD)) {
 		pr_err("Attempt to release alive packet socket: %p\n", sk);
 		return;
@@ -2516,14 +2519,12 @@ static void tpacket_destruct_skb(struct sk_buff *skb)
 		__u32 ts;
 
 		ph = skb_zcopy_get_nouarg(skb);
-		packet_dec_pending(&po->tx_ring);
 
 		ts = __packet_set_timestamp(po, ph, skb);
 		__packet_set_status(po, ph, TP_STATUS_AVAILABLE | ts);
-
+		packet_dec_pending(&po->tx_ring);
 		complete(&po->skb_completion);
 	}
-
 	sock_wfree(skb);
 }
 
@@ -3182,7 +3183,6 @@ static int packet_release(struct socket *sock)
 	/* Purge queues */
 
 	skb_queue_purge(&sk->sk_receive_queue);
-	packet_free_pending(po);
 
 	sock_put(sk);
 	return 0;
@@ -4345,8 +4345,16 @@ static const struct vm_operations_struct packet_mmap_ops = {
 	.close	=	packet_mm_close,
 };
 
-static void free_pg_vec(struct pgv *pg_vec, unsigned int order,
-			unsigned int len)
+struct packet_pg_vec_free {
+	struct delayed_work work;
+	struct sock *sk;
+	struct pgv *pg_vec;
+	unsigned int order;
+	unsigned int len;
+};
+
+static void __free_pg_vec(struct pgv *pg_vec, unsigned int order,
+			  unsigned int len)
 {
 	int i;
 
@@ -4363,6 +4371,69 @@ static void free_pg_vec(struct pgv *pg_vec, unsigned int order,
 	kfree(pg_vec);
 }
 
+static void packet_wait_for_tx_skbs(struct packet_sock *po)
+{
+	while (sk_wmem_alloc_get(&po->sk))
+		wait_for_completion_timeout(&po->skb_completion, 1);
+}
+
+static void packet_free_pg_vec_work(struct work_struct *work)
+{
+	struct packet_pg_vec_free *deferred;
+	struct sock *sk;
+
+	deferred = container_of(to_delayed_work(work),
+				struct packet_pg_vec_free, work);
+	sk = deferred->sk;
+	if (sk_wmem_alloc_get(sk)) {
+		queue_delayed_work(system_long_wq, &deferred->work, 1);
+		return;
+	}
+
+	__free_pg_vec(deferred->pg_vec, deferred->order, deferred->len);
+	kfree(deferred);
+	sock_put(sk);
+}
+
+static bool pg_vec_has_vmalloc(struct pgv *pg_vec, unsigned int len)
+{
+	int i;
+
+	for (i = 0; i < len; i++)
+		if (pg_vec[i].buffer && is_vmalloc_addr(pg_vec[i].buffer))
+			return true;
+
+	return false;
+}
+
+static void free_pg_vec(struct packet_sock *po, struct pgv *pg_vec,
+			unsigned int order, unsigned int len)
+{
+	struct packet_pg_vec_free *deferred;
+
+	if (!pg_vec_has_vmalloc(pg_vec, len) ||
+	    !packet_read_pending(&po->tx_ring)) {
+		__free_pg_vec(pg_vec, order, len);
+		return;
+	}
+
+	deferred = kmalloc(sizeof(*deferred), GFP_KERNEL);
+	if (!deferred) {
+		packet_wait_for_tx_skbs(po);
+		__free_pg_vec(pg_vec, order, len);
+		return;
+	}
+
+	INIT_DELAYED_WORK(&deferred->work, packet_free_pg_vec_work);
+	deferred->sk = &po->sk;
+	deferred->pg_vec = pg_vec;
+	deferred->order = order;
+	deferred->len = len;
+
+	sock_hold(deferred->sk);
+	queue_delayed_work(system_long_wq, &deferred->work, 0);
+}
+
 static char *alloc_one_pg_vec_page(unsigned long order)
 {
 	char *buffer;
@@ -4408,7 +4479,7 @@ static struct pgv *alloc_pg_vec(struct tpacket_req *req, int order)
 	return pg_vec;
 
 out_free_pgvec:
-	free_pg_vec(pg_vec, order, block_nr);
+	__free_pg_vec(pg_vec, order, block_nr);
 	pg_vec = NULL;
 	goto out;
 }
@@ -4574,7 +4645,10 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
 out_free_pg_vec:
 	if (pg_vec) {
 		bitmap_free(rx_owner_map);
-		free_pg_vec(pg_vec, order, req->tp_block_nr);
+		if (tx_ring && closing)
+			free_pg_vec(po, pg_vec, order, req->tp_block_nr);
+		else
+			__free_pg_vec(pg_vec, order, req->tp_block_nr);
 	}
 out:
 	return err;
-- 
2.54.0

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

[Eric Dumazet]

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)?

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,

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

[Willem de Bruijn]

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,