[PATCH net v3] xsk: freeze deferred pool teardown without blocking unregister

[PATCH net v3] xsk: freeze deferred pool teardown without blocking unregister

[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 per netdev with netdev operations locking and detach
remaining pools after the notifier socket walk, including copy-mode pools.
This also covers leased queues without scanning pools from unrelated
devices or network namespaces. 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 put.

The lifetime problem was identified by code inspection of the deferred
release and system-sleep paths.

Fixes: 1c1efc2af158 ("xsk: Create and free buffer pool independently from umem")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
Changes in v3:
- Protect the per-netdev pool list with netdev operations locking, as
  requested by Stanislav Fomichev and Björn Töpel.
- Assert operations locking for copy-mode assignment too, and share a
  locked detach helper so unregister holds the lock across the pool walk.
- Rebase onto current net.
- Link to v2: https://patch.msgid.link/20261005-xsk-suspend-teardown-v2-1-2d87228f4329@gmail.com

Changes in v2:
- Track assigned pools per netdev instead of scanning a global pool list.
- Keep deferred releases visible across queue changes and queue leases.
- Rebase onto current net.
- Link to v1: https://patch.msgid.link/20260930-xsk-suspend-teardown-v1-1-a6cac8c030be@gmail.com
---
 include/linux/netdevice.h   |  5 +++++
 include/net/xsk_buff_pool.h |  3 +++
 net/xdp/xsk.c               |  5 +++++
 net/xdp/xsk_buff_pool.c     | 46 ++++++++++++++++++++++++++++++++++++++-------
 4 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3cff2174dc03..ac257221c7c5 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2545,6 +2545,11 @@ struct net_device {
 	/* protected by rtnl_lock */
 	struct bpf_xdp_entity	xdp_state[__MAX_XDP_MODE];
 
+#ifdef CONFIG_XDP_SOCKETS
+	/** @xsk_pools: assigned AF_XDP pools, ops protected */
+	struct hlist_head	xsk_pools;
+#endif
+
 	u8 dev_addr_shadow[MAX_ADDR_LEN];
 	netdevice_tracker	linkwatch_dev_tracker;
 	netdevice_tracker	watchdog_dev_tracker;
diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
index a7df573784fd..02e89350faf9 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; ops protected. */
+	struct hlist_node dev_node;
 	/* 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..781ba46eb152 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -2145,6 +2145,11 @@ 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..5b9dcafd6882 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -173,6 +173,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
 	int err = 0;
 
 	ASSERT_RTNL();
+	netdev_assert_locked_ops_compat(netdev);
 
 	force_zc = flags & XDP_ZEROCOPY;
 	force_copy = flags & XDP_COPY;
@@ -204,6 +205,11 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
 	pool->cached_need_wakeup = XDP_WAKEUP_TX;
 
 	dev_hold(netdev);
+	/*
+	 * Socket removal precedes the final pool put. Keep the pool visible to
+	 * NETDEV_UNREGISTER even while release work is waiting for process thaw.
+	 */
+	hlist_add_head(&pool->dev_node, &netdev->xsk_pools);
 
 	if (force_copy)
 		/* For copy-mode, we are done. */
@@ -244,7 +250,6 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
 	bpf.xsk.pool = pool;
 	bpf.xsk.queue_id = queue_id;
 
-	netdev_assert_locked_ops_compat(netdev);
 	err = netdev->netdev_ops->ndo_bpf(netdev, &bpf);
 	if (err)
 		goto err_unreg_pool;
@@ -265,6 +270,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);
+		hlist_del(&pool->dev_node);
+		pool->netdev = NULL;
 		dev_put(netdev);
 	}
 	return err;
@@ -287,19 +294,40 @@ int xp_assign_dev_shared(struct xsk_buff_pool *pool, struct xdp_sock *umem_xs,
 	return xp_assign_dev(pool, dev, queue_id, flags);
 }
 
