* [PATCH v2 1/2] wifi: mac80211: export ieee80211_purge_tx_queue() for drivers
@ 2024-08-22 1:42 Ping-Ke Shih
2024-08-22 1:42 ` [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb Ping-Ke Shih
0 siblings, 1 reply; 12+ messages in thread
From: Ping-Ke Shih @ 2024-08-22 1:42 UTC (permalink / raw)
To: johannes, linux-wireless; +Cc: pbrobinson
Drivers need to purge TX SKB when stopping. Using skb_queue_purge() can't
report TX status to mac80211, causing ieee80211_free_ack_frame() warns
"Have pending ack frames!". Export ieee80211_purge_tx_queue() for drivers
to not have to reimplement it.
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
v2:
- correct wording in commit message, and mention reason why we export
the symbol
- add comment to note ieee80211_purge_tx_queue() doesn't take locks
- remove prototype from net/mac80211/ieee80211_i.h
---
include/net/mac80211.h | 13 +++++++++++++
net/mac80211/ieee80211_i.h | 2 --
net/mac80211/status.c | 1 +
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 0a04eaf5343c..651fa926c94a 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -3180,6 +3180,19 @@ ieee80211_get_alt_retry_rate(const struct ieee80211_hw *hw,
*/
void ieee80211_free_txskb(struct ieee80211_hw *hw, struct sk_buff *skb);
+/**
+ * ieee80211_purge_tx_queue - purge TX skb queue
+ * @hw: the hardware
+ * @skbs: the skbs
+ *
+ * Free a set of transmit skbs. Use this function when device is going to stop
+ * but some transmit skbs without TX status are still queued.
+ * This function does not take the list lock and the caller must hold the
+ * relevant locks to use it.
+ */
+void ieee80211_purge_tx_queue(struct ieee80211_hw *hw,
+ struct sk_buff_head *skbs);
+
/**
* DOC: Hardware crypto acceleration
*
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index a3485e4c6132..da705bf50d93 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -2072,8 +2072,6 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb,
u32 info_flags,
u32 ctrl_flags,
u64 *cookie);
-void ieee80211_purge_tx_queue(struct ieee80211_hw *hw,
- struct sk_buff_head *skbs);
struct sk_buff *
ieee80211_build_data_template(struct ieee80211_sub_if_data *sdata,
struct sk_buff *skb, u32 info_flags);
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index dd8f857a1fbc..d1cf987de13b 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -1301,3 +1301,4 @@ void ieee80211_purge_tx_queue(struct ieee80211_hw *hw,
while ((skb = __skb_dequeue(skbs)))
ieee80211_free_txskb(hw, skb);
}
+EXPORT_SYMBOL(ieee80211_purge_tx_queue);
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
2024-08-22 1:42 [PATCH v2 1/2] wifi: mac80211: export ieee80211_purge_tx_queue() for drivers Ping-Ke Shih
@ 2024-08-22 1:42 ` Ping-Ke Shih
2024-08-22 1:48 ` Ping-Ke Shih
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Ping-Ke Shih @ 2024-08-22 1:42 UTC (permalink / raw)
To: johannes, linux-wireless; +Cc: pbrobinson
When removing kernel modules by:
rmmod rtw88_8723cs rtw88_8703b rtw88_8723x rtw88_sdio rtw88_core
Driver uses skb_queue_purge() to purge TX skb, but not report tx status
causing "Have pending ack frames!" warning. Use ieee80211_purge_tx_queue()
to correct this.
Since ieee80211_purge_tx_queue() doesn't take locks, to prevent racing
between TX work and purge TX queue, flush and destroy TX work in advance.
wlan0: deauthenticating from aa:f5:fd:60:4c:a8 by local
choice (Reason: 3=DEAUTH_LEAVING)
------------[ cut here ]------------
Have pending ack frames!
WARNING: CPU: 3 PID: 9232 at net/mac80211/main.c:1691
ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
CPU: 3 PID: 9232 Comm: rmmod Tainted: G C
6.10.1-200.fc40.aarch64 #1
Hardware name: pine64 Pine64 PinePhone Braveheart
(1.1)/Pine64 PinePhone Braveheart (1.1), BIOS 2024.01 01/01/2024
pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
lr : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
sp : ffff80008c1b37b0
x29: ffff80008c1b37b0 x28: ffff000003be8000 x27: 0000000000000000
x26: 0000000000000000 x25: ffff000003dc14b8 x24: ffff80008c1b37d0
x23: ffff000000ff9f80 x22: 0000000000000000 x21: 000000007fffffff
x20: ffff80007c7e93d8 x19: ffff00006e66f400 x18: 0000000000000000
x17: ffff7ffffd2b3000 x16: ffff800083fc0000 x15: 0000000000000000
x14: 0000000000000000 x13: 2173656d61726620 x12: 6b636120676e6964
x11: 0000000000000000 x10: 000000000000005d x9 : ffff8000802af2b0
x8 : ffff80008c1b3430 x7 : 0000000000000001 x6 : 0000000000000001
x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000003be8000
Call trace:
ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
idr_for_each+0x74/0x110
ieee80211_free_hw+0x44/0xe8 [mac80211]
rtw_sdio_remove+0x9c/0xc0 [rtw88_sdio]
sdio_bus_remove+0x44/0x180
device_remove+0x54/0x90
device_release_driver_internal+0x1d4/0x238
driver_detach+0x54/0xc0
bus_remove_driver+0x78/0x108
driver_unregister+0x38/0x78
sdio_unregister_driver+0x2c/0x40
rtw_8723cs_driver_exit+0x18/0x1000 [rtw88_8723cs]
__do_sys_delete_module.isra.0+0x190/0x338
__arm64_sys_delete_module+0x1c/0x30
invoke_syscall+0x74/0x100
el0_svc_common.constprop.0+0x48/0xf0
do_el0_svc+0x24/0x38
el0_svc+0x3c/0x158
el0t_64_sync_handler+0x120/0x138
el0t_64_sync+0x194/0x198
---[ end trace 0000000000000000 ]---
Reported-by: Peter Robinson <pbrobinson@gmail.com>
Closes: https://lore.kernel.org/linux-wireless/CALeDE9OAa56KMzgknaCD3quOgYuEHFx9_hcT=OFgmMAb+8MPyA@mail.gmail.com/
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
v2:
- flush and destroy TX work before doing purge_tx_queue
- remove "Tested-by: Peter Robinson" since code changed
---
drivers/net/wireless/realtek/rtw88/sdio.c | 6 +++---
drivers/net/wireless/realtek/rtw88/usb.c | 5 +++--
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index 21d0754dd7f6..b67e551fcee3 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -1297,12 +1297,12 @@ static void rtw_sdio_deinit_tx(struct rtw_dev *rtwdev)
struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
int i;
- for (i = 0; i < RTK_MAX_TX_QUEUE_NUM; i++)
- skb_queue_purge(&rtwsdio->tx_queue[i]);
-
flush_workqueue(rtwsdio->txwq);
destroy_workqueue(rtwsdio->txwq);
kfree(rtwsdio->tx_handler_data);
+
+ for (i = 0; i < RTK_MAX_TX_QUEUE_NUM; i++)
+ ieee80211_purge_tx_queue(rtwdev->hw, &rtwsdio->tx_queue[i]);
}
int rtw_sdio_probe(struct sdio_func *sdio_func,
diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index e83ab6fb83f5..dbc7d8d73494 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -423,10 +423,11 @@ static void rtw_usb_tx_handler(struct work_struct *work)
static void rtw_usb_tx_queue_purge(struct rtw_usb *rtwusb)
{
+ struct rtw_dev *rtwdev = rtwusb->rtwdev;
int i;
for (i = 0; i < ARRAY_SIZE(rtwusb->tx_queue); i++)
- skb_queue_purge(&rtwusb->tx_queue[i]);
+ ieee80211_purge_tx_queue(rtwdev->hw, &rtwusb->tx_queue[i]);
}
static void rtw_usb_write_port_complete(struct urb *urb)
@@ -889,9 +890,9 @@ static void rtw_usb_deinit_tx(struct rtw_dev *rtwdev)
{
struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
- rtw_usb_tx_queue_purge(rtwusb);
flush_workqueue(rtwusb->txwq);
destroy_workqueue(rtwusb->txwq);
+ rtw_usb_tx_queue_purge(rtwusb);
}
static int rtw_usb_intf_init(struct rtw_dev *rtwdev,
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
2024-08-22 1:42 ` [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb Ping-Ke Shih
@ 2024-08-22 1:48 ` Ping-Ke Shih
2024-09-20 6:27 ` Ping-Ke Shih
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Ping-Ke Shih @ 2024-08-22 1:48 UTC (permalink / raw)
To: pbrobinson@gmail.com
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org
Hi Peter,
Could you please test v2 again? Thanks.
Ping-Ke Shih <pkshih@realtek.com> wrote:
> When removing kernel modules by:
> rmmod rtw88_8723cs rtw88_8703b rtw88_8723x rtw88_sdio rtw88_core
>
> Driver uses skb_queue_purge() to purge TX skb, but not report tx status
> causing "Have pending ack frames!" warning. Use ieee80211_purge_tx_queue()
> to correct this.
>
> Since ieee80211_purge_tx_queue() doesn't take locks, to prevent racing
> between TX work and purge TX queue, flush and destroy TX work in advance.
>
> wlan0: deauthenticating from aa:f5:fd:60:4c:a8 by local
> choice (Reason: 3=DEAUTH_LEAVING)
> ------------[ cut here ]------------
> Have pending ack frames!
> WARNING: CPU: 3 PID: 9232 at net/mac80211/main.c:1691
> ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> CPU: 3 PID: 9232 Comm: rmmod Tainted: G C
> 6.10.1-200.fc40.aarch64 #1
> Hardware name: pine64 Pine64 PinePhone Braveheart
> (1.1)/Pine64 PinePhone Braveheart (1.1), BIOS 2024.01 01/01/2024
> pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> lr : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> sp : ffff80008c1b37b0
> x29: ffff80008c1b37b0 x28: ffff000003be8000 x27: 0000000000000000
> x26: 0000000000000000 x25: ffff000003dc14b8 x24: ffff80008c1b37d0
> x23: ffff000000ff9f80 x22: 0000000000000000 x21: 000000007fffffff
> x20: ffff80007c7e93d8 x19: ffff00006e66f400 x18: 0000000000000000
> x17: ffff7ffffd2b3000 x16: ffff800083fc0000 x15: 0000000000000000
> x14: 0000000000000000 x13: 2173656d61726620 x12: 6b636120676e6964
> x11: 0000000000000000 x10: 000000000000005d x9 : ffff8000802af2b0
> x8 : ffff80008c1b3430 x7 : 0000000000000001 x6 : 0000000000000001
> x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000003be8000
> Call trace:
> ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> idr_for_each+0x74/0x110
> ieee80211_free_hw+0x44/0xe8 [mac80211]
> rtw_sdio_remove+0x9c/0xc0 [rtw88_sdio]
> sdio_bus_remove+0x44/0x180
> device_remove+0x54/0x90
> device_release_driver_internal+0x1d4/0x238
> driver_detach+0x54/0xc0
> bus_remove_driver+0x78/0x108
> driver_unregister+0x38/0x78
> sdio_unregister_driver+0x2c/0x40
> rtw_8723cs_driver_exit+0x18/0x1000 [rtw88_8723cs]
> __do_sys_delete_module.isra.0+0x190/0x338
> __arm64_sys_delete_module+0x1c/0x30
> invoke_syscall+0x74/0x100
> el0_svc_common.constprop.0+0x48/0xf0
> do_el0_svc+0x24/0x38
> el0_svc+0x3c/0x158
> el0t_64_sync_handler+0x120/0x138
> el0t_64_sync+0x194/0x198
> ---[ end trace 0000000000000000 ]---
>
> Reported-by: Peter Robinson <pbrobinson@gmail.com>
> Closes:
> https://lore.kernel.org/linux-wireless/CALeDE9OAa56KMzgknaCD3quOgYuEHFx9_hcT=OFgmMAb+8MPyA@mail.gmail.
> com/
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> ---
> v2:
> - flush and destroy TX work before doing purge_tx_queue
> - remove "Tested-by: Peter Robinson" since code changed
> ---
> drivers/net/wireless/realtek/rtw88/sdio.c | 6 +++---
> drivers/net/wireless/realtek/rtw88/usb.c | 5 +++--
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
> index 21d0754dd7f6..b67e551fcee3 100644
> --- a/drivers/net/wireless/realtek/rtw88/sdio.c
> +++ b/drivers/net/wireless/realtek/rtw88/sdio.c
> @@ -1297,12 +1297,12 @@ static void rtw_sdio_deinit_tx(struct rtw_dev *rtwdev)
> struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
> int i;
>
> - for (i = 0; i < RTK_MAX_TX_QUEUE_NUM; i++)
> - skb_queue_purge(&rtwsdio->tx_queue[i]);
> -
> flush_workqueue(rtwsdio->txwq);
> destroy_workqueue(rtwsdio->txwq);
> kfree(rtwsdio->tx_handler_data);
> +
> + for (i = 0; i < RTK_MAX_TX_QUEUE_NUM; i++)
> + ieee80211_purge_tx_queue(rtwdev->hw, &rtwsdio->tx_queue[i]);
> }
>
> int rtw_sdio_probe(struct sdio_func *sdio_func,
> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> index e83ab6fb83f5..dbc7d8d73494 100644
> --- a/drivers/net/wireless/realtek/rtw88/usb.c
> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> @@ -423,10 +423,11 @@ static void rtw_usb_tx_handler(struct work_struct *work)
>
> static void rtw_usb_tx_queue_purge(struct rtw_usb *rtwusb)
> {
> + struct rtw_dev *rtwdev = rtwusb->rtwdev;
> int i;
>
> for (i = 0; i < ARRAY_SIZE(rtwusb->tx_queue); i++)
> - skb_queue_purge(&rtwusb->tx_queue[i]);
> + ieee80211_purge_tx_queue(rtwdev->hw, &rtwusb->tx_queue[i]);
> }
>
> static void rtw_usb_write_port_complete(struct urb *urb)
> @@ -889,9 +890,9 @@ static void rtw_usb_deinit_tx(struct rtw_dev *rtwdev)
> {
> struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
>
> - rtw_usb_tx_queue_purge(rtwusb);
> flush_workqueue(rtwusb->txwq);
> destroy_workqueue(rtwusb->txwq);
> + rtw_usb_tx_queue_purge(rtwusb);
> }
>
> static int rtw_usb_intf_init(struct rtw_dev *rtwdev,
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
2024-08-22 1:42 ` [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb Ping-Ke Shih
2024-08-22 1:48 ` Ping-Ke Shih
@ 2024-09-20 6:27 ` Ping-Ke Shih
2024-09-20 6:33 ` Ping-Ke Shih
2024-11-05 10:54 ` Ladislav Michl
3 siblings, 0 replies; 12+ messages in thread
From: Ping-Ke Shih @ 2024-09-20 6:27 UTC (permalink / raw)
To: pbrobinson@gmail.com
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org
Ping-Ke Shih wrote:
>
> Hi Peter,
>
> Could you please test v2 again? Thanks.
Still no reply from peter though.
>
> Ping-Ke Shih <pkshih@realtek.com> wrote:
> > When removing kernel modules by:
> > rmmod rtw88_8723cs rtw88_8703b rtw88_8723x rtw88_sdio rtw88_core
> >
> > Driver uses skb_queue_purge() to purge TX skb, but not report tx status
> > causing "Have pending ack frames!" warning. Use ieee80211_purge_tx_queue()
> > to correct this.
> >
> > Since ieee80211_purge_tx_queue() doesn't take locks, to prevent racing
> > between TX work and purge TX queue, flush and destroy TX work in advance.
> >
> > wlan0: deauthenticating from aa:f5:fd:60:4c:a8 by local
> > choice (Reason: 3=DEAUTH_LEAVING)
> > ------------[ cut here ]------------
> > Have pending ack frames!
> > WARNING: CPU: 3 PID: 9232 at net/mac80211/main.c:1691
> > ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> > CPU: 3 PID: 9232 Comm: rmmod Tainted: G C
> > 6.10.1-200.fc40.aarch64 #1
> > Hardware name: pine64 Pine64 PinePhone Braveheart
> > (1.1)/Pine64 PinePhone Braveheart (1.1), BIOS 2024.01 01/01/2024
> > pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > pc : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> > lr : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> > sp : ffff80008c1b37b0
> > x29: ffff80008c1b37b0 x28: ffff000003be8000 x27: 0000000000000000
> > x26: 0000000000000000 x25: ffff000003dc14b8 x24: ffff80008c1b37d0
> > x23: ffff000000ff9f80 x22: 0000000000000000 x21: 000000007fffffff
> > x20: ffff80007c7e93d8 x19: ffff00006e66f400 x18: 0000000000000000
> > x17: ffff7ffffd2b3000 x16: ffff800083fc0000 x15: 0000000000000000
> > x14: 0000000000000000 x13: 2173656d61726620 x12: 6b636120676e6964
> > x11: 0000000000000000 x10: 000000000000005d x9 : ffff8000802af2b0
> > x8 : ffff80008c1b3430 x7 : 0000000000000001 x6 : 0000000000000001
> > x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> > x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000003be8000
> > Call trace:
> > ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> > idr_for_each+0x74/0x110
> > ieee80211_free_hw+0x44/0xe8 [mac80211]
> > rtw_sdio_remove+0x9c/0xc0 [rtw88_sdio]
> > sdio_bus_remove+0x44/0x180
> > device_remove+0x54/0x90
> > device_release_driver_internal+0x1d4/0x238
> > driver_detach+0x54/0xc0
> > bus_remove_driver+0x78/0x108
> > driver_unregister+0x38/0x78
> > sdio_unregister_driver+0x2c/0x40
> > rtw_8723cs_driver_exit+0x18/0x1000 [rtw88_8723cs]
> > __do_sys_delete_module.isra.0+0x190/0x338
> > __arm64_sys_delete_module+0x1c/0x30
> > invoke_syscall+0x74/0x100
> > el0_svc_common.constprop.0+0x48/0xf0
> > do_el0_svc+0x24/0x38
> > el0_svc+0x3c/0x158
> > el0t_64_sync_handler+0x120/0x138
> > el0t_64_sync+0x194/0x198
> > ---[ end trace 0000000000000000 ]---
> >
> > Reported-by: Peter Robinson <pbrobinson@gmail.com>
> > Closes:
> >
> https://lore.kernel.org/linux-wireless/CALeDE9OAa56KMzgknaCD3quOgYuEHFx9_hcT=OFgmMAb+8MPyA@mail.gmail.
> > com/
> > Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
I added some test code to be easier to reproduce this problem on USB device
named 8723DU, and this patch can fix the problem, so
Tested-by: Ping-Ke Shih <pkshih@realtek.com> # 8723DU
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
2024-08-22 1:42 ` [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb Ping-Ke Shih
2024-08-22 1:48 ` Ping-Ke Shih
2024-09-20 6:27 ` Ping-Ke Shih
@ 2024-09-20 6:33 ` Ping-Ke Shih
2024-09-20 12:58 ` Bitterblue Smith
2024-11-05 10:54 ` Ladislav Michl
3 siblings, 1 reply; 12+ messages in thread
From: Ping-Ke Shih @ 2024-09-20 6:33 UTC (permalink / raw)
To: Ping-Ke Shih, johannes, linux-wireless; +Cc: pbrobinson
Ping-Ke Shih <pkshih@realtek.com> wrote:
> When removing kernel modules by:
> rmmod rtw88_8723cs rtw88_8703b rtw88_8723x rtw88_sdio rtw88_core
>
> Driver uses skb_queue_purge() to purge TX skb, but not report tx status
> causing "Have pending ack frames!" warning. Use ieee80211_purge_tx_queue()
> to correct this.
>
> Since ieee80211_purge_tx_queue() doesn't take locks, to prevent racing
> between TX work and purge TX queue, flush and destroy TX work in advance.
>
> wlan0: deauthenticating from aa:f5:fd:60:4c:a8 by local
> choice (Reason: 3=DEAUTH_LEAVING)
> ------------[ cut here ]------------
> Have pending ack frames!
> WARNING: CPU: 3 PID: 9232 at net/mac80211/main.c:1691
> ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> CPU: 3 PID: 9232 Comm: rmmod Tainted: G C
> 6.10.1-200.fc40.aarch64 #1
> Hardware name: pine64 Pine64 PinePhone Braveheart
> (1.1)/Pine64 PinePhone Braveheart (1.1), BIOS 2024.01 01/01/2024
> pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> lr : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> sp : ffff80008c1b37b0
> x29: ffff80008c1b37b0 x28: ffff000003be8000 x27: 0000000000000000
> x26: 0000000000000000 x25: ffff000003dc14b8 x24: ffff80008c1b37d0
> x23: ffff000000ff9f80 x22: 0000000000000000 x21: 000000007fffffff
> x20: ffff80007c7e93d8 x19: ffff00006e66f400 x18: 0000000000000000
> x17: ffff7ffffd2b3000 x16: ffff800083fc0000 x15: 0000000000000000
> x14: 0000000000000000 x13: 2173656d61726620 x12: 6b636120676e6964
> x11: 0000000000000000 x10: 000000000000005d x9 : ffff8000802af2b0
> x8 : ffff80008c1b3430 x7 : 0000000000000001 x6 : 0000000000000001
> x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000003be8000
> Call trace:
> ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> idr_for_each+0x74/0x110
> ieee80211_free_hw+0x44/0xe8 [mac80211]
> rtw_sdio_remove+0x9c/0xc0 [rtw88_sdio]
> sdio_bus_remove+0x44/0x180
> device_remove+0x54/0x90
> device_release_driver_internal+0x1d4/0x238
> driver_detach+0x54/0xc0
> bus_remove_driver+0x78/0x108
> driver_unregister+0x38/0x78
> sdio_unregister_driver+0x2c/0x40
> rtw_8723cs_driver_exit+0x18/0x1000 [rtw88_8723cs]
> __do_sys_delete_module.isra.0+0x190/0x338
> __arm64_sys_delete_module+0x1c/0x30
> invoke_syscall+0x74/0x100
> el0_svc_common.constprop.0+0x48/0xf0
> do_el0_svc+0x24/0x38
> el0_svc+0x3c/0x158
> el0t_64_sync_handler+0x120/0x138
> el0t_64_sync+0x194/0x198
> ---[ end trace 0000000000000000 ]---
>
> Reported-by: Peter Robinson <pbrobinson@gmail.com>
> Closes: https://lore.kernel.org/linux-wireless/CALeDE9OAa56KMzgknaCD3quOgYuEHFx9_hcT=OFgmMAb+8MPyA@mail.gmail.com/
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> Tested-by: Ping-Ke Shih <pkshih@realtek.com> # 8723DU
1 patch(es) applied to rtw-next branch of rtw.git, thanks.
3e5e4a801aaf wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
---
https://github.com/pkshih/rtw.git
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
2024-09-20 6:33 ` Ping-Ke Shih
@ 2024-09-20 12:58 ` Bitterblue Smith
2024-09-20 13:11 ` Ping-Ke Shih
0 siblings, 1 reply; 12+ messages in thread
From: Bitterblue Smith @ 2024-09-20 12:58 UTC (permalink / raw)
To: Ping-Ke Shih, johannes, linux-wireless; +Cc: pbrobinson
On 20/09/2024 09:33, Ping-Ke Shih wrote:
> Ping-Ke Shih <pkshih@realtek.com> wrote:
>
>> When removing kernel modules by:
>> rmmod rtw88_8723cs rtw88_8703b rtw88_8723x rtw88_sdio rtw88_core
>>
>> Driver uses skb_queue_purge() to purge TX skb, but not report tx status
>> causing "Have pending ack frames!" warning. Use ieee80211_purge_tx_queue()
>> to correct this.
>>
>> Since ieee80211_purge_tx_queue() doesn't take locks, to prevent racing
>> between TX work and purge TX queue, flush and destroy TX work in advance.
>>
>> wlan0: deauthenticating from aa:f5:fd:60:4c:a8 by local
>> choice (Reason: 3=DEAUTH_LEAVING)
>> ------------[ cut here ]------------
>> Have pending ack frames!
>> WARNING: CPU: 3 PID: 9232 at net/mac80211/main.c:1691
>> ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
>> CPU: 3 PID: 9232 Comm: rmmod Tainted: G C
>> 6.10.1-200.fc40.aarch64 #1
>> Hardware name: pine64 Pine64 PinePhone Braveheart
>> (1.1)/Pine64 PinePhone Braveheart (1.1), BIOS 2024.01 01/01/2024
>> pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
>> lr : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
>> sp : ffff80008c1b37b0
>> x29: ffff80008c1b37b0 x28: ffff000003be8000 x27: 0000000000000000
>> x26: 0000000000000000 x25: ffff000003dc14b8 x24: ffff80008c1b37d0
>> x23: ffff000000ff9f80 x22: 0000000000000000 x21: 000000007fffffff
>> x20: ffff80007c7e93d8 x19: ffff00006e66f400 x18: 0000000000000000
>> x17: ffff7ffffd2b3000 x16: ffff800083fc0000 x15: 0000000000000000
>> x14: 0000000000000000 x13: 2173656d61726620 x12: 6b636120676e6964
>> x11: 0000000000000000 x10: 000000000000005d x9 : ffff8000802af2b0
>> x8 : ffff80008c1b3430 x7 : 0000000000000001 x6 : 0000000000000001
>> x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
>> x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000003be8000
>> Call trace:
>> ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
>> idr_for_each+0x74/0x110
>> ieee80211_free_hw+0x44/0xe8 [mac80211]
>> rtw_sdio_remove+0x9c/0xc0 [rtw88_sdio]
>> sdio_bus_remove+0x44/0x180
>> device_remove+0x54/0x90
>> device_release_driver_internal+0x1d4/0x238
>> driver_detach+0x54/0xc0
>> bus_remove_driver+0x78/0x108
>> driver_unregister+0x38/0x78
>> sdio_unregister_driver+0x2c/0x40
>> rtw_8723cs_driver_exit+0x18/0x1000 [rtw88_8723cs]
>> __do_sys_delete_module.isra.0+0x190/0x338
>> __arm64_sys_delete_module+0x1c/0x30
>> invoke_syscall+0x74/0x100
>> el0_svc_common.constprop.0+0x48/0xf0
>> do_el0_svc+0x24/0x38
>> el0_svc+0x3c/0x158
>> el0t_64_sync_handler+0x120/0x138
>> el0t_64_sync+0x194/0x198
>> ---[ end trace 0000000000000000 ]---
>>
>> Reported-by: Peter Robinson <pbrobinson@gmail.com>
>> Closes: https://lore.kernel.org/linux-wireless/CALeDE9OAa56KMzgknaCD3quOgYuEHFx9_hcT=OFgmMAb+8MPyA@mail.gmail.com/
>> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
>> Tested-by: Ping-Ke Shih <pkshih@realtek.com> # 8723DU
>
> 1 patch(es) applied to rtw-next branch of rtw.git, thanks.
>
> 3e5e4a801aaf wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
>
> ---
> https://github.com/pkshih/rtw.git
>
>
A small problem: patch 1/2 is not applied yet:
linux/drivers/net/wireless/realtek/rtw88/usb.c:430:17: error: implicit declaration of function ‘ieee80211_purge_tx_queue’; did you mean ‘ieee80211_wake_queue’? [-Wimplicit-function-declaration]
430 | ieee80211_purge_tx_queue(rtwdev->hw, &rtwusb->tx_queue[i]);
| ^~~~~~~~~~~~~~~~~~~~~~~~
| ieee80211_wake_queue
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
2024-09-20 12:58 ` Bitterblue Smith
@ 2024-09-20 13:11 ` Ping-Ke Shih
2024-09-20 14:44 ` Bitterblue Smith
0 siblings, 1 reply; 12+ messages in thread
From: Ping-Ke Shih @ 2024-09-20 13:11 UTC (permalink / raw)
To: Bitterblue Smith, johannes@sipsolutions.net,
linux-wireless@vger.kernel.org
Cc: pbrobinson@gmail.com
Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> On 20/09/2024 09:33, Ping-Ke Shih wrote:
> > Ping-Ke Shih <pkshih@realtek.com> wrote:
> >
> >> When removing kernel modules by:
> >> rmmod rtw88_8723cs rtw88_8703b rtw88_8723x rtw88_sdio rtw88_core
> >>
> >> Driver uses skb_queue_purge() to purge TX skb, but not report tx status
> >> causing "Have pending ack frames!" warning. Use ieee80211_purge_tx_queue()
> >> to correct this.
> >>
> >> Since ieee80211_purge_tx_queue() doesn't take locks, to prevent racing
> >> between TX work and purge TX queue, flush and destroy TX work in advance.
> >>
> >> wlan0: deauthenticating from aa:f5:fd:60:4c:a8 by local
> >> choice (Reason: 3=DEAUTH_LEAVING)
> >> ------------[ cut here ]------------
> >> Have pending ack frames!
> >> WARNING: CPU: 3 PID: 9232 at net/mac80211/main.c:1691
> >> ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> >> CPU: 3 PID: 9232 Comm: rmmod Tainted: G C
> >> 6.10.1-200.fc40.aarch64 #1
> >> Hardware name: pine64 Pine64 PinePhone Braveheart
> >> (1.1)/Pine64 PinePhone Braveheart (1.1), BIOS 2024.01 01/01/2024
> >> pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> >> pc : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> >> lr : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> >> sp : ffff80008c1b37b0
> >> x29: ffff80008c1b37b0 x28: ffff000003be8000 x27: 0000000000000000
> >> x26: 0000000000000000 x25: ffff000003dc14b8 x24: ffff80008c1b37d0
> >> x23: ffff000000ff9f80 x22: 0000000000000000 x21: 000000007fffffff
> >> x20: ffff80007c7e93d8 x19: ffff00006e66f400 x18: 0000000000000000
> >> x17: ffff7ffffd2b3000 x16: ffff800083fc0000 x15: 0000000000000000
> >> x14: 0000000000000000 x13: 2173656d61726620 x12: 6b636120676e6964
> >> x11: 0000000000000000 x10: 000000000000005d x9 : ffff8000802af2b0
> >> x8 : ffff80008c1b3430 x7 : 0000000000000001 x6 : 0000000000000001
> >> x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> >> x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000003be8000
> >> Call trace:
> >> ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
> >> idr_for_each+0x74/0x110
> >> ieee80211_free_hw+0x44/0xe8 [mac80211]
> >> rtw_sdio_remove+0x9c/0xc0 [rtw88_sdio]
> >> sdio_bus_remove+0x44/0x180
> >> device_remove+0x54/0x90
> >> device_release_driver_internal+0x1d4/0x238
> >> driver_detach+0x54/0xc0
> >> bus_remove_driver+0x78/0x108
> >> driver_unregister+0x38/0x78
> >> sdio_unregister_driver+0x2c/0x40
> >> rtw_8723cs_driver_exit+0x18/0x1000 [rtw88_8723cs]
> >> __do_sys_delete_module.isra.0+0x190/0x338
> >> __arm64_sys_delete_module+0x1c/0x30
> >> invoke_syscall+0x74/0x100
> >> el0_svc_common.constprop.0+0x48/0xf0
> >> do_el0_svc+0x24/0x38
> >> el0_svc+0x3c/0x158
> >> el0t_64_sync_handler+0x120/0x138
> >> el0t_64_sync+0x194/0x198
> >> ---[ end trace 0000000000000000 ]---
> >>
> >> Reported-by: Peter Robinson <pbrobinson@gmail.com>
> >> Closes: https://lore.kernel.org/linux-wireless/CALeDE9OAa56KMzgknaCD3quOgYuEHFx9_hcT=OFgmMAb+8MPyA@mail.gmail.com/
> >> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> >> Tested-by: Ping-Ke Shih <pkshih@realtek.com> # 8723DU
> >
> > 1 patch(es) applied to rtw-next branch of rtw.git, thanks.
> >
> > 3e5e4a801aaf wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
> >
> > ---
> > https://github.com/pkshih/rtw.git
> >
> >
>
> A small problem: patch 1/2 is not applied yet:
>
> linux/drivers/net/wireless/realtek/rtw88/usb.c:430:17: error: implicit declaration of function ‘ieee80211_purge_tx_queue’; did you mean ‘ieee80211_wake_queue’? [-Wimplicit-function-declaration]
> 430 | ieee80211_purge_tx_queue(rtwdev->hw, &rtwusb->tx_queue[i]);
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> | ieee80211_wake_queue
>
Patch 1/2 is merged [1]. Have you update kernel from rtw tree?
[1] 53bc1b73b678 ("wifi: mac80211: export ieee80211_purge_tx_queue() for drivers")
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
2024-09-20 13:11 ` Ping-Ke Shih
@ 2024-09-20 14:44 ` Bitterblue Smith
0 siblings, 0 replies; 12+ messages in thread
From: Bitterblue Smith @ 2024-09-20 14:44 UTC (permalink / raw)
To: Ping-Ke Shih, johannes@sipsolutions.net,
linux-wireless@vger.kernel.org
Cc: pbrobinson@gmail.com
On 20/09/2024 16:11, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>> On 20/09/2024 09:33, Ping-Ke Shih wrote:
>>> Ping-Ke Shih <pkshih@realtek.com> wrote:
>>>
>>>> When removing kernel modules by:
>>>> rmmod rtw88_8723cs rtw88_8703b rtw88_8723x rtw88_sdio rtw88_core
>>>>
>>>> Driver uses skb_queue_purge() to purge TX skb, but not report tx status
>>>> causing "Have pending ack frames!" warning. Use ieee80211_purge_tx_queue()
>>>> to correct this.
>>>>
>>>> Since ieee80211_purge_tx_queue() doesn't take locks, to prevent racing
>>>> between TX work and purge TX queue, flush and destroy TX work in advance.
>>>>
>>>> wlan0: deauthenticating from aa:f5:fd:60:4c:a8 by local
>>>> choice (Reason: 3=DEAUTH_LEAVING)
>>>> ------------[ cut here ]------------
>>>> Have pending ack frames!
>>>> WARNING: CPU: 3 PID: 9232 at net/mac80211/main.c:1691
>>>> ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
>>>> CPU: 3 PID: 9232 Comm: rmmod Tainted: G C
>>>> 6.10.1-200.fc40.aarch64 #1
>>>> Hardware name: pine64 Pine64 PinePhone Braveheart
>>>> (1.1)/Pine64 PinePhone Braveheart (1.1), BIOS 2024.01 01/01/2024
>>>> pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>>>> pc : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
>>>> lr : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
>>>> sp : ffff80008c1b37b0
>>>> x29: ffff80008c1b37b0 x28: ffff000003be8000 x27: 0000000000000000
>>>> x26: 0000000000000000 x25: ffff000003dc14b8 x24: ffff80008c1b37d0
>>>> x23: ffff000000ff9f80 x22: 0000000000000000 x21: 000000007fffffff
>>>> x20: ffff80007c7e93d8 x19: ffff00006e66f400 x18: 0000000000000000
>>>> x17: ffff7ffffd2b3000 x16: ffff800083fc0000 x15: 0000000000000000
>>>> x14: 0000000000000000 x13: 2173656d61726620 x12: 6b636120676e6964
>>>> x11: 0000000000000000 x10: 000000000000005d x9 : ffff8000802af2b0
>>>> x8 : ffff80008c1b3430 x7 : 0000000000000001 x6 : 0000000000000001
>>>> x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
>>>> x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000003be8000
>>>> Call trace:
>>>> ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
>>>> idr_for_each+0x74/0x110
>>>> ieee80211_free_hw+0x44/0xe8 [mac80211]
>>>> rtw_sdio_remove+0x9c/0xc0 [rtw88_sdio]
>>>> sdio_bus_remove+0x44/0x180
>>>> device_remove+0x54/0x90
>>>> device_release_driver_internal+0x1d4/0x238
>>>> driver_detach+0x54/0xc0
>>>> bus_remove_driver+0x78/0x108
>>>> driver_unregister+0x38/0x78
>>>> sdio_unregister_driver+0x2c/0x40
>>>> rtw_8723cs_driver_exit+0x18/0x1000 [rtw88_8723cs]
>>>> __do_sys_delete_module.isra.0+0x190/0x338
>>>> __arm64_sys_delete_module+0x1c/0x30
>>>> invoke_syscall+0x74/0x100
>>>> el0_svc_common.constprop.0+0x48/0xf0
>>>> do_el0_svc+0x24/0x38
>>>> el0_svc+0x3c/0x158
>>>> el0t_64_sync_handler+0x120/0x138
>>>> el0t_64_sync+0x194/0x198
>>>> ---[ end trace 0000000000000000 ]---
>>>>
>>>> Reported-by: Peter Robinson <pbrobinson@gmail.com>
>>>> Closes: https://lore.kernel.org/linux-wireless/CALeDE9OAa56KMzgknaCD3quOgYuEHFx9_hcT=OFgmMAb+8MPyA@mail.gmail.com/
>>>> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
>>>> Tested-by: Ping-Ke Shih <pkshih@realtek.com> # 8723DU
>>>
>>> 1 patch(es) applied to rtw-next branch of rtw.git, thanks.
>>>
>>> 3e5e4a801aaf wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
>>>
>>> ---
>>> https://github.com/pkshih/rtw.git
>>>
>>>
>>
>> A small problem: patch 1/2 is not applied yet:
>>
>> linux/drivers/net/wireless/realtek/rtw88/usb.c:430:17: error: implicit declaration of function ‘ieee80211_purge_tx_queue’; did you mean ‘ieee80211_wake_queue’? [-Wimplicit-function-declaration]
>> 430 | ieee80211_purge_tx_queue(rtwdev->hw, &rtwusb->tx_queue[i]);
>> | ^~~~~~~~~~~~~~~~~~~~~~~~
>> | ieee80211_wake_queue
>>
>
> Patch 1/2 is merged [1]. Have you update kernel from rtw tree?
>
> [1] 53bc1b73b678 ("wifi: mac80211: export ieee80211_purge_tx_queue() for drivers")
>
>
Oh, right. This is what happens when I try to compile just rtw88.
I didn't see an email from Johannes or Kalle saying the patch was
applied, so I assumed it wasn't. Sorry about that.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
2024-08-22 1:42 ` [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb Ping-Ke Shih
` (2 preceding siblings ...)
2024-09-20 6:33 ` Ping-Ke Shih
@ 2024-11-05 10:54 ` Ladislav Michl
2024-11-06 3:43 ` Ping-Ke Shih
3 siblings, 1 reply; 12+ messages in thread
From: Ladislav Michl @ 2024-11-05 10:54 UTC (permalink / raw)
To: Ping-Ke Shih; +Cc: johannes, linux-wireless, pbrobinson
On Thu, Aug 22, 2024 at 09:42:55AM +0800, Ping-Ke Shih wrote:
> When removing kernel modules by:
> rmmod rtw88_8723cs rtw88_8703b rtw88_8723x rtw88_sdio rtw88_core
>
> Driver uses skb_queue_purge() to purge TX skb, but not report tx status
> causing "Have pending ack frames!" warning. Use ieee80211_purge_tx_queue()
> to correct this.
>
> Since ieee80211_purge_tx_queue() doesn't take locks, to prevent racing
> between TX work and purge TX queue, flush and destroy TX work in advance.
A the very same problem exists in 6.1 kernel series, where this patch
becomes oneliner:
---
drivers/net/wireless/realtek/rtw88/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 81f3112923f1..d7d4b0f05da9 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -2145,7 +2145,7 @@ void rtw_core_deinit(struct rtw_dev *rtwdev)
destroy_workqueue(rtwdev->tx_wq);
spin_lock_irqsave(&rtwdev->tx_report.q_lock, flags);
- skb_queue_purge(&rtwdev->tx_report.queue);
+ ieee80211_purge_tx_queue(rtwdev->hw, &rtwdev->tx_report.queue);
skb_queue_purge(&rtwdev->coex.queue);
spin_unlock_irqrestore(&rtwdev->tx_report.q_lock, flags);
--
2.39.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
2024-11-05 10:54 ` Ladislav Michl
@ 2024-11-06 3:43 ` Ping-Ke Shih
2024-11-06 9:33 ` Ladislav Michl
0 siblings, 1 reply; 12+ messages in thread
From: Ping-Ke Shih @ 2024-11-06 3:43 UTC (permalink / raw)
To: Ladislav Michl
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org,
pbrobinson@gmail.com
Ladislav Michl <oss-lists@triops.cz> wrote:
> On Thu, Aug 22, 2024 at 09:42:55AM +0800, Ping-Ke Shih wrote:
> > When removing kernel modules by:
> > rmmod rtw88_8723cs rtw88_8703b rtw88_8723x rtw88_sdio rtw88_core
> >
> > Driver uses skb_queue_purge() to purge TX skb, but not report tx status
> > causing "Have pending ack frames!" warning. Use ieee80211_purge_tx_queue()
> > to correct this.
> >
> > Since ieee80211_purge_tx_queue() doesn't take locks, to prevent racing
> > between TX work and purge TX queue, flush and destroy TX work in advance.
>
> A the very same problem exists in 6.1 kernel series, where this patch
> becomes oneliner:
linux-6.1.y branch doesn't have patch that exports ieee80211_purge_tx_queue().
> ---
> drivers/net/wireless/realtek/rtw88/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
> index 81f3112923f1..d7d4b0f05da9 100644
> --- a/drivers/net/wireless/realtek/rtw88/main.c
> +++ b/drivers/net/wireless/realtek/rtw88/main.c
> @@ -2145,7 +2145,7 @@ void rtw_core_deinit(struct rtw_dev *rtwdev)
>
> destroy_workqueue(rtwdev->tx_wq);
> spin_lock_irqsave(&rtwdev->tx_report.q_lock, flags);
> - skb_queue_purge(&rtwdev->tx_report.queue);
> + ieee80211_purge_tx_queue(rtwdev->hw, &rtwdev->tx_report.queue);
> skb_queue_purge(&rtwdev->coex.queue);
> spin_unlock_irqrestore(&rtwdev->tx_report.q_lock, flags);
>
Seemingly, we need below also?
diff --git a/drivers/net/wireless/realtek/rtw88/tx.c b/drivers/net/wireless/realtek/rtw88/tx.c
index 6ed470dd6f22..224678b9c38b 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.c
+++ b/drivers/net/wireless/realtek/rtw88/tx.c
@@ -188,7 +188,7 @@ void rtw_tx_report_purge_timer(struct timer_list *t)
rtw_warn(rtwdev, "failed to get tx report from firmware\n");
spin_lock_irqsave(&tx_report->q_lock, flags);
- skb_queue_purge(&tx_report->queue);
+ ieee80211_purge_tx_queue(&tx_report->queue);
spin_unlock_irqrestore(&tx_report->q_lock, flags);
}
Have you seen "Have pending ack frames!" in your side?
If so, use above changes to verify again on the latest kernel, and send a patch?
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
2024-11-06 3:43 ` Ping-Ke Shih
@ 2024-11-06 9:33 ` Ladislav Michl
2024-11-07 0:17 ` Ping-Ke Shih
0 siblings, 1 reply; 12+ messages in thread
From: Ladislav Michl @ 2024-11-06 9:33 UTC (permalink / raw)
To: Ping-Ke Shih
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org,
pbrobinson@gmail.com
On Wed, Nov 06, 2024 at 03:43:24AM +0000, Ping-Ke Shih wrote:
> Ladislav Michl <oss-lists@triops.cz> wrote:
> > On Thu, Aug 22, 2024 at 09:42:55AM +0800, Ping-Ke Shih wrote:
> > > When removing kernel modules by:
> > > rmmod rtw88_8723cs rtw88_8703b rtw88_8723x rtw88_sdio rtw88_core
> > >
> > > Driver uses skb_queue_purge() to purge TX skb, but not report tx status
> > > causing "Have pending ack frames!" warning. Use ieee80211_purge_tx_queue()
> > > to correct this.
> > >
> > > Since ieee80211_purge_tx_queue() doesn't take locks, to prevent racing
> > > between TX work and purge TX queue, flush and destroy TX work in advance.
> >
> > A the very same problem exists in 6.1 kernel series, where this patch
> > becomes oneliner:
>
> linux-6.1.y branch doesn't have patch that exports ieee80211_purge_tx_queue().
Sure, but that one applies with a little fuzz.
> > ---
> > drivers/net/wireless/realtek/rtw88/main.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
> > index 81f3112923f1..d7d4b0f05da9 100644
> > --- a/drivers/net/wireless/realtek/rtw88/main.c
> > +++ b/drivers/net/wireless/realtek/rtw88/main.c
> > @@ -2145,7 +2145,7 @@ void rtw_core_deinit(struct rtw_dev *rtwdev)
> >
> > destroy_workqueue(rtwdev->tx_wq);
> > spin_lock_irqsave(&rtwdev->tx_report.q_lock, flags);
> > - skb_queue_purge(&rtwdev->tx_report.queue);
> > + ieee80211_purge_tx_queue(rtwdev->hw, &rtwdev->tx_report.queue);
> > skb_queue_purge(&rtwdev->coex.queue);
> > spin_unlock_irqrestore(&rtwdev->tx_report.q_lock, flags);
> >
>
> Seemingly, we need below also?
Perhaps I was lucky enough I never hit it here, but for a sake of completeness
it should be there as well.
> diff --git a/drivers/net/wireless/realtek/rtw88/tx.c b/drivers/net/wireless/realtek/rtw88/tx.c
> index 6ed470dd6f22..224678b9c38b 100644
> --- a/drivers/net/wireless/realtek/rtw88/tx.c
> +++ b/drivers/net/wireless/realtek/rtw88/tx.c
> @@ -188,7 +188,7 @@ void rtw_tx_report_purge_timer(struct timer_list *t)
> rtw_warn(rtwdev, "failed to get tx report from firmware\n");
>
> spin_lock_irqsave(&tx_report->q_lock, flags);
> - skb_queue_purge(&tx_report->queue);
> + ieee80211_purge_tx_queue(&tx_report->queue);
> spin_unlock_irqrestore(&tx_report->q_lock, flags);
> }
>
> Have you seen "Have pending ack frames!" in your side?
Yes.
> If so, use above changes to verify again on the latest kernel, and send a patch?
Do you mean to send a patch against linux-6.1.y stable tree? I can see your
patch in wireless-next only, so I do not think sending it to stable now would
work. But in general, having this fix in the stable trees is a good idea.
ladis
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb
2024-11-06 9:33 ` Ladislav Michl
@ 2024-11-07 0:17 ` Ping-Ke Shih
0 siblings, 0 replies; 12+ messages in thread
From: Ping-Ke Shih @ 2024-11-07 0:17 UTC (permalink / raw)
To: Ladislav Michl
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org,
pbrobinson@gmail.com
Ladislav Michl <oss-lists@triops.cz> wrote:
> On Wed, Nov 06, 2024 at 03:43:24AM +0000, Ping-Ke Shih wrote:
> > Ladislav Michl <oss-lists@triops.cz> wrote:
> > > On Thu, Aug 22, 2024 at 09:42:55AM +0800, Ping-Ke Shih wrote:
> > > > When removing kernel modules by:
> > > > rmmod rtw88_8723cs rtw88_8703b rtw88_8723x rtw88_sdio rtw88_core
> > > >
> > > > Driver uses skb_queue_purge() to purge TX skb, but not report tx status
> > > > causing "Have pending ack frames!" warning. Use ieee80211_purge_tx_queue()
> > > > to correct this.
> > > >
> > > > Since ieee80211_purge_tx_queue() doesn't take locks, to prevent racing
> > > > between TX work and purge TX queue, flush and destroy TX work in advance.
> > >
> > > A the very same problem exists in 6.1 kernel series, where this patch
> > > becomes oneliner:
> >
> > linux-6.1.y branch doesn't have patch that exports ieee80211_purge_tx_queue().
>
> Sure, but that one applies with a little fuzz.
>
> > > ---
> > > drivers/net/wireless/realtek/rtw88/main.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
> > > index 81f3112923f1..d7d4b0f05da9 100644
> > > --- a/drivers/net/wireless/realtek/rtw88/main.c
> > > +++ b/drivers/net/wireless/realtek/rtw88/main.c
> > > @@ -2145,7 +2145,7 @@ void rtw_core_deinit(struct rtw_dev *rtwdev)
> > >
> > > destroy_workqueue(rtwdev->tx_wq);
> > > spin_lock_irqsave(&rtwdev->tx_report.q_lock, flags);
> > > - skb_queue_purge(&rtwdev->tx_report.queue);
> > > + ieee80211_purge_tx_queue(rtwdev->hw, &rtwdev->tx_report.queue);
> > > skb_queue_purge(&rtwdev->coex.queue);
> > > spin_unlock_irqrestore(&rtwdev->tx_report.q_lock, flags);
> > >
> >
> > Seemingly, we need below also?
>
> Perhaps I was lucky enough I never hit it here, but for a sake of completeness
> it should be there as well.
>
> > diff --git a/drivers/net/wireless/realtek/rtw88/tx.c b/drivers/net/wireless/realtek/rtw88/tx.c
> > index 6ed470dd6f22..224678b9c38b 100644
> > --- a/drivers/net/wireless/realtek/rtw88/tx.c
> > +++ b/drivers/net/wireless/realtek/rtw88/tx.c
> > @@ -188,7 +188,7 @@ void rtw_tx_report_purge_timer(struct timer_list *t)
> > rtw_warn(rtwdev, "failed to get tx report from firmware\n");
> >
> > spin_lock_irqsave(&tx_report->q_lock, flags);
> > - skb_queue_purge(&tx_report->queue);
> > + ieee80211_purge_tx_queue(&tx_report->queue);
> > spin_unlock_irqrestore(&tx_report->q_lock, flags);
> > }
> >
> > Have you seen "Have pending ack frames!" in your side?
>
> Yes.
>
> > If so, use above changes to verify again on the latest kernel, and send a patch?
>
> Do you mean to send a patch against linux-6.1.y stable tree? I can see your
> patch in wireless-next only, so I do not think sending it to stable now would
> work. But in general, having this fix in the stable trees is a good idea.
I meant to send a patch to wireless-next. After it gets merged, send another
backport patchset related to the warning to 6.1 and 6.6.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-11-07 0:17 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-22 1:42 [PATCH v2 1/2] wifi: mac80211: export ieee80211_purge_tx_queue() for drivers Ping-Ke Shih
2024-08-22 1:42 ` [PATCH v2 2/2] wifi: rtw88: use ieee80211_purge_tx_queue() to purge TX skb Ping-Ke Shih
2024-08-22 1:48 ` Ping-Ke Shih
2024-09-20 6:27 ` Ping-Ke Shih
2024-09-20 6:33 ` Ping-Ke Shih
2024-09-20 12:58 ` Bitterblue Smith
2024-09-20 13:11 ` Ping-Ke Shih
2024-09-20 14:44 ` Bitterblue Smith
2024-11-05 10:54 ` Ladislav Michl
2024-11-06 3:43 ` Ping-Ke Shih
2024-11-06 9:33 ` Ladislav Michl
2024-11-07 0:17 ` Ping-Ke Shih
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).