Re: [PATCH 4/5] wifi: ath12k: avoid IDR mutation during pending mgmt TX cleanup

[Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>]

On 9/26/2026 9:18 PM, Jiale Yao wrote:
> ath12k_mac_tx_mgmt_pending_free() is passed as an idr_for_each()
> callback and removes the current entry from txmgmt_idr. This can
> invalidate the radix-tree iterator retained by idr_for_each().
> 
> Both callers destroy the IDR immediately after the walk, so removing
> each entry in the callback is unnecessary. Split skb release from IDR
> removal and let the callback release the supplied skb without updating
> the IDR. The following idr_destroy() tears down the IDR itself.
> 
> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>   drivers/net/wireless/ath/ath12k/mac.c | 25 +++++++++++++++----------
>   1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 99bf5cf79d10..7695b21149d1 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -9166,18 +9166,11 @@ static void ath12k_mgmt_over_wmi_tx_drop(struct ath12k *ar, struct sk_buff *skb)
>   		wake_up(&ar->txmgmt_empty_waitq);
>   }
>   
> -static void ath12k_mac_tx_mgmt_free(struct ath12k *ar, int buf_id)
> +static void ath12k_mac_tx_mgmt_free_skb(struct ath12k *ar,
> +					struct sk_buff *msdu)
>   {
> -	struct sk_buff *msdu;
>   	struct ieee80211_tx_info *info;
>   
> -	spin_lock_bh(&ar->txmgmt_idr_lock);
> -	msdu = idr_remove(&ar->txmgmt_idr, buf_id);
> -	spin_unlock_bh(&ar->txmgmt_idr_lock);
> -
> -	if (!msdu)
> -		return;
> -
>   	dma_unmap_single(ar->ab->dev, ATH12K_SKB_CB(msdu)->paddr, msdu->len,
>   			 DMA_TO_DEVICE);
>   
> @@ -9187,11 +9180,23 @@ static void ath12k_mac_tx_mgmt_free(struct ath12k *ar, int buf_id)
>   	ath12k_mgmt_over_wmi_tx_drop(ar, msdu);
>   }
>   
> +static void ath12k_mac_tx_mgmt_free(struct ath12k *ar, int buf_id)
> +{
> +	struct sk_buff *msdu;
> +
> +	spin_lock_bh(&ar->txmgmt_idr_lock);
> +	msdu = idr_remove(&ar->txmgmt_idr, buf_id);
> +	spin_unlock_bh(&ar->txmgmt_idr_lock);
> +
> +	if (msdu)
> +		ath12k_mac_tx_mgmt_free_skb(ar, msdu);
> +}
> +
>   int ath12k_mac_tx_mgmt_pending_free(int buf_id, void *skb, void *ctx)
>   {
>   	struct ath12k *ar = ctx;
>   
> -	ath12k_mac_tx_mgmt_free(ar, buf_id);
> +	ath12k_mac_tx_mgmt_free_skb(ar, skb);

this now keeps the freed skbs in the idr for the whole time 
idr_for_each() runs as opposed to previous implementation which removed 
the idr entry as well which looked safe.


>   
>   	return 0;
>   }


-- 
Ramesh