[PATCH rtw-next v3 0/3] wifi: rtl8xxxu: keep RX requests available across transient errors

[PATCH rtw-next v3 0/3] wifi: rtl8xxxu: keep RX requests available across transient errors

[Kim Wooseok via B4 Relay]

rtl8xxxu keeps 32 RX URBs in circulation. Completed requests wait on a
pending list until more than eight are ready to be submitted again.
Completion errors free their URBs, however, so a finite run of errors
can leave too few requests to reach that threshold. At that point,
reception can stay stopped even after the errors end.

This series fixes that loss of RX requests, along with two problems in
the submission and startup paths: a failed submission can leak its skb,
and a failed start can leave earlier requests active or report success
after only part of the pool was allocated.

In v3, normal completions and recoverable failures return requests to
one pending list, serviced by one delayed work item. An error queues a
100 ms retry. When a normal completion takes the pending count above
eight, mod_delayed_work(..., 0) brings the work forward. ENOMEM/EAGAIN
during submission uses the same retry path.

The delay limits repeated retries when normal reception is not
progressing; it is not a minimum wait for each URB. During stop, the RX
lock prevents new reservations once shutdown is set. Stop then cancels
the delayed work before draining active and pending requests.

Changes since v2:
- Keep patch 1's code unchanged and retain Ping-Ke's Reviewed-by.
- Share the startup and worker submission loop, preserving their
  different handling of fatal errors.
- Replace the separate retry list and worker with one pending list
  and one delayed work item; normal completions can advance a retry.
- Remove the shutdown initialization added in probe.
- Drop the KUnit patch and its test-only hooks from this series.
- Use the confirmed author name and verify the Fixes references.

I built the driver at baseline and after each patch with W=1 under ARM64
allmodconfig and allyesconfig. Both full kernel builds also passed, and
the baseline/final sparse comparisons produced no new diagnostics.

The hardware tests used an RTL8192EU USB adapter on a Raspberry Pi. I
adapted the RX changes to the Pi's existing Linux 6.18.46 RT kernel to
run them on the device. V3 passed 52 tests covering allocation and
submission failures, all six completion errors, repeated failures,
scheduling, stop and normal RX/down-up. Two interrupt-URB tests were not
run because this device does not use that path.

Nine of the passing tests failed the first start after a fresh module
load, at the first, middle or last TX/RX allocation or fatal RX
submission. Each began with no previous start, cleaned up after the
injected failure, and received packets after the next start.

For the same 24 injected EPROTO completions in a 32-URB pool, the
unpatched driver stopped with eight pending requests and none in flight.
V3 kept all 32 and received 1,225 packets over 30 seconds without
restarting the interface.

The fault-injection changes are kept outside the series and exercise the
actual allocation, submission, completion, worker and stop paths.

v2: https://lore.kernel.org/linux-wireless/20260913-codex-rtw-rx-v2-v2-0-f09c964e0b96@khu.ac.kr/

Assisted-by: GPT-6 Astra
Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>

---
Kim Wooseok (3):
      wifi: rtl8xxxu: free RX skb when URB submission fails
      wifi: rtl8xxxu: unwind incomplete receive startup
      wifi: rtl8xxxu: preserve RX requests across recoverable transfer errors

 drivers/net/wireless/realtek/rtl8xxxu/core.c     | 153 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------------------------------
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h |   2 +-
 2 files changed, 86 insertions(+), 69 deletions(-)
---
base-commit: 73e3b1c94c7d5b4a113e7cbb42665dc2fe3b9d78
change-id: 20260920-codex-rtl-rx-v3-submit-1b4ea6921111

Best regards,
--  
Kim Wooseok <5mghybrid@khu.ac.kr>

[PATCH rtw-next v3 1/3] wifi: rtl8xxxu: free RX skb when URB submission fails

[Kim Wooseok via B4 Relay]

From: Kim Wooseok <5mghybrid@khu.ac.kr>

When usb_submit_urb() fails, rtl8xxxu_submit_rx_urb() unanchors the URB
but leaves the newly allocated skb in urb.context. For ENOMEM/EAGAIN,
the RX worker puts the request back on the pending list. The next
submission allocates another skb and overwrites that pointer, leaking
the previous buffer. Stopping before the retry also leaks it, because
pending-list cleanup frees only the URB.

