mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net] xsk: freeze deferred pool teardown without blocking unregister
@ 2026-10-01  5:09 James Hilliard
  2026-10-01  5:13 ` netdev-bot+sinfo
  2026-10-05  5:31 ` netdev-bot+sashiko
  0 siblings, 2 replies; 4+ messages in thread
From: James Hilliard @ 2026-10-01  5:09 UTC (permalink / raw)
  To: netdev, Paolo Abeni, Jakub Kicinski, Magnus Karlsson,
	Maciej Fijalkowski, Stanislav Fomichev, Simon Horman,
	Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
	John Fastabend
  Cc: Eric Dumazet, David S. Miller, Björn Töpel, bpf,
	linux-kernel, James Hilliard

Deferred pool destruction calls ndo_bpf() under RTNL. system_wq is
not frozen during system sleep, so that callback can run after a
device has suspended and gated its clocks. Use system_freezable_wq
so running destruction finishes before device suspend and new work
waits until process thaw.

Keep assigned pools visible to NETDEV_UNREGISTER independently of
the socket list. A released socket has already left that list, but
its final pool put can queue destruction after workqueues freeze.
A resume-time unregister would then wait for a device reference
whose release cannot run until the resume completes.

Track assigned pools under RTNL and detach remaining pools after the
notifier socket walk, including copy-mode pools. Remove the entry on
assignment failure and normal teardown. The deferred worker still
owns the pool and later observes the cleared device pointer, avoiding
a second driver detach or device put.

Fixes: 1c1efc2af158 ("xsk: Create and free buffer pool independently from umem")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 include/net/xsk_buff_pool.h |  3 +++
 net/xdp/xsk.c               |  4 ++++
 net/xdp/xsk_buff_pool.c     | 25 ++++++++++++++++++++++++-
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
index a7df573784fd..cea6e883f7cd 100644
--- a/include/net/xsk_buff_pool.h
+++ b/include/net/xsk_buff_pool.h
@@ -48,6 +48,8 @@ struct xsk_buff_pool {
 	struct device *dev;
 	struct net_device *netdev;
 	struct list_head xsk_tx_list;
+	/* Assigned pools, including deferred releases; protected by RTNL. */
+	struct list_head dev_list;
 	/* Protects modifications to the xsk_tx_list */
 	spinlock_t xsk_tx_list_lock;
 	refcount_t users;
@@ -119,6 +121,7 @@ bool xp_put_pool(struct xsk_buff_pool *pool);
 void xp_clear_dev(struct xsk_buff_pool *pool);
 void xp_add_xsk(struct xsk_buff_pool *pool, struct xdp_sock *xs);
 void xp_del_xsk(struct xsk_buff_pool *pool, struct xdp_sock *xs);
+void xp_clear_dev_all(struct net_device *dev);
 
 /* AF_XDP, and XDP core. */
 void xp_free(struct xdp_buff_xsk *xskb);
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 33475b180ea6..a2ae0e10d893 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -2145,6 +2145,10 @@ static int xsk_notifier(struct notifier_block *this,
 			mutex_unlock(&xs->mutex);
 		}
 		mutex_unlock(&net->xdp.lock);
+		/* A released socket is no longer on xdp.list. Its pool can still
+		 * hold a device reference on the frozen release workqueue.
+		 */
+		xp_clear_dev_all(dev);
 		break;
 	}
 	return NOTIFY_DONE;
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 9d2d94f1fb75..b5fd0850db5b 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -12,6 +12,11 @@
 
 #define ETH_PAD_LEN (ETH_HLEN + 2 * VLAN_HLEN  + ETH_FCS_LEN)
 
