On 11/6/2024 10:26 PM, Kalle Valo wrote:
> +static void ath12k_mac_unassign_link_sta(struct ath12k_hw *ah,
> + struct ath12k_sta *ahsta,
> + u8 link_id)
> +{
> + lockdep_assert_wiphy(ah->hw->wiphy);
> +
> + ahsta->links_map &= ~BIT(link_id);
> + rcu_assign_pointer(ahsta->link[link_id], NULL);
> +
> + synchronize_rcu();
this looks strange: generally we call synchronize_rcu() to wait for any RCU readers to finish, such that we can then safely free something. but here we do nothing ...
> +}
> +
> +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?
> +
> + if (arsta != &ahsta->deflink)
> + kfree(arsta);
I know the actual free happens here, but why split them?
these two hunks give me the impression that we may (in the future?) have cases to call ath12k_mac_unassign_link_sta() alone somewhere else rather than directly calling ath12k_mac_free_unassign_link_sta(). am I feeling right? what are those cases?
> +}
> +