Free the skb and clear urb.context in rtl8xxxu_submit_rx_urb() when
submission fails. This keeps buffer allocation and failure cleanup in
the same function, so a request returned for retry or teardown no longer
owns an skb. Remove the corresponding cleanup from start and the RX
worker; they only need to decide whether to retry or free the URB.

Fixes: 26f1fad29ad9 ("New driver: rtl8xxxu (mac80211)")
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
Assisted-by: GPT-6 Astra
Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
---
 drivers/net/wireless/realtek/rtl8xxxu/core.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
index bddbd0990de72..795a5ec2f8cd4 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
@@ -5864,7 +5864,6 @@ static void rtl8xxxu_rx_urb_work(struct work_struct *work)
 	struct rtl8xxxu_priv *priv;
 	struct rtl8xxxu_rx_urb *rx_urb, *tmp;
 	struct list_head local;
-	struct sk_buff *skb;
 	unsigned long flags;
 	int ret;
 
@@ -5896,8 +5895,6 @@ static void rtl8xxxu_rx_urb_work(struct work_struct *work)
 		default:
 			dev_warn(&priv->udev->dev,
 				 "failed to requeue urb with error %i\n", ret);
-			skb = (struct sk_buff *)rx_urb->urb.context;
-			dev_kfree_skb(skb);
 			usb_free_urb(&rx_urb->urb);
 		}
 	}
@@ -6596,8 +6593,11 @@ static int rtl8xxxu_submit_rx_urb(struct rtl8xxxu_priv *priv,
 			  skb_size, rtl8xxxu_rx_complete, skb);
 	usb_anchor_urb(&rx_urb->urb, &priv->rx_anchor);
 	ret = usb_submit_urb(&rx_urb->urb, GFP_ATOMIC);
-	if (ret)
+	if (ret) {
 		usb_unanchor_urb(&rx_urb->urb);
+		dev_kfree_skb(skb);
+		rx_urb->urb.context = NULL;
+	}
 	return ret;
 }
 
@@ -7410,7 +7410,6 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
 	struct rtl8xxxu_priv *priv = hw->priv;
 	struct rtl8xxxu_rx_urb *rx_urb;
 	struct rtl8xxxu_tx_urb *tx_urb;
-	struct sk_buff *skb;
 	unsigned long flags;
 	int ret, i;
 
@@ -7461,13 +7460,8 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
 		rx_urb->hw = hw;
 
 		ret = rtl8xxxu_submit_rx_urb(priv, rx_urb);
-		if (ret) {
-			if (ret != -ENOMEM) {
-				skb = (struct sk_buff *)rx_urb->urb.context;
-				dev_kfree_skb(skb);
-			}
+		if (ret)
 			rtl8xxxu_queue_rx_urb(priv, rx_urb);
-		}
 	}
 
 	schedule_delayed_work(&priv->ra_watchdog, 2 * HZ);

-- 
2.53.0

[PATCH rtw-next v3 2/3] wifi: rtl8xxxu: unwind incomplete receive startup

[Kim Wooseok via B4 Relay]

From: Kim Wooseok <5mghybrid@khu.ac.kr>

rtl8xxxu_start() allocates and submits RX URBs one at a time. If a later
allocation fails, the error path frees the TX pool but leaves earlier RX
requests active. A partial TX or RX allocation failure can also leave
ret at zero, so start reports success despite the incomplete setup.

Allocate the RX pool before submitting any of it, and return ENOMEM
whenever a pool allocation fails. Pass start errors through
rtl8xxxu_stop(), including interrupt URB submission failures, so the
existing stop path cancels work and releases both active requests and
partially allocated pools.

Share the submission loop with the RX worker. Both callers keep requests
that fail with ENOMEM/EAGAIN. For a fatal error during startup, free the
rest of the unsubmitted batch and return the error so start can unwind.
In the worker, free only the failed request and continue with the rest
of the batch.

Keep the allocation loop in a small helper so the setup and error paths
in start remain easy to follow.

Fixes: 26f1fad29ad9 ("New driver: rtl8xxxu (mac80211)")
Assisted-by: GPT-6 Astra
Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
---
 drivers/net/wireless/realtek/rtl8xxxu/core.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------
 1 file changed, 54 insertions(+), 42 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
