Re: [PATCH v2 3/3] wifi: ath12k: implement custom wake_tx_queue with flow control
Tamizh Raja <[email protected]> Thu, 16 Jul 2026 07:27:30 +0530
| Newsgroups | org.infradead.lists.ath11k,org.infradead.lists.ath12k,org.kernel.vger.linux-kernel,org.kernel.vger.linux-wireless |
|---|---|
| Message-ID | <CABkEBKa26vR52mX85t4xuZbrXz0FiRc0O_NeDFSNAQpSx=RR7g@mail.gmail.com> |
On Wed, Jul 15, 2026 at 6:21 PM Jose Ignacio Tornos Martinez <[email protected]> wrote: > > Under heavy traffic, ath12k can hang and experiences -ENOMEM errors > ("failed to transmit frame -12") when the hardware TCL ring fills up. > This issue is more commonly observed in VMs with PCIe passthrough but > also occurs on bare metal systems. > > Implement a custom wake_tx_queue operation that: > > 1. Checks hardware ring space before dequeuing packets from mac80211 > 2. Uses per-ring locking to serialize ring access and prevent races > 3. Syncs with hardware state to get accurate free slot count > 4. Uses ieee80211_tx_peek() to determine the exact target ring via > get_ring_selector(), matching dp_tx on all platforms > 5. Returns early during firmware crash in the same way as other > tx paths > > This approach follows the pattern used in the iwlwifi driver, adapted > for ath12k's hardware ring architecture. > > This prevents hangs, eliminates -ENOMEM errors, and improves throughput > by optimizing resource usage and preventing unnecessary packet drops. > > Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]> ..... > drivers/net/wireless/ath/ath12k/dp.c | 1 + > drivers/net/wireless/ath/ath12k/dp.h | 2 + > drivers/net/wireless/ath/ath12k/hal.c | 1 + > drivers/net/wireless/ath/ath12k/wifi7/hw.c | 65 +++++++++++++++++++++- > 4 files changed, 68 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/ath/ath12k/dp.c b/drivers/net/wireless/ath/ath12k/dp.c > index af5f11fc1d84..3d46cfbf0a1c 100644 > --- a/drivers/net/wireless/ath/ath12k/dp.c > +++ b/drivers/net/wireless/ath/ath12k/dp.c > @@ -1539,6 +1539,7 @@ static int ath12k_dp_setup(struct ath12k_base *ab) > } > > for (i = 0; i < ab->hw_params->max_tx_ring; i++) { > + spin_lock_init(&dp->tx_ring[i].wake_tx_lock); > dp->tx_ring[i].tcl_data_ring_id = i; > > dp->tx_ring[i].tx_status_head = 0; > diff --git a/drivers/net/wireless/ath/ath12k/dp.h b/drivers/net/wireless/ath/ath12k/dp.h > index f8cfc7bb29dd..68d2020be9b8 100644 > --- a/drivers/net/wireless/ath/ath12k/dp.h > +++ b/drivers/net/wireless/ath/ath12k/dp.h > @@ -58,6 +58,8 @@ struct dp_tx_ring { > u8 tcl_data_ring_id; > struct dp_srng tcl_data_ring; > struct dp_srng tcl_comp_ring; > + /* Serializes wake_tx_queue operations for this ring */ > + spinlock_t wake_tx_lock; > struct hal_wbm_completion_ring_tx *tx_status; > int tx_status_head; > int tx_status_tail; > diff --git a/drivers/net/wireless/ath/ath12k/hal.c b/drivers/net/wireless/ath/ath12k/hal.c > index a164563fff28..c1c656e4550b 100644 > --- a/drivers/net/wireless/ath/ath12k/hal.c > +++ b/drivers/net/wireless/ath/ath12k/hal.c > @@ -390,6 +390,7 @@ int ath12k_hal_srng_src_num_free(struct ath12k_base *ab, struct hal_srng *srng, > else > return ((srng->ring_size - hp + tp) / srng->entry_size) - 1; > } > +EXPORT_SYMBOL(ath12k_hal_srng_src_num_free); > > void *ath12k_hal_srng_src_next_peek(struct ath12k_base *ab, > struct hal_srng *srng) > diff --git a/drivers/net/wireless/ath/ath12k/wifi7/hw.c b/drivers/net/wireless/ath/ath12k/wifi7/hw.c > index d9fdd2fc8298..306d51da3ea1 100644 > --- a/drivers/net/wireless/ath/ath12k/wifi7/hw.c > +++ b/drivers/net/wireless/ath/ath12k/wifi7/hw.c > @@ -1100,9 +1100,72 @@ static void ath12k_wifi7_mac_op_tx(struct ieee80211_hw *hw, > } > } > > +static void ath12k_wifi7_mac_op_wake_tx_queue(struct ieee80211_hw *hw, > + struct ieee80211_txq *txq) > +{ > + struct ath12k_vif *ahvif = ath12k_vif_to_ahvif(txq->vif); Is txq->vif always valid? Do we need a NULL check here? > + struct ath12k_link_vif *arvif = &ahvif->deflink; > + struct ieee80211_tx_control control = { > + .sta = txq->sta, > + }; > + const struct sk_buff *peek_skb; > + struct dp_tx_ring *tx_ring; > + struct hal_srng *tcl_ring; > + struct ath12k_dp *dp; > + struct sk_buff *skb; > + struct ath12k *ar; > + u32 ring_selector; > + int num_free; > + u8 ring_id; > + > + ar = arvif->ar; Shouldn't we check the link_id for fetching proper arvif? this code always chooses deflink ar and dp for packet transmission. This needs to use the same link selection logic as op_tx. > + if (!ar) > + return; > + > + dp = ar->ab->dp; > + > + while (1) { > + if (unlikely(test_bit(ATH12K_FLAG_CRASH_FLUSH, > + &ar->ab->dev_flags))) > + break; > + > + peek_skb = ieee80211_tx_peek(hw, txq); > + if (!peek_skb) > + break; > + > + ring_selector = dp->hw_params->hw_ops->get_ring_selector( > + (struct sk_buff *)peek_skb); > + ring_id = ring_selector % dp->hw_params->max_tx_ring; > + > + tx_ring = &dp->tx_ring[ring_id]; > + tcl_ring = &dp->hal->srng_list[tx_ring->tcl_data_ring.ring_id]; > + > + spin_lock_bh(&tx_ring->wake_tx_lock); > + > + spin_lock(&tcl_ring->lock); > + num_free = ath12k_hal_srng_src_num_free(ar->ab, tcl_ring, true); > + spin_unlock(&tcl_ring->lock); > + > + if (num_free == 0) { > + spin_unlock_bh(&tx_ring->wake_tx_lock); > + break; > + } > + > + skb = ieee80211_tx_dequeue(hw, txq); > + if (!skb) { > + spin_unlock_bh(&tx_ring->wake_tx_lock); > + break; > + } > + > + ath12k_wifi7_mac_op_tx(hw, &control, skb); > + > + spin_unlock_bh(&tx_ring->wake_tx_lock); > + } > +} > + > static const struct ieee80211_ops ath12k_ops_wifi7 = { > .tx = ath12k_wifi7_mac_op_tx, > - .wake_tx_queue = ieee80211_handle_wake_tx_queue, > + .wake_tx_queue = ath12k_wifi7_mac_op_wake_tx_queue, > .start = ath12k_mac_op_start, > .stop = ath12k_mac_op_stop, > .reconfig_complete = ath12k_mac_op_reconfig_complete, > -- > 2.54.0 > > -- - Tamizh.