+/* Socket removal precedes the final pool put. Keep assigned pools visible to
+ * NETDEV_UNREGISTER even while their release work is waiting for process thaw.
+ */
+static LIST_HEAD(xsk_dev_pools);
+
 void xp_add_xsk(struct xsk_buff_pool *pool, struct xdp_sock *xs)
 {
 	if (!xs->tx)
@@ -204,6 +209,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
 	pool->cached_need_wakeup = XDP_WAKEUP_TX;
 
 	dev_hold(netdev);
+	list_add_tail(&pool->dev_list, &xsk_dev_pools);
 
 	if (force_copy)
 		/* For copy-mode, we are done. */
@@ -265,6 +271,8 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
 		err = 0; /* fallback to copy mode */
 	if (err) {
 		xsk_clear_pool_at_qid(netdev, queue_id);
+		list_del(&pool->dev_list);
+		pool->netdev = NULL;
 		dev_put(netdev);
 	}
 	return err;
@@ -291,17 +299,29 @@ void xp_clear_dev(struct xsk_buff_pool *pool)
 {
 	struct net_device *netdev = pool->netdev;
 
+	ASSERT_RTNL();
 	if (!pool->netdev)
 		return;
 
 	netdev_lock_ops(netdev);
 	xp_disable_drv_zc(pool);
 	xsk_clear_pool_at_qid(pool->netdev, pool->queue_id);
+	list_del(&pool->dev_list);
 	pool->netdev = NULL;
 	netdev_unlock_ops(netdev);
 	dev_put(netdev);
 }
 
+void xp_clear_dev_all(struct net_device *dev)
+{
+	struct xsk_buff_pool *pool, *next;
+
+	ASSERT_RTNL();
+	list_for_each_entry_safe(pool, next, &xsk_dev_pools, dev_list)
+		if (pool->netdev == dev)
+			xp_clear_dev(pool);
+}
+
 static void xp_release_deferred(struct work_struct *work)
 {
 	struct xsk_buff_pool *pool = container_of(work, struct xsk_buff_pool,
@@ -337,7 +357,10 @@ bool xp_put_pool(struct xsk_buff_pool *pool)
 
 	if (refcount_dec_and_test(&pool->users)) {
 		INIT_WORK(&pool->work, xp_release_deferred);
-		schedule_work(&pool->work);
+		/* Teardown calls ndo_bpf(), which may need powered hardware.
+		 * RTNL alone does not exclude the device's system PM callbacks.
+		 */
+		queue_work(system_freezable_wq, &pool->work);
 		return true;
 	}
 

---
base-commit: a7bfaba4823e3c165bb2004c74eff7c096672bc7
change-id: 20260930-xsk-suspend-teardown-902f98408c93

Best regards,
--  
James Hilliard <james.hilliard1@gmail.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] xsk: freeze deferred pool teardown without blocking unregister
  2026-10-01  5:09 [PATCH net] xsk: freeze deferred pool teardown without blocking unregister James Hilliard
@ 2026-10-01  5:13 ` netdev-bot+sinfo
  2026-10-06  4:38   ` James Hilliard
  2026-10-05  5:31 ` netdev-bot+sashiko
  1 sibling, 1 reply; 4+ messages in thread
From: netdev-bot+sinfo @ 2026-10-01  5:13 UTC (permalink / raw)
  To: James Hilliard
  Cc: netdev, Paolo Abeni, Jakub Kicinski, Magnus Karlsson,
	Maciej Fijalkowski, Stanislav Fomichev, Simon Horman,
	Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
	John Fastabend, Eric Dumazet, David S. Miller,
	Björn Töpel, bpf, linux-kernel

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] xsk: freeze deferred pool teardown without blocking unregister
  2026-10-01  5:09 [PATCH net] xsk: freeze deferred pool teardown without blocking unregister James Hilliard
  2026-10-01  5:13 ` netdev-bot+sinfo
@ 2026-10-05  5:31 ` netdev-bot+sashiko
  1 sibling, 0 replies; 4+ messages in thread
From: netdev-bot+sashiko @ 2026-10-05  5:31 UTC (permalink / raw)
  To: james.hilliard1
  Cc: netdev, pabeni, kuba, magnus.karlsson, maciej.fijalkowski, sdf,
	horms, ast, daniel, hawk, john.fastabend, edumazet, davem, bjorn,
	bpf, linux-kernel

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch adds one global list, xsk_dev_pools
  (net/xdp/xsk_buff_pool.c), that holds every assigned XSK buffer pool…

--

xsk: freeze deferred pool teardown without blocking unregister

This moves deferred XSK buffer pool destruction from system_wq to
system_freezable_wq, so ndo_bpf() is not called on a suspended device. It
also adds a global list of assigned pools. NETDEV_UNREGISTER uses that
list to detach pools whose sockets have already left net->xdp.list.

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 33475b180ea6f..a2ae0e10d8933 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -2145,6 +2145,10 @@ static int xsk_notifier(struct notifier_block *this,
>  			mutex_unlock(&xs->mutex);
>  		}
>  		mutex_unlock(&net->xdp.lock);
> +		/* A released socket is no longer on xdp.list. Its pool can still
> +		 * hold a device reference on the frozen release workqueue.
> +		 */
> +		xp_clear_dev_all(dev);
>  		break;
>  	}
>  	return NOTIFY_DONE;
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 9d2d94f1fb759..b5fd0850db5bb 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -12,6 +12,11 @@
>  
>  #define ETH_PAD_LEN (ETH_HLEN + 2 * VLAN_HLEN  + ETH_FCS_LEN)
>  
> +/* Socket removal precedes the final pool put. Keep assigned pools visible to
> + * NETDEV_UNREGISTER even while their release work is waiting for process thaw.
> + */
> +static LIST_HEAD(xsk_dev_pools);
> +

[ ... ]

> +void xp_clear_dev_all(struct net_device *dev)
> +{
> +	struct xsk_buff_pool *pool, *next;
> +
> +	ASSERT_RTNL();
> +	list_for_each_entry_safe(pool, next, &xsk_dev_pools, dev_list)
> +		if (pool->netdev == dev)
> +			xp_clear_dev(pool);
> +}

[Severity: Medium]
Should xsk_dev_pools be scoped per netns or per netdev, rather than being
one global list?

As written, xsk_dev_pools holds every assigned pool from every network
namespace. xsk_notifier()->xp_clear_dev_all() walks the whole list under
RTNL on every NETDEV_UNREGISTER, in any namespace.

Before this patch, the notifier only walked net->xdp.list of the namespace
that owns the device. With this change, each unregister costs O(total pools
in the system). A batched unregister, such as netns teardown, costs
O(devices x global pools), all while RTNL is held.

xsk_create() only checks ns_capable(net->user_ns, CAP_NET_RAW). A user
inside a user namespace can therefore create many pools, for example with
copy-mode binds to veth or dummy queues, or with XDP_SHARED_UMEM backing
many pools from one umem. That would lengthen RTNL hold times during device
and netns teardown in unrelated namespaces.

Would tracking the pools per netdev or per netns limit the walk to pools
that can actually match dev?

>  static void xp_release_deferred(struct work_struct *work)
>  {
>  	struct xsk_buff_pool *pool = container_of(work, struct xsk_buff_pool,

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930-xsk-suspend-teardown-v1-1-a6cac8c030be%40gmail.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] xsk: freeze deferred pool teardown without blocking unregister
  2026-10-01  5:13 ` netdev-bot+sinfo
@ 2026-10-06  4:38   ` James Hilliard
  0 siblings, 0 replies; 4+ messages in thread
From: James Hilliard @ 2026-10-06  4:38 UTC (permalink / raw)
  To: netdev-bot+sinfo
  Cc: netdev, Paolo Abeni, Jakub Kicinski, Magnus Karlsson,
	Maciej Fijalkowski, Stanislav Fomichev, Simon Horman,
	Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
	John Fastabend, Eric Dumazet, David S. Miller,
	Björn Töpel, bpf, linux-kernel

On Wed, Sep 30, 2026 at 11:13 PM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.

Found by source review of AF_XDP teardown during system sleep while
working on the larger stmmac MTU/resume recovery series. Further
unregister issues were identified during follow-up review.

>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.

These are code-inspection findings. I have not reproduced the
suspended-device access or freezer/unregister deadlock in a running
kernel.

> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-06  4:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-01  5:09 [PATCH net] xsk: freeze deferred pool teardown without blocking unregister James Hilliard
2026-10-01  5:13 ` netdev-bot+sinfo
2026-10-06  4:38   ` James Hilliard
2026-10-05  5:31 ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®