index 795a5ec2f8cd4..5ae3dbf5034ac 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
@@ -58,6 +58,7 @@ MODULE_PARM_DESC(dma_agg_pages, "Set DMA aggregation pages (range 1-127, 0 to di
 #define RTL8XXXU_TX_URB_LOW_WATER	25
 #define RTL8XXXU_TX_URB_HIGH_WATER	32
 
+static void rtl8xxxu_stop(struct ieee80211_hw *hw, bool suspend);
 static int rtl8xxxu_submit_rx_urb(struct rtl8xxxu_priv *priv,
 				  struct rtl8xxxu_rx_urb *rx_urb);
 
@@ -5832,6 +5833,27 @@ static void rtl8xxxu_free_rx_resources(struct rtl8xxxu_priv *priv)
 	spin_unlock_irqrestore(&priv->rx_urb_lock, flags);
 }
 
+static int rtl8xxxu_alloc_rx_urbs(struct rtl8xxxu_priv *priv)
+{
+	struct rtl8xxxu_rx_urb *rx_urb;
+	int i;
+
+	/* No RX work is active until the complete pool has been allocated. */
+	for (i = 0; i < RTL8XXXU_RX_URBS; i++) {
+		rx_urb = kmalloc_obj(struct rtl8xxxu_rx_urb);
+		if (!rx_urb)
+			return -ENOMEM;
+
+		usb_init_urb(&rx_urb->urb);
+		INIT_LIST_HEAD(&rx_urb->list);
+		rx_urb->hw = priv->hw;
+		list_add_tail(&rx_urb->list, &priv->rx_urb_pending_list);
+		priv->rx_urb_pending_count++;
+	}
+
+	return 0;
+}
+
 static void rtl8xxxu_queue_rx_urb(struct rtl8xxxu_priv *priv,
 				  struct rtl8xxxu_rx_urb *rx_urb)
 {
@@ -5859,32 +5881,21 @@ static void rtl8xxxu_queue_rx_urb(struct rtl8xxxu_priv *priv,
 	spin_unlock_irqrestore(&priv->rx_urb_lock, flags);
 }
 
-static void rtl8xxxu_rx_urb_work(struct work_struct *work)
+static int rtl8xxxu_submit_rx_urbs(struct rtl8xxxu_priv *priv, bool startup)
 {
-	struct rtl8xxxu_priv *priv;
 	struct rtl8xxxu_rx_urb *rx_urb, *tmp;
-	struct list_head local;
 	unsigned long flags;
+	LIST_HEAD(local);
 	int ret;
 
-	priv = container_of(work, struct rtl8xxxu_priv, rx_urb_wq);
-	INIT_LIST_HEAD(&local);
-
 	spin_lock_irqsave(&priv->rx_urb_lock, flags);
-
 	list_splice_init(&priv->rx_urb_pending_list, &local);
 	priv->rx_urb_pending_count = 0;
-
 	spin_unlock_irqrestore(&priv->rx_urb_lock, flags);
 
 	list_for_each_entry_safe(rx_urb, tmp, &local, list) {
 		list_del_init(&rx_urb->list);
 		ret = rtl8xxxu_submit_rx_urb(priv, rx_urb);
-		/*
-		 * If out of memory or temporary error, put it back on the
-		 * queue and try again. Otherwise the device is dead/gone
-		 * and we should drop it.
-		 */
 		switch (ret) {
 		case 0:
 			break;
@@ -5893,11 +5904,30 @@ static void rtl8xxxu_rx_urb_work(struct work_struct *work)
 			rtl8xxxu_queue_rx_urb(priv, rx_urb);
 			break;
 		default:
+			usb_free_urb(&rx_urb->urb);
+			if (startup)
+				goto free_remaining;
 			dev_warn(&priv->udev->dev,
 				 "failed to requeue urb with error %i\n", ret);
-			usb_free_urb(&rx_urb->urb);
 		}
 	}
+
+	return 0;
+
+free_remaining:
+	list_for_each_entry_safe(rx_urb, tmp, &local, list) {
+		list_del(&rx_urb->list);
+		usb_free_urb(&rx_urb->urb);
+	}
+	return ret;
+}
+
+static void rtl8xxxu_rx_urb_work(struct work_struct *work)
+{
+	struct rtl8xxxu_priv *priv;
+
+	priv = container_of(work, struct rtl8xxxu_priv, rx_urb_wq);
+	rtl8xxxu_submit_rx_urbs(priv, false);
 }
 
 /*
@@ -7408,7 +7438,6 @@ static void rtl8xxxu_watchdog_callback(struct work_struct *work)
 static int rtl8xxxu_start(struct ieee80211_hw *hw)
 {
 	struct rtl8xxxu_priv *priv = hw->priv;
-	struct rtl8xxxu_rx_urb *rx_urb;
 	struct rtl8xxxu_tx_urb *tx_urb;
 	unsigned long flags;
 	int ret, i;
@@ -7423,14 +7452,13 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
 	if (priv->usb_interrupts) {
 		ret = rtl8xxxu_submit_int_urb(hw);
 		if (ret)
-			goto exit;
+			goto error_out;
 	}
 
 	for (i = 0; i < RTL8XXXU_TX_URBS; i++) {
 		tx_urb = kmalloc_obj(struct rtl8xxxu_tx_urb);
 		if (!tx_urb) {
-			if (!i)
-				ret = -ENOMEM;
+			ret = -ENOMEM;
 
 			goto error_out;
 		}
@@ -7441,31 +7469,21 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
 		priv->tx_urb_free_count++;
 	}
 
+	ret = rtl8xxxu_alloc_rx_urbs(priv);
+	if (ret)
+		goto error_out;
+
 	priv->tx_stopped = false;
 
 	spin_lock_irqsave(&priv->rx_urb_lock, flags);
 	priv->shutdown = false;
 	spin_unlock_irqrestore(&priv->rx_urb_lock, flags);
 
-	for (i = 0; i < RTL8XXXU_RX_URBS; i++) {
-		rx_urb = kmalloc_obj(struct rtl8xxxu_rx_urb);
-		if (!rx_urb) {
-			if (!i)
-				ret = -ENOMEM;
-
-			goto error_out;
-		}
-		usb_init_urb(&rx_urb->urb);
-		INIT_LIST_HEAD(&rx_urb->list);
-		rx_urb->hw = hw;
-
-		ret = rtl8xxxu_submit_rx_urb(priv, rx_urb);
-		if (ret)
-			rtl8xxxu_queue_rx_urb(priv, rx_urb);
-	}
+	ret = rtl8xxxu_submit_rx_urbs(priv, true);
+	if (ret)
+		goto error_out;
 
 	schedule_delayed_work(&priv->ra_watchdog, 2 * HZ);
-exit:
 	/*
 	 * Accept all data and mgmt frames
 	 */
@@ -7478,13 +7496,7 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
 	return ret;
 
 error_out:
-	rtl8xxxu_free_tx_resources(priv);
-	/*
-	 * Disable all data and mgmt frames
-	 */
-	rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x0000);
-	rtl8xxxu_write16(priv, REG_RXFLTMAP0, 0x0000);
-
+	rtl8xxxu_stop(hw, false);
 	return ret;
 }
 

