Commit f1a0898b authored by Hugh Dickins's avatar Hugh Dickins Committed by Johannes Berg

wifi: iwlwifi: mvm: spin_lock_bh() to fix lockdep regression

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: a8938bc8 ("wifi: iwlwifi: mvm: Add locking to the rate read flow")
Signed-off-by: default avatarHugh Dickins <hughd@google.com>
Link: https://lore.kernel.org/r/79ffcc22-9775-cb6d-3ffd-1a517c40beef@google.comSigned-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
parent d094482c
...@@ -2692,7 +2692,7 @@ static void rs_drv_get_rate(void *mvm_r, struct ieee80211_sta *sta, ...@@ -2692,7 +2692,7 @@ static void rs_drv_get_rate(void *mvm_r, struct ieee80211_sta *sta,
lq_sta = mvm_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, iwl_mvm_hwrate_to_tx_rate_v1(lq_sta->last_rate_n_flags,
info->band, &info->control.rates[0]); info->band, &info->control.rates[0]);
info->control.rates[0].count = 1; info->control.rates[0].count = 1;
...@@ -2707,7 +2707,7 @@ static void rs_drv_get_rate(void *mvm_r, struct ieee80211_sta *sta, ...@@ -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, iwl_mvm_hwrate_to_tx_rate_v1(last_ucode_rate, info->band,
&txrc->reported_rate); &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, 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, ...@@ -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 /* If it's locked we are in middle of init flow
* just wait for next tx status to update the lq_sta data * 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; return;
__iwl_mvm_rs_tx_status(mvm, sta, tid, info, ndp); __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 #ifdef CONFIG_MAC80211_DEBUGFS
...@@ -4117,9 +4117,9 @@ void iwl_mvm_rs_rate_init(struct iwl_mvm *mvm, ...@@ -4117,9 +4117,9 @@ void iwl_mvm_rs_rate_init(struct iwl_mvm *mvm,
} else { } else {
struct iwl_mvm_sta *mvmsta = iwl_mvm_sta_from_mac80211(sta); 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); 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);
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment