[PATCH] wifi: brcmfmac: avoid sleeping tx locks in netpoll context

[PATCH] wifi: brcmfmac: avoid sleeping tx locks in netpoll context

[Karl Mehltretter]

With the default fcmode=0, netpoll calls ndo_start_xmit() with hard
interrupts disabled and reaches brcmf_sdio_bus_txdata() directly. The
function takes txq_lock with spin_lock_bh(), and the queue helper takes
the embedded sk_buff_head lock. These locks may sleep on PREEMPT_RT.
On non-RT, spin_unlock_bh() can run pending networking softirqs before
netpoll releases the transmit lock, causing a recursive transmit
deadlock.

Use spin_trylock() for IRQ-disabled calls and enqueue with the unlocked
skb helper while holding txq_lock. On PREEMPT_RT, reject hard IRQ and NMI
callers, where rt-spinlocks cannot be acquired. If the lock is busy or
the queue is full, return through the existing drop path. Do not evict an
older packet from this context. Suppress the queue-full printk because
netconsole can recursively enter this path.

The flow-control callback takes another spinlock, so defer it when an
IRQ-disabled enqueue reaches TXHI. The data worker rechecks the bus state
and queue length under txq_lock before stopping the queue. Existing TXLOW
handling wakes the queue after it drains.

This fixes the direct SDIO transmit path used by fcmode=0. Modes 1 and 2
take the FWS lock first and need a separate change.

Fixes: ac3d9dd034e5 ("netpoll: make ndo_poll_controller() optional")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
Testing:

- This revision passed W=1 sdio.o builds on x86_64 PREEMPT_RT and
  x86_64 PREEMPT kernels.
- This exact revision, together with the netpoll v2 fixes, passed 10/10
  counted physical boots across a Pi 400 and Pi 500+: three RT and two
  non-RT boots per board. The softirq trigger delivered 64/64 records on
  every boot with no driver drop. Every lock-contention run freed all 64
  skbs without timeout, and every TXHI queue stop subsequently woke.
- Each board and kernel flavor completed a 1,800-second numbered stream,
  ten repeated TXHI cycles and a Wi-Fi reconnect. No counted boot
  reported an atomic-sleep warning, new lockdep splat, stall or lockup.
- The exact revision passed combined RT and non-RT QEMU softirq,
  lock-contention and TXHI stop/drain/wake tests.
- An unpatched PREEMPT_RT Pi 400 reproduced the atomic-sleep report. An
  unpatched non-RT Pi 400 deadlocked and produced 32 RCU stall reports.
- An unpatched PREEMPT_RT Pi 500+ reproduced 21 atomic-sleep reports.
- All brcmfmac tests used the default fcmode=0.

 .../broadcom/brcm80211/brcmfmac/sdio.c        | 91 +++++++++++++++++--
 1 file changed, 85 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 381801af3a..170450306d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -516,6 +516,7 @@ struct brcmf_sdio {
 	bool dpc_running;
 
 	bool txoff;		/* Transmit flow-controlled */
+	bool txoff_pending;	/* Deferred transmit flow control */
 	struct brcmf_sdio_count sdcnt;
 	bool sr_enabled; /* SaveRestore enabled */
 	bool sleeping;
@@ -2796,8 +2797,53 @@ static bool brcmf_sdio_prec_enq(struct pktq *q, struct sk_buff *pkt, int prec)
 	return p != NULL;
 }
 
