[PATCH rdma-next] IB/IPoIB: Cancel normal flush work on device stop

[PATCH rdma-next] IB/IPoIB: Cancel normal flush work on device stop

[Edward Srouji]

From: Carolina Jubran <cjubran@nvidia.com>

ipoib_ib_dev_flush_normal() can race with ipoib_stop() because the normal
flush work does not run under RTNL.

Before commit a77a57a1a22a ("IPoIB: Fix deadlock on RTNL in
ipoib_stop()"), ipoib_stop() flushed ipoib_workqueue after clearing
IPOIB_FLAG_ADMIN_UP and before calling ipoib_ib_dev_down(). This ensured
that any in-flight flush work completed before ipoib_ib_dev_down()
performed the final clear of IPOIB_FLAG_OPER_UP.

After that commit removed the workqueue flush to avoid a deadlock with
workqueue tasks taking RTNL, a normal flush can continue running
concurrently with ipoib_stop() after IPOIB_FLAG_ADMIN_UP has been
cleared.

The flush may observe IPOIB_FLAG_ADMIN_UP set and continue running while
ipoib_stop() clears IPOIB_FLAG_ADMIN_UP and brings the device down. The
flush can then call ipoib_ib_dev_up() and set IPOIB_FLAG_OPER_UP again
after the device has already been stopped:

  flush_normal (ipoib_workqueue)    ipoib_stop() (RTNL held)
  ------------------------------    ------------------------
  test_bit(ADMIN_UP) == 1
                                    clear_bit(ADMIN_UP)
                                    ipoib_ib_dev_down()
                                      clear_bit(OPER_UP)
  ipoib_ib_dev_up()
    set_bit(OPER_UP)

This leaves the interface with ADMIN_UP clear and OPER_UP set. During
device removal, ipoib_ndo_uninit() waits in destroy_workqueue() while
holding RTNL, while ipoib_mcast_carrier_on_task() may keep retrying
rtnl_trylock() as long as OPER_UP remains set, resulting in a deadlock.

Cancel the normal flush work from ipoib_stop() after clearing
IPOIB_FLAG_ADMIN_UP and before bringing the device down. A pending normal
flush is canceled, while an already running flush is allowed to finish
before ipoib_ib_dev_down() performs the final transition to OPER_UP
clear.

Only the normal flush needs this synchronization. The heavy flush is
already serialized with ipoib_stop() by RTNL, while the light flush does
not bring the device back up and therefore cannot recreate this race.

Fixes: a77a57a1a22a ("IPoIB: Fix deadlock on RTNL in ipoib_stop()")
Reported-by: John Popplewell <john.popplewell@nmc2.ai>
Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Edward Srouji <edwards@nvidia.com>
---
 drivers/infiniband/ulp/ipoib/ipoib_main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 6c14246befb1f96483a1c634e533a60c0b171fdb..f0b72d65fdeb71fd1b9680b90342a5a756bcac1d 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -230,6 +230,13 @@ static int ipoib_stop(struct net_device *dev)
 
 	clear_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags);
 
+	/* Normal flush runs without RTNL and may bring the device back
+	 * up. Cancel it before the device down transition.
+	 * Heavy flush is already serialized by RTNL and must not be
+	 * waited on here while RTNL is held.
+	 */
+	cancel_work_sync(&priv->flush_normal);
+
 	netif_stop_queue(dev);
 
 	ipoib_ib_dev_down(dev);

---
base-commit: d37e98844afea39a9614cac0de837701263c90f7
change-id: 20261014-ipoib-cancel-work-1d526f705a9c

Best regards,
-- 
Edward Srouji <edwards@nvidia.com>

Re: [PATCH rdma-next] IB/IPoIB: Cancel normal flush work on device stop

[sashiko-bot]

> From: Carolina Jubran <cjubran@nvidia.com>
> 
> ipoib_ib_dev_flush_normal() can race with ipoib_stop() because the normal
> flush work does not run under RTNL.
> 
> Before commit a77a57a1a22a ("IPoIB: Fix deadlock on RTNL in
> ipoib_stop()"), ipoib_stop() flushed ipoib_workqueue after clearing
> IPOIB_FLAG_ADMIN_UP and before calling ipoib_ib_dev_down(). This ensured
> that any in-flight flush work completed before ipoib_ib_dev_down()
> performed the final clear of IPOIB_FLAG_OPER_UP.
> 
> After that commit removed the workqueue flush to avoid a deadlock with
> workqueue tasks taking RTNL, a normal flush can continue running
> concurrently with ipoib_stop() after IPOIB_FLAG_ADMIN_UP has been
> cleared.
> [ ... ]
> Fixes: a77a57a1a22a ("IPoIB: Fix deadlock on RTNL in ipoib_stop()")
> Reported-by: John Popplewell <john.popplewell@nmc2.ai>
> Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
> Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
> Signed-off-by: Edward Srouji <edwards@nvidia.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-ipoib-cancel-work-v1-1-6c0faac016b8@nvidia.com?part=1