Baochen Qiang <quic_bqiang@xxxxxxxxxxx> writes:
>>>> +static void ath12k_mac_free_unassign_link_sta(struct ath12k_hw *ah,
>>>> + struct ath12k_sta *ahsta,
>>>> + u8 link_id)
>>>> +{
>>>> + struct ath12k_link_sta *arsta;
>>>> +
>>>> + lockdep_assert_wiphy(ah->hw->wiphy);
>>>> +
>>>> + if (WARN_ON(link_id >= IEEE80211_MLD_MAX_NUM_LINKS))
>>>> + return;
>>>> +
>>>> + arsta = wiphy_dereference(ah->hw->wiphy, ahsta->link[link_id]);
>>>> +
>>>> + if (WARN_ON(!arsta))
>>>> + return;
>>>> +
>>>> + ath12k_mac_unassign_link_sta(ah, ahsta, link_id);
>>>> +
>>>> + arsta->link_id = ATH12K_INVALID_LINK_ID;
>>>> + arsta->ahsta = NULL;
>>>> + arsta->arvif = NULL;
>>>
>>> if arsta is not deflink and would be freed, can we avoid these
>>> cleanup?
>>
>> I think that's something we can cleanup later if needed. Sure, it's
>> extra assignments but it's not really doing any harm.
> exactly, but ideally we should avoid unnecessary effort if possible.
>
>>
>>>> + if (arsta != &ahsta->deflink)
>>>> + kfree(arsta);
>>>
>>> I know the actual free happens here, but why split them?
>>
>> You mean why have a separate function ath12k_mac_unassign_link_sta() and
>> instead just have all code the in
>> ath12k_mac_free_unassign_link_sta()?
>
> yes. such that we can have synchronize_rcu() and kfree() located together.
Ok, I think I now get what you mean. Does this look better:
static void ath12k_mac_free_unassign_link_sta(struct ath12k_hw *ah,
struct ath12k_sta *ahsta,
u8 link_id)
{
struct ath12k_link_sta *arsta;
lockdep_assert_wiphy(ah->hw->wiphy);
if (WARN_ON(link_id >= IEEE80211_MLD_MAX_NUM_LINKS))
return;
arsta = wiphy_dereference(ah->hw->wiphy, ahsta->link[link_id]);
if (WARN_ON(!arsta))
return;
ahsta->links_map &= ~BIT(link_id);
rcu_assign_pointer(ahsta->link[link_id], NULL);
if (arsta == &ahsta->deflink) {
arsta->link_id = ATH12K_INVALID_LINK_ID;
arsta->ahsta = NULL;
arsta->arvif = NULL;
return;
}
synchronize_rcu();
kfree(arsta);
}
BTW your lines are really long, please check your settings:
https://patchwork.kernel.org/project/linux-wireless/patch/20241106142617.660901-2-kvalo@xxxxxxxxxx/
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
