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

[Jeff Johnson]

On 10/9/2026 6:31 PM, Jeff Johnson wrote:
> 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@xxxxxxx>
>> ---
>>  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.

BTW no need to send a new version to address this unless reviewers have other
issues -- I'll make this change when applying the patches to my 'pending' branch