Karthikeyan Periyasamy <quic_periyasa@xxxxxxxxxxx> writes:
>>> static void ath12k_mac_op_cancel_hw_scan(struct ieee80211_hw *hw,
>>> struct ieee80211_vif *vif)
>>> {
>>> - struct ath12k *ar = hw->priv;
>>> + struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>>> + struct ath12k *ar;
>>> +
>>> + mutex_lock(&ah->conf_mutex);
>>> +
>>> + ar = ath12k_ah_to_ar(ah);
>>> mutex_lock(&ar->conf_mutex);
>>> ath12k_scan_abort(ar);
>>> mutex_unlock(&ar->conf_mutex);
>>> + mutex_unlock(&ah->conf_mutex);
>>> +
>>> cancel_delayed_work_sync(&ar->scan.timeout);
>>> }
>>
>> Do we really need two mutexes? I don't see any analysis about that. And
>> even if we do, I feel that it should be added in a separate patch.
>
> Yes, ah->conf_mutex protect the concurrent mac80211 operation. But
> there is other places like radio/link specific synchronous operation
> (ie MGMT tx wait for the vdev deletion) is needed. To fulfill this
> need, we also need radio/link specific (ar) mutex instead of all link
> (ah) mutex for efficient lock/unlock.
Are there any numbers to show the inefficiency? Anyway, I consider
adding new mutexes as an optimisation which could be done in a separate
patch with proper analysis.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
