On 2024/5/23 20:55, Kalle Valo wrote:
Lingbo Kong <quic_lingbok@xxxxxxxxxxx> writes:
+struct ath12k_vif *ath12k_mac_get_vif_up(struct ath12k_base *ab)
+{
+ struct ath12k *ar;
+ struct ath12k_pdev *pdev;
+ struct ath12k_vif *arvif;
+ int i;
+
+ for (i = 0; i < ab->num_radios; i++) {
+ pdev = &ab->pdevs[i];
+ ar = pdev->ar;
+ list_for_each_entry(arvif, &ar->arvifs, list) {
+ if (arvif->is_up)
+ return arvif;
+ }
+ }
+
+ return NULL;
+}
I'm not seeing any protection here, is that on purpose?
you means there need to add lockdep_assert_held(&ar->conf_mutex)?
I mean what's the locking design here? Is it safe to concurrectly access
ab->pdevs and arvif->is_up?
oh, i've seen other places use ar->conf_mutex to protect when accessing
arvif->is_up.
but according to the ath12k_mac_get_vif_up()'s call stack, the
ath12k_write_htt_stats_reset() and ath12k_open_htt_stats() have already
executed mutex_lock(&ar->conf_mutex);
so it's best to add lockdep_assert_held(&ar->conf_mutex) here to
determine whether conf_mutex is obtained.
/lingbo kong