-- 
2.53.0

RE: [PATCH rtw-next v3 2/3] wifi: rtl8xxxu: unwind incomplete receive startup

[Ping-Ke Shih]

Kim Wooseok via B4 Relay <devnull+5mghybrid.khu.ac.kr@kernel.org> wrote:
> From: Kim Wooseok <5mghybrid@khu.ac.kr>
> 
> rtl8xxxu_start() allocates and submits RX URBs one at a time. If a later
> allocation fails, the error path frees the TX pool but leaves earlier RX
> requests active. A partial TX or RX allocation failure can also leave
> ret at zero, so start reports success despite the incomplete setup.
> 
> Allocate the RX pool before submitting any of it, and return ENOMEM
> whenever a pool allocation fails. Pass start errors through
> rtl8xxxu_stop(), including interrupt URB submission failures, so the
> existing stop path cancels work and releases both active requests and
> partially allocated pools.
> 
> Share the submission loop with the RX worker. Both callers keep requests
> that fail with ENOMEM/EAGAIN. For a fatal error during startup, free the
> rest of the unsubmitted batch and return the error so start can unwind.
> In the worker, free only the failed request and continue with the rest
> of the batch.
> 
> Keep the allocation loop in a small helper so the setup and error paths
> in start remain easy to follow.
> 
> Fixes: 26f1fad29ad9 ("New driver: rtl8xxxu (mac80211)")
> Assisted-by: GPT-6 Astra
> Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>

