[PATCH rtw-next] wifi: rtw88: pci: recover from a stalled TX ring

Abdurrahman Karadag posted 1 patch 3 weeks, 3 days ago
drivers/net/wireless/realtek/rtw88/hci.h  |  7 +++
drivers/net/wireless/realtek/rtw88/main.c |  2 +
drivers/net/wireless/realtek/rtw88/pci.c  | 67 +++++++++++++++++++++++
drivers/net/wireless/realtek/rtw88/pci.h  |  3 +
4 files changed, 79 insertions(+)
[PATCH rtw-next] wifi: rtw88: pci: recover from a stalled TX ring
Posted by Abdurrahman Karadag 3 weeks, 3 days ago
The hardware read pointer of a PCIe TX ring can stop advancing while the
driver keeps queueing descriptors. Once the ring fills up,
rtw_pci_tx_write() stops the corresponding mac80211 queue, and the only
ieee80211_wake_queue() for that ring lives inside the completion loop of
rtw_pci_tx_isr():

	count = cur_rp - ring->r.rp;
	while (count--) {
		...
		if (ring->queue_stopped && avail_desc(...) > 4)
			ieee80211_wake_queue(hw, q_map);
		...
	}

When the read pointer is frozen, count is zero, the loop body never runs
and the queue is never woken again. The interface stays associated, RX
keeps working, and TX is silently dead until the interface is taken down
and up again or the machine is rebooted. Nothing is logged and no
counter reflects it.

This was observed several times on an RTL8821CE. Dumps taken at the
moment of the failure, before touching the interface, show the BE queue
stopped with reason IEEE80211_QUEUE_STOP_REASON_DRIVER, a per-TID
backlog that keeps growing, a TXBD read index that is identical in
samples two seconds and two minutes apart, and a frozen tx packet
counter while beacon and other RX counters keep increasing.

Detect the condition from rtw_watch_dog_work(): for every TX ring that
has something in flight, remember the hardware read pointer and, if it
has not advanced for three consecutive rounds (about six seconds), run
the kick-off for that queue again. Once the hardware resumes consuming
descriptors, rtw_pci_tx_isr() runs with a non-zero count and wakes the
queue through the existing path.

Rings with nothing in flight are skipped before the register read, so an
idle device is not touched, and the beacon and H2C rings are left alone
because they are not flow controlled through mac80211. The warning is
printed once per stall, while the kick-off is retried as long as the
ring does not drain.

This does not address why the hardware stops consuming descriptors; it
turns an unrecoverable state into a recoverable one and makes it visible
in the log.

Tested on an RTL8821CE by stalling the BE ring on purpose (skipping the
doorbell write until the ring filled up). Without this change the queue
stayed stopped for as long as it was observed; with it the ring drained
and traffic resumed within one watchdog round.

Link: https://lore.kernel.org/linux-wireless/20260826162514.80580-1-abdurrahmankaradag19@gmail.com/
Signed-off-by: Abdurrahman Karadag <abdurrahmankaradag19@gmail.com>
---
 drivers/net/wireless/realtek/rtw88/hci.h  |  7 +++
 drivers/net/wireless/realtek/rtw88/main.c |  2 +
 drivers/net/wireless/realtek/rtw88/pci.c  | 67 +++++++++++++++++++++++
 drivers/net/wireless/realtek/rtw88/pci.h  |  3 +
 4 files changed, 79 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/hci.h b/drivers/net/wireless/realtek/rtw88/hci.h
index d4bee9c3e..42e6cefa3 100644
--- a/drivers/net/wireless/realtek/rtw88/hci.h
+++ b/drivers/net/wireless/realtek/rtw88/hci.h
@@ -11,6 +11,7 @@ struct rtw_hci_ops {
 			struct rtw_tx_pkt_info *pkt_info,
 			struct sk_buff *skb);
 	void (*tx_kick_off)(struct rtw_dev *rtwdev);
+	void (*tx_stall_check)(struct rtw_dev *rtwdev);
 	void (*flush_queues)(struct rtw_dev *rtwdev, u32 queues, bool drop);
 	int (*setup)(struct rtw_dev *rtwdev);
 	int (*start)(struct rtw_dev *rtwdev);
@@ -45,6 +46,12 @@ static inline void rtw_hci_tx_kick_off(struct rtw_dev *rtwdev)
 	return rtwdev->hci.ops->tx_kick_off(rtwdev);
 }
 
