| Message ID | 20221229124845.1155429-1-martin.blumenstingl@googlemail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | rtw88: Four fixes found while working on SDIO support | expand |
On 12/29/22 06:48, Martin Blumenstingl wrote: > This series consists of three patches which are fixing existing > behavior (meaning: it either affects PCIe or USB or both) in the rtw88 > driver. > > The first change adds the packed attribute to the eFuse structs. This > was spotted by Ping-Ke while reviewing the SDIO support patches from > [0]. > > The remaining three changes relate to locking (barrier hold) problems. > We previously had discussed patches for this for SDIO support, but the > problem never ocurred while testing USB cards. It turns out that these > are still needed and I think that they also fix the same problems for > USB users (it's not clear how often it happens there though). > > The issue fixed by the second and third patches have been spotted by a > user who tested rtw88 SDIO support. Everything is working fine for him > but there are warnings [1] and [2] in the kernel log stating "Voluntary > context switch within RCU read-side critical section!". > > The solution in the third and fourth patch was actually suggested by > Ping-Ke in [3]. Thanks again! > > These fixes are indepdent of my other series adding SDIO support to the > rtw88 driver, meaning they can be added to the wireless driver tree on > top of Linux 6.2-rc1 or linux-next. > > > Changes since v1 at [4]: > - Keep the u8 bitfields in patch 1 but split the res2 field into res2_1 > and res2_2 as suggested by Ping-Ke > - Added Ping-Ke's reviewed-by to patches 2-4 - thank you! > - Added a paragraph in the cover-letter to avoid confusion whether > these patches depend on the rtw88 SDIO support series > > > [0] https://lore.kernel.org/linux-wireless/695c976e02ed44a2b2345a3ceb226fc4@realtek.com/ > [1] https://github.com/LibreELEC/LibreELEC.tv/pull/7301#issuecomment-1366421445 > [2] https://github.com/LibreELEC/LibreELEC.tv/pull/7301#issuecomment-1366610249 > [3] https://lore.kernel.org/lkml/e0aa1ba4336ab130712e1fcb425e6fd0adca4145.camel@realtek.com/ > > > Martin Blumenstingl (4): > rtw88: Add packed attribute to the eFuse structs > rtw88: Configure the registers from rtw_bf_assoc() outside the RCU > lock > rtw88: Use rtw_iterate_vifs() for rtw_vif_watch_dog_iter() > rtw88: Use non-atomic rtw_iterate_stas() in rtw_ra_mask_info_update() > > drivers/net/wireless/realtek/rtw88/bf.c | 13 +++++++------ > drivers/net/wireless/realtek/rtw88/mac80211.c | 4 +++- > drivers/net/wireless/realtek/rtw88/main.c | 6 ++++-- > drivers/net/wireless/realtek/rtw88/main.h | 6 +++--- > drivers/net/wireless/realtek/rtw88/rtw8723d.h | 6 +++--- > drivers/net/wireless/realtek/rtw88/rtw8821c.h | 9 +++++---- > drivers/net/wireless/realtek/rtw88/rtw8822b.h | 9 +++++---- > drivers/net/wireless/realtek/rtw88/rtw8822c.h | 9 +++++---- > 8 files changed, 35 insertions(+), 27 deletions(-) > Martin, I do not feel qualified to review these contributions, but I have some suggestions. The first is that the subject should start with wifi: rtw88: .... That is a fairly recent change that you likely did not catch. My second comment is that changed patches should have a version number to identify that they are new patches. You can generate the correct form using the "-v N" option in 'git format-email'. Once you have generated the patches, you should then edit them to indicate what change was made to each patch in the various versions. Such explanations should go below the --- following the Signed-off-by line, and end with another ---. With these additions, the community, and more importantly Kalle, can keep track of the various versions, and know what reviewer's comments have been addressed. I know of several people that have asked about SDIO versions of these drivers. They will be pleased to see them become available. Larry
Hello Larry, On Thu, Dec 29, 2022 at 6:41 PM Larry Finger <Larry.Finger@lwfinger.net> wrote: [...] > I do not feel qualified to review these contributions, but I have some suggestions. > > The first is that the subject should start with wifi: rtw88: .... That is a > fairly recent change that you likely did not catch. Oh, this is something that I missed. I'll wait until tomorrow to see if I can get Ping-Ke's Reviewed-by on patch 1 and then re-send the whole series with fixed subjects. > My second comment is that changed patches should have a version number to > identify that they are new patches. This series had four patches from the beginning. So no patches were added/removed during the lifecycle of this patchset. I think the cover-letter subject is a bit misleading as it contains the words "SDIO support". In fact the issues (which are fixed by this series) were found while working on SDIO support, but they also apply to existing PCIe/USB support. > [...] Once you have generated the patches, you > should then edit them to indicate what change was made to each patch in the > various versions. Such explanations should go below the --- following the > Signed-off-by line, and end with another ---. With these additions, the > community, and more importantly Kalle, can keep track of the various versions, > and know what reviewer's comments have been addressed. Noted. I will take care of this in v3 along with the updated subjects. > I know of several people that have asked about SDIO versions of these drivers. > They will be pleased to see them become available. Thanks for the motivating words :-) Best regards, Martin
On Thu, Dec 29, 2022 at 01:48:41PM +0100, Martin Blumenstingl wrote: > This series consists of three patches which are fixing existing > behavior (meaning: it either affects PCIe or USB or both) in the rtw88 > driver. > > The first change adds the packed attribute to the eFuse structs. This > was spotted by Ping-Ke while reviewing the SDIO support patches from > [0]. > > The remaining three changes relate to locking (barrier hold) problems. > We previously had discussed patches for this for SDIO support, but the > problem never ocurred while testing USB cards. It turns out that these > are still needed and I think that they also fix the same problems for > USB users (it's not clear how often it happens there though). Indeed I haven't stumbled over any of the issues you fix in this series. I briefly looked into it and it might be that at least some of the code paths are only used with beam forming support. Could it be that my cheapo USB sticks do not support that? For what it's worth, I gave this series a quick test and didn't see any regressions, so: Tested-by: Sascha Hauer <s.hauer@pengutronix.de> Sascha
In the course of 3 weeks my Shuttle DS10U bare bone running Ubuntu 22.04 server locked up 3 times. I'm using the Realtek RTL8822CE PCIe module in access point mode. Below a dump of the first lock up. There is no log from the other two lockups, possibly because of 'options rtw88_pci disable_aspm=1' in rtw88_pci.conf I hope this is of any use to you. Best regards, Gert Erkelens Dec 29 22:24:29 shuttle kernel: [98328.813880] BUG: scheduling while atomic: kworker/u4:0/7592/0x00000700 Dec 29 22:24:29 shuttle kernel: [98328.813889] Modules linked in: tls ccm xt_nat xt_state xt_MASQUERADE nft_reject_ipv4 nft_reject nft_ct nft_masq nft_chain_nat nf_nat_h323 nf_conntrack_h323 nf_nat_pptp nf_conntrack_pptp nf_nat_tftp nf_conntrack_tftp nf_nat_sip nf_conntrack_sip nf_nat_irc nf_conntrack_irc nf_nat_ftp nf_conntrack_ftp iptable_nat nf_nat ip6t_REJECT nf_reject_ipv6 xt_hl ip6_tables ip6t_rt ipt_REJECT nf_reject_ipv4 xt_LOG nf_log_syslog nft_limit binfmt_misc xt_limit xt_addrtype xt_tcpudp xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nft_compat nft_counter nf_tables nfnetlink nls_iso8859_1 snd_sof_pci_intel_cnl snd_sof_intel_hda_common soundwire_intel rtw88_8822ce soundwire_generic_allocation rtw88_8822c soundwire_cadence snd_sof_intel_hda rtw88_pci snd_sof_pci intel_tcc_cooling snd_sof_xtensa_dsp x86_pkg_temp_thermal intel_powerclamp rtw88_core coretemp snd_sof snd_soc_hdac_hda snd_hda_ext_core mac80211 snd_soc_acpi_intel_match kvm_intel snd_soc_acpi mei_hdcp intel_rapl_msr soundwire_bus kvm Dec 29 22:24:29 shuttle kernel: [98328.813949] snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_soc_core snd_compress ac97_bus snd_pcm_dmaengine snd_hda_intel snd_intel_dspcfg btusb rapl intel_cstate snd_intel_sdw_acpi btrtl cfg80211 snd_hda_codec btbcm snd_hda_core btintel snd_hwdep snd_pcm wmi_bmof processor_thermal_device_pci_legacy snd_timer processor_thermal_device bluetooth libarc4 processor_thermal_rfim snd ee1004 processor_thermal_mbox ecdh_generic soundcore processor_thermal_rapl mei_me ecc intel_rapl_common mei int340x_thermal_zone intel_pch_thermal intel_soc_dts_iosf mac_hid acpi_pad acpi_tad ramoops pstore_blk reed_solomon pstore_zone efi_pstore dm_multipath scsi_dh_rdac scsi_dh_emc sch_fq_codel scsi_dh_alua msr ip_tables x_tables autofs4 btrfs blake2b_generic zstd_compress raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c raid1 raid0 multipath linear i915 ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops Dec 29 22:24:29 shuttle kernel: [98328.814014] crct10dif_pclmul cec crc32_pclmul ghash_clmulni_intel igb rc_core aesni_intel i2c_i801 drm crypto_simd cryptd dca i2c_smbus i2c_algo_bit e1000e xhci_pci xhci_pci_renesas ahci libahci wmi video pinctrl_cannonlake Dec 29 22:24:29 shuttle kernel: [98328.814032] CPU: 0 PID: 7592 Comm: kworker/u4:0 Not tainted 5.15.0-56-generic #62-Ubuntu Dec 29 22:24:29 shuttle kernel: [98328.814037] Hardware name: Shuttle Inc. DS10U/DS10U, BIOS 1.07 11/18/2022 Dec 29 22:24:29 shuttle kernel: [98328.814038] Workqueue: phy0 rtw_watch_dog_work [rtw88_core] Dec 29 22:24:29 shuttle kernel: [98328.814054] Call Trace: Dec 29 22:24:29 shuttle kernel: [98328.814056] <IRQ> Dec 29 22:24:29 shuttle kernel: [98328.814059] show_stack+0x52/0x5c Dec 29 22:24:29 shuttle kernel: [98328.814064] dump_stack_lvl+0x4a/0x63 Dec 29 22:24:29 shuttle kernel: [98328.814070] dump_stack+0x10/0x16 Dec 29 22:24:29 shuttle kernel: [98328.814074] __schedule_bug.cold+0x4e/0x60 Dec 29 22:24:29 shuttle kernel: [98328.814077] __schedule+0x449/0x590 Dec 29 22:24:29 shuttle kernel: [98328.814081] schedule+0x69/0x110 Dec 29 22:24:29 shuttle kernel: [98328.814084] schedule_preempt_disabled+0xe/0x20 Dec 29 22:24:29 shuttle kernel: [98328.814088] __mutex_lock.constprop.0+0x484/0x490 Dec 29 22:24:29 shuttle kernel: [98328.814091] ? __qdisc_run+0x96/0xe0 Dec 29 22:24:29 shuttle kernel: [98328.814097] __mutex_lock_slowpath+0x13/0x20 Dec 29 22:24:29 shuttle kernel: [98328.814100] mutex_lock+0x38/0x50 Dec 29 22:24:29 shuttle kernel: [98328.814104] rtw_ops_set_tim+0x20/0x40 [rtw88_core] Dec 29 22:24:29 shuttle kernel: [98328.814117] __sta_info_recalc_tim+0x1a1/0x3d0 [mac80211] Dec 29 22:24:29 shuttle kernel: [98328.814172] sta_info_recalc_tim+0x10/0x20 [mac80211] Dec 29 22:24:29 shuttle kernel: [98328.814221] ieee80211_tx_h_unicast_ps_buf+0x225/0x310 [mac80211] Dec 29 22:24:29 shuttle kernel: [98328.814283] invoke_tx_handlers_early+0xbf/0x360 [mac80211] Dec 29 22:24:29 shuttle kernel: [98328.814341] ieee80211_tx+0x9c/0x160 [mac80211] Dec 29 22:24:29 shuttle kernel: [98328.814398] ieee80211_xmit+0xc0/0x100 [mac80211] Dec 29 22:24:29 shuttle kernel: [98328.814454] __ieee80211_subif_start_xmit+0x1f6/0x360 [mac80211] Dec 29 22:24:29 shuttle kernel: [98328.814509] ? consume_skb+0x43/0xb0 Dec 29 22:24:29 shuttle kernel: [98328.814515] ieee80211_subif_start_xmit+0x47/0x190 [mac80211] Dec 29 22:24:29 shuttle kernel: [98328.814570] ? dev_queue_xmit_nit+0x275/0x2b0 Dec 29 22:24:29 shuttle kernel: [98328.814574] xmit_one.constprop.0+0x99/0x160 Dec 29 22:24:29 shuttle kernel: [98328.814576] dev_hard_start_xmit+0x45/0x90 Dec 29 22:24:29 shuttle kernel: [98328.814580] __dev_queue_xmit+0x46d/0x510 Dec 29 22:24:29 shuttle kernel: [98328.814583] ? neigh_add_timer+0x37/0x60 Dec 29 22:24:29 shuttle kernel: [98328.814588] dev_queue_xmit+0x10/0x20 Dec 29 22:24:29 shuttle kernel: [98328.814591] neigh_resolve_output+0x114/0x1b0 Dec 29 22:24:29 shuttle kernel: [98328.814594] ip_finish_output2+0x17a/0x460 Dec 29 22:24:29 shuttle kernel: [98328.814597] __ip_finish_output+0xb7/0x180 Dec 29 22:24:29 shuttle kernel: [98328.814599] ip_finish_output+0x2e/0xc0 Dec 29 22:24:29 shuttle kernel: [98328.814602] ip_output+0x78/0x100 Dec 29 22:24:29 shuttle kernel: [98328.814604] ? __ip_finish_output+0x180/0x180 Dec 29 22:24:29 shuttle kernel: [98328.814606] ip_forward_finish+0x8b/0xb0 Dec 29 22:24:29 shuttle kernel: [98328.814612] ip_forward+0x3fc/0x550 Dec 29 22:24:29 shuttle kernel: [98328.814616] ? __build_flow_key.constprop.0+0x92/0xf0 Dec 29 22:24:29 shuttle kernel: [98328.814620] ? ip_expire+0x1a0/0x1a0 Dec 29 22:24:29 shuttle kernel: [98328.814624] ip_sublist_rcv_finish+0x6f/0x80 Dec 29 22:24:29 shuttle kernel: [98328.814628] ip_sublist_rcv+0x17c/0x200 Dec 29 22:24:29 shuttle kernel: [98328.814633] ? ip_sublist_rcv+0x200/0x200 Dec 29 22:24:29 shuttle kernel: [98328.814637] ip_list_rcv+0xf9/0x120 Dec 29 22:24:29 shuttle kernel: [98328.814641] __netif_receive_skb_list_core+0x218/0x240 Dec 29 22:24:29 shuttle kernel: [98328.814645] netif_receive_skb_list_internal+0x18e/0x2a0 Dec 29 22:24:29 shuttle kernel: [98328.814649] ? e1000_clean_rx_irq+0x34d/0x3a0 [e1000e] Dec 29 22:24:29 shuttle kernel: [98328.814669] napi_complete_done+0x7a/0x1c0 Dec 29 22:24:29 shuttle kernel: [98328.814672] e1000e_poll+0xcf/0x2d0 [e1000e] Dec 29 22:24:29 shuttle kernel: [98328.814690] __napi_poll+0x30/0x180 Dec 29 22:24:29 shuttle kernel: [98328.814693] net_rx_action+0x126/0x280 Dec 29 22:24:29 shuttle kernel: [98328.814697] __do_softirq+0xd6/0x2e7 Dec 29 22:24:29 shuttle kernel: [98328.814701] irq_exit_rcu+0x94/0xc0 Dec 29 22:24:29 shuttle kernel: [98328.814706] common_interrupt+0x8e/0xa0 Dec 29 22:24:29 shuttle kernel: [98328.814710] </IRQ> Dec 29 22:24:29 shuttle kernel: [98328.814712] <TASK> Dec 29 22:24:29 shuttle kernel: [98328.814713] asm_common_interrupt+0x27/0x40 Dec 29 22:24:29 shuttle kernel: [98328.814715] RIP: 0010:rtw_pci_read32+0x14/0x20 [rtw88_pci] Dec 29 22:24:29 shuttle kernel: [98328.814721] Code: b7 30 7a 00 00 48 89 e5 66 8b 06 5d c3 cc cc cc cc 0f 1f 44 00 00 0f 1f 44 00 00 55 89 f6 48 03 b7 30 7a 00 00 48 89 e5 8b 06 <5d> c3 cc cc cc cc 66 0f 1f 44 00 00 0f 1f 44 00 00 55 89 f6 48 03 Dec 29 22:24:29 shuttle kernel: [98328.814724] RSP: 0018:ffffbff902427da0 EFLAGS: 00000282 Dec 29 22:24:29 shuttle kernel: [98328.814727] RAX: 00000000c0000000 RBX: 0000000000000002 RCX: 000000000000000c Dec 29 22:24:29 shuttle kernel: [98328.814729] RDX: 0000000012baa000 RSI: ffffbff900a11d2c RDI: ffff9b370b9b2020 Dec 29 22:24:29 shuttle kernel: [98328.814731] RBP: ffffbff902427da0 R08: 0000000000000000 R09: 0000000000000000 Dec 29 22:24:29 shuttle kernel: [98328.814732] R10: 0000000000000005 R11: 0000000000000000 R12: ffff9b370b9b2020 Dec 29 22:24:29 shuttle kernel: [98328.814734] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 Dec 29 22:24:29 shuttle kernel: [98328.814737] rtw8822c_false_alarm_statistics+0x26d/0x350 [rtw88_8822c] Dec 29 22:24:29 shuttle kernel: [98328.814744] rtw_phy_dynamic_mechanism+0x71/0x320 [rtw88_core] Dec 29 22:24:29 shuttle kernel: [98328.814761] rtw_watch_dog_work+0x1cd/0x260 [rtw88_core] Dec 29 22:24:29 shuttle kernel: [98328.814773] process_one_work+0x228/0x3d0 Dec 29 22:24:29 shuttle kernel: [98328.814776] worker_thread+0x53/0x420 Dec 29 22:24:29 shuttle kernel: [98328.814779] ? process_one_work+0x3d0/0x3d0 Dec 29 22:24:29 shuttle kernel: [98328.814781] kthread+0x127/0x150 Dec 29 22:24:29 shuttle kernel: [98328.814785] ? set_kthread_struct+0x50/0x50 Dec 29 22:24:29 shuttle kernel: [98328.814789] ret_from_fork+0x1f/0x30 Dec 29 22:24:29 shuttle kernel: [98328.814796] </TASK> Dec 29 22:24:29 shuttle kernel: [98328.814811] NOHZ tick-stop error: Non-RCU local softirq work is pending, handler #88!!! Dec 29 22:24:35 shuttle kernel: [98333.925935] Dead loop on virtual device wlp2s0, fix it urgently! Dec 29 22:24:36 shuttle kernel: [98334.949866] Dead loop on virtual device wlp2s0, fix it urgently! Dec 29 22:24:37 shuttle kernel: [98335.974049] Dead loop on virtual device wlp2s0, fix it urgently!
Hi Gert, On Sat, Jan 7, 2023 at 7:23 PM gert erkelens <g.erkelens5@xs4all.nl> wrote: > > > In the course of 3 weeks my Shuttle DS10U bare bone running Ubuntu 22.04 server locked up 3 times. > I'm using the Realtek RTL8822CE PCIe module in access point mode. > Below a dump of the first lock up. There is no log from the other two lockups, possibly because of > 'options rtw88_pci disable_aspm=1' in rtw88_pci.conf > > I hope this is of any use to you. Did you try patches 2-4 from this series? Patch 3 should solve this specific issue. Best regards, Martin
> -----Original Message----- > From: gert erkelens <g.erkelens5@xs4all.nl> > Sent: Sunday, January 8, 2023 2:23 AM > To: martin.blumenstingl@googlemail.com > Cc: kvalo@kernel.org; linux-kernel@vger.kernel.org; linux-wireless@vger.kernel.org; > netdev@vger.kernel.org; Ping-Ke Shih <pkshih@realtek.com>; s.hauer@pengutronix.de; > tony0620emma@gmail.com > Subject: Re: [PATCH 0/4] rtw88: Four fixes found while working on SDIO support > > In the course of 3 weeks my Shuttle DS10U bare bone running Ubuntu 22.04 server locked up 3 times. > I'm using the Realtek RTL8822CE PCIe module in access point mode. > Below a dump of the first lock up. There is no log from the other two lockups, possibly because of > 'options rtw88_pci disable_aspm=1' in rtw88_pci.conf > > I hope this is of any use to you. > > Best regards, > Gert Erkelens > > > Dec 29 22:24:29 shuttle kernel: [98328.813880] BUG: scheduling while atomic: > kworker/u4:0/7592/0x00000700 [...] > Dec 29 22:24:29 shuttle kernel: [98328.814032] CPU: 0 PID: 7592 Comm: kworker/u4:0 Not tainted > 5.15.0-56-generic #62-Ubuntu The trace below is very similar to this fix 7711fe713a49 ("wifi: rtw88: add a work to correct atomic scheduling warning of ::set_tim") Please check if the driver you are using includes it. By the way, ::set_time is added after 5.19, but your kernel is 5.15.0-56-generic Ping-Ke
This series consists of three patches which are fixing existing behavior (meaning: it either affects PCIe or USB or both) in the rtw88 driver. The first change adds the packed attribute to the eFuse structs. This was spotted by Ping-Ke while reviewing the SDIO support patches from [0]. The remaining three changes relate to locking (barrier hold) problems. We previously had discussed patches for this for SDIO support, but the problem never ocurred while testing USB cards. It turns out that these are still needed and I think that they also fix the same problems for USB users (it's not clear how often it happens there though). The issue fixed by the second and third patches have been spotted by a user who tested rtw88 SDIO support. Everything is working fine for him but there are warnings [1] and [2] in the kernel log stating "Voluntary context switch within RCU read-side critical section!". The solution in the third and fourth patch was actually suggested by Ping-Ke in [3]. Thanks again! These fixes are indepdent of my other series adding SDIO support to the rtw88 driver, meaning they can be added to the wireless driver tree on top of Linux 6.2-rc1 or linux-next. Changes since v1 at [4]: - Keep the u8 bitfields in patch 1 but split the res2 field into res2_1 and res2_2 as suggested by Ping-Ke - Added Ping-Ke's reviewed-by to patches 2-4 - thank you! - Added a paragraph in the cover-letter to avoid confusion whether these patches depend on the rtw88 SDIO support series [0] https://lore.kernel.org/linux-wireless/695c976e02ed44a2b2345a3ceb226fc4@realtek.com/ [1] https://github.com/LibreELEC/LibreELEC.tv/pull/7301#issuecomment-1366421445 [2] https://github.com/LibreELEC/LibreELEC.tv/pull/7301#issuecomment-1366610249 [3] https://lore.kernel.org/lkml/e0aa1ba4336ab130712e1fcb425e6fd0adca4145.camel@realtek.com/ Martin Blumenstingl (4): rtw88: Add packed attribute to the eFuse structs rtw88: Configure the registers from rtw_bf_assoc() outside the RCU lock rtw88: Use rtw_iterate_vifs() for rtw_vif_watch_dog_iter() rtw88: Use non-atomic rtw_iterate_stas() in rtw_ra_mask_info_update() drivers/net/wireless/realtek/rtw88/bf.c | 13 +++++++------ drivers/net/wireless/realtek/rtw88/mac80211.c | 4 +++- drivers/net/wireless/realtek/rtw88/main.c | 6 ++++-- drivers/net/wireless/realtek/rtw88/main.h | 6 +++--- drivers/net/wireless/realtek/rtw88/rtw8723d.h | 6 +++--- drivers/net/wireless/realtek/rtw88/rtw8821c.h | 9 +++++---- drivers/net/wireless/realtek/rtw88/rtw8822b.h | 9 +++++---- drivers/net/wireless/realtek/rtw88/rtw8822c.h | 9 +++++---- 8 files changed, 35 insertions(+), 27 deletions(-)