Please fix some minor comments, and take my reviewed-by to next version.

Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>

> ---
>  drivers/net/wireless/realtek/rtl8xxxu/core.c | 96
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------
>  1 file changed, 54 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> index 795a5ec2f8cd4..5ae3dbf5034ac 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> @@ -58,6 +58,7 @@ MODULE_PARM_DESC(dma_agg_pages, "Set DMA aggregation pages (range 1-127, 0 to di
>  #define RTL8XXXU_TX_URB_LOW_WATER      25
>  #define RTL8XXXU_TX_URB_HIGH_WATER     32
> 
> +static void rtl8xxxu_stop(struct ieee80211_hw *hw, bool suspend);
>  static int rtl8xxxu_submit_rx_urb(struct rtl8xxxu_priv *priv,
>                                   struct rtl8xxxu_rx_urb *rx_urb);
> 
> @@ -5832,6 +5833,27 @@ static void rtl8xxxu_free_rx_resources(struct rtl8xxxu_priv *priv)
>         spin_unlock_irqrestore(&priv->rx_urb_lock, flags);
>  }
> 
> +static int rtl8xxxu_alloc_rx_urbs(struct rtl8xxxu_priv *priv)
> +{
> +       struct rtl8xxxu_rx_urb *rx_urb;
> +       int i;
> +
> +       /* No RX work is active until the complete pool has been allocated. */
> +       for (i = 0; i < RTL8XXXU_RX_URBS; i++) {
> +               rx_urb = kmalloc_obj(struct rtl8xxxu_rx_urb);
> +               if (!rx_urb)
> +                       return -ENOMEM;
> +
> +               usb_init_urb(&rx_urb->urb);
> +               INIT_LIST_HEAD(&rx_urb->list);
> +               rx_urb->hw = priv->hw;

nit: add a blank line

> +               list_add_tail(&rx_urb->list, &priv->rx_urb_pending_list);
> +               priv->rx_urb_pending_count++;
> +       }
> +
> +       return 0;
> +}
> +
>  static void rtl8xxxu_queue_rx_urb(struct rtl8xxxu_priv *priv,
>                                   struct rtl8xxxu_rx_urb *rx_urb)
>  {

[...]

> +
> +static void rtl8xxxu_rx_urb_work(struct work_struct *work)
> +{
> +       struct rtl8xxxu_priv *priv;
> +
> +       priv = container_of(work, struct rtl8xxxu_priv, rx_urb_wq);

Just assign this at declaration. 

> +       rtl8xxxu_submit_rx_urbs(priv, false);
>  }
> 
>  /*
> @@ -7408,7 +7438,6 @@ static void rtl8xxxu_watchdog_callback(struct work_struct *work)
>  static int rtl8xxxu_start(struct ieee80211_hw *hw)
>  {
>         struct rtl8xxxu_priv *priv = hw->priv;
> -       struct rtl8xxxu_rx_urb *rx_urb;
>         struct rtl8xxxu_tx_urb *tx_urb;
>         unsigned long flags;
>         int ret, i;
> @@ -7423,14 +7452,13 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
>         if (priv->usb_interrupts) {
>                 ret = rtl8xxxu_submit_int_urb(hw);
>                 if (ret)
> -                       goto exit;
> +                       goto error_out;

I have a request in v2 that please make sure all error path can
properly handled by rtl8xxxu_stop(). Can you confirm and ack me?
Or I missed?

>         }
> 
>         for (i = 0; i < RTL8XXXU_TX_URBS; i++) {
>                 tx_urb = kmalloc_obj(struct rtl8xxxu_tx_urb);
>                 if (!tx_urb) {
> -                       if (!i)
> -                               ret = -ENOMEM;
> +                       ret = -ENOMEM;
> 
>                         goto error_out;
>                 }

Re: [PATCH rtw-next v3 2/3] wifi: rtl8xxxu: unwind incomplete receive startup

[Kim Wooseok]

Hi Ping-Ke,

> I have a request in v2 that please make sure all error path can
> properly handled by rtl8xxxu_stop(). Can you confirm and ack me?
> Or I missed?

Yes. More precisely, apart from interrupt URB allocation and submission
failures (this adapter does not use that path, so those two cases were
excluded from hardware testing), I exercised all remaining error exits
from rtl8xxxu_start() on the device.

TX and RX URB allocation failures and fatal RX submission failures were
tested at the first, middle and last positions. The same tests were run
both after a successful activation and on the first start after a
fresh probe.

In each case, start returned the expected error and rtl8xxxu_stop()
cleaned up the remaining resources and RX work. Reception resumed
normally on the next start.

I will add the blank line, initialize priv at its declaration, and
carry your Reviewed-by into the next version. Thank you.

Thanks,
Kim Wooseok

[PATCH rtw-next v3 3/3] wifi: rtl8xxxu: preserve RX requests across recoverable transfer errors

[Kim Wooseok via B4 Relay]

From: Kim Wooseok <5mghybrid@khu.ac.kr>

rtl8xxxu resubmits completed RX requests when more than eight URBs are
waiting on the pending list. It starts with 32 URBs, but frees them on
completion errors. After enough errors, the remaining pool can no longer
reach the submission threshold. Reception then stays stopped even after
the errors end. Temporary submission failures can leave a small batch
waiting with no further work scheduled, too.

Keep the URB when a completion reports EPROTO, EILSEQ, ETIME, EOVERFLOW,
ECOMM or ENOSR. Free its receive buffer and return the request to the
same pending list used by normal completions. ENOMEM/EAGAIN from startup
or worker submission uses this path as well, so a submission failure
cannot strand a request during recovery.

Use one delayed work item to submit the pending requests. An error
queues a retry after 100 ms without moving an existing reservation back.
If a normal completion takes the pending count above eight,
mod_delayed_work(..., 0) brings the work forward. This limits repeated
retries when reception is not progressing, while allowing normal traffic
to replenish the pool promptly. The 100 ms delay is therefore not a
minimum wait for each failed URB.

Keep the shutdown check, queue insertion and scheduling under the RX
lock. Stop sets shutdown under that lock, cancels the delayed work
synchronously, then drains active and pending requests. Cancellation and
device removal continue to free their URBs.

Fixes: 26f1fad29ad9 ("New driver: rtl8xxxu (mac80211)")
Assisted-by: GPT-6 Astra
Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
---
 drivers/net/wireless/realtek/rtl8xxxu/core.c     | 45 ++++++++++++++++++++++++++++-----------------
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h |  2 +-
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
index 5ae3dbf5034ac..8e513a0cb2e24 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
@@ -54,6 +54,7 @@ MODULE_PARM_DESC(dma_agg_pages, "Set DMA aggregation pages (range 1-127, 0 to di
 #define USB_VENDOR_ID_REALTEK		0x0bda
 #define RTL8XXXU_RX_URBS		32
 #define RTL8XXXU_RX_URB_PENDING_WATER	8
+#define RTL8XXXU_RX_URB_RETRY_DELAY_MS	100
 #define RTL8XXXU_TX_URBS		64
 #define RTL8XXXU_TX_URB_LOW_WATER	25
 #define RTL8XXXU_TX_URB_HIGH_WATER	32
@@ -5855,9 +5856,8 @@ static int rtl8xxxu_alloc_rx_urbs(struct rtl8xxxu_priv *priv)
 }
 
 static void rtl8xxxu_queue_rx_urb(struct rtl8xxxu_priv *priv,
-				  struct rtl8xxxu_rx_urb *rx_urb)
+				  struct rtl8xxxu_rx_urb *rx_urb, bool retry)
 {
-	struct sk_buff *skb;
 	unsigned long flags;
 
 	spin_lock_irqsave(&priv->rx_urb_lock, flags);
@@ -5865,16 +5865,13 @@ static void rtl8xxxu_queue_rx_urb(struct rtl8xxxu_priv *priv,
 	if (!priv->shutdown) {
 		list_add_tail(&rx_urb->list, &priv->rx_urb_pending_list);
 		priv->rx_urb_pending_count++;
-		/*
-		 * Arm the worker under rx_urb_lock so this is atomic with the
-		 * shutdown check: moving it out of the lock would let a
-		 * completion arm the work after rtl8xxxu_stop() canceled it.
-		 */
-		if (priv->rx_urb_pending_count > RTL8XXXU_RX_URB_PENDING_WATER)
-			schedule_work(&priv->rx_urb_wq);
+		/* Serialize scheduling with the shutdown check and cancellation. */
+		if (retry)
+			queue_delayed_work(system_wq, &priv->rx_urb_wq,
+					   msecs_to_jiffies(RTL8XXXU_RX_URB_RETRY_DELAY_MS));
+		else if (priv->rx_urb_pending_count > RTL8XXXU_RX_URB_PENDING_WATER)
+			mod_delayed_work(system_wq, &priv->rx_urb_wq, 0);
 	} else {
-		skb = (struct sk_buff *)rx_urb->urb.context;
-		dev_kfree_skb_irq(skb);
 		usb_free_urb(&rx_urb->urb);
 	}
 