+static inline void rtw_hci_tx_stall_check(struct rtw_dev *rtwdev)
+{
+	if (rtwdev->hci.ops->tx_stall_check)
+		rtwdev->hci.ops->tx_stall_check(rtwdev);
+}
+
 static inline int rtw_hci_setup(struct rtw_dev *rtwdev)
 {
 	return rtwdev->hci.ops->setup(rtwdev);
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index cd9254370..35b3682e2 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -273,6 +273,8 @@ static void rtw_watch_dog_work(struct work_struct *work)
 
 	/* make sure BB/RF is working for dynamic mech */
 	rtw_leave_lps(rtwdev);
+
+	rtw_hci_tx_stall_check(rtwdev);
 	rtw_coex_wl_status_check(rtwdev);
 	rtw_coex_query_bt_hid_list(rtwdev);
 	rtw_coex_active_query_bt_info(rtwdev);
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 66d2e5f51..f9875cb07 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -787,6 +787,72 @@ static void rtw_pci_tx_kick_off_queue(struct rtw_dev *rtwdev,
 	spin_unlock_bh(&rtwpci->irq_lock);
 }
 
+/* The hardware read pointer of a TX ring can stop advancing while the driver
+ * keeps queueing descriptors. Once the ring is full, rtw_pci_tx_write() stops
+ * the mac80211 queue, and because the only wake up is inside the completion
+ * loop of rtw_pci_tx_isr(), which does not run while the read pointer is
+ * frozen, the queue would stay stopped forever. Detect a ring that is not
+ * draining and kick it off again.
+ */
+#define RTW_PCI_TX_STALL_LIMIT 3
+
+static void rtw_pci_tx_stall_check(struct rtw_dev *rtwdev)
+{
+	struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
+	struct rtw_pci_tx_ring *ring;
+	enum rtw_tx_queue_type queue;
+	u32 bd_idx, cur_rp, wp;
+	bool pending, kick, warn, stopped;
+
+	for (queue = 0; queue < RTK_MAX_TX_QUEUE_NUM; queue++) {
+		/* BCN is the reserved page and H2C is managed by the
+		 * firmware, neither is flow controlled through mac80211
+		 */
+		if (queue == RTW_TX_QUEUE_BCN || queue == RTW_TX_QUEUE_H2C)
+			continue;
+
+		ring = &rtwpci->tx_rings[queue];
+		kick = false;
+		warn = false;
+
+		spin_lock_bh(&rtwpci->irq_lock);
+
+		/* nothing in flight, do not touch the device */
+		if (skb_queue_empty(&ring->queue) && !ring->queue_stopped) {
+			ring->stall_cnt = 0;
+			ring->stall_warned = false;
+			spin_unlock_bh(&rtwpci->irq_lock);
+			continue;
+		}
+
+		bd_idx = rtw_read32(rtwdev, rtw_pci_tx_queue_idx_addr[queue]);
+		cur_rp = (bd_idx >> 16) & TRX_BD_IDX_MASK;
+		wp = ring->r.wp;
+		pending = ring->queue_stopped || cur_rp != wp;
+
+		if (pending && cur_rp == ring->last_rp) {
+			if (++ring->stall_cnt >= RTW_PCI_TX_STALL_LIMIT) {
+				ring->stall_cnt = 0;
+				kick = true;
+				warn = !ring->stall_warned;
+				ring->stall_warned = true;
+			}
+		} else {
+			ring->stall_cnt = 0;
+			ring->stall_warned = false;
+		}
+		stopped = ring->queue_stopped;
+		ring->last_rp = cur_rp;
+		spin_unlock_bh(&rtwpci->irq_lock);
+
+		if (warn)
+			rtw_warn(rtwdev, "TX queue %d stalled (rp %u wp %u%s), kicking\n",
+				 queue, cur_rp, wp, stopped ? ", stopped" : "");
+		if (kick)
+			rtw_pci_tx_kick_off_queue(rtwdev, queue);
+	}
+}
+
 static void rtw_pci_tx_kick_off(struct rtw_dev *rtwdev)
 {
 	struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
@@ -1605,6 +1671,7 @@ static void rtw_pci_destroy(struct rtw_dev *rtwdev, struct pci_dev *pdev)
 static const struct rtw_hci_ops rtw_pci_ops = {
 	.tx_write = rtw_pci_tx_write,
 	.tx_kick_off = rtw_pci_tx_kick_off,
+	.tx_stall_check = rtw_pci_tx_stall_check,
 	.flush_queues = rtw_pci_flush_queues,
 	.setup = rtw_pci_setup,
 	.start = rtw_pci_start,
diff --git a/drivers/net/wireless/realtek/rtw88/pci.h b/drivers/net/wireless/realtek/rtw88/pci.h
index 8ffdea113..ddeefa220 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.h
+++ b/drivers/net/wireless/realtek/rtw88/pci.h
@@ -188,6 +188,9 @@ struct rtw_pci_tx_ring {
 	struct rtw_pci_ring r;
 	struct sk_buff_head queue;
 	bool queue_stopped;
+	u32 last_rp;
+	u8 stall_cnt;
+	bool stall_warned;
 };
 
 struct rtw_pci_rx_buffer_desc {
-- 
2.55.0
RE: [PATCH rtw-next] wifi: rtw88: pci: recover from a stalled TX ring
Posted by Ping-Ke Shih 2 weeks, 6 days ago
Abdurrahman Karadag <abdurrahmankaradag19@gmail.com> wrote:

[...]

> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
> index 66d2e5f51..f9875cb07 100644
> --- a/drivers/net/wireless/realtek/rtw88/pci.c
> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> @@ -787,6 +787,72 @@ static void rtw_pci_tx_kick_off_queue(struct rtw_dev *rtwdev,
>         spin_unlock_bh(&rtwpci->irq_lock);
>  }
> 
> +/* The hardware read pointer of a TX ring can stop advancing while the driver

First line of comment block should be empty. We don't use special style for
networking subsystem anymroe.

> + * keeps queueing descriptors. Once the ring is full, rtw_pci_tx_write() stops
> + * the mac80211 queue, and because the only wake up is inside the completion
> + * loop of rtw_pci_tx_isr(), which does not run while the read pointer is
> + * frozen, the queue would stay stopped forever. Detect a ring that is not
> + * draining and kick it off again.
> + */
> +#define RTW_PCI_TX_STALL_LIMIT 3
> +
> +static void rtw_pci_tx_stall_check(struct rtw_dev *rtwdev)
> +{
> +       struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
> +       struct rtw_pci_tx_ring *ring;
> +       enum rtw_tx_queue_type queue;
> +       u32 bd_idx, cur_rp, wp;
> +       bool pending, kick, warn, stopped;
> +
> +       for (queue = 0; queue < RTK_MAX_TX_QUEUE_NUM; queue++) {
> +               /* BCN is the reserved page and H2C is managed by the
> +                * firmware, neither is flow controlled through mac80211
> +                */
> +               if (queue == RTW_TX_QUEUE_BCN || queue == RTW_TX_QUEUE_H2C)
> +                       continue;
> +
> +               ring = &rtwpci->tx_rings[queue];
> +               kick = false;
> +               warn = false;
> +
> +               spin_lock_bh(&rtwpci->irq_lock);
> +
> +               /* nothing in flight, do not touch the device */
> +               if (skb_queue_empty(&ring->queue) && !ring->queue_stopped) {
> +                       ring->stall_cnt = 0;
> +                       ring->stall_warned = false;
> +                       spin_unlock_bh(&rtwpci->irq_lock);
> +                       continue;
> +               }
> +
> +               bd_idx = rtw_read32(rtwdev, rtw_pci_tx_queue_idx_addr[queue]);
> +               cur_rp = (bd_idx >> 16) & TRX_BD_IDX_MASK;
> +               wp = ring->r.wp;
> +               pending = ring->queue_stopped || cur_rp != wp;
> +
> +               if (pending && cur_rp == ring->last_rp) {
> +                       if (++ring->stall_cnt >= RTW_PCI_TX_STALL_LIMIT) {
> +                               ring->stall_cnt = 0;
> +                               kick = true;
> +                               warn = !ring->stall_warned;
> +                               ring->stall_warned = true;
> +                       }
> +               } else {
> +                       ring->stall_cnt = 0;
> +                       ring->stall_warned = false;
> +               }
> +               stopped = ring->queue_stopped;
> +               ring->last_rp = cur_rp;

Does software pointer cause driver doesn't restart queue?
Or without changes of this chunk, the stuck can be resolved?

> +               spin_unlock_bh(&rtwpci->irq_lock);
> +
> +               if (warn)
> +                       rtw_warn(rtwdev, "TX queue %d stalled (rp %u wp %u%s), kicking\n",
> +                                queue, cur_rp, wp, stopped ? ", stopped" : "");
> +               if (kick)
> +                       rtw_pci_tx_kick_off_queue(rtwdev, queue);

I mean just doing this kick is enough to resolve the stuck?

> +       }
> +}
> +
>  static void rtw_pci_tx_kick_off(struct rtw_dev *rtwdev)
>  {
>         struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
Re: [PATCH rtw-next] wifi: rtw88: pci: recover from a stalled TX ring
Posted by abkarada 2 days, 19 hours ago
Sorry for the slow reply. I wanted to answer your questions with
measurements rather than with an opinion, and that took a while.

> > +               stopped = ring->queue_stopped;
> > +               ring->last_rp = cur_rp;
>
> Does software pointer cause driver doesn't restart queue?
> Or without changes of this chunk, the stuck can be resolved?

No. last_rp and stall_cnt are detection state only; they never touch the
data path. The only action in the patch is the kick below.

> I mean just doing this kick is enough to resolve the stuck?

No, and you were right to ask twice.

I taught my watchdog to rewrite the TXBD host index during a real stall
and then measure whether the hardware read index moves. Three live
occurrences since:

  2026-09-06 03:17   BE rp 0x00b7 before and after the rewrite
  2026-09-14 17:48   BE rp 0x00c9 before and after (wp 0x6c -> 0x6d)
  2026-09-14 19:09   BE rp 0x009e before and after (wp 0x50 -> 0x51)

In all three the read index never moved and traffic stayed at 100% loss.
The premise of this patch is wrong: the hardware is not waiting to be
told about new descriptors, it has stopped consuming them, and writing
the doorbell again changes nothing.

Please drop this patch. I will not resend it in this form.

I have sent a longer reply in the bug thread with the rest of the data,
including the fact that the stall also happens on 7.0.12, so my earlier
good/bad version claim was wrong as well. It also proposes a different
direction for the driver side, which I would rather agree with you
before writing.

Thanks for the review, and for pushing back on the mechanism.