Skip to contentSkip to conversation

wifi: ath11k: isolate RXDMA page-frag lifetimes

RFC PATCH In review Mark Ruvald Pedersen 2 participants 6 messages Jul 19 – Jul 29 Qualcomm Atheros ATH11K wireless driver

lore
Share & tools
lore
#0 lore raw

cover wifi: ath11k: isolate RXDMA page-frag lifetimes

ath11k fills RXDMA rings with skb heads allocated by dev_alloc_skb(). Those allocations use shared per-CPU page-frag caches, so unrelated RXDMA rings and other network users can place fragments with different lifetimes on the same backing page.

On an IPQ8074-based Linksys MX4200, repeater/AP traffic made available memory fall at roughly 12-16 MiB/min until OOM. Ring and IDR ownership remained bounded while page-frag backing accumulated. Two ownership captures found fragments from multiple allocation origins on 86-87% of the final tracker-visible backing pages.

This RFC:

1. fixes an existing no-op alignment attempt on empty RX skbs;
2. makes sequential external IRQ/NAPI lifecycle transitions idempotent;
3. quiesces non-reset crash recovery before DP teardown, gives each RXDMA ring a private page-frag cache, and drains it after the ring's DMA mappings and skbs have been released.

No RX buffer or ring sizes are changed, and the existing per-buffer DMA map/unmap lifecycle is unchanged.

page_pool should also be viable, but it is not a mechanical allocator substitution here. These skb heads need about 2.5 KiB of backing, so an order-0 pool uses one 4 KiB page per buffer. An order-1 fragmented pool restores packing density but lacks page_frag_cache's lower-order fallback under fragmentation. page_pool also adds fallible setup/unwind and requires every skb return path to become pool-aware. This RFC keeps those behaviors unchanged; I can prototype page_pool if that is the preferred ath11k direction.

The AHB version was built as an OpenWrt backport and remained bounded in natural-load, resident, and high-packet-rate pressure runs. PCI has not been runtime-tested. The non-reset firmware-recovery path has not been validated in isolation, so feedback on patches 2 and 3 is especially welcome. The IRQ state guards make repeated sequential calls safe; they do not add synchronization between concurrent lifecycle callers.

OpenWrt discussion:
https://github.com/openwrt/openwrt/pull/24254

Mark Ruvald Pedersen (3):
wifi: ath11k: reserve headroom when aligning RX buffers
wifi: ath11k: make external IRQ control idempotent
wifi: ath11k: use private page-frag caches for RXDMA rings

---
base-commit: 189721a4afa1804315e7dcfca9ca0539c7b1d7af

#1 lore raw

RFC PATCH 1/3 wifi: ath11k: reserve headroom when aligning RX buffers

RXDMA buffers are allocated with extra space for 128-byte alignment, but the driver tries to move skb->data with skb_pull(). The skb is empty at that point, so skb_pull() rejects every positive delta and leaves the data pointer unchanged.

Use skb_reserve() while the skb is empty. The existing extra allocation space leaves at least DP_RX_BUFFER_SIZE bytes of DMA tailroom after the reservation.

Fixes d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")

Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.9.0.1-02146-QCAHKSWPL_SILICONZ-1

---