@@ -5901,7 +5898,7 @@ static int rtl8xxxu_submit_rx_urbs(struct rtl8xxxu_priv *priv, bool startup)
 			break;
 		case -ENOMEM:
 		case -EAGAIN:
-			rtl8xxxu_queue_rx_urb(priv, rx_urb);
+			rtl8xxxu_queue_rx_urb(priv, rx_urb, true);
 			break;
 		default:
 			usb_free_urb(&rx_urb->urb);
@@ -5926,7 +5923,8 @@ static void rtl8xxxu_rx_urb_work(struct work_struct *work)
 {
 	struct rtl8xxxu_priv *priv;
 
-	priv = container_of(work, struct rtl8xxxu_priv, rx_urb_wq);
+	priv = container_of(to_delayed_work(work), struct rtl8xxxu_priv,
+			    rx_urb_wq);
 	rtl8xxxu_submit_rx_urbs(priv, false);
 }
 
@@ -6585,10 +6583,23 @@ static void rtl8xxxu_rx_complete(struct urb *urb)
 
 		skb = NULL;
 		rx_urb->urb.context = NULL;
-		rtl8xxxu_queue_rx_urb(priv, rx_urb);
+		rtl8xxxu_queue_rx_urb(priv, rx_urb, false);
 	} else {
 		dev_dbg(dev, "%s: status %i\n",	__func__, urb->status);
-		goto cleanup;
+		switch (urb->status) {
+		case -EPROTO:
+		case -EILSEQ:
+		case -ETIME:
+		case -EOVERFLOW:
+		case -ECOMM:
+		case -ENOSR:
+			dev_kfree_skb(skb);
+			urb->context = NULL;
+			rtl8xxxu_queue_rx_urb(priv, rx_urb, true);
+			return;
+		default:
+			goto cleanup;
+		}
 	}
 	return;
 