+static void __xp_clear_dev(struct xsk_buff_pool *pool)
+{
+	struct net_device *netdev = pool->netdev;
+
+	netdev_assert_locked_ops_compat(netdev);
+	xp_disable_drv_zc(pool);
+	xsk_clear_pool_at_qid(netdev, pool->queue_id);
+	hlist_del(&pool->dev_node);
+	pool->netdev = NULL;
+	dev_put(netdev);
+}
+
 void xp_clear_dev(struct xsk_buff_pool *pool)
 {
 	struct net_device *netdev = pool->netdev;
 
-	if (!pool->netdev)
+	if (!netdev)
 		return;
 
 	netdev_lock_ops(netdev);
-	xp_disable_drv_zc(pool);
-	xsk_clear_pool_at_qid(pool->netdev, pool->queue_id);
-	pool->netdev = NULL;
+	__xp_clear_dev(pool);
 	netdev_unlock_ops(netdev);
-	dev_put(netdev);
+}
+
+void xp_clear_dev_all(struct net_device *dev)
+{
+	struct xsk_buff_pool *pool;
+	struct hlist_node *next;
+
+	netdev_lock_ops(dev);
+	netdev_assert_locked_ops_compat(dev);
+	hlist_for_each_entry_safe(pool, next, &dev->xsk_pools, dev_node)
+		__xp_clear_dev(pool);
+	netdev_unlock_ops(dev);
 }
 
 static void xp_release_deferred(struct work_struct *work)
@@ -337,7 +365,11 @@ 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: 513444cc62e231a11c9b15ea88de0890da9b2b4f
change-id: 20260930-xsk-suspend-teardown-902f98408c93

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

Re: [PATCH net v3] xsk: freeze deferred pool teardown without blocking unregister

[Björn Töpel]

James Hilliard <james.hilliard1@gmail.com> writes:

> 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 per netdev with netdev operations locking and detach
> remaining pools after the notifier socket walk, including copy-mode pools.
> This also covers leased queues without scanning pools from unrelated
> devices or network namespaces. 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 put.
>
> The lifetime problem was identified by code inspection of the deferred
> release and system-sleep paths.
>
> Fixes: 1c1efc2af158 ("xsk: Create and free buffer pool independently from umem")
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Thanks, James!

Reviewed-by: Björn Töpel <bjorn@kernel.org>

Re: [PATCH net v3] xsk: freeze deferred pool teardown without blocking unregister

[Jason Xing]

On Wed, Oct 14, 2026 at 2:38 AM James Hilliard
<james.hilliard1@gmail.com> wrote:
>
> 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 per netdev with netdev operations locking and detach
> remaining pools after the notifier socket walk, including copy-mode pools.
> This also covers leased queues without scanning pools from unrelated
> devices or network namespaces. 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 put.
>
> The lifetime problem was identified by code inspection of the deferred
> release and system-sleep paths.
>
> Fixes: 1c1efc2af158 ("xsk: Create and free buffer pool independently from umem")
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Admittedly it's a bit hard to review the complex logic. Thanks for the work!

Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>

Re: [PATCH net v3] xsk: freeze deferred pool teardown without blocking unregister

[Stanislav Fomichev]

