Re: [PATCH RFC v2] wifi: ath10k: make in-order rx amsdu buffers persistent

[Richard Acayan <mailingradian@gmail.com>]

On Thu, Jul 30, 2026 at 07:24:00PM -0700, Jeff Johnson wrote:
> On 7/19/2026 2:45 PM, David Heidelberg via B4 Relay wrote:
> > From: Richard Acayan <mailingradian@gmail.com>
> > 
> > The WCN3990 might split MSDUs among multiple "in-order" indications. The
> > driver needs information from previous indications to handle MPDUs that
> > are not started by the same indications that complete them. Move the
> > list that tracks unprocessed MSDUs to the driver state so the driver can
> > handle MPDUs that are split in this way and be less confused.
> > 
> > Fixes: c545070e404b ("ath10k: implement rx reorder support")
> > Signed-off-by: Richard Acayan <mailingradian@gmail.com>
> > Co-developed-by: David Heidelberg <david@ixit.cz>
> > Signed-off-by: David Heidelberg <david@ixit.cz>
> > ---
> > This one we have in-tree forever.
> 
> I'm transcribing a few comments from my review agent (which may overlap
> Sashiko). I have not vetted them for correctness. Hopefully I placed them at
> the correct spots!
> 
> > 
> > Changes in v2:
> > - checkpatch & style. (Jeff)
> > - Improve comments.
> > - Link to v1: https://lore.kernel.org/linux-wireless/20260210021249.12132-2-mailingradian@gmail.com/
> > ---
> >  drivers/net/wireless/ath/ath10k/htt.h    |  4 +++
> >  drivers/net/wireless/ath/ath10k/htt_rx.c | 50 +++++++++++++++++++++++++-------
> >  2 files changed, 43 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
> > index 25c6b2e2f81c8..07c3dc088f07b 100644
> > --- a/drivers/net/wireless/ath/ath10k/htt.h
> > +++ b/drivers/net/wireless/ath/ath10k/htt.h
> > @@ -1924,16 +1924,20 @@ struct ath10k_htt {
> >  
> >  	bool tx_mem_allocated;
> >  	const struct ath10k_htt_tx_ops *tx_ops;
> >  	const struct ath10k_htt_rx_ops *rx_ops;
> >  	bool disable_tx_comp;
> >  	bool bundle_tx;
> >  	struct sk_buff_head tx_req_head;
> >  	struct sk_buff_head tx_complete_head;
> > +
> > +	u8 rx_in_ord_split_tid;
> > +	u16 rx_in_ord_split_peer_id;
> > +	struct sk_buff_head rx_in_ord_split;
> >  };
> >  
> >  struct ath10k_htt_tx_ops {
> >  	int (*htt_send_rx_ring_cfg)(struct ath10k_htt *htt);
> >  	int (*htt_send_frag_desc_bank_cfg)(struct ath10k_htt *htt);
> >  	int (*htt_alloc_frag_desc)(struct ath10k_htt *htt);
> >  	void (*htt_free_frag_desc)(struct ath10k_htt *htt);
> >  	int (*htt_tx)(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txmode,
> > diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> > index ab2d373b4750d..732bd3a2f9992 100644
> > --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> > +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> > @@ -291,16 +291,18 @@ void ath10k_htt_rx_free(struct ath10k_htt *htt)
> >  		return;
> >  
> >  	timer_delete_sync(&htt->rx_ring.refill_retry_timer);
> >  
> >  	skb_queue_purge(&htt->rx_msdus_q);
> >  	skb_queue_purge(&htt->rx_in_ord_compl_q);
> >  	skb_queue_purge(&htt->tx_fetch_ind_q);
> >  
> > +	skb_queue_purge(&htt->rx_in_ord_split);
> > +
> >  	spin_lock_bh(&htt->rx_ring.lock);
> >  	ath10k_htt_rx_ring_free(htt);
> >  	spin_unlock_bh(&htt->rx_ring.lock);
> >  
> >  	dma_free_coherent(htt->ar->dev,
> >  			  ath10k_htt_get_rx_ring_size(htt),
> >  			  ath10k_htt_get_vaddr_ring(htt),
> >  			  htt->rx_ring.base_paddr);
> > @@ -841,16 +843,18 @@ int ath10k_htt_rx_alloc(struct ath10k_htt *htt)
> >  	htt->rx_ring.sw_rd_idx.msdu_payld = 0;
> >  	hash_init(htt->rx_ring.skb_table);
> >  
> >  	skb_queue_head_init(&htt->rx_msdus_q);
> >  	skb_queue_head_init(&htt->rx_in_ord_compl_q);
> >  	skb_queue_head_init(&htt->tx_fetch_ind_q);
> >  	atomic_set(&htt->num_mpdus_ready, 0);
> >  
> > +	skb_queue_head_init(&htt->rx_in_ord_split);
> > +
> >  	ath10k_dbg(ar, ATH10K_DBG_BOOT, "htt rx ring size %d fill_level %d\n",
> >  		   htt->rx_ring.size, htt->rx_ring.fill_level);
> >  	return 0;
> >  
> >  err_dma_idx:
> >  	dma_free_coherent(htt->ar->dev,
> >  			  ath10k_htt_get_rx_ring_size(htt),
> >  			  vaddr_ring,
> > @@ -3156,16 +3160,20 @@ static int ath10k_htt_rx_extract_amsdu(struct ath10k_hw_params *hw,
> >  	struct rx_msdu_end_common *rxd_msdu_end_common;
> >  
> >  	if (skb_queue_empty(list))
> >  		return -ENOBUFS;
> >  
> >  	if (WARN_ON(!skb_queue_empty(amsdu)))
> >  		return -EINVAL;
> >  
> > +	msdu = skb_peek(list);
> > +	rxd = HTT_RX_BUF_TO_RX_DESC(hw,
> > +				    (void *)msdu->data - hw->rx_desc_ops->rx_desc_size);
> 
> Dead rxd computation before the loop — VALID, MINOR
> 
> Lines 3168–3170 compute rxd via skb_peek(), but the while loop at 3172
> immediately dequeues the same SKB and unconditionally recomputes rxd at lines
> 3175–3177. The pre-loop assignment is never read. It's dead code. The original
> patch must have introduced this when restructuring (the pre-existing code
> likely used rxd from before the loop). It should be removed.

This was removed on 20 January 2026 before v1 was submitted. It seems
this v2 is based on applying the commits to the pre-2026 patch.