@@ -7519,7 +7530,7 @@ static void rtl8xxxu_stop(struct ieee80211_hw *hw, bool suspend)
 	 * it drained via rtl8xxxu_submit_rx_urb(), so a worker still running
 	 * after the kill could submit a URB that escapes it.
 	 */
-	cancel_work_sync(&priv->rx_urb_wq);
+	cancel_delayed_work_sync(&priv->rx_urb_wq);
 
 	usb_kill_anchored_urbs(&priv->rx_anchor);
 	usb_kill_anchored_urbs(&priv->tx_anchor);
@@ -7832,7 +7843,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
 	spin_lock_init(&priv->tx_urb_lock);
 	INIT_LIST_HEAD(&priv->rx_urb_pending_list);
 	spin_lock_init(&priv->rx_urb_lock);
-	INIT_WORK(&priv->rx_urb_wq, rtl8xxxu_rx_urb_work);
+	INIT_DELAYED_WORK(&priv->rx_urb_wq, rtl8xxxu_rx_urb_work);
 	INIT_DELAYED_WORK(&priv->ra_watchdog, rtl8xxxu_watchdog_callback);
 	INIT_DELAYED_WORK(&priv->update_beacon_work, rtl8xxxu_update_beacon_work_callback);
 	skb_queue_head_init(&priv->c2hcmd_queue);
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index eeb18eb0e4c0f..c6953051d9c40 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1809,7 +1809,7 @@ struct rtl8xxxu_priv {
 	struct list_head rx_urb_pending_list;
 	int rx_urb_pending_count;
 	bool shutdown;
-	struct work_struct rx_urb_wq;
+	struct delayed_work rx_urb_wq;
 
 	u8 mac_addr[ETH_ALEN];
 	char chip_name[8];

-- 
2.53.0

RE: [PATCH rtw-next v3 3/3] wifi: rtl8xxxu: preserve RX requests across recoverable transfer errors

[Ping-Ke Shih]

Kim Wooseok via B4 Relay <devnull+5mghybrid.khu.ac.kr@kernel.org> wrote:

[...]

> @@ -5855,9 +5856,8 @@ static int rtl8xxxu_alloc_rx_urbs(struct rtl8xxxu_priv *priv)
>  }
> 
>  static void rtl8xxxu_queue_rx_urb(struct rtl8xxxu_priv *priv,
> -                                 struct rtl8xxxu_rx_urb *rx_urb)
> +                                 struct rtl8xxxu_rx_urb *rx_urb, bool retry)

