wifi: iwlwifi: mvm: spin_lock_bh() to fix lockdep regression
authorHugh Dickins <hughd@google.com>
Fri, 9 Jun 2023 21:29:39 +0000 (14:29 -0700)
committerJohannes Berg <johannes.berg@intel.com>
Wed, 14 Jun 2023 07:05:51 +0000 (09:05 +0200)
Lockdep on 6.4-rc on ThinkPad X1 Carbon 5th says
=====================================================
WARNING: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected
6.4.0-rc5 #1 Not tainted
-----------------------------------------------------
kworker/3:1/49 [HC0[0]:SC0[4]:HE1:SE0] is trying to acquire:
ffff8881066fa368 (&mvm_sta->deflink.lq_sta.rs_drv.pers.lock){+.+.}-{2:2}, at: rs_drv_get_rate+0x46/0xe7

and this task is already holding:
ffff8881066f80a8 (&sta->rate_ctrl_lock){+.-.}-{2:2}, at: rate_control_get_rate+0xbd/0x126
which would create a new lock dependency:
 (&sta->rate_ctrl_lock){+.-.}-{2:2} -> (&mvm_sta->deflink.lq_sta.rs_drv.pers.lock){+.+.}-{2:2}

but this new dependency connects a SOFTIRQ-irq-safe lock:
 (&sta->rate_ctrl_lock){+.-.}-{2:2}
etc. etc. etc.

Changing the spin_lock() in rs_drv_get_rate() to spin_lock_bh() was not
enough to pacify lockdep, but changing them all on pers.lock has worked.

Fixes: a8938bc881d2 ("wifi: iwlwifi: mvm: Add locking to the rate read flow")
Signed-off-by: Hugh Dickins <hughd@google.com>
Link: https://lore.kernel.org/r/79ffcc22-9775-cb6d-3ffd-1a517c40beef@google.com
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
drivers/net/wireless/intel/iwlwifi/mvm/rs.c

index 23266d0..9a20468 100644 (file)
@@ -2692,7 +2692,7 @@ static void rs_drv_get_rate(void *mvm_r, struct ieee80211_sta *sta,
 
        lq_sta = mvm_sta;
 
-       spin_lock(&lq_sta->pers.lock);
+       spin_lock_bh(&lq_sta->pers.lock);
        iwl_mvm_hwrate_to_tx_rate_v1(lq_sta->last_rate_n_flags,
                                     info->band, &info->control.rates[0]);
        info->control.rates[0].count = 1;
@@ -2707,7 +2707,7 @@ static void rs_drv_get_rate(void *mvm_r, struct ieee80211_sta *sta,
                iwl_mvm_hwrate_to_tx_rate_v1(last_ucode_rate, info->band,
                                             &txrc->reported_rate);
        }
-       spin_unlock(&lq_sta->pers.lock);
+       spin_unlock_bh(&lq_sta->pers.lock);
 }
 
 static void *rs_drv_alloc_sta(void *mvm_rate, struct ieee80211_sta *sta,
@@ -3264,11 +3264,11 @@ void iwl_mvm_rs_tx_status(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
        /* If it's locked we are in middle of init flow
         * just wait for next tx status to update the lq_sta data
         */
-       if (!spin_trylock(&mvmsta->deflink.lq_sta.rs_drv.pers.lock))
+       if (!spin_trylock_bh(&mvmsta->deflink.lq_sta.rs_drv.pers.lock))
                return;
 
        __iwl_mvm_rs_tx_status(mvm, sta, tid, info, ndp);
-       spin_unlock(&mvmsta->deflink.lq_sta.rs_drv.pers.lock);
+       spin_unlock_bh(&mvmsta->deflink.lq_sta.rs_drv.pers.lock);
 }
 
 #ifdef CONFIG_MAC80211_DEBUGFS
@@ -4117,9 +4117,9 @@ void iwl_mvm_rs_rate_init(struct iwl_mvm *mvm,
        } else {
                struct iwl_mvm_sta *mvmsta = iwl_mvm_sta_from_mac80211(sta);
 
-               spin_lock(&mvmsta->deflink.lq_sta.rs_drv.pers.lock);
+               spin_lock_bh(&mvmsta->deflink.lq_sta.rs_drv.pers.lock);
                rs_drv_rate_init(mvm, sta, band);
-               spin_unlock(&mvmsta->deflink.lq_sta.rs_drv.pers.lock);
+               spin_unlock_bh(&mvmsta->deflink.lq_sta.rs_drv.pers.lock);
        }
 }