On 10/13, James Hilliard wrote:
> 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 per netdev with netdev operations locking and detach
> remaining pools after the notifier socket walk, including copy-mode pools.
> This also covers leased queues without scanning pools from unrelated
> devices or network namespaces. 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 put.
> 
> The lifetime problem was identified by code inspection of the deferred
> release and system-sleep paths.
> 
> Fixes: 1c1efc2af158 ("xsk: Create and free buffer pool independently from umem")
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
> Changes in v3:
> - Protect the per-netdev pool list with netdev operations locking, as
>   requested by Stanislav Fomichev and Björn Töpel.
> - Assert operations locking for copy-mode assignment too, and share a
>   locked detach helper so unregister holds the lock across the pool walk.
> - Rebase onto current net.
> - Link to v2: https://patch.msgid.link/20261005-xsk-suspend-teardown-v2-1-2d87228f4329@gmail.com
> 
> Changes in v2:
> - Track assigned pools per netdev instead of scanning a global pool list.
> - Keep deferred releases visible across queue changes and queue leases.
> - Rebase onto current net.
> - Link to v1: https://patch.msgid.link/20260930-xsk-suspend-teardown-v1-1-a6cac8c030be@gmail.com
> ---
>  include/linux/netdevice.h   |  5 +++++
>  include/net/xsk_buff_pool.h |  3 +++
>  net/xdp/xsk.c               |  5 +++++
>  net/xdp/xsk_buff_pool.c     | 46 ++++++++++++++++++++++++++++++++++++++-------
>  4 files changed, 52 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 3cff2174dc03..ac257221c7c5 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2545,6 +2545,11 @@ struct net_device {
>  	/* protected by rtnl_lock */
>  	struct bpf_xdp_entity	xdp_state[__MAX_XDP_MODE];
>  
> +#ifdef CONFIG_XDP_SOCKETS
> +	/** @xsk_pools: assigned AF_XDP pools, ops protected */
> +	struct hlist_head	xsk_pools;
> +#endif
> +
>  	u8 dev_addr_shadow[MAX_ADDR_LEN];
>  	netdevice_tracker	linkwatch_dev_tracker;
>  	netdevice_tracker	watchdog_dev_tracker;
> diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
> index a7df573784fd..02e89350faf9 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; ops protected. */
> +	struct hlist_node dev_node;
>  	/* 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..781ba46eb152 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -2145,6 +2145,11 @@ 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..5b9dcafd6882 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -173,6 +173,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
>  	int err = 0;
 

[..]

>  	ASSERT_RTNL();
> +	netdev_assert_locked_ops_compat(netdev);

nit: can now drop ASSERT_RTNL? netdev_assert_locked_ops_compat should be
enough.

>  
>  	force_zc = flags & XDP_ZEROCOPY;
>  	force_copy = flags & XDP_COPY;
> @@ -204,6 +205,11 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
>  	pool->cached_need_wakeup = XDP_WAKEUP_TX;
>  
>  	dev_hold(netdev);
> +	/*
> +	 * Socket removal precedes the final pool put. Keep the pool visible to
> +	 * NETDEV_UNREGISTER even while release work is waiting for process thaw.
> +	 */
> +	hlist_add_head(&pool->dev_node, &netdev->xsk_pools);
>  
>  	if (force_copy)
>  		/* For copy-mode, we are done. */
> @@ -244,7 +250,6 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
>  	bpf.xsk.pool = pool;
>  	bpf.xsk.queue_id = queue_id;
>  
> -	netdev_assert_locked_ops_compat(netdev);
>  	err = netdev->netdev_ops->ndo_bpf(netdev, &bpf);
>  	if (err)
>  		goto err_unreg_pool;
> @@ -265,6 +270,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);
> +		hlist_del(&pool->dev_node);
> +		pool->netdev = NULL;
>  		dev_put(netdev);
>  	}
>  	return err;
> @@ -287,19 +294,40 @@ int xp_assign_dev_shared(struct xsk_buff_pool *pool, struct xdp_sock *umem_xs,
>  	return xp_assign_dev(pool, dev, queue_id, flags);
>  }
>  
> +static void __xp_clear_dev(struct xsk_buff_pool *pool)
> +{
> +	struct net_device *netdev = pool->netdev;
> +
> +	netdev_assert_locked_ops_compat(netdev);
> +	xp_disable_drv_zc(pool);
> +	xsk_clear_pool_at_qid(netdev, pool->queue_id);
> +	hlist_del(&pool->dev_node);
> +	pool->netdev = NULL;
> +	dev_put(netdev);
> +}
> +
>  void xp_clear_dev(struct xsk_buff_pool *pool)
>  {
>  	struct net_device *netdev = pool->netdev;
>  
> -	if (!pool->netdev)
> +	if (!netdev)
>  		return;
>  
>  	netdev_lock_ops(netdev);
> -	xp_disable_drv_zc(pool);
> -	xsk_clear_pool_at_qid(pool->netdev, pool->queue_id);
> -	pool->netdev = NULL;
> +	__xp_clear_dev(pool);
>  	netdev_unlock_ops(netdev);
> -	dev_put(netdev);
> +}
> +
> +void xp_clear_dev_all(struct net_device *dev)
> +{
> +	struct xsk_buff_pool *pool;
> +	struct hlist_node *next;
> +

[..]

> +	netdev_lock_ops(dev);
> +	netdev_assert_locked_ops_compat(dev);

And here also no need for netdev_assert_locked_ops_compat right after
netdev_lock_ops. You have one in __xp_clear_dev, that should be enough.
With that:

Acked-by: Stanislav Fomichev <sdf@fomichev.me>