diff mbox series

[net,v4,04/15] xsk: freeze deferred pool teardown during system sleep

Message ID 20260926-submit-stmmac-reset-fixes-v1-v4-4-ec1c0250b3c9@gmail.com (mailing list archive)
State New
Headers
Series net: stmmac: preserve datapath state across MTU and resume failures |

Commit Message

James Hilliard Sept. 26, 2026, 3:48 p.m. UTC 
Pool destruction calls the driver under RTNL from system_wq. That queue
is not frozen during system sleep, so ndo_bpf() can run after the device
suspend callback has gated its clocks or after the noirq phase.

Use the freezable workqueue for deferred pool release. Work already
running completes before device suspend, and newly queued destruction
waits until device resume. The pool, UMEM and netdev references remain
owned by the work until then. This does not replace driver error
handling after a failed resume.

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

Comments

netdev-bot+sashiko@kernel.org Sept. 30, 2026, 4:51 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · 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).

- [High] Resume can hang forever when a netdev is unregistered from a
  device PM callback while an xsk pool release is waiting on the frozen…

--

xsk: freeze deferred pool teardown during system sleep

This patch moves the deferred xsk buffer pool release from system_wq to
system_freezable_wq. The goal is to stop ndo_bpf() from being called
while the device is suspended.

> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 9d2d94f1fb759..c58f56f24a9c7 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -337,7 +337,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);

[Severity: High]
Can this hang system resume if the same netdev is unregistered from a PM
callback while this work is still queued on the frozen workqueue?

This work is now the only path to the dev_put() that balances the
dev_hold() in xp_assign_dev():

xp_release_deferred()
  xp_clear_dev()
    dev_put(netdev);

The final put can arrive after the workqueues are frozen:

- xsk_release() has already taken the socket off net->xdp.list with
  sk_del_node_init_rcu().

- xsk_destruct() -> xp_put_pool() runs from the SOCK_RCU_FREE callback
  once sk_wmem_alloc drains. That drain can happen when pending copy-mode
  TX skbs are freed via xsk_destruct_skb() -> sock_wfree(). Two ways this
  can happen are a driver freeing TX skbs in its suspend callback and a
  qdisc reset during unregister.

If this final put happens after freeze_workqueues_begin(), the work does
not run until thaw_workqueues(). For suspend-to-RAM that is
suspend_finish() -> suspend_thaw_processes(), which runs after
dpm_resume_end().

Now suppose a resume callback unregisters that same netdev:

usb_resume()
  unbind_marked_interfaces()
    usb_forced_unbind_intf()
      usbnet_disconnect()
        unregister_netdev()
          netdev_run_todo()
            netdev_wait_allrefs_any()

This path is taken when usb_resume_interface() sets needs_binding on a
reset-resume and the driver has no .reset_resume. Several usbnet drivers
lack one, for example dm9601, rndis_host, cdc_eem, cdc_subset and aqc111.

netdev_wait_allrefs_any() loops with no timeout:

- Its NETDEV_UNREGISTER rebroadcasts reach xsk_notifier(), but that only
  walks net->xdp.list, so it cannot find the released socket's pool.

- Its rcu_barrier() does not run frozen work.

The resume thread never reaches thaw_workqueues(), so the netdev
reference is never dropped. With system_wq the work would have run and
called dev_put().

Copy-mode pools also take the freezable path, even though
xp_disable_drv_zc() only calls ndo_bpf() when pool->umem->zc is set.

Would it work to defer only the driver ndo_bpf() teardown to the
freezable queue, and never the dev_put()? Another option might be to let
xsk_notifier() handle NETDEV_UNREGISTER for pools whose release is still
pending, for example by tracking them per netdev.

Separately, the commit message says:

    newly queued destruction waits until device resume

Is this accurate? As far as I can tell, the work waits until process
thaw, which comes after every device resume and complete callback has
finished.

>  		return true;
>  	}
>
diff mbox series

Patch

diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 9d2d94f1fb75..c58f56f24a9c 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -337,7 +337,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;
 	}