Crash when rmmod the rtw88_8723cs driver

Crash when rmmod the rtw88_8723cs driver

[Peter Robinson]

Hi Folks,

When playing around with the original Pinephone and the new support
for the rtl8723cs (thanks Fiona!) I had to rmmod the driver to test
suspend (side note there's issues with the driver here) I got the
crash below in the 6.10.x kernel.

Happy to test any patches.

Regards,
Peter

# rmmod rtw88_8723cs rtw88_8703b rtw88_8723x rtw88_sdio rtw88_core
[ 3698.067354] wlan0: deauthenticating from aa:f5:fd:60:4c:a8 by local
choice (Reason: 3=DEAUTH_LEAVING)
[ 3698.300937] ------------[ cut here ]------------
[ 3698.305987] Have pending ack frames!
[ 3698.310853] WARNING: CPU: 3 PID: 9232 at net/mac80211/main.c:1691
ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
[ 3698.324249] Modules linked in: rfcomm snd_seq_dummy snd_hrtimer
nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet
nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat
nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set nf_tables bnep
joydev panel_sitronix_st7703 rtw88_8723cs(-) hci_uart rtw88_8703b
rtw88_8723x btqca rtw88_sdio btrtl rtw88_core btintel mac80211 btbcm
st_magn_spi axp20x_adc st_sensors_spi libarc4 axp20x_pek bluetooth
regmap_spi snd_soc_simple_card gpio_vibra sun50i_codec_analog
snd_soc_simple_card_utils ff_memless sun8i_adda_pr_regmap cfg80211
inv_mpu6050_i2c inv_mpu6050 sun8i_di st_magn_i2c inv_sensors_timestamp
st_magn i2c_mux st_sensors_i2c st_sensors
industrialio_triggered_buffer sun6i_mipi_dsi stk3310 sunxi_cedrus(C)
kfifo_buf goodix_ts snd_soc_simple_amplifier sun8i_rotate
sun8i_thermal sun4i_i2s sun8i_codec v4l2_mem2mem snd_soc_bt_sco
videobuf2_dma_contig videobuf2_memops rfkill videobuf2_v4l2
snd_soc_core videodev snd_compress ac97_bus snd_pcm_dmaengine snd_seq
[ 3698.325088]  videobuf2_common snd_seq_device snd_pcm sun8i_mixer mc
snd_timer snd leds_sgm3140 leds_gpio led_class_flash soundcore vfat
fat pkcs8_key_parser fuse loop nfnetlink zram mmc_block
axp20x_usb_power axp20x_battery pinctrl_axp209 industrialio
axp20x_regulator axp20x_rsb crct10dif_ce polyval_ce polyval_generic
ghash_ce lima governor_simpleondemand sunxi sunxi_rsb pwm_sun4i
sun8i_a33_mbus gpu_sched musb_hdrc phy_sun6i_mipi_dphy i2c_mv64xxx
sunxi_wdt ohci_platform ehci_platform des_generic ohci_hcd libdes
phy_generic udc_core sun4i_drm sun8i_ce nvmem_sunxi_sid sunxi_mmc
phy_sun4i_usb sun6i_dma sun4i_tcon sun8i_tcon_top drm_dma_helper
pwm_bl cpufreq_dt dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua
[ 3698.477870] CPU: 3 PID: 9232 Comm: rmmod Tainted: G         C
  6.10.1-200.fc40.aarch64 #1
[ 3698.486780] Hardware name: pine64 Pine64 PinePhone Braveheart
(1.1)/Pine64 PinePhone Braveheart (1.1), BIOS 2024.01 01/01/2024
[ 3698.498252] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 3698.505306] pc : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
[ 3698.514865] lr : ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
[ 3698.524187] sp : ffff80008c1b37b0
[ 3698.527577] x29: ffff80008c1b37b0 x28: ffff000003be8000 x27: 0000000000000000
[ 3698.534830] x26: 0000000000000000 x25: ffff000003dc14b8 x24: ffff80008c1b37d0
[ 3698.542093] x23: ffff000000ff9f80 x22: 0000000000000000 x21: 000000007fffffff
[ 3698.549342] x20: ffff80007c7e93d8 x19: ffff00006e66f400 x18: 0000000000000000
[ 3698.556597] x17: ffff7ffffd2b3000 x16: ffff800083fc0000 x15: 0000000000000000
[ 3698.563861] x14: 0000000000000000 x13: 2173656d61726620 x12: 6b636120676e6964
[ 3698.571081] x11: 0000000000000000 x10: 000000000000005d x9 : ffff8000802af2b0
[ 3698.578301] x8 : ffff80008c1b3430 x7 : 0000000000000001 x6 : 0000000000000001
[ 3698.585555] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
[ 3698.592942] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000003be8000
[ 3698.600234] Call trace:
[ 3698.602774]  ieee80211_free_ack_frame+0x5c/0x90 [mac80211]
[ 3698.611524]  idr_for_each+0x74/0x110
[ 3698.615226]  ieee80211_free_hw+0x44/0xe8 [mac80211]
[ 3698.623476]  rtw_sdio_remove+0x9c/0xc0 [rtw88_sdio]
[ 3698.628563]  sdio_bus_remove+0x44/0x180
[ 3698.632624]  device_remove+0x54/0x90
[ 3698.636324]  device_release_driver_internal+0x1d4/0x238
[ 3698.641628]  driver_detach+0x54/0xc0
[ 3698.645270]  bus_remove_driver+0x78/0x108
[ 3698.649355]  driver_unregister+0x38/0x78
[ 3698.653374]  sdio_unregister_driver+0x2c/0x40
[ 3698.657808]  rtw_8723cs_driver_exit+0x18/0x1000 [rtw88_8723cs]
[ 3698.663747]  __do_sys_delete_module.isra.0+0x190/0x338
[ 3698.669042]  __arm64_sys_delete_module+0x1c/0x30
[ 3698.673869]  invoke_syscall+0x74/0x100
[ 3698.673924]  el0_svc_common.constprop.0+0x48/0xf0
[ 3698.673935]  do_el0_svc+0x24/0x38
[ 3698.673945]  el0_svc+0x3c/0x158
[ 3698.673971]  el0t_64_sync_handler+0x120/0x138
[ 3698.673994]  el0t_64_sync+0x194/0x198
[ 3698.674027] ---[ end trace 0000000000000000 ]---

