]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
Reapply "bnxt_en: bring back rtnl_lock() in the bnxt_open() path"
authorJakub Kicinski <kuba@kernel.org>
Wed, 3 Jun 2026 19:58:45 +0000 (12:58 -0700)
committerJakub Kicinski <kuba@kernel.org>
Thu, 4 Jun 2026 16:02:37 +0000 (09:02 -0700)
This reverts commit 850d9248d2eac662f869c766a598c877690c74e5.
This reapplies commit 325eb217e41f ("bnxt_en: bring back rtnl_lock()
in the bnxt_open() path").

Breno reports a lockdep warning in bnxt. During FW reset the driver
may end up calling netif_set_real_num_tx_queues() (if queue count
changes), so calls to bnxt_open() still require rtnl_lock.

  net/sched/sch_generic.c:1416 suspicious rcu_dereference_protected() usage!

   dev_qdisc_change_real_num_tx+0x54/0xe0
   netif_set_real_num_tx_queues+0x4ed/0xa80
   __bnxt_open_nic+0x9cb/0x3490
   bnxt_open+0x1cb/0x370
   bnxt_fw_reset_task+0x80d/0x1e80
   process_scheduled_works+0x9c1/0x13b0

The reverted commit was just an optimization / experiment
so let's go back to taking the lock.

Reported-by: Breno Leitao <leitao@debian.org>
Link: https://lore.kernel.org/ah726OtFX-Qw3U-R@gmail.com
Fixes: 850d9248d2ea ("Revert "bnxt_en: bring back rtnl_lock() in the bnxt_open() path"")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Reviewed-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20260603195845.2574426-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
drivers/net/ethernet/broadcom/bnxt/bnxt.c

index 008c34cff7b46cd9e799a3a2a031ed75705fa9be..35e1f8f663c78ed757b872028ff13d1b19f98e7a 100644 (file)
@@ -14388,13 +14388,28 @@ static void bnxt_unlock_sp(struct bnxt *bp)
        netdev_unlock(bp->dev);
 }
 
+/* Same as bnxt_lock_sp() with additional rtnl_lock */
+static void bnxt_rtnl_lock_sp(struct bnxt *bp)
+{
+       clear_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
+       rtnl_lock();
+       netdev_lock(bp->dev);
+}
+
+static void bnxt_rtnl_unlock_sp(struct bnxt *bp)
+{
+       set_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
+       netdev_unlock(bp->dev);
+       rtnl_unlock();
+}
+
 /* Only called from bnxt_sp_task() */
 static void bnxt_reset(struct bnxt *bp, bool silent)
 {
-       bnxt_lock_sp(bp);
+       bnxt_rtnl_lock_sp(bp);
        if (test_bit(BNXT_STATE_OPEN, &bp->state))
                bnxt_reset_task(bp, silent);
-       bnxt_unlock_sp(bp);
+       bnxt_rtnl_unlock_sp(bp);
 }
 
 /* Only called from bnxt_sp_task() */
@@ -14402,9 +14417,9 @@ static void bnxt_rx_ring_reset(struct bnxt *bp)
 {
        int i;
 
-       bnxt_lock_sp(bp);
+       bnxt_rtnl_lock_sp(bp);
        if (!test_bit(BNXT_STATE_OPEN, &bp->state)) {
-               bnxt_unlock_sp(bp);
+               bnxt_rtnl_unlock_sp(bp);
                return;
        }
        /* Disable and flush TPA before resetting the RX ring */
@@ -14443,7 +14458,7 @@ static void bnxt_rx_ring_reset(struct bnxt *bp)
        }
        if (bp->flags & BNXT_FLAG_TPA)
                bnxt_set_tpa(bp, true);
-       bnxt_unlock_sp(bp);
+       bnxt_rtnl_unlock_sp(bp);
 }
 
 static void bnxt_fw_fatal_close(struct bnxt *bp)
@@ -15358,15 +15373,17 @@ static void bnxt_fw_reset_task(struct work_struct *work)
                bp->fw_reset_state = BNXT_FW_RESET_STATE_OPENING;
                fallthrough;
        case BNXT_FW_RESET_STATE_OPENING:
-               while (!netdev_trylock(bp->dev)) {
+               while (!rtnl_trylock()) {
                        bnxt_queue_fw_reset_work(bp, HZ / 10);
                        return;
                }
+               netdev_lock(bp->dev);
                rc = bnxt_open(bp->dev);
                if (rc) {
                        netdev_err(bp->dev, "bnxt_open() failed during FW reset\n");
                        bnxt_fw_reset_abort(bp, rc);
                        netdev_unlock(bp->dev);
+                       rtnl_unlock();
                        goto ulp_start;
                }
 
@@ -15386,6 +15403,7 @@ static void bnxt_fw_reset_task(struct work_struct *work)
                        bnxt_dl_health_fw_status_update(bp, true);
                }
                netdev_unlock(bp->dev);
+               rtnl_unlock();
                bnxt_ulp_start(bp);
                bnxt_reenable_sriov(bp);
                netdev_lock(bp->dev);
@@ -16379,7 +16397,7 @@ err_reset:
                   rc);
        napi_enable_locked(&bnapi->napi);
        bnxt_db_nq_arm(bp, &cpr->cp_db, cpr->cp_raw_cons);
-       bnxt_reset_task(bp, true);
+       netif_close(dev);
        return rc;
 }
 
@@ -17230,6 +17248,7 @@ static int bnxt_resume(struct device *device)
        struct bnxt *bp = netdev_priv(dev);
        int rc = 0;
 
+       rtnl_lock();
        netdev_lock(dev);
        rc = pci_enable_device(bp->pdev);
        if (rc) {
@@ -17274,6 +17293,7 @@ static int bnxt_resume(struct device *device)
 
 resume_exit:
        netdev_unlock(bp->dev);
+       rtnl_unlock();
        if (!rc) {
                bnxt_ulp_start(bp);
                bnxt_reenable_sriov(bp);
@@ -17445,6 +17465,7 @@ static void bnxt_io_resume(struct pci_dev *pdev)
        int err;
 
        netdev_info(bp->dev, "PCI Slot Resume\n");
+       rtnl_lock();
        netdev_lock(netdev);
 
        err = bnxt_hwrm_func_qcaps(bp);
@@ -17462,6 +17483,7 @@ static void bnxt_io_resume(struct pci_dev *pdev)
                netif_device_attach(netdev);
 
        netdev_unlock(netdev);
+       rtnl_unlock();
        if (!err) {
                bnxt_ulp_start(bp);
                bnxt_reenable_sriov(bp);