'retry' --> 'defer_schedule'?

>  {
> -       struct sk_buff *skb;
>         unsigned long flags;
> 
>         spin_lock_irqsave(&priv->rx_urb_lock, flags);
> @@ -5865,16 +5865,13 @@ static void rtl8xxxu_queue_rx_urb(struct rtl8xxxu_priv *priv,
>         if (!priv->shutdown) {
>                 list_add_tail(&rx_urb->list, &priv->rx_urb_pending_list);
>                 priv->rx_urb_pending_count++;
> -               /*
> -                * Arm the worker under rx_urb_lock so this is atomic with the
> -                * shutdown check: moving it out of the lock would let a
> -                * completion arm the work after rtl8xxxu_stop() canceled it.
> -                */
> -               if (priv->rx_urb_pending_count > RTL8XXXU_RX_URB_PENDING_WATER)
> -                       schedule_work(&priv->rx_urb_wq);
> +               /* Serialize scheduling with the shutdown check and cancellation. */
> +               if (retry)
> +                       queue_delayed_work(system_wq, &priv->rx_urb_wq,
> +                                          msecs_to_jiffies(RTL8XXXU_RX_URB_RETRY_DELAY_MS));
> +               else if (priv->rx_urb_pending_count > RTL8XXXU_RX_URB_PENDING_WATER)
> +                       mod_delayed_work(system_wq, &priv->rx_urb_wq, 0);

Not prefer using bare system_wq.
Can both use schedule_delayed_work()? 

>         } else {
> -               skb = (struct sk_buff *)rx_urb->urb.context;
> -               dev_kfree_skb_irq(skb);
>                 usb_free_urb(&rx_urb->urb);
>         }
> 

[...]

> @@ -6585,10 +6583,23 @@ static void rtl8xxxu_rx_complete(struct urb *urb)
> 
>                 skb = NULL;
>                 rx_urb->urb.context = NULL;
> -               rtl8xxxu_queue_rx_urb(priv, rx_urb);
> +               rtl8xxxu_queue_rx_urb(priv, rx_urb, false);
>         } else {
>                 dev_dbg(dev, "%s: status %i\n", __func__, urb->status);

nit: a blank line

> -               goto cleanup;
> +               switch (urb->status) {
> +               case -EPROTO:
> +               case -EILSEQ:
> +               case -ETIME:
> +               case -EOVERFLOW:
> +               case -ECOMM:
> +               case -ENOSR:
> +                       dev_kfree_skb(skb);
> +                       urb->context = NULL;
> +                       rtl8xxxu_queue_rx_urb(priv, rx_urb, true);
> +                       return;
> +               default:
> +                       goto cleanup;
> +               }
>         }
>         return;
> 

[...]

Re: [PATCH rtw-next v3 3/3] wifi: rtl8xxxu: preserve RX requests across recoverable transfer errors

[Kim Wooseok]

Hi Ping-Ke,

> 'retry' --> 'defer_schedule'?

Agreed, I will rename it to defer_schedule.

> Not prefer using bare system_wq.
> Can both use schedule_delayed_work()?

In v3, the error path uses queue_delayed_work(system_wq, ..., delay),
while normal completions use mod_delayed_work(system_wq, ..., 0).

I can replace the error-path call with schedule_delayed_work().
However, the normal-completion path is intended to bring an existing
delayed retry forward when the pending count exceeds the batching
threshold, which schedule_delayed_work(..., 0) cannot do because it
leaves an already pending timer unchanged.

Would it be OK to retain mod_delayed_work() for normal completions,
changing its queue argument to system_percpu_wq?

With that change, schedule_delayed_work() would select system_percpu_wq
internally, and mod_delayed_work() would use the same queue explicitly.

> nit: a blank line

I will add it before the switch.

Thanks,
Kim Wooseok