mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] wifi: brcmfmac: avoid sleeping tx locks in netpoll context
@ 2026-09-30 18:54 Karl Mehltretter
  2026-10-01  7:57 ` Sebastian Andrzej Siewior
  2026-10-04 12:29 ` [PATCH] wifi: brcmfmac: avoid sleeping tx locks in netpoll context Arend van Spriel
  0 siblings, 2 replies; 6+ messages in thread
From: Karl Mehltretter @ 2026-09-30 18:54 UTC (permalink / raw)
  To: linux-wireless
  Cc: Karl Mehltretter, Arend van Spriel, Sebastian Andrzej Siewior,
	Clark Williams, Steven Rostedt, David S. Miller, Eric Dumazet,
	brcm80211, brcm80211-dev-list.pdl, linux-kernel, linux-rt-devel,
	stable

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-06  8:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-30 18:54 [PATCH] wifi: brcmfmac: avoid sleeping tx locks in netpoll context Karl Mehltretter
2026-10-01  7:57 ` Sebastian Andrzej Siewior
2026-10-01 23:41   ` [PATCH] wifi: brcmfmac: avoid sleeping tx locks in netpoll contextg Karl Mehltretter
2026-10-04 12:29 ` [PATCH] wifi: brcmfmac: avoid sleeping tx locks in netpoll context Arend van Spriel
2026-10-06  4:18   ` Karl Mehltretter
2026-10-06  8:27     ` Arend van Spriel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®