drivers/net/wireless/ath/ath11k/dp_rx.c +4 -4
index 33425707c..e4ea0c216 100644
@@ -378,7 +378,7 @@ int ath11k_dp_rxbufs_replenish(struct ath11k_base *ab, int mac_id,
 
 		if (!IS_ALIGNED((unsigned long)skb->data,
 				DP_RX_BUFFER_ALIGN_SIZE)) {
-			skb_pull(skb,
-				 PTR_ALIGN(skb->data, DP_RX_BUFFER_ALIGN_SIZE) -
-				 skb->data);
+			skb_reserve(skb,
+				    PTR_ALIGN(skb->data, DP_RX_BUFFER_ALIGN_SIZE) -
+				    skb->data);
 		}
@@ -2875,7 +2875,7 @@ static struct sk_buff *ath11k_dp_rx_alloc_mon_status_buf(struct ath11k_base *ab,
 
 	if (!IS_ALIGNED((unsigned long)skb->data,
 			DP_RX_BUFFER_ALIGN_SIZE)) {
-		skb_pull(skb, PTR_ALIGN(skb->data, DP_RX_BUFFER_ALIGN_SIZE) -
+		skb_reserve(skb, PTR_ALIGN(skb->data, DP_RX_BUFFER_ALIGN_SIZE) -
 			 skb->data);
 	}
 
#2 lore raw

RFC PATCH 2/3 wifi: ath11k: make external IRQ control idempotent

In reply to Mark Ruvald Pedersen

ath11k tracks each external IRQ group's NAPI lifecycle with napi_enabled, but calls the physical IRQ enable and disable helpers outside the state guards. Repeating a lifecycle disable therefore increments Linux's IRQ disable depth on AHB and multi-MSI PCI even when NAPI is already disabled. One later enable leaves the IRQ masked.

Move each physical group transition inside the matching NAPI state transition. Preserve the ordering: mask IRQs before synchronizing and disabling NAPI, then enable NAPI before unmasking IRQs. The outer synchronize_irq() calls remain unchanged.

This makes sequential lifecycle transitions idempotent. It does not change the temporary disable/enable pairing in interrupt handlers and NAPI poll completion.

Fixes d943fdad7589 ("ath11k: Fix napi related hang")
Fixes bbfdc5a751a6 ("ath11k: Refactor PCI code to support WCN6750")

Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.9.0.1-02146-QCAHKSWPL_SILICONZ-1

---

drivers/net/wireless/ath/ath11k/ahb.c +3 -3
index 1e1dea485..1f1562ff3 100644
@@ -235,9 +235,9 @@ static void __ath11k_ahb_ext_irq_disable(struct ath11k_base *ab)
 	for (i = 0; i < ATH11K_EXT_IRQ_GRP_NUM_MAX; i++) {
 		struct ath11k_ext_irq_grp *irq_grp = &ab->ext_irq_grp[i];
 
-		ath11k_ahb_ext_grp_disable(irq_grp);
-
 		if (irq_grp->napi_enabled) {
+			ath11k_ahb_ext_grp_disable(irq_grp);
+
 			napi_synchronize(&irq_grp->napi);
 			napi_disable(&irq_grp->napi);
 			irq_grp->napi_enabled = false;
@@ -380,8 +380,8 @@ static void ath11k_ahb_ext_irq_enable(struct ath11k_base *ab)
 		if (!irq_grp->napi_enabled) {
 			napi_enable(&irq_grp->napi);
 			irq_grp->napi_enabled = true;
+			ath11k_ahb_ext_grp_enable(irq_grp);
 		}
-		ath11k_ahb_ext_grp_enable(irq_grp);
 	}
 }
 
drivers/net/wireless/ath/ath11k/pcic.c +3 -3
index 2259adc3b..bf367f60b 100644
@@ -455,9 +455,9 @@ static void __ath11k_pcic_ext_irq_disable(struct ath11k_base *ab)
 	for (i = 0; i < ATH11K_EXT_IRQ_GRP_NUM_MAX; i++) {
 		struct ath11k_ext_irq_grp *irq_grp = &ab->ext_irq_grp[i];
 
-		ath11k_pcic_ext_grp_disable(irq_grp);
-
 		if (irq_grp->napi_enabled) {
+			ath11k_pcic_ext_grp_disable(irq_grp);
+
 			napi_synchronize(&irq_grp->napi);
 			napi_disable(&irq_grp->napi);
 			irq_grp->napi_enabled = false;
@@ -490,8 +490,8 @@ void ath11k_pcic_ext_irq_enable(struct ath11k_base *ab)
 		if (!irq_grp->napi_enabled) {
 			napi_enable(&irq_grp->napi);
 			irq_grp->napi_enabled = true;
+			ath11k_pcic_ext_grp_enable(irq_grp);
 		}
-		ath11k_pcic_ext_grp_enable(irq_grp);
 	}
 
 	set_bit(ATH11K_FLAG_EXT_IRQ_ENABLED, &ab->dev_flags);
#3 lore raw

Re: [RFC PATCH 2/3] wifi: ath11k: make external IRQ control idempotent

Quoted 5 lines · Mark Ruvald Pedersen

ath11k tracks each external IRQ group's NAPI lifecycle with napi_enabled, but calls the physical IRQ enable and disable helpers outside the state guards. Repeating a lifecycle disable therefore increments Linux's IRQ disable depth on AHB and multi-MSI PCI even when NAPI is already disabled. One later enable leaves the IRQ masked.

can you elaborate the scenario where the repeated lifecycle disable happen for better understand ?

Signature · 3 lines
Karthikeyan Periyasamy
--
கார்த்திகேயன் பெரியசாமி
#4 lore raw

Re: [RFC PATCH 2/3] wifi: ath11k: make external IRQ control idempotent

Quoted 2 lines

can you elaborate the scenario where the repeated lifecycle disable happen for better understand ?

Hey Karthikeyan, the scenario is a failed reset retry:

ath11k_core_reset()
disable IRQ/NAPI depth 0 -> 1
power-cycle
recovery does not complete
another reset_work is queued
reset_count > 1
wait_for(reset_complete) times out
disable IRQ/NAPI again depth 1 -> 2

later recovery succeeds
enable IRQ/NAPI depth 2 -> 1

NAPI is then enabled, but the IRQ remains masked. The patch makes the second lifecycle disable a no-op - ie idempotent.

This is an error/retry path, mainly the PCI/MHI reset path.

I haven't reproduced that exact sequence on hardware, but it was under suspicion during testing, reloading of modules and kexec etc. So this patch is more of a belt-and-suspenders kind of thing. Idempotency helps me sleep at night.

regards Mark

#5 lore raw

RFC PATCH 3/3 wifi: ath11k: use private page-frag caches for RXDMA rings

In reply to Mark Ruvald Pedersen

ath11k allocates RXDMA skb heads with dev_alloc_skb(), which draws from shared per-CPU page-frag caches. RX buffers from different rings and unrelated networking allocations can therefore share a high-order backing page despite having different ownership lifetimes.

On IPQ8074, RX ownership tracing found bounded ring and IDR occupancy while page-frag backing grew. Two captures found current or historical cross-origin fragments on 86-87% of the final tracker-visible physical pages.

Give each RXDMA ring its own page_frag_cache and use it for regular and monitor-status buffers. New fragments from distinct rings can no longer share backing pages with one another or with unrelated users. The monitor-destination path that intentionally uses the data refill ring also uses that ring's cache.

Commit d455e805de70 ("wifi: ath11k: rearrange IRQ enable/disable in reset path") moved HIF IRQ shutdown from the common crash-reconfiguration path into the reset worker. The non-reset QMI recovery path can consequently reach DP teardown while NAPI can still refill an RXDMA ring.

Quiesce HIF IRQ/NAPI processing on that non-reset path before teardown. The reset worker already does so before power cycling, therefore keep the existing reset path unchanged. This makes the cache change self-contained without relying on repeated IRQ lifecycle transitions.

The refill paths serialize each cache with the corresponding SRNG lock. Drain the cache only after ring-owned skbs have been DMA-unmapped and freed and the IDR has been destroyed. Keep the dev_alloc_skb() allocation flags and headroom behavior; build_skb() propagates head-frag and pfmemalloc metadata from the backing page.

This leaves buffer and ring sizes and all DMA map/unmap operations unchanged. It isolates cross-owner lifetime mixing; it does not prevent lifetime variation among buffers belonging to the same ring.

With the private caches, page-frag backing remained bounded in natural load, resident, and high-packet-rate pressure runs on IPQ8074. The non-reset recovery path has not been runtime-tested in isolation.

Fixes d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
Fixes d455e805de70 ("wifi: ath11k: rearrange IRQ enable/disable in reset path")

Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.9.0.1-02146-QCAHKSWPL_SILICONZ-1

---

drivers/net/wireless/ath/ath11k/core.c +3 -0
index 8dacc878c006..e71a22ce332d 100644
@@ -2330,6 +2330,9 @@ static int ath11k_core_reconfigure_on_crash(struct ath11k_base *ab)
 {
 	int ret;
 
+	if (!ab->is_reset)
+		ath11k_hif_irq_disable(ab);
+
 	mutex_lock(&ab->core_lock);
 	ath11k_thermal_unregister(ab);
 	ath11k_dp_pdev_free(ab);
drivers/net/wireless/ath/ath11k/dp.h +3 -0
index 84f66839f0c6..e69dc23b2051 100644
@@ -7,6 +7,8 @@
 #ifndef ATH11K_DP_H
 #define ATH11K_DP_H
 
+#include <linux/page_frag_cache.h>
+
 #include "hal_rx.h"
 
 #define MAX_RXDMA_PER_PDEV     2
@@ -72,6 +74,7 @@ struct dp_srng {
 
 struct dp_rxdma_ring {
 	struct dp_srng refill_buf_ring;
+	struct page_frag_cache frag_cache;
 	struct idr bufs_idr;
 	/* Protects bufs_idr */
 	spinlock_t idr_lock;
drivers/net/wireless/ath/ath11k/dp_rx.c +38 -4

75 lines not on this page · raw message