On 7/9/2024 5:06 AM, Rameshkumar Sundaram wrote:
...
> @@ -4569,12 +4685,17 @@ static int ath12k_mac_op_sta_state(struct ieee80211_hw *hw,
> enum ieee80211_sta_state old_state,
> enum ieee80211_sta_state new_state)
> {
> + struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
> + struct ath12k_vif *ahvif = ath12k_vif_to_ahvif(vif);
> struct ath12k *ar;
> - struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
> struct ath12k_sta *arsta = ath12k_sta_to_arsta(sta);
> + struct ath12k_link_vif *arvif;
> struct ath12k_peer *peer;
> int ret = 0;
>
> + mutex_lock(&ah->conf_mutex);
> + arvif = &ahvif->deflink;
> +
> /* cancel must be done outside the mutex to avoid deadlock */
this is now inside the ah->conf_mutex -- is that ok?
seems this comment should be updated to explain *which* mutex(es) the cancel
must be done outside of?
> if ((old_state == IEEE80211_STA_NONE &&
> new_state == IEEE80211_STA_NOTEXIST))
> @@ -4583,9 +4704,9 @@ static int ath12k_mac_op_sta_state(struct ieee80211_hw *hw,
> ar = ath12k_get_ar_by_vif(hw, vif);
> if (!ar) {
> WARN_ON_ONCE(1);
> + mutex_unlock(&ah->conf_mutex);
unlock before the WARN -- the WARN is not part of the critical section
> return -EINVAL;
> }
> -
> mutex_lock(&ar->conf_mutex);
>
> if (old_state == IEEE80211_STA_NOTEXIST &&