RE: Crash when rmmod the rtw88_8723cs driver

[Ping-Ke Shih]

Hi Peter,

> 
> # rmmod rtw88_8723cs rtw88_8703b rtw88_8723x rtw88_sdio rtw88_core
> [ 3698.067354] wlan0: deauthenticating from aa:f5:fd:60:4c:a8 by local
> choice (Reason: 3=DEAUTH_LEAVING)
> [ 3698.300937] ------------[ cut here ]------------
> [ 3698.305987] Have pending ack frames!

I think that is because TX skb isn't acked to mac80211. Please apply below 
changes to see if it can fix the problem.


diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index 763aa8212a4b..7b1ec5ba80e6 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -1297,7 +1297,7 @@ static void rtw_sdio_deinit_tx(struct rtw_dev *rtwdev)
        int i;

        for (i = 0; i < RTK_MAX_TX_QUEUE_NUM; i++)
-               skb_queue_purge(&rtwsdio->tx_queue[i]);
+               ieee80211_purge_tx_queue(rtwdev->hw, &rtwsdio->tx_queue[i]);

        flush_workqueue(rtwsdio->txwq);
        destroy_workqueue(rtwsdio->txwq);
diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index 9145c11a063e..e6b244135ca3 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)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 0a04eaf5343c..f11844f0c80f 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -3180,6 +3180,17 @@ 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.
+ */
+void ieee80211_purge_tx_queue(struct ieee80211_hw *hw,
+                             struct sk_buff_head *skbs);
+
 /**
  * DOC: Hardware crypto acceleration
  *
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);

Re: Crash when rmmod the rtw88_8723cs driver

[Peter Robinson]

Hi Ping-Ke,

On Wed, 7 Aug 2024 at 04:49, Ping-Ke Shih <pkshih@realtek.com> wrote:
>
> Hi Peter,
>
> >
> > # rmmod rtw88_8723cs rtw88_8703b rtw88_8723x rtw88_sdio rtw88_core
> > [ 3698.067354] wlan0: deauthenticating from aa:f5:fd:60:4c:a8 by local
> > choice (Reason: 3=DEAUTH_LEAVING)
> > [ 3698.300937] ------------[ cut here ]------------
> > [ 3698.305987] Have pending ack frames!
>
> I think that is because TX skb isn't acked to mac80211. Please apply below
> changes to see if it can fix the problem.
>
>
> diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
> index 763aa8212a4b..7b1ec5ba80e6 100644
> --- a/drivers/net/wireless/realtek/rtw88/sdio.c
> +++ b/drivers/net/wireless/realtek/rtw88/sdio.c
> @@ -1297,7 +1297,7 @@ static void rtw_sdio_deinit_tx(struct rtw_dev *rtwdev)
>         int i;
>
>         for (i = 0; i < RTK_MAX_TX_QUEUE_NUM; i++)
> -               skb_queue_purge(&rtwsdio->tx_queue[i]);
> +               ieee80211_purge_tx_queue(rtwdev->hw, &rtwsdio->tx_queue[i]);
>
>         flush_workqueue(rtwsdio->txwq);
>         destroy_workqueue(rtwsdio->txwq);
> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> index 9145c11a063e..e6b244135ca3 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]);
>  }

The USB bit doesn't compile because rtwdev is undeclared:

drivers/net/wireless/realtek/rtw88/usb.c: In function
‘rtw_usb_tx_queue_purge’:
drivers/net/wireless/realtek/rtw88/usb.c:424:42: error: ‘rtwdev’
undeclared (first use in this function); did you mean ‘rtw_dev’?
  424 |                 ieee80211_purge_tx_queue(rtwdev->hw,
&rtwusb->tx_queue[i]);
      |                                          ^~~~~~
      |                                          rtw_dev
drivers/net/wireless/realtek/rtw88/usb.c:424:42: note: each undeclared
identifier is reported only once for each function it appears in

I dropped that snippet of the patch as the sdio bit doesn't have that
problem so I could still test and provide feedback.

With just the sdio piece on the Pinephone I no longer get a crash with
6.11.-rc2 when testing with the wifi not connected to an AP as well as
connected and doing a few cycles with different bits works fine
without issue and the wifi still works so apart from a fix for the USB
piece I can give a:

Tested-by: Peter Robinson <pbrobinson@gmail.com> # Pinephone

Regards,
Peter

>  static void rtw_usb_write_port_complete(struct urb *urb)
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 0a04eaf5343c..f11844f0c80f 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -3180,6 +3180,17 @@ 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.
> + */
> +void ieee80211_purge_tx_queue(struct ieee80211_hw *hw,
> +                             struct sk_buff_head *skbs);
> +
>  /**
>   * DOC: Hardware crypto acceleration
>   *
> 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);
>
>