Re: [PATCH v2 1/5] wifi: ath10k: avoid IDR mutation during TX cleanup

[Jeff Johnson <jeff.johnson@oss.qualcomm.com>]

On 10/3/2026 2:20 AM, Jiale Yao wrote:
> ath10k_htt_flush_tx_queue() walks pending_tx with idr_for_each().
> Its callback calls ath10k_txrx_tx_unref(), which removes the current
> entry from pending_tx through ath10k_htt_tx_free_msdu_id().
> 
> idr_for_each() keeps radix-tree iterator state across the callback.
> Removing the current entry can therefore invalidate that state and
> make the remaining walk unsafe.
> 
> Use idr_for_each_entry(), which starts a fresh lookup after each
> callback.  The current entry can then be removed safely, while the
> following idr_destroy() continues to release the IDR itself.
> 
> Fixes: 89d6d83565e9 ("ath10k: use idr api for msdu_ids")
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  drivers/net/wireless/ath/ath10k/htt_tx.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
> index e46f579d745d..9993c5a2f96c 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
> @@ -535,8 +535,13 @@ void ath10k_htt_tx_destroy(struct ath10k_htt *htt)
>  
>  static void ath10k_htt_flush_tx_queue(struct ath10k_htt *htt)
>  {
> +	struct sk_buff *msdu;
> +	int msdu_id;
> +
>  	ath10k_htc_stop_hl(htt->ar);
> -	idr_for_each(&htt->pending_tx, ath10k_htt_tx_clean_up_pending, htt->ar);
> +
> +	idr_for_each_entry(&htt->pending_tx, msdu, msdu_id)
> +		ath10k_htt_tx_clean_up_pending(msdu_id, msdu, htt->ar);

my review agent notes:
**Actionable issue:** `ath10k_htt_tx_clean_up_pending` retains its old
`idr_for_each`-callback signature even though it's now called directly:

```c
static int ath10k_htt_tx_clean_up_pending(int msdu_id, void *skb, void *ctx)
```

The `skb` parameter is completely unused inside the function — it's not
read anywhere. Since this is a `static` function no longer used as a
callback, the signature should be cleaned up in the same patch:

- Remove the unused `void *skb` parameter
- Change `void *ctx` to `struct ath10k *ar`
- Make the return type `void` (the `return 0` is vestigial)

As-is, the call site `ath10k_htt_tx_clean_up_pending(msdu_id, msdu, htt->ar)`
passes `msdu` as a `void *` that the callee silently discards — confusing
for future readers.

>  }
>  
>  void ath10k_htt_tx_stop(struct ath10k_htt *htt)