Baochen Qiang <quic_bqiang@xxxxxxxxxxx> writes:
> On 11/6/2024 10:26 PM, Kalle Valo wrote:
>> +static int ath12k_mac_assign_link_sta(struct ath12k_hw *ah,
>> + struct ath12k_sta *ahsta,
>> + struct ath12k_link_sta *arsta,
>> + struct ath12k_vif *ahvif,
>> + u8 link_id)
>> +{
>> + struct ieee80211_sta *sta = ath12k_ahsta_to_sta(ahsta);
>> + struct ieee80211_link_sta *link_sta;
>> + struct ath12k_link_vif *arvif;
>> +
>> + lockdep_assert_wiphy(ah->hw->wiphy);
>> +
>> + if (!arsta || link_id >= IEEE80211_MLD_MAX_NUM_LINKS)
>> + return -EINVAL;
>> +
>> + arvif = wiphy_dereference(ah->hw->wiphy, ahvif->link[link_id]);
>> + if (!arvif)
>> + return -EINVAL;
>> +
>> + memset(arsta, 0, sizeof(*arsta));
>> +
>> + link_sta = wiphy_dereference(ah->hw->wiphy, sta->link[link_id]);
>> + if (!link_sta)
>> + return -EINVAL;
>> +
>> + ether_addr_copy(arsta->addr, link_sta->addr);
>> +
>> + /* logical index of the link sta in order of creation */
>> + arsta->link_idx = ahsta->num_peer++;
>> +
>> + arsta->link_id = link_id;
>> + ahsta->links_map |= BIT(arsta->link_id);
>
> would be better to put this after rcu_assign_pointer()?
My thinking is that it's racy anyway so it doesn't really matter.
links_map is not really protected properly right now but luckily there's
only one function which accesses outside of the wiphy lock. My plan is
to fix that in a later patch.
>> + arsta->arvif = arvif;
>> + arsta->ahsta = ahsta;
>> + arsta->state = IEEE80211_STA_NONE;
>> + wiphy_work_init(&arsta->update_wk, ath12k_sta_rc_update_wk);
>> +
>> + rcu_assign_pointer(ahsta->link[link_id], arsta);
>> +
>> + synchronize_rcu();
>
> what are we waiting for here?
That's a good question. I didn't analyse that thoroughly but I'm just
making sure here that all readers have access to the new arsta before we
return to mac80211. The delay shouldn't be that long anyway, I hope.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