+/*
+ * The caller holds txq_lock with hard IRQs disabled. Avoid the skb queue
+ * lock, which may sleep on PREEMPT_RT.
+ */
+static bool brcmf_sdio_prec_enq_irqoff(struct pktq *q, struct sk_buff *pkt,
+				       int prec)
+{
+	struct sk_buff_head *list = &q->q[prec].skblist;
+
+	if (pktq_pfull(q, prec) || pktq_full(q))
+		return false;
+
+	__skb_queue_tail(list, pkt);
+	q->len++;
+	if (q->hi_prec < prec)
+		q->hi_prec = prec;
+
+	return true;
+}
+
+static bool brcmf_sdio_txq_lock(struct brcmf_sdio *bus, bool irq_off)
+		__cond_acquires(true, &bus->txq_lock)
+{
+	if (irq_off) {
+		if (IS_ENABLED(CONFIG_PREEMPT_RT) &&
+		    (in_hardirq() || in_nmi()))
+			return false;
+		return spin_trylock(&bus->txq_lock);
+	}
+
+	spin_lock_bh(&bus->txq_lock);
+	return true;
+}
+
+static void brcmf_sdio_txq_unlock(struct brcmf_sdio *bus, bool irq_off)
+		__releases(&bus->txq_lock)
+{
+	if (irq_off)
+		spin_unlock(&bus->txq_lock);
+	else
+		spin_unlock_bh(&bus->txq_lock);
+}
+
 static int brcmf_sdio_bus_txdata(struct device *dev, struct sk_buff *pkt)
 {
+	bool irq_off = irqs_disabled();
+	bool enqueued;
 	int ret = -EBADE;
 	uint prec;
 	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
@@ -2826,22 +2872,39 @@ static int brcmf_sdio_bus_txdata(struct device *dev, struct sk_buff *pkt)
 	bus->sdcnt.fcqueued++;
 
 	/* Priority based enq */
-	spin_lock_bh(&bus->txq_lock);
+	if (!brcmf_sdio_txq_lock(bus, irq_off)) {
+		skb_pull(pkt, bus->tx_hdrlen);
+		return -EBUSY;
+	}
+
 	/* reset bus_flags in packet cb */
 	*(u16 *)(pkt->cb) = 0;
-	if (!brcmf_sdio_prec_enq(&bus->txq, pkt, prec)) {
+	if (irq_off)
+		enqueued = brcmf_sdio_prec_enq_irqoff(&bus->txq, pkt, prec);
+	else
+		enqueued = brcmf_sdio_prec_enq(&bus->txq, pkt, prec);
+
+	if (!enqueued) {
 		skb_pull(pkt, bus->tx_hdrlen);
-		brcmf_err("out of bus->txq !!!\n");
+		/* Avoid netconsole recursion. */
+		if (!irq_off)
+			brcmf_err("out of bus->txq !!!\n");
 		ret = -ENOSR;
 	} else {
 		ret = 0;
 	}
 
 	if (pktq_len(&bus->txq) >= TXHI) {
-		bus->txoff = true;
-		brcmf_proto_bcdc_txflowblock(dev, true);
+		if (irq_off) {
+			WRITE_ONCE(bus->txoff_pending, true);
+		} else {
+			WRITE_ONCE(bus->txoff_pending, false);
+			bus->txoff = true;
+			brcmf_proto_bcdc_txflowblock(dev, true);
+		}
 	}
-	spin_unlock_bh(&bus->txq_lock);
+
+	brcmf_sdio_txq_unlock(bus, irq_off);
 
 #ifdef DEBUG
 	if (pktq_plen(&bus->txq, prec) > qcount[prec])
@@ -3754,6 +3817,21 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 	}
 }
 
+static void brcmf_sdio_deferred_txflowblock(struct brcmf_sdio *bus)
+{
+	if (!READ_ONCE(bus->txoff_pending))
+		return;
+
+	spin_lock_bh(&bus->txq_lock);
+	WRITE_ONCE(bus->txoff_pending, false);
+	if (bus->sdiodev->state == BRCMF_SDIOD_DATA && !bus->txoff &&
+	    pktq_len(&bus->txq) >= TXHI) {
+		bus->txoff = true;
+		brcmf_proto_bcdc_txflowblock(bus->sdiodev->dev, true);
+	}
+	spin_unlock_bh(&bus->txq_lock);
+}
+
 static void brcmf_sdio_dataworker(struct work_struct *work)
 {
 	struct brcmf_sdio *bus = container_of(work, struct brcmf_sdio,
@@ -3763,6 +3841,7 @@ static void brcmf_sdio_dataworker(struct work_struct *work)
 	wmb();
 	while (READ_ONCE(bus->dpc_triggered)) {
 		bus->dpc_triggered = false;
+		brcmf_sdio_deferred_txflowblock(bus);
 		brcmf_sdio_dpc(bus);
 		bus->idlecount = 0;
 	}

base-commit: 6f63e919fe1e335b8abcb3a28bfd4804a98d875a
-- 
2.53.0

Re: [PATCH] wifi: brcmfmac: avoid sleeping tx locks in netpoll context

[Sebastian Andrzej Siewior]

On 2026-09-30 20:54:17 [+0200], Karl Mehltretter wrote:
> With the default fcmode=0, netpoll calls ndo_start_xmit() with hard
> interrupts disabled and reaches brcmf_sdio_bus_txdata() directly. The
> function takes txq_lock with spin_lock_bh(), and the queue helper takes
> the embedded sk_buff_head lock. These locks may sleep on PREEMPT_RT.
> On non-RT, spin_unlock_bh() can run pending networking softirqs before
> netpoll releases the transmit lock, causing a recursive transmit
> deadlock.
> 
> Use spin_trylock() for IRQ-disabled calls and enqueue with the unlocked
> skb helper while holding txq_lock. On PREEMPT_RT, reject hard IRQ and NMI
> callers, where rt-spinlocks cannot be acquired. If the lock is busy or
> the queue is full, return through the existing drop path. Do not evict an
> older packet from this context. Suppress the queue-full printk because
> netconsole can recursively enter this path.
> 
> The flow-control callback takes another spinlock, so defer it when an
> IRQ-disabled enqueue reaches TXHI. The data worker rechecks the bus state
> and queue length under txq_lock before stopping the queue. Existing TXLOW
> handling wakes the queue after it drains.
> 
> This fixes the direct SDIO transmit path used by fcmode=0. Modes 1 and 2
> take the FWS lock first and need a separate change.

Is this the only affected driver?
Do you have maybe a backtrace?

> Fixes: ac3d9dd034e5 ("netpoll: make ndo_poll_controller() optional")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>

Sebastian