ath10k fixes for warns
Subsystems (3)
cover ath10k fixes for warns
I have been seeing lockdep asserts for a couple of months and finally found time to debug and fix the problems. The dmesg looks clean with these fixes.
Enabling LOCKDEP and ATH10K_DEBUGFS triggers the lockdep assert and RCU warns.
The first two patches in this series are fixes to lockdep assert and RCU usage bugs.
The last patch (5/5) is a fix to reduce invalid ht params rate message noise. Patch 3/4 changes a message from debug to warn. Patch 4 adds detect to assert not calling ath10k_drain_tx() holding conf_mutex.
Shuah Khan (5):
ath10k: fix conf_mutex lock assert in ath10k_debug_fw_stats_request()
ath10k: fix WARNING: suspicious RCU usage
ath10k: change ath10k_offchan_tx_work() peer present msg to a warn
ath10k: detect conf_mutex held ath10k_drain_tx() calls
ath10k: reduce invalid ht params rate message noise
drivers/net/wireless/ath/ath10k/mac.c | 13 ++++++++----- drivers/net/wireless/ath/ath10k/wmi-tlv.c | 15 +++++++++++---- 2 files changed, 19 insertions(+), 9 deletions(-)
Signature · 7 lines
2.27.0 _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
PATCH 1/5 ath10k: fix conf_mutex lock assert in ath10k_debug_fw_stats_request()
ath10k_debug_fw_stats_request() is called ath10k_sta_statistics() without holding conf_mutex. ath10k_debug_fw_stats_request() simply returns when CONFIG_ATH10K_DEBUGFS is disabled.
When CONFIG_ATH10K_DEBUGFS is enabled, ath10k_debug_fw_stats_request() code path isn't protected. This assert is triggered when CONFIG_LOCKDEP and CONFIG_ATH10K_DEBUGFS are enabled.
All other ath10k_debug_fw_stats_request() callers hold conf_mutex. Fix ath10k_sta_statistics() to do the same.
WARNING: CPU: 5 PID: 696 at drivers/net/wireless/ath/ath10k/debug.c:357 ath10k_debug_fw_stats_request+0x29a/0x2d0 [ath10k_core] Modules linked in: rfcomm ccm fuse cmac algif_hash algif_skcipher af_alg bnep binfmt_misc nls_iso8859_1 intel_rapl_msr intel_rapl_common snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hda_core snd_hwdep snd_pcm amdgpu snd_seq_midi snd_seq_midi_event snd_rawmidi edac_mce_amd snd_seq ath10k_pci ath10k_core aesni_intel gpu_sched drm_ttm_helper btusb ttm glue_helper crypto_simd btrtl ath cryptd drm_kms_helper snd_seq_device btbcm snd_timer rapl btintel cec i2c_algo_bit mac80211 bluetooth fb_sys_fops input_leds ecdh_generic snd wmi_bmof syscopyarea ecc serio_raw efi_pstore ccp k10temp sysfillrect soundcore sysimgblt snd_pci_acp3x cfg80211 ipmi_devintf libarc4 ipmi_msghandler mac_hid sch_fq_codel parport_pc ppdev lp parport drm ip_tables x_tables autofs4 hid_generic usbhid hid crc32_pclmul psmouse ahci nvme libahci i2c_piix4 nvme_core r8169 realtek wmi video CPU: 5 PID: 696 Comm: NetworkManager Tainted: G W 5.11.0-rc7+ #20 Hardware name: LENOVO 10VGCTO1WW/3130, BIOS M1XKT45A 08/21/2019 RIP: 0010:ath10k_debug_fw_stats_request+0x29a/0x2d0 [ath10k_core] Code: 83 c4 10 44 89 f8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 48 8d bf e8 20 00 00 be ff ff ff ff e8 de 2d 47 fa 85 c0 0f 85 8d fd ff ff <0f> 0b e9 86 fd ff ff 41 bf a1 ff ff ff 44 89 fa 48 c7 c6 2c 71 c4 RSP: 0018:ffffaffbc124b7d0 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff93d02e4fec70 RCX: 0000000000000001 RDX: 0000000000000000 RSI: ffff93d00cba5248 RDI: ffff93d00ab309a0 RBP: ffffaffbc124b808 R08: 0000000000000000 R09: ffff93d02e4fec70 R10: 0000000000000001 R11: 0000000000000246 R12: ffff93d00cba3160 R13: ffff93d00cba3160 R14: ffff93d02e4fe4f0 R15: 0000000000000001 FS: 00007f7ce8d50bc0(0000) GS:ffff93d137d40000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fc3595ad160 CR3: 000000010d492000 CR4: 00000000003506e0 Call Trace: ? sta_info_get_bss+0xeb/0x1f0 [mac80211] ath10k_sta_statistics+0x4f/0x280 [ath10k_core] sta_set_sinfo+0xda/0xd20 [mac80211] ieee80211_get_station+0x58/0x80 [mac80211] nl80211_get_station+0xbd/0x340 [cfg80211] genl_family_rcv_msg_doit+0xe7/0x150 genl_rcv_msg+0xe2/0x1e0 ? nl80211_dump_station+0x3a0/0x3a0 [cfg80211] ? nl80211_send_station+0xef0/0xef0 [cfg80211] ? genl_get_cmd+0xd0/0xd0 netlink_rcv_skb+0x55/0x100 genl_rcv+0x29/0x40 netlink_unicast+0x1a8/0x270 netlink_sendmsg+0x253/0x480 sock_sendmsg+0x65/0x70 ____sys_sendmsg+0x219/0x260 ? __import_iovec+0x32/0x170 ___sys_sendmsg+0xb7/0x100 ? end_opal_session+0x39/0xd0 ? __fget_files+0xe0/0x1d0 ? find_held_lock+0x31/0x90 ? __fget_files+0xe0/0x1d0 ? __fget_files+0x103/0x1d0 ? __fget_light+0x32/0x80 __sys_sendmsg+0x5a/0xa0 ? syscall_enter_from_user_mode+0x21/0x60 __x64_sys_sendmsg+0x1f/0x30 do_syscall_64+0x38/0x50 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7f7cea2c791d Code: 28 89 54 24 1c 48 89 74 24 10 89 7c 24 08 e8 4a ee ff ff 8b 54 24 1c 48 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 33 44 89 c7 48 89 44 24 08 e8 9e ee ff ff 48 RSP: 002b:00007ffedf612a30 EFLAGS: 00000293 ORIG_RAX: 000000000000002e RAX: ffffffffffffffda RBX: 00005618c4cfec00 RCX: 00007f7cea2c791d RDX: 0000000000000000 RSI: 00007ffedf612a80 RDI: 000000000000000b RBP: 00007ffedf612a80 R08: 0000000000000000 R09: 00005618c4e74000 R10: 00005618c4da0590 R11: 0000000000000293 R12: 00005618c4cfec00 R13: 00005618c4cfe2c0 R14: 00007f7cea32ef80 R15: 00005618c4cff340 irq event stamp: 520897 hardirqs last enabled at (520903): [<ffffffffba501cc5>] console_unlock+0x4e5/0x5d0 hardirqs last disabled at (520908): [<ffffffffba501c38>] console_unlock+0x458/0x5d0 softirqs last enabled at (520722): [<ffffffffbb201002>] asm_call_irq_on_stack+0x12/0x20 softirqs last disabled at (520717): [<ffffffffbb201002>] asm_call_irq_on_stack+0x12/0x20
--- drivers/net/wireless/ath/ath10k/mac.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/net/wireless/ath/ath10k/mac.c +2 -0
@@ -9117,7 +9117,9 @@ static void ath10k_sta_statistics(struct ieee80211_hw *hw,if (!ath10k_peer_stats_enabled (ar))return ; +mutex_lock (&ar->conf_mutex);ath10k_debug_fw_stats_request (ar); +mutex_unlock (&ar->conf_mutex); sinfo->rx_duration = arsta->rx_duration; sinfo->filled |=BIT_ULL (NL80211_STA_INFO_RX_DURATION );
Signature · 7 lines
2.27.0 _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 1/5] ath10k: fix conf_mutex lock assert in ath10k_debug_fw_stats_request()
Quoted 70 lines · Shuah Khan
ath10k_debug_fw_stats_request()is calledath10k_sta_statistics()without holdingconf_mutex.ath10k_debug_fw_stats_request()simply returns whenCONFIG_ATH10K_DEBUGFSis disabled.When
CONFIG_ATH10K_DEBUGFSis enabled,ath10k_debug_fw_stats_request()code path isn't protected. This assert is triggered whenCONFIG_LOCKDEPandCONFIG_ATH10K_DEBUGFSare enabled.All other
ath10k_debug_fw_stats_request()callers holdconf_mutex. Fixath10k_sta_statistics()to do the same.WARNING: CPU: 5 PID: 696 at drivers/net/wireless/ath/ath10k/debug.c:357 ath10k_debug_fw_stats_request+0x29a/0x2d0 [ath10k_core] Modules linked in: rfcomm ccm fuse cmac algif_hash algif_skcipher af_alg bnep binfmt_misc nls_iso8859_1 intel_rapl_msr intel_rapl_common snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hda_core snd_hwdep snd_pcm amdgpu snd_seq_midi snd_seq_midi_event snd_rawmidi edac_mce_amd snd_seq ath10k_pci ath10k_core aesni_intel gpu_sched drm_ttm_helper btusb ttm glue_helper crypto_simd btrtl ath cryptd drm_kms_helper snd_seq_device btbcm snd_timer rapl btintel cec i2c_algo_bit mac80211 bluetooth fb_sys_fops input_leds ecdh_generic snd wmi_bmof syscopyarea ecc serio_raw efi_pstore ccp k10temp sysfillrect soundcore sysimgblt snd_pci_acp3x cfg80211 ipmi_devintf libarc4 ipmi_msghandler mac_hid sch_fq_codel parport_pc ppdev lp parport drm ip_tables x_tables autofs4 hid_generic usbhid hid crc32_pclmul psmouse ahci nvme libahci i2c_piix4 nvme_core r8169 realtek wmi video CPU: 5 PID: 696 Comm: NetworkManager Tainted: G W 5.11.0-rc7+ #20 Hardware name: LENOVO 10VGCTO1WW/3130, BIOS M1XKT45A 08/21/2019 RIP: 0010:ath10k_debug_fw_stats_request+0x29a/0x2d0 [ath10k_core] Code: 83 c4 10 44 89 f8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 48 8d bf e8 20 00 00 be ff ff ff ff e8 de 2d 47 fa 85 c0 0f 85 8d fd ff ff <0f> 0b e9 86 fd ff ff 41 bf a1 ff ff ff 44 89 fa 48 c7 c6 2c 71 c4 RSP: 0018:ffffaffbc124b7d0 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff93d02e4fec70 RCX: 0000000000000001 RDX: 0000000000000000 RSI: ffff93d00cba5248 RDI: ffff93d00ab309a0 RBP: ffffaffbc124b808 R08: 0000000000000000 R09: ffff93d02e4fec70 R10: 0000000000000001 R11: 0000000000000246 R12: ffff93d00cba3160 R13: ffff93d00cba3160 R14: ffff93d02e4fe4f0 R15: 0000000000000001 FS: 00007f7ce8d50bc0(0000) GS:ffff93d137d40000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fc3595ad160 CR3: 000000010d492000 CR4: 00000000003506e0 Call Trace: ? sta_info_get_bss+0xeb/0x1f0 [mac80211] ath10k_sta_statistics+0x4f/0x280 [ath10k_core] sta_set_sinfo+0xda/0xd20 [mac80211] ieee80211_get_station+0x58/0x80 [mac80211] nl80211_get_station+0xbd/0x340 [cfg80211] genl_family_rcv_msg_doit+0xe7/0x150 genl_rcv_msg+0xe2/0x1e0 ? nl80211_dump_station+0x3a0/0x3a0 [cfg80211] ? nl80211_send_station+0xef0/0xef0 [cfg80211] ? genl_get_cmd+0xd0/0xd0 netlink_rcv_skb+0x55/0x100 genl_rcv+0x29/0x40 netlink_unicast+0x1a8/0x270 netlink_sendmsg+0x253/0x480 sock_sendmsg+0x65/0x70 ____sys_sendmsg+0x219/0x260 ? __import_iovec+0x32/0x170 ___sys_sendmsg+0xb7/0x100 ? end_opal_session+0x39/0xd0 ? __fget_files+0xe0/0x1d0 ? find_held_lock+0x31/0x90 ? __fget_files+0xe0/0x1d0 ? __fget_files+0x103/0x1d0 ? __fget_light+0x32/0x80 __sys_sendmsg+0x5a/0xa0 ? syscall_enter_from_user_mode+0x21/0x60 __x64_sys_sendmsg+0x1f/0x30 do_syscall_64+0x38/0x50 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7f7cea2c791d Code: 28 89 54 24 1c 48 89 74 24 10 89 7c 24 08 e8 4a ee ff ff 8b 54 24 1c 48 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 33 44 89 c7 48 89 44 24 08 e8 9e ee ff ff 48 RSP: 002b:00007ffedf612a30 EFLAGS: 00000293 ORIG_RAX: 000000000000002e RAX: ffffffffffffffda RBX: 00005618c4cfec00 RCX: 00007f7cea2c791d RDX: 0000000000000000 RSI: 00007ffedf612a80 RDI: 000000000000000b RBP: 00007ffedf612a80 R08: 0000000000000000 R09: 00005618c4e74000 R10: 00005618c4da0590 R11: 0000000000000293 R12: 00005618c4cfec00 R13: 00005618c4cfe2c0 R14: 00007f7cea32ef80 R15: 00005618c4cff340 irq event stamp: 520897 hardirqs last enabled at (520903): [<ffffffffba501cc5>] console_unlock+0x4e5/0x5d0 hardirqs last disabled at (520908): [<ffffffffba501c38>] console_unlock+0x458/0x5d0 softirqs last enabled at (520722): [<ffffffffbb201002>] asm_call_irq_on_stack+0x12/0x20 softirqs last disabled at (520717): [<ffffffffbb201002>] asm_call_irq_on_stack+0x12/0x20
Quoted 1 line
Signed-off-by Shuah Khan <[email protected]>
Bad timing, just yesterday I applied an identical patch:
Patch set to Superseded.
Signature · 9 lines
https://patchwork.kernel.org/project/linux-wireless/patch/1c38ef6d39ed89a564bc817d964d923ff0676c53.1612915444.git.skhan@linuxfoundation.org/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 1/5] ath10k: fix conf_mutex lock assert in ath10k_debug_fw_stats_request()
In reply to Shuah KhanQuoted 70 lines · Shuah Khan
ath10k_debug_fw_stats_request()is calledath10k_sta_statistics()without holdingconf_mutex.ath10k_debug_fw_stats_request()simply returns whenCONFIG_ATH10K_DEBUGFSis disabled.When
CONFIG_ATH10K_DEBUGFSis enabled,ath10k_debug_fw_stats_request()code path isn't protected. This assert is triggered whenCONFIG_LOCKDEPandCONFIG_ATH10K_DEBUGFSare enabled.All other
ath10k_debug_fw_stats_request()callers holdconf_mutex. Fixath10k_sta_statistics()to do the same.WARNING: CPU: 5 PID: 696 at drivers/net/wireless/ath/ath10k/debug.c:357 ath10k_debug_fw_stats_request+0x29a/0x2d0 [ath10k_core] Modules linked in: rfcomm ccm fuse cmac algif_hash algif_skcipher af_alg bnep binfmt_misc nls_iso8859_1 intel_rapl_msr intel_rapl_common snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hda_core snd_hwdep snd_pcm amdgpu snd_seq_midi snd_seq_midi_event snd_rawmidi edac_mce_amd snd_seq ath10k_pci ath10k_core aesni_intel gpu_sched drm_ttm_helper btusb ttm glue_helper crypto_simd btrtl ath cryptd drm_kms_helper snd_seq_device btbcm snd_timer rapl btintel cec i2c_algo_bit mac80211 bluetooth fb_sys_fops input_leds ecdh_generic snd wmi_bmof syscopyarea ecc serio_raw efi_pstore ccp k10temp sysfillrect soundcore sysimgblt snd_pci_acp3x cfg80211 ipmi_devintf libarc4 ipmi_msghandler mac_hid sch_fq_codel parport_pc ppdev lp parport drm ip_tables x_tables autofs4 hid_generic usbhid hid crc32_pclmul psmouse ahci nvme libahci i2c_piix4 nvme_core r8169 realtek wmi video CPU: 5 PID: 696 Comm: NetworkManager Tainted: G W 5.11.0-rc7+ #20 Hardware name: LENOVO 10VGCTO1WW/3130, BIOS M1XKT45A 08/21/2019 RIP: 0010:ath10k_debug_fw_stats_request+0x29a/0x2d0 [ath10k_core] Code: 83 c4 10 44 89 f8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 48 8d bf e8 20 00 00 be ff ff ff ff e8 de 2d 47 fa 85 c0 0f 85 8d fd ff ff <0f> 0b e9 86 fd ff ff 41 bf a1 ff ff ff 44 89 fa 48 c7 c6 2c 71 c4 RSP: 0018:ffffaffbc124b7d0 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff93d02e4fec70 RCX: 0000000000000001 RDX: 0000000000000000 RSI: ffff93d00cba5248 RDI: ffff93d00ab309a0 RBP: ffffaffbc124b808 R08: 0000000000000000 R09: ffff93d02e4fec70 R10: 0000000000000001 R11: 0000000000000246 R12: ffff93d00cba3160 R13: ffff93d00cba3160 R14: ffff93d02e4fe4f0 R15: 0000000000000001 FS: 00007f7ce8d50bc0(0000) GS:ffff93d137d40000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fc3595ad160 CR3: 000000010d492000 CR4: 00000000003506e0 Call Trace: ? sta_info_get_bss+0xeb/0x1f0 [mac80211] ath10k_sta_statistics+0x4f/0x280 [ath10k_core] sta_set_sinfo+0xda/0xd20 [mac80211] ieee80211_get_station+0x58/0x80 [mac80211] nl80211_get_station+0xbd/0x340 [cfg80211] genl_family_rcv_msg_doit+0xe7/0x150 genl_rcv_msg+0xe2/0x1e0 ? nl80211_dump_station+0x3a0/0x3a0 [cfg80211] ? nl80211_send_station+0xef0/0xef0 [cfg80211] ? genl_get_cmd+0xd0/0xd0 netlink_rcv_skb+0x55/0x100 genl_rcv+0x29/0x40 netlink_unicast+0x1a8/0x270 netlink_sendmsg+0x253/0x480 sock_sendmsg+0x65/0x70 ____sys_sendmsg+0x219/0x260 ? __import_iovec+0x32/0x170 ___sys_sendmsg+0xb7/0x100 ? end_opal_session+0x39/0xd0 ? __fget_files+0xe0/0x1d0 ? find_held_lock+0x31/0x90 ? __fget_files+0xe0/0x1d0 ? __fget_files+0x103/0x1d0 ? __fget_light+0x32/0x80 __sys_sendmsg+0x5a/0xa0 ? syscall_enter_from_user_mode+0x21/0x60 __x64_sys_sendmsg+0x1f/0x30 do_syscall_64+0x38/0x50 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7f7cea2c791d Code: 28 89 54 24 1c 48 89 74 24 10 89 7c 24 08 e8 4a ee ff ff 8b 54 24 1c 48 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 33 44 89 c7 48 89 44 24 08 e8 9e ee ff ff 48 RSP: 002b:00007ffedf612a30 EFLAGS: 00000293 ORIG_RAX: 000000000000002e RAX: ffffffffffffffda RBX: 00005618c4cfec00 RCX: 00007f7cea2c791d RDX: 0000000000000000 RSI: 00007ffedf612a80 RDI: 000000000000000b RBP: 00007ffedf612a80 R08: 0000000000000000 R09: 00005618c4e74000 R10: 00005618c4da0590 R11: 0000000000000293 R12: 00005618c4cfec00 R13: 00005618c4cfe2c0 R14: 00007f7cea32ef80 R15: 00005618c4cff340 irq event stamp: 520897 hardirqs last enabled at (520903): [<ffffffffba501cc5>] console_unlock+0x4e5/0x5d0 hardirqs last disabled at (520908): [<ffffffffba501c38>] console_unlock+0x458/0x5d0 softirqs last enabled at (520722): [<ffffffffbb201002>] asm_call_irq_on_stack+0x12/0x20 softirqs last disabled at (520717): [<ffffffffbb201002>] asm_call_irq_on_stack+0x12/0x20
Quoted 1 line
Signed-off-by Shuah Khan <[email protected]>
Bad timing, just yesterday I applied an identical patch:
Patch set to Superseded.
Signature · 3 lines
https://patchwork.kernel.org/project/linux-wireless/patch/1c38ef6d39ed89a564bc817d964d923ff0676c53.1612915444.git.skhan@linuxfoundation.org/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [PATCH 1/5] ath10k: fix conf_mutex lock assert in ath10k_debug_fw_stats_request()
Quoted 72 lines · Kalle Valo
Quoted 70 lines · Shuah Khan
ath10k_debug_fw_stats_request()is calledath10k_sta_statistics()without holdingconf_mutex.ath10k_debug_fw_stats_request()simply returns whenCONFIG_ATH10K_DEBUGFSis disabled.When
CONFIG_ATH10K_DEBUGFSis enabled,ath10k_debug_fw_stats_request()code path isn't protected. This assert is triggered whenCONFIG_LOCKDEPandCONFIG_ATH10K_DEBUGFSare enabled.All other
ath10k_debug_fw_stats_request()callers holdconf_mutex. Fixath10k_sta_statistics()to do the same.WARNING: CPU: 5 PID: 696 at drivers/net/wireless/ath/ath10k/debug.c:357 ath10k_debug_fw_stats_request+0x29a/0x2d0 [ath10k_core] Modules linked in: rfcomm ccm fuse cmac algif_hash algif_skcipher af_alg bnep binfmt_misc nls_iso8859_1 intel_rapl_msr intel_rapl_common snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hda_core snd_hwdep snd_pcm amdgpu snd_seq_midi snd_seq_midi_event snd_rawmidi edac_mce_amd snd_seq ath10k_pci ath10k_core aesni_intel gpu_sched drm_ttm_helper btusb ttm glue_helper crypto_simd btrtl ath cryptd drm_kms_helper snd_seq_device btbcm snd_timer rapl btintel cec i2c_algo_bit mac80211 bluetooth fb_sys_fops input_leds ecdh_generic snd wmi_bmof syscopyarea ecc serio_raw efi_pstore ccp k10temp sysfillrect soundcore sysimgblt snd_pci_acp3x cfg80211 ipmi_devintf libarc4 ipmi_msghandler mac_hid sch_fq_codel parport_pc ppdev lp parport drm ip_tables x_tables autofs4 hid_generic usbhid hid crc32_pclmul psmouse ahci nvme libahci i2c_piix4 nvme_core r8169 realtek wmi video CPU: 5 PID: 696 Comm: NetworkManager Tainted: G W 5.11.0-rc7+ #20 Hardware name: LENOVO 10VGCTO1WW/3130, BIOS M1XKT45A 08/21/2019 RIP: 0010:ath10k_debug_fw_stats_request+0x29a/0x2d0 [ath10k_core] Code: 83 c4 10 44 89 f8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 48 8d bf e8 20 00 00 be ff ff ff ff e8 de 2d 47 fa 85 c0 0f 85 8d fd ff ff <0f> 0b e9 86 fd ff ff 41 bf a1 ff ff ff 44 89 fa 48 c7 c6 2c 71 c4 RSP: 0018:ffffaffbc124b7d0 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff93d02e4fec70 RCX: 0000000000000001 RDX: 0000000000000000 RSI: ffff93d00cba5248 RDI: ffff93d00ab309a0 RBP: ffffaffbc124b808 R08: 0000000000000000 R09: ffff93d02e4fec70 R10: 0000000000000001 R11: 0000000000000246 R12: ffff93d00cba3160 R13: ffff93d00cba3160 R14: ffff93d02e4fe4f0 R15: 0000000000000001 FS: 00007f7ce8d50bc0(0000) GS:ffff93d137d40000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fc3595ad160 CR3: 000000010d492000 CR4: 00000000003506e0 Call Trace: ? sta_info_get_bss+0xeb/0x1f0 [mac80211] ath10k_sta_statistics+0x4f/0x280 [ath10k_core] sta_set_sinfo+0xda/0xd20 [mac80211] ieee80211_get_station+0x58/0x80 [mac80211] nl80211_get_station+0xbd/0x340 [cfg80211] genl_family_rcv_msg_doit+0xe7/0x150 genl_rcv_msg+0xe2/0x1e0 ? nl80211_dump_station+0x3a0/0x3a0 [cfg80211] ? nl80211_send_station+0xef0/0xef0 [cfg80211] ? genl_get_cmd+0xd0/0xd0 netlink_rcv_skb+0x55/0x100 genl_rcv+0x29/0x40 netlink_unicast+0x1a8/0x270 netlink_sendmsg+0x253/0x480 sock_sendmsg+0x65/0x70 ____sys_sendmsg+0x219/0x260 ? __import_iovec+0x32/0x170 ___sys_sendmsg+0xb7/0x100 ? end_opal_session+0x39/0xd0 ? __fget_files+0xe0/0x1d0 ? find_held_lock+0x31/0x90 ? __fget_files+0xe0/0x1d0 ? __fget_files+0x103/0x1d0 ? __fget_light+0x32/0x80 __sys_sendmsg+0x5a/0xa0 ? syscall_enter_from_user_mode+0x21/0x60 __x64_sys_sendmsg+0x1f/0x30 do_syscall_64+0x38/0x50 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7f7cea2c791d Code: 28 89 54 24 1c 48 89 74 24 10 89 7c 24 08 e8 4a ee ff ff 8b 54 24 1c 48 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 33 44 89 c7 48 89 44 24 08 e8 9e ee ff ff 48 RSP: 002b:00007ffedf612a30 EFLAGS: 00000293 ORIG_RAX: 000000000000002e RAX: ffffffffffffffda RBX: 00005618c4cfec00 RCX: 00007f7cea2c791d RDX: 0000000000000000 RSI: 00007ffedf612a80 RDI: 000000000000000b RBP: 00007ffedf612a80 R08: 0000000000000000 R09: 00005618c4e74000 R10: 00005618c4da0590 R11: 0000000000000293 R12: 00005618c4cfec00 R13: 00005618c4cfe2c0 R14: 00007f7cea32ef80 R15: 00005618c4cff340 irq event stamp: 520897 hardirqs last enabled at (520903): [<ffffffffba501cc5>] console_unlock+0x4e5/0x5d0 hardirqs last disabled at (520908): [<ffffffffba501c38>] console_unlock+0x458/0x5d0 softirqs last enabled at (520722): [<ffffffffbb201002>] asm_call_irq_on_stack+0x12/0x20 softirqs last disabled at (520717): [<ffffffffbb201002>] asm_call_irq_on_stack+0x12/0x20
Quoted 4 lines
Quoted 1 line
Signed-off-by Shuah Khan <[email protected]>Bad timing, just yesterday I applied an identical patch:
Yes it is.
Quoted 2 lines
No worries. I will apply yours to my repo
thanks,
-- Shuah
_______________________________________________
ath10k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath10k
PATCH 2/5 ath10k: fix WARNING: suspicious RCU usage
In reply to Shuah Khanieee80211_find_sta_by_ifaddr() must be called under the RCU lock and the resulting pointer is only valid under RCU lock as well.
Fix ath10k_wmi_tlv_parse_peer_stats_info() to hold RCU lock before it calls ieee80211_find_sta_by_ifaddr() and release it when the resulting pointer is no longer needed. The log below shows the problem.
While at it, fix ath10k_wmi_tlv_op_pull_peer_stats_info() to do the same.
=============================
WARNING: suspicious RCU usage
5.11.0-rc7+ #20 Tainted: G W
-----------------------------include/linux/rhashtable.h:594 suspicious rcu_dereference_check() usage! other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
no locks held by ksoftirqd/5/44.
stack backtrace: CPU: 5 PID: 44 Comm: ksoftirqd/5 Tainted: G W 5.11.0-rc7+ #20 Hardware name: LENOVO 10VGCTO1WW/3130, BIOS M1XKT45A 08/21/2019 Call Trace: dump_stack+0x7d/0x9f lockdep_rcu_suspicious+0xdb/0xe5 __rhashtable_lookup+0x1eb/0x260 [mac80211] ieee80211_find_sta_by_ifaddr+0x5b/0xc0 [mac80211] ath10k_wmi_tlv_parse_peer_stats_info+0x3e/0x90 [ath10k_core] ath10k_wmi_tlv_iter+0x6a/0xc0 [ath10k_core] ? ath10k_wmi_tlv_op_pull_mgmt_tx_bundle_compl_ev+0xe0/0xe0 [ath10k_core] ath10k_wmi_tlv_op_rx+0x5da/0xda0 [ath10k_core] ? trace_hardirqs_on+0x54/0xf0 ? ath10k_ce_completed_recv_next+0x4e/0x60 [ath10k_core] ath10k_wmi_process_rx+0x1d/0x40 [ath10k_core] ath10k_htc_rx_completion_handler+0x115/0x180 [ath10k_core] ath10k_pci_process_rx_cb+0x149/0x1b0 [ath10k_pci] ? ath10k_htc_process_trailer+0x2d0/0x2d0 [ath10k_core] ? ath10k_pci_sleep.part.0+0x6a/0x80 [ath10k_pci] ath10k_pci_htc_rx_cb+0x15/0x20 [ath10k_pci] ath10k_ce_per_engine_service+0x61/0x80 [ath10k_core] ath10k_ce_per_engine_service_any+0x7d/0xa0 [ath10k_core] ath10k_pci_napi_poll+0x48/0x120 [ath10k_pci] net_rx_action+0x136/0x500 __do_softirq+0xc6/0x459 ? smpboot_thread_fn+0x2b/0x1f0 run_ksoftirqd+0x2b/0x60 smpboot_thread_fn+0x116/0x1f0 kthread+0x14b/0x170 ? smpboot_register_percpu_thread+0xe0/0xe0 ? __kthread_bind_mask+0x70/0x70 ret_from_fork+0x22/0x30
--- drivers/net/wireless/ath/ath10k/wmi-tlv.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
drivers/net/wireless/ath/ath10k/wmi-tlv.c +11 -4
@@ -225,6 +225,7 @@ static int ath10k_wmi_tlv_parse_peer_stats_info(struct ath10k *ar, u16 tag, u16const struct wmi_tlv_peer_stats_info *stat = ptr;struct ieee80211_sta *sta;struct ath10k_sta *arsta; +int ret =0 ;if (tag !=WMI_TLV_TAG_STRUCT_PEER_STATS_INFO )return -EPROTO ;
@@ -240,10 +241,12 @@ static int ath10k_wmi_tlv_parse_peer_stats_info(struct ath10k *ar, u16 tag, u16__le32_to_cpu (stat->last_tx_rate_code), __le32_to_cpu(stat->last_tx_bitrate_kbps)); +rcu_read_lock (); sta =ieee80211_find_sta_by_ifaddr (ar->hw, stat->peer_macaddr.addr,NULL );if (!sta) {ath10k_warn (ar,"not found station for peer stats\n" ); -return -EINVAL ; + ret = -EINVAL ; + goto exit; } arsta = (struct ath10k_sta *)sta->drv_priv;
@@ -252,7 +255,9 @@ static int ath10k_wmi_tlv_parse_peer_stats_info(struct ath10k *ar, u16 tag, u16 arsta->tx_rate_code =__le32_to_cpu (stat->last_tx_rate_code); arsta->tx_bitrate_kbps =__le32_to_cpu (stat->last_tx_bitrate_kbps); -return 0 ; +exit: +rcu_read_unlock (); +return ret; } static int ath10k_wmi_tlv_op_pull_peer_stats_info(struct ath10k *ar,
@@ -573,13 +578,13 @@ static void ath10k_wmi_event_tdls_peer(struct ath10k *ar, struct sk_buff *skb)case WMI_TDLS_TEARDOWN_REASON_TX :case WMI_TDLS_TEARDOWN_REASON_RSSI :case WMI_TDLS_TEARDOWN_REASON_PTR_TIMEOUT : +rcu_read_lock (); station =ieee80211_find_sta_by_ifaddr (ar->hw, ev->peer_macaddr.addr,NULL );if (!station) {ath10k_warn (ar,"did not find station from tdls peer event" ); -kfree (tb); -return ; + goto exit; } arvif =ath10k_get_arvif (ar,__le32_to_cpu (ev->vdev_id)); ieee80211_tdls_oper_request(
@@ -590,6 +595,8 @@ static void ath10k_wmi_event_tdls_peer(struct ath10k *ar, struct sk_buff *skb) );break ; } +exit: +rcu_read_unlock ();kfree (tb); }
Signature · 7 lines
2.27.0 _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 2/5] ath10k: fix WARNING: suspicious RCU usage
Quoted 51 lines · Shuah Khan
ieee80211_find_sta_by_ifaddr()must be called under the RCU lock and the resulting pointer is only valid under RCU lock as well.Fix
ath10k_wmi_tlv_parse_peer_stats_info()to hold RCU lock before it callsieee80211_find_sta_by_ifaddr()and release it when the resulting pointer is no longer needed. The log below shows the problem.While at it, fix
ath10k_wmi_tlv_op_pull_peer_stats_info()to do the same.=============================
WARNING: suspicious RCU usage
5.11.0-rc7+ #20 Tainted: G W
-----------------------------include/linux/rhashtable.h:594suspiciousrcu_dereference_check()usage! other info that might help us debug this:
rcu_scheduler_active= 2,debug_locks= 1
no locks held by ksoftirqd/5/44.stack backtrace: CPU: 5 PID: 44 Comm: ksoftirqd/5 Tainted: G W 5.11.0-rc7+ #20 Hardware name: LENOVO 10VGCTO1WW/3130, BIOS M1XKT45A 08/21/2019 Call Trace: dump_stack+0x7d/0x9f lockdep_rcu_suspicious+0xdb/0xe5 __rhashtable_lookup+0x1eb/0x260 [mac80211] ieee80211_find_sta_by_ifaddr+0x5b/0xc0 [mac80211] ath10k_wmi_tlv_parse_peer_stats_info+0x3e/0x90 [ath10k_core] ath10k_wmi_tlv_iter+0x6a/0xc0 [ath10k_core] ? ath10k_wmi_tlv_op_pull_mgmt_tx_bundle_compl_ev+0xe0/0xe0 [ath10k_core] ath10k_wmi_tlv_op_rx+0x5da/0xda0 [ath10k_core] ? trace_hardirqs_on+0x54/0xf0 ? ath10k_ce_completed_recv_next+0x4e/0x60 [ath10k_core] ath10k_wmi_process_rx+0x1d/0x40 [ath10k_core] ath10k_htc_rx_completion_handler+0x115/0x180 [ath10k_core] ath10k_pci_process_rx_cb+0x149/0x1b0 [ath10k_pci] ? ath10k_htc_process_trailer+0x2d0/0x2d0 [ath10k_core] ? ath10k_pci_sleep.part.0+0x6a/0x80 [ath10k_pci] ath10k_pci_htc_rx_cb+0x15/0x20 [ath10k_pci] ath10k_ce_per_engine_service+0x61/0x80 [ath10k_core] ath10k_ce_per_engine_service_any+0x7d/0xa0 [ath10k_core] ath10k_pci_napi_poll+0x48/0x120 [ath10k_pci] net_rx_action+0x136/0x500 __do_softirq+0xc6/0x459 ? smpboot_thread_fn+0x2b/0x1f0 run_ksoftirqd+0x2b/0x60 smpboot_thread_fn+0x116/0x1f0 kthread+0x14b/0x170 ? smpboot_register_percpu_thread+0xe0/0xe0 ? __kthread_bind_mask+0x70/0x70 ret_from_fork+0x22/0x30
Quoted 1 line
Signed-off-by Shuah Khan <[email protected]>
Unlucky timing also on this one, it conflicts with a patch I applied yesterday:
Can you redo the patch to only change ath10k_wmi_event_tdls_peer()?
error: patch failed: drivers/net/wireless/ath/ath10k/wmi-tlv.c:240
error: drivers/net/wireless/ath/ath10k/wmi-tlv.c: patch does not apply stg import: Diff does not apply cleanly
Patch set to Changes Requested.
Signature · 9 lines
https://patchwork.kernel.org/project/linux-wireless/patch/23a1333dfb0367cc69e7177a2e373df0b6d42980.1612915444.git.skhan@linuxfoundation.org/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 2/5] ath10k: fix WARNING: suspicious RCU usage
In reply to Shuah KhanQuoted 51 lines · Shuah Khan
ieee80211_find_sta_by_ifaddr()must be called under the RCU lock and the resulting pointer is only valid under RCU lock as well.Fix
ath10k_wmi_tlv_parse_peer_stats_info()to hold RCU lock before it callsieee80211_find_sta_by_ifaddr()and release it when the resulting pointer is no longer needed. The log below shows the problem.While at it, fix
ath10k_wmi_tlv_op_pull_peer_stats_info()to do the same.=============================
WARNING: suspicious RCU usage
5.11.0-rc7+ #20 Tainted: G W
-----------------------------include/linux/rhashtable.h:594suspiciousrcu_dereference_check()usage! other info that might help us debug this:
rcu_scheduler_active= 2,debug_locks= 1
no locks held by ksoftirqd/5/44.stack backtrace: CPU: 5 PID: 44 Comm: ksoftirqd/5 Tainted: G W 5.11.0-rc7+ #20 Hardware name: LENOVO 10VGCTO1WW/3130, BIOS M1XKT45A 08/21/2019 Call Trace: dump_stack+0x7d/0x9f lockdep_rcu_suspicious+0xdb/0xe5 __rhashtable_lookup+0x1eb/0x260 [mac80211] ieee80211_find_sta_by_ifaddr+0x5b/0xc0 [mac80211] ath10k_wmi_tlv_parse_peer_stats_info+0x3e/0x90 [ath10k_core] ath10k_wmi_tlv_iter+0x6a/0xc0 [ath10k_core] ? ath10k_wmi_tlv_op_pull_mgmt_tx_bundle_compl_ev+0xe0/0xe0 [ath10k_core] ath10k_wmi_tlv_op_rx+0x5da/0xda0 [ath10k_core] ? trace_hardirqs_on+0x54/0xf0 ? ath10k_ce_completed_recv_next+0x4e/0x60 [ath10k_core] ath10k_wmi_process_rx+0x1d/0x40 [ath10k_core] ath10k_htc_rx_completion_handler+0x115/0x180 [ath10k_core] ath10k_pci_process_rx_cb+0x149/0x1b0 [ath10k_pci] ? ath10k_htc_process_trailer+0x2d0/0x2d0 [ath10k_core] ? ath10k_pci_sleep.part.0+0x6a/0x80 [ath10k_pci] ath10k_pci_htc_rx_cb+0x15/0x20 [ath10k_pci] ath10k_ce_per_engine_service+0x61/0x80 [ath10k_core] ath10k_ce_per_engine_service_any+0x7d/0xa0 [ath10k_core] ath10k_pci_napi_poll+0x48/0x120 [ath10k_pci] net_rx_action+0x136/0x500 __do_softirq+0xc6/0x459 ? smpboot_thread_fn+0x2b/0x1f0 run_ksoftirqd+0x2b/0x60 smpboot_thread_fn+0x116/0x1f0 kthread+0x14b/0x170 ? smpboot_register_percpu_thread+0xe0/0xe0 ? __kthread_bind_mask+0x70/0x70 ret_from_fork+0x22/0x30
Quoted 1 line
Signed-off-by Shuah Khan <[email protected]>
Unlucky timing also on this one, it conflicts with a patch I applied yesterday:
Can you redo the patch to only change ath10k_wmi_event_tdls_peer()?
error: patch failed: drivers/net/wireless/ath/ath10k/wmi-tlv.c:240
error: drivers/net/wireless/ath/ath10k/wmi-tlv.c: patch does not apply stg import: Diff does not apply cleanly
Patch set to Changes Requested.
Signature · 3 lines
https://patchwork.kernel.org/project/linux-wireless/patch/23a1333dfb0367cc69e7177a2e373df0b6d42980.1612915444.git.skhan@linuxfoundation.org/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [PATCH 2/5] ath10k: fix WARNING: suspicious RCU usage
Quoted 53 lines · Kalle Valo
Quoted 51 lines · Shuah Khan
ieee80211_find_sta_by_ifaddr()must be called under the RCU lock and the resulting pointer is only valid under RCU lock as well.Fix
ath10k_wmi_tlv_parse_peer_stats_info()to hold RCU lock before it callsieee80211_find_sta_by_ifaddr()and release it when the resulting pointer is no longer needed. The log below shows the problem.While at it, fix
ath10k_wmi_tlv_op_pull_peer_stats_info()to do the same.=============================
WARNING: suspicious RCU usage
5.11.0-rc7+ #20 Tainted: G W
-----------------------------include/linux/rhashtable.h:594suspiciousrcu_dereference_check()usage! other info that might help us debug this:
rcu_scheduler_active= 2,debug_locks= 1
no locks held by ksoftirqd/5/44.stack backtrace: CPU: 5 PID: 44 Comm: ksoftirqd/5 Tainted: G W 5.11.0-rc7+ #20 Hardware name: LENOVO 10VGCTO1WW/3130, BIOS M1XKT45A 08/21/2019 Call Trace: dump_stack+0x7d/0x9f lockdep_rcu_suspicious+0xdb/0xe5 __rhashtable_lookup+0x1eb/0x260 [mac80211] ieee80211_find_sta_by_ifaddr+0x5b/0xc0 [mac80211] ath10k_wmi_tlv_parse_peer_stats_info+0x3e/0x90 [ath10k_core] ath10k_wmi_tlv_iter+0x6a/0xc0 [ath10k_core] ? ath10k_wmi_tlv_op_pull_mgmt_tx_bundle_compl_ev+0xe0/0xe0 [ath10k_core] ath10k_wmi_tlv_op_rx+0x5da/0xda0 [ath10k_core] ? trace_hardirqs_on+0x54/0xf0 ? ath10k_ce_completed_recv_next+0x4e/0x60 [ath10k_core] ath10k_wmi_process_rx+0x1d/0x40 [ath10k_core] ath10k_htc_rx_completion_handler+0x115/0x180 [ath10k_core] ath10k_pci_process_rx_cb+0x149/0x1b0 [ath10k_pci] ? ath10k_htc_process_trailer+0x2d0/0x2d0 [ath10k_core] ? ath10k_pci_sleep.part.0+0x6a/0x80 [ath10k_pci] ath10k_pci_htc_rx_cb+0x15/0x20 [ath10k_pci] ath10k_ce_per_engine_service+0x61/0x80 [ath10k_core] ath10k_ce_per_engine_service_any+0x7d/0xa0 [ath10k_core] ath10k_pci_napi_poll+0x48/0x120 [ath10k_pci] net_rx_action+0x136/0x500 __do_softirq+0xc6/0x459 ? smpboot_thread_fn+0x2b/0x1f0 run_ksoftirqd+0x2b/0x60 smpboot_thread_fn+0x116/0x1f0 kthread+0x14b/0x170 ? smpboot_register_percpu_thread+0xe0/0xe0 ? __kthread_bind_mask+0x70/0x70 ret_from_fork+0x22/0x30
Quoted 8 lines
Quoted 1 line
Signed-off-by Shuah Khan <[email protected]>Unlucky timing also on this one, it conflicts with a patch I applied yesterday:
Can you redo the patch to only change
ath10k_wmi_event_tdls_peer()?
Yes. I will send the patch just for ath10k_wmi_event_tdls_peer() on top of your patch.
Quoted 6 lines
error: patch failed:
drivers/net/wireless/ath/ath10k/wmi-tlv.c:240
error:drivers/net/wireless/ath/ath10k/wmi-tlv.c: patch does not apply stg import: Diff does not apply cleanlyPatch set to Changes Requested.
thanks,
-- Shuah
_______________________________________________
ath10k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath10k
PATCH 3/5 ath10k: change ath10k_offchan_tx_work() peer present msg to a warn
In reply to Shuah KhanBased on the comment block in this function and the FIXME for this, peer being present for the offchannel tx is unlikely. Peer is deleted once tx is complete. Change peer present msg to a warn to detect this condition.
--- drivers/net/wireless/ath/ath10k/mac.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/net/wireless/ath/ath10k/mac.c +2 -3
@@ -3954,9 +3954,8 @@ void ath10k_offchan_tx_work(struct work_struct *work)spin_unlock_bh (&ar->data_lock);if (peer) -/* FIXME: should this use ath10k_warn()? */ -ath10k_dbg (ar,ATH10K_DBG_MAC ,"peer %pM on vdev %d already present\n" , - peer_addr, vdev_id); +ath10k_warn (ar,"peer %pM on vdev %d already present\n" , + peer_addr, vdev_id);if (!peer) { ret = ath10k_peer_create(ar, NULL, NULL, vdev_id,
Signature · 7 lines
2.27.0 _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 3/5] ath10k: change ath10k_offchan_tx_work() peer present msg to a warn
Quoted 6 lines · Shuah Khan
Based on the comment block in this function and the FIXME for this, peer being present for the offchannel tx is unlikely. Peer is deleted once tx is complete. Change peer present msg to a warn to detect this condition.
Signed-off-by Shuah Khan <[email protected]>Signed-off-by Kalle Valo <[email protected]>
Patch applied to ath-next branch of ath.git, thanks.
83bae26532ca ath10k: change ath10k_offchan_tx_work() peer present msg to a warn
Signature · 9 lines
https://patchwork.kernel.org/project/linux-wireless/patch/3b1f71272d56ee1d7f567fbce13bdb56cc06d342.1612915444.git.skhan@linuxfoundation.org/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 3/5] ath10k: change ath10k_offchan_tx_work() peer present msg to a warn
In reply to Shuah KhanQuoted 6 lines · Shuah Khan
Based on the comment block in this function and the FIXME for this, peer being present for the offchannel tx is unlikely. Peer is deleted once tx is complete. Change peer present msg to a warn to detect this condition.
Signed-off-by Shuah Khan <[email protected]>Signed-off-by Kalle Valo <[email protected]>
Patch applied to ath-next branch of ath.git, thanks.
83bae26532ca ath10k: change ath10k_offchan_tx_work() peer present msg to a warn
Signature · 3 lines
https://patchwork.kernel.org/project/linux-wireless/patch/3b1f71272d56ee1d7f567fbce13bdb56cc06d342.1612915444.git.skhan@linuxfoundation.org/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
PATCH 4/5 ath10k: detect conf_mutex held ath10k_drain_tx() calls
In reply to Shuah Khanath10k_drain_tx() must not be called with conf_mutex held as workers can use that also. Add check to detect conf_mutex held calls.
--- drivers/net/wireless/ath/ath10k/mac.c | 1 + 1 file changed, 1 insertion(+)
drivers/net/wireless/ath/ath10k/mac.c +1 -0
@@ -4566,6 +4566,7 @@ static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,/* Must not be called with conf_mutex held as workers can use that also. */ void ath10k_drain_tx (struct ath10k *ar) { +WARN_ON (lockdep_is_held (&ar->conf_mutex));/* make sure rcu-protected mac80211 tx path itself is drained */ synchronize_net ();
Signature · 7 lines
2.27.0 _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
kernel test robot
build botRe: [PATCH 4/5] ath10k: detect conf_mutex held ath10k_drain_tx() calls
I love your patch! Yet something to improve:
Hi Shuah,
I love your patch! Yet something to improve:
[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on wireless-drivers/master v5.11-rc7 next-20210125] [cannot apply to ath6kl/ath-next]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Shuah-Khan/ath10k-fixes-for-warns/20210210-085259 base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master config: arm64-defconfig (attached as .config) compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/5a96c0c9dfec2bd59e680b7ec8ade9378b654c4c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Shuah-Khan/ath10k-fixes-for-warns/20210210-085259 git checkout 5a96c0c9dfec2bd59e680b7ec8ade9378b654c4c
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "lockdep_is_held" [drivers/net/wireless/ath/ath10k/ath10k_core.ko] undefined!
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
Re: [PATCH 4/5] ath10k: detect conf_mutex held ath10k_drain_tx() calls
In reply to Shuah KhanQuoted 4 lines · Shuah Khan
ath10k_drain_tx()must not be called withconf_mutexheld as workers can use that also. Add check to detectconf_mutexheld calls.Signed-off-by Shuah Khan <[email protected]>
The commit log does not answer to "Why?". How did you find this? What actual problem are you trying to solve?
drivers/net/wireless/ath/ath10k/mac.c static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw, · line 4569
Quoted 13 lines
--- drivers/net/wireless/ath/ath10k/mac.c | 1 + 1 file changed, 1 insertion(+)drivers/net/wireless/ath/ath10k/mac.c +1
-0@@ -4566,6 +4566,7 @@ static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,/* Must not be called with conf_mutex held as workers can use that also. */ void ath10k_drain_tx (struct ath10k *ar) { +WARN_ON (lockdep_is_held (&ar->conf_mutex));
Empty line after WARN_ON().
Shouldn't this check debug_locks similarly lockdep_assert_held() does?
#define lockdep_assert_held(l) do { \
WARN_ON(debug_locks && !lockdep_is_held(l)); \
} while (0)And I suspect you need #ifdef CONFIG_LOCKDEP which should fix the kbuild bot error.
But honestly I would prefer to have lockdep_assert_not_held() in include/linux/lockdep.h, much cleaner that way. Also i915_gem_object_lookup_rcu() could then use the same macro.
Signature · 8 lines
https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 4/5] ath10k: detect conf_mutex held ath10k_drain_tx() calls
Quoted 10 lines · Kalle Valo
Quoted 4 lines · Shuah Khan
ath10k_drain_tx()must not be called withconf_mutexheld as workers can use that also. Add check to detectconf_mutexheld calls.Signed-off-by Shuah Khan <[email protected]>The commit log does not answer to "Why?". How did you find this? What actual problem are you trying to solve?
I came across the comment block above the ath10k_drain_tx() as I was reviewing at conf_mutex holds while I was debugging the conf_mutex lock assert in ath10k_debug_fw_stats_request().
My reasoning is that having this will help detect incorrect usages of ath10k_drain_tx() while holding conf_mutex which could lead to locking problems when async worker routines try to call this routine.
Quoted 4 lines
Quoted 4 lines
--- drivers/net/wireless/ath/ath10k/mac.c | 1 + 1 file changed, 1 insertion(+)
Quoted 12 lines
drivers/net/wireless/ath/ath10k/mac.c static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw, · line 4569
Quoted 9 lines
drivers/net/wireless/ath/ath10k/mac.c +1
-0@@ -4566,6 +4566,7 @@ static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,/* Must not be called with conf_mutex held as workers can use that also. */ void ath10k_drain_tx (struct ath10k *ar) { +WARN_ON (lockdep_is_held (&ar->conf_mutex));Empty line after
WARN_ON().
Will do.
Quoted 9 lines
Shouldn't this check
debug_lockssimilarlylockdep_assert_held()does?#define lockdep_assert_held(l) do { \ WARN_ON(debug_locks && !lockdep_is_held(l)); \ } while (0)And I suspect you need #ifdef
CONFIG_LOCKDEPwhich should fix the kbuild bot error.
Yes.
Quoted 4 lines
But honestly I would prefer to have
lockdep_assert_not_held()ininclude/linux/lockdep.h, much cleaner that way. Alsoi915_gem_object_lookup_rcu()could then use the same macro.
Right. This is the right way to go. That was first instinct and decided to have the discussion evolve in that direction. Now that it has, I will combine this change with include/linux/lockdep.h and add lockdep_assert_not_held()
I think we might have other places in the kernel that could use lockdep_assert_not_held() in addition to i915_gem_object_lookup_rcu()
thanks,
-- Shuah
_______________________________________________
ath10k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 4/5] ath10k: detect conf_mutex held ath10k_drain_tx() calls
Quoted 12 lines · Shuah Khan
Quoted 10 lines · Kalle Valo
Quoted 4 lines · Shuah Khan
ath10k_drain_tx()must not be called withconf_mutexheld as workers can use that also. Add check to detectconf_mutexheld calls.Signed-off-by Shuah Khan <[email protected]>The commit log does not answer to "Why?". How did you find this? What actual problem are you trying to solve?
Quoted 7 lines
I came across the comment block above the
ath10k_drain_tx()as I was reviewing atconf_mutexholds while I was debugging theconf_mutexlock assert inath10k_debug_fw_stats_request().My reasoning is that having this will help detect incorrect usages of
ath10k_drain_tx()while holdingconf_mutexwhich could lead to locking problems when async worker routines try to call this routine.
Ok, makes sense. I prefer having this background info in the commit log, for example "found by code review" or something like that. Or just copy what you wrote above :)
Quoted 31 lines
Quoted 11 lines
drivers/net/wireless/ath/ath10k/mac.c static void · line 4570
Quoted 8 lines
drivers/net/wireless/ath/ath10k/mac.c +1
-0@@ -4566,6 +4566,7 @@ static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,/* Must not be called with conf_mutex held as workers can use that also. */ void ath10k_drain_tx(struct ath10k *ar) { + WARN_ON(lockdep_is_held(&ar->conf_mutex));Empty line after
WARN_ON().Will do.
Quoted 9 lines
Shouldn't this check
debug_lockssimilarlylockdep_assert_held()does?#define lockdep_assert_held(l) do { \ WARN_ON(debug_locks && !lockdep_is_held(l)); \ } while (0)And I suspect you need #ifdef
CONFIG_LOCKDEPwhich should fix the kbuild bot error.Yes.
Quoted 4 lines
But honestly I would prefer to have
lockdep_assert_not_held()ininclude/linux/lockdep.h, much cleaner that way. Alsoi915_gem_object_lookup_rcu()could then use the same macro.
Quoted 7 lines
Right. This is the right way to go. That was first instinct and decided to have the discussion evolve in that direction. Now that it has, I will combine this change with
include/linux/lockdep.hand addlockdep_assert_not_held()I think we might have other places in the kernel that could use
lockdep_assert_not_held()in addition toi915_gem_object_lookup_rcu()
Great, thank you. The only problem is that lockdep.h changes have to go via some other tree, I just don't know which :) I think it would be easiest if also the ath10k patch goes via that other tree, I can ack the ath10k changes.
Another option is that I'll apply the ath10k patch after the lockdep.h change has trickled down to my tree, but that usually happens only after the merge window and means weeks of waiting. Either is fine for me.
Signature · 8 lines
https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 4/5] ath10k: detect conf_mutex held ath10k_drain_tx() calls
Quoted 18 lines · Kalle Valo
Quoted 16 lines · Shuah Khan
Quoted 10 lines · Kalle Valo
Quoted 4 lines · Shuah Khan
ath10k_drain_tx()must not be called withconf_mutexheld as workers can use that also. Add check to detectconf_mutexheld calls.Signed-off-by Shuah Khan <[email protected]>The commit log does not answer to "Why?". How did you find this? What actual problem are you trying to solve?
I came across the comment block above the
ath10k_drain_tx()as I was reviewing atconf_mutexholds while I was debugging theconf_mutexlock assert inath10k_debug_fw_stats_request().
Quoted 8 lines
Quoted 3 lines
My reasoning is that having this will help detect incorrect usages of
ath10k_drain_tx()while holdingconf_mutexwhich could lead to locking problems when async worker routines try to call this routine.Ok, makes sense. I prefer having this background info in the commit log, for example "found by code review" or something like that. Or just copy what you wrote above :)
Thanks. I will do that.
Quoted 31 lines
Quoted 31 lines
Quoted 11 lines
drivers/net/wireless/ath/ath10k/mac.c static void · line 4570
Quoted 8 lines
drivers/net/wireless/ath/ath10k/mac.c +1
-0@@ -4566,6 +4566,7 @@ static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,/* Must not be called with conf_mutex held as workers can use that also. */ void ath10k_drain_tx(struct ath10k *ar) { + WARN_ON(lockdep_is_held(&ar->conf_mutex));Empty line after
WARN_ON().Will do.
Quoted 9 lines
Shouldn't this check
debug_lockssimilarlylockdep_assert_held()does?#define lockdep_assert_held(l) do { \ WARN_ON(debug_locks && !lockdep_is_held(l)); \ } while (0)And I suspect you need #ifdef
CONFIG_LOCKDEPwhich should fix the kbuild bot error.Yes.
Quoted 4 lines
But honestly I would prefer to have
lockdep_assert_not_held()ininclude/linux/lockdep.h, much cleaner that way. Alsoi915_gem_object_lookup_rcu()could then use the same macro.
Quoted 8 lines
Quoted 7 lines
Right. This is the right way to go. That was first instinct and decided to have the discussion evolve in that direction. Now that it has, I will combine this change with
include/linux/lockdep.hand addlockdep_assert_not_held()I think we might have other places in the kernel that could use
lockdep_assert_not_held()in addition toi915_gem_object_lookup_rcu()
I looked at i915_gem_object_lookup_rcu(). The following can be replaced by lockdep_assert_held().
#ifdef CONFIG_LOCKDEP
WARN_ON(debug_locks && !lock_is_held(&rcu_lock_map));#endif
Quoted 9 lines
Great, thank you. The only problem is that
lockdep.hchanges have to go via some other tree, I just don't know which :) I think it would be easiest if also the ath10k patch goes via that other tree, I can ack the ath10k changes.Another option is that I'll apply the ath10k patch after the
lockdep.hchange has trickled down to my tree, but that usually happens only after the merge window and means weeks of waiting. Either is fine for me.
I will send the include/linux/lockdep.h and ath10k patch together and we will take it from there.
thanks,
-- Shuah
_______________________________________________
ath10k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath10k
kernel test robot
build botRe: [PATCH 4/5] ath10k: detect conf_mutex held ath10k_drain_tx() calls
In reply to Shuah KhanI love your patch! Yet something to improve:
Hi Shuah,
I love your patch! Yet something to improve:
[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on wireless-drivers/master v5.11-rc7 next-20210125] [cannot apply to ath6kl/ath-next]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Shuah-Khan/ath10k-fixes-for-warns/20210210-085259 base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master config: m68k-allyesconfig (attached as .config) compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/5a96c0c9dfec2bd59e680b7ec8ade9378b654c4c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Shuah-Khan/ath10k-fixes-for-warns/20210210-085259 git checkout 5a96c0c9dfec2bd59e680b7ec8ade9378b654c4c
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>):
m68k-linux-ld: drivers/net/wireless/ath/ath10k/mac.o: in function `ath10k_drain_tx':
>> (.text+0xc8b2): undefined reference to `lockdep_is_held'
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
PATCH 5/5 ath10k: reduce invalid ht params rate message noise
In reply to Shuah Khanath10k_mac_get_rate_flags_ht() floods dmesg with the following messages, when it fails to find a match for mcs=7 and rate=1440.
supported_ht_mcs_rate_nss2:
{7, {1300, 2700, 1444, 3000} }
ath10k_pci 0000:02:00.0: invalid ht params rate 1440 100kbps nss 2 mcs 7
dev_warn_ratelimited() isn't helping the noise. Use dev_warn_once() instead.
--- drivers/net/wireless/ath/ath10k/mac.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/net/wireless/ath/ath10k/mac.c +3 -2
@@ -8970,8 +8970,9 @@ static void ath10k_mac_get_rate_flags_ht(struct ath10k *ar, u32 rate, u8 nss, u8 *bw |=RATE_INFO_BW_40 ; *flags |=RATE_INFO_FLAGS_SHORT_GI ; }else { -ath10k_warn (ar,"invalid ht params rate %d 100kbps nss %d mcs %d" , - rate, nss, mcs); +dev_warn_once (ar->dev, +"invalid ht params rate %d 100kbps nss %d mcs %d" , + rate, nss, mcs); } }
Signature · 7 lines
2.27.0 _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 5/5] ath10k: reduce invalid ht params rate message noise
Quoted 23 lines · Shuah Khan
ath10k_mac_get_rate_flags_ht()floods dmesg with the following messages, when it fails to find a match for mcs=7 and rate=1440.
supported_ht_mcs_rate_nss2:
{7, {1300, 2700, 1444, 3000} }
ath10k_pci0000:02:00.0: invalid ht params rate 1440 100kbps nss 2 mcs 7
dev_warn_ratelimited()isn't helping the noise. Usedev_warn_once()instead.Signed-off-by Shuah Khan <[email protected]>--- drivers/net/wireless/ath/ath10k/mac.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)drivers/net/wireless/ath/ath10k/mac.c +0
-0@@ -8970,8 +8970,9 @@ static void ath10k_mac_get_rate_flags_ht(structath10k *ar, u32 rate, u8 nss, u8
drivers/net/wireless/ath/ath10k/mac.c static void ath10k_mac_get_rate_flags_ht(struct ath10k *ar, u32 rate, u8 nss, u8 · line 8977
Quoted 10 lines
drivers/net/wireless/ath/ath10k/mac.c +3
-2*bw |=RATE_INFO_BW_40 ; *flags |=RATE_INFO_FLAGS_SHORT_GI ; }else { -ath10k_warn (ar,"invalid ht params rate %d 100kbps nss %d mcs %d" , - rate, nss, mcs); +dev_warn_once (ar->dev, +"invalid ht params rate %d 100kbps nss %d mcs %d" , + rate, nss, mcs); } }
The {7, {1300, 2700, 1444, 3000} } is a correct value.
The 1440 is report from firmware, its a wrong value, it has fixed in firmware.
If change it to dev_warn_once, then it will have no chance to find the other wrong values which report by firmware, and it indicate a wrong value to mac80211/cfg80211 and lead "iw wlan0 station dump" get a wrong bitrate.
_______________________________________________
ath10k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 5/5] ath10k: reduce invalid ht params rate message noise
Quoted 32 lines · Wen Gong
Quoted 31 lines · Shuah Khan
ath10k_mac_get_rate_flags_ht()floods dmesg with the following messages,
when it fails to find a match for mcs=7 and rate=1440.
supported_ht_mcs_rate_nss2:
{7, {1300, 2700, 1444, 3000} }
ath10k_pci0000:02:00.0: invalid ht params rate 1440 100kbps nss 2 mcs 7
dev_warn_ratelimited()isn't helping the noise. Usedev_warn_once()instead.Signed-off-by Shuah Khan <[email protected]>--- drivers/net/wireless/ath/ath10k/mac.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)drivers/net/wireless/ath/ath10k/mac.c +0
-0@@ -8970,8 +8970,9 @@ static void ath10k_mac_get_rate_flags_ht(structath10k *ar, u32 rate, u8 nss, u8
drivers/net/wireless/ath/ath10k/mac.c +1
-2*bw |=RATE_INFO_BW_40 ; *flags |=RATE_INFO_FLAGS_SHORT_GI ; }else { -ath10k_warn (ar,"invalid ht params rate %d 100kbps nss %d mcs %d" , - rate, nss, mcs); + dev_warn_once(ar->dev,
Quoted 8 lines
drivers/net/wireless/ath/ath10k/mac.c static void ath10k_mac_get_rate_flags_ht(struct ath10k *ar, u32 rate, u8 nss, u8 · line 8977
Quoted 4 lines
drivers/net/wireless/ath/ath10k/mac.c +2
-0+"invalid ht params rate %d 100kbps nss %d mcs %d" , + rate, nss, mcs); } }The {7, {1300, 2700, 1444, 3000} } is a correct value. The 1440 is report from firmware, its a wrong value, it has fixed in firmware.
In what version?
Quoted 4 lines
If change it to
dev_warn_once, then it will have no chance to find the other wrong values which report by firmware, and it indicate a wrong value to mac80211/cfg80211 and lead "iw wlan0 station dump" get a wrong bitrate.
I agree, we should keep this warning. If the firmware still keeps sending invalid rates we should add a specific check to ignore the known invalid values, but not all of them.
Signature · 8 lines
https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 5/5] ath10k: reduce invalid ht params rate message noise
Quoted 43 lines · Kalle Valo
Quoted 32 lines · Wen Gong
Quoted 31 lines · Shuah Khan
ath10k_mac_get_rate_flags_ht()floods dmesg with the following messages,
when it fails to find a match for mcs=7 and rate=1440.
supported_ht_mcs_rate_nss2:
{7, {1300, 2700, 1444, 3000} }
ath10k_pci0000:02:00.0: invalid ht params rate 1440 100kbps nss 2 mcs 7
dev_warn_ratelimited()isn't helping the noise. Usedev_warn_once()instead.Signed-off-by Shuah Khan <[email protected]>--- drivers/net/wireless/ath/ath10k/mac.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)drivers/net/wireless/ath/ath10k/mac.c +0
-0@@ -8970,8 +8970,9 @@ static void ath10k_mac_get_rate_flags_ht(structath10k *ar, u32 rate, u8 nss, u8
*bw |=RATE_INFO_BW_40; *flags |=RATE_INFO_FLAGS_SHORT_GI;
} else {drivers/net/wireless/ath/ath10k/mac.c +1
-2-ath10k_warn (ar,"invalid ht params rate %d 100kbps nss %d mcs %d" , - rate, nss, mcs); + dev_warn_once(ar->dev,Quoted 8 lines
drivers/net/wireless/ath/ath10k/mac.c static void ath10k_mac_get_rate_flags_ht(struct ath10k *ar, u32 rate, u8 nss, u8 · line 8977
Quoted 4 lines
drivers/net/wireless/ath/ath10k/mac.c +2
-0+"invalid ht params rate %d 100kbps nss %d mcs %d" , + rate, nss, mcs); } }The {7, {1300, 2700, 1444, 3000} } is a correct value. The 1440 is report from firmware, its a wrong value, it has fixed in firmware.
Quoted 2 lines
In what version?
Here is the info:
ath10k_pci 0000:02:00.0: qca6174 hw3.2 target 0x05030000 chip_id 0x00340aff sub 17aa:0827
ath10k_pci 0000:02:00.0: firmware ver WLAN.RM.4.4.1-00140-QCARMSWPZ-1 api 6 features wowlan,ignore-otp,mfp crc32 29eb8ca1
ath10k_pci 0000:02:00.0: board_file api 2 bmi_id N/A crc32 4ac0889b
ath10k_pci 0000:02:00.0: htt-ver 3.60 wmi-op 4 htt-op 3 cal otp max-sta 32 raw 0 hwcrypto 1
Quoted 5 lines
Quoted 4 lines
If change it to
dev_warn_once, then it will have no chance to find the other wrong values which report by firmware, and it indicate a wrong value to mac80211/cfg80211 and lead "iw wlan0 station dump" get a wrong bitrate.
Agreed.
Quoted 4 lines
I agree, we should keep this warning. If the firmware still keeps sending invalid rates we should add a specific check to ignore the known invalid values, but not all of them.
Would it be helpful to adjust the default rate limits and set the to a higher value instead. It might be difficult to account all possible invalid values?
Something like, ath10k_warn_ratelimited() to adjust the
DEFAULT_RATELIMIT_INTERVAL and DEFAULT_RATELIMIT_BURST using DEFINE_RATELIMIT_STATE
Let me know if you like this idea. I can send a patch in to do this. I will hang on to this firmware version for a little but longer, so we have a test case. :)
thanks,
-- Shuah
_______________________________________________
ath10k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 5/5] ath10k: reduce invalid ht params rate message noise
Quoted 77 lines · Shuah Khan
Quoted 43 lines · Kalle Valo
Quoted 32 lines · Wen Gong
Quoted 31 lines · Shuah Khan
ath10k_mac_get_rate_flags_ht()floods dmesg with the following messages,
when it fails to find a match for mcs=7 and rate=1440.
supported_ht_mcs_rate_nss2:
{7, {1300, 2700, 1444, 3000} }
ath10k_pci0000:02:00.0: invalid ht params rate 1440 100kbps nss 2 mcs 7
dev_warn_ratelimited()isn't helping the noise. Usedev_warn_once()instead.Signed-off-by Shuah Khan <[email protected]>--- drivers/net/wireless/ath/ath10k/mac.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)drivers/net/wireless/ath/ath10k/mac.c +0
-0@@ -8970,8 +8970,9 @@ static void ath10k_mac_get_rate_flags_ht(structath10k *ar, u32 rate, u8 nss, u8
*bw |=RATE_INFO_BW_40; *flags |=RATE_INFO_FLAGS_SHORT_GI;
} else {drivers/net/wireless/ath/ath10k/mac.c +1
-2-ath10k_warn (ar,"invalid ht params rate %d 100kbps nss %d mcs %d" , - rate, nss, mcs); + dev_warn_once(ar->dev,Quoted 8 lines
drivers/net/wireless/ath/ath10k/mac.c static void ath10k_mac_get_rate_flags_ht(struct ath10k *ar, u32 rate, u8 nss, u8 · line 8977
Quoted 4 lines
drivers/net/wireless/ath/ath10k/mac.c +2
-0+"invalid ht params rate %d 100kbps nss %d mcs %d" , + rate, nss, mcs); } }The {7, {1300, 2700, 1444, 3000} } is a correct value. The 1440 is report from firmware, its a wrong value, it has fixed in firmware.
Quoted 2 lines
In what version?
Here is the info:
ath10k_pci0000:02:00.0: qca6174 hw3.2 target0x05030000chip_id0x00340affsub 17aa:0827
ath10k_pci0000:02:00.0: firmware ver WLAN.RM.4.4.1-00140-QCARMSWPZ-1 api 6 features wowlan,ignore-otp,mfp crc3229eb8ca1
ath10k_pci0000:02:00.0:board_fileapi 2bmi_idN/A crc324ac0889b
ath10k_pci0000:02:00.0: htt-ver 3.60 wmi-op 4 htt-op 3 cal otp max-sta 32 raw 0 hwcrypto 1Quoted 5 lines
Quoted 4 lines
If change it to
dev_warn_once, then it will have no chance to find the other wrong values which report by firmware, and it indicate a wrong value to mac80211/cfg80211 and lead "iw wlan0 station dump" get a wrong bitrate.Agreed.
Quoted 4 lines
I agree, we should keep this warning. If the firmware still keeps sending invalid rates we should add a specific check to ignore the known invalid values, but not all of them.
Would it be helpful to adjust the default rate limits and set the to a higher value instead. It might be difficult to account all possible invalid values?
Quoted 8 lines
Something like,
ath10k_warn_ratelimited()to adjust the
DEFAULT_RATELIMIT_INTERVALandDEFAULT_RATELIMIT_BURSTusingDEFINE_RATELIMIT_STATELet me know if you like this idea. I can send a patch in to do this. I will hang on to this firmware version for a little but longer, so we have a test case. :)
I would rather first try to fix the root cause, which is the firmware sending invalid rates. Wen, you mentioned there's a fix in firmware. Do you know which firmware version (and branch) has the fix?
Signature · 8 lines
https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches _______________________________________________ ath10k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 5/5] ath10k: reduce invalid ht params rate message noise
Quoted 84 lines · Kalle Valo
Quoted 82 lines · Shuah Khan
Quoted 43 lines · Kalle Valo
Quoted 32 lines · Wen Gong
Quoted 31 lines · Shuah Khan
ath10k_mac_get_rate_flags_ht()floods dmesg with the following messages,
when it fails to find a match for mcs=7 and rate=1440.
supported_ht_mcs_rate_nss2:
{7, {1300, 2700, 1444, 3000} }
ath10k_pci0000:02:00.0: invalid ht params rate 1440 100kbps nss 2 mcs 7
dev_warn_ratelimited()isn't helping the noise. Usedev_warn_once()instead.Signed-off-by Shuah Khan <[email protected]>--- drivers/net/wireless/ath/ath10k/mac.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)drivers/net/wireless/ath/ath10k/mac.c +0
-0@@ -8970,8 +8970,9 @@ static void ath10k_mac_get_rate_flags_ht(structath10k *ar, u32 rate, u8 nss, u8
*bw |= RATE_INFO_BW_40; *flags |= RATE_INFO_FLAGS_SHORT_GI; } else {drivers/net/wireless/ath/ath10k/mac.c +1
-2-ath10k_warn (ar,"invalid ht params rate %d 100kbps nss %d mcs %d" , - rate, nss, mcs); + dev_warn_once(ar->dev,Quoted 8 lines
drivers/net/wireless/ath/ath10k/mac.c static void ath10k_mac_get_rate_flags_ht(struct ath10k *ar, u32 rate, u8 nss, u8 · line 8977
Quoted 4 lines
drivers/net/wireless/ath/ath10k/mac.c +2
-0+"invalid ht params rate %d 100kbps nss %d mcs %d" , + rate, nss, mcs); } }The {7, {1300, 2700, 1444, 3000} } is a correct value. The 1440 is report from firmware, its a wrong value, it has fixed in firmware.
Quoted 2 lines
In what version?
Here is the info:
ath10k_pci0000:02:00.0: qca6174 hw3.2 target0x05030000chip_id0x00340affsub 17aa:0827
ath10k_pci0000:02:00.0: firmware ver WLAN.RM.4.4.1-00140-QCARMSWPZ-1 api 6 features wowlan,ignore-otp,mfp crc3229eb8ca1
ath10k_pci0000:02:00.0:board_fileapi 2bmi_idN/A crc324ac0889b
ath10k_pci0000:02:00.0: htt-ver 3.60 wmi-op 4 htt-op 3 cal otp max-sta 32 raw 0 hwcrypto 1Quoted 5 lines
Quoted 4 lines
If change it to
dev_warn_once, then it will have no chance to find the other wrong values which report by firmware, and it indicate a wrong value to mac80211/cfg80211 and lead "iw wlan0 station dump" get a wrong bitrate.Agreed.
Quoted 4 lines
I agree, we should keep this warning. If the firmware still keeps sending invalid rates we should add a specific check to ignore the known invalid values, but not all of them.
Would it be helpful to adjust the default rate limits and set the to a higher value instead. It might be difficult to account all possible invalid values?
Something like,
ath10k_warn_ratelimited()to adjust the
DEFAULT_RATELIMIT_INTERVALandDEFAULT_RATELIMIT_BURSTusingDEFINE_RATELIMIT_STATE
Quoted 8 lines
Quoted 3 lines
Let me know if you like this idea. I can send a patch in to do this. I will hang on to this firmware version for a little but longer, so we have a test case. :)
I would rather first try to fix the root cause, which is the firmware sending invalid rates. Wen, you mentioned there's a fix in firmware. Do you know which firmware version (and branch) has the fix?
Picking this back up. Wen, which firmware version has this fix? I can test this on my system and get rid of the noisy messages. :)
thanks,
-- Shuah
_______________________________________________
ath10k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 5/5] ath10k: reduce invalid ht params rate message noise
Quoted 97 lines · Shuah Khan
Quoted 87 lines · Kalle Valo
Quoted 85 lines · Shuah Khan
Quoted 46 lines · Kalle Valo
Quoted 35 lines · Wen Gong
Quoted 34 lines · Shuah Khan
ath10k_mac_get_rate_flags_ht()floods dmesg with the following messages,
when it fails to find a match for mcs=7 and rate=1440.
supported_ht_mcs_rate_nss2:
{7, {1300, 2700, 1444, 3000} }
ath10k_pci0000:02:00.0: invalid ht params rate 1440 100kbps nss 2 mcs 7
dev_warn_ratelimited()isn't helping the noise. Usedev_warn_once()instead.Signed-off-by Shuah Khan <[email protected]>---
drivers/net/wireless/ath/ath10k/mac.c| 5 +++--1 file changed, 3 insertions(+), 2 deletions(-)drivers/net/wireless/ath/ath10k/mac.c +0
-0@@ -8970,8 +8970,9 @@ static voidath10k_mac_get_rate_flags_ht(struct
ath10k *ar, u32 rate, u8 nss, u8
*bw |=RATE_INFO_BW_40;
*flags |=RATE_INFO_FLAGS_SHORT_GI;
} else {
- ath10k_warn(ar, "invalid ht params rate %d 100kbps nss %d mcs %d",
- rate, nss, mcs);
+ dev_warn_once(ar->dev,
+ "invalid ht params rate %d 100kbps nss %d mcsQuoted 8 lines
Quoted 4 lines
%d",
+ rate, nss, mcs);
}
}The {7, {1300, 2700, 1444, 3000} } is a correct value. The 1440 is report from firmware, its a wrong value, it has fixed in firmware.
Quoted 2 lines
In what version?
Here is the info:
ath10k_pci0000:02:00.0: qca6174 hw3.2 target0x05030000chip_id0x00340affsub 17aa:0827
ath10k_pci0000:02:00.0: firmware ver WLAN.RM.4.4.1-00140-QCARMSWPZ-1 api 6 features wowlan,ignore-otp,mfp crc3229eb8ca1
ath10k_pci0000:02:00.0:board_fileapi 2bmi_idN/A crc324ac0889b
ath10k_pci0000:02:00.0: htt-ver 3.60 wmi-op 4 htt-op 3 cal otp max-sta 32 raw 0 hwcrypto 1Quoted 5 lines
Quoted 4 lines
If change it to
dev_warn_once, then it will have no chance to find the other wrong values which report by firmware, and it indicate a wrong value to mac80211/cfg80211 and lead "iw wlan0 station dump" get a wrong bitrate.Agreed.
Quoted 4 lines
I agree, we should keep this warning. If the firmware still keeps sending invalid rates we should add a specific check to ignore the known invalid values, but not all of them.
Would it be helpful to adjust the default rate limits and set the to a higher value instead. It might be difficult to account all possible invalid values?
Something like,
ath10k_warn_ratelimited()to adjust the
DEFAULT_RATELIMIT_INTERVALandDEFAULT_RATELIMIT_BURSTusingDEFINE_RATELIMIT_STATEQuoted 8 lines
Quoted 3 lines
Let me know if you like this idea. I can send a patch in to do this. I will hang on to this firmware version for a little but longer, so we have a test case. :)
I would rather first try to fix the root cause, which is the firmware sending invalid rates. Wen, you mentioned there's a fix in firmware. Do you know which firmware version (and branch) has the fix?
Quoted 5 lines
Picking this back up. Wen, which firmware version has this fix? I can test this on my system and get rid of the noisy messages. :)
thanks,
-- Shuah
I know its been years, but reading through this Wen mentioned there is a fix in the firmware? I haven't tried all of the firmware binaries in Kalle's tree but the most recent definitely still spam the logs with this message. Is there a specific version I can use to get rid of these?
One thing to note is the older "firmware-4.bin" did not have this problem, but was met with worse problems like driver/firmware crashes.
Thanks,
James
Quoted 6 lines
_______________________________________________
ath10k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 5/5] ath10k: reduce invalid ht params rate message noise
(just a resend with Wen's current e-mail address, no further comments)
Quoted 101 lines · James Prestwood
Quoted 100 lines · Shuah Khan
Quoted 87 lines · Kalle Valo
Quoted 85 lines · Shuah Khan
Quoted 46 lines · Kalle Valo
Quoted 35 lines · Wen Gong
Quoted 34 lines · Shuah Khan
ath10k_mac_get_rate_flags_ht()floods dmesg with the following messages,
when it fails to find a match for mcs=7 and rate=1440.
supported_ht_mcs_rate_nss2:
{7, {1300, 2700, 1444, 3000} }
ath10k_pci0000:02:00.0: invalid ht params rate 1440 100kbps nss 2 mcs 7
dev_warn_ratelimited()isn't helping the noise. Usedev_warn_once()instead.Signed-off-by Shuah Khan <[email protected]>---
drivers/net/wireless/ath/ath10k/mac.c| 5 +++--1 file changed, 3 insertions(+), 2 deletions(-)drivers/net/wireless/ath/ath10k/mac.c +0
-0@@ -8970,8 +8970,9 @@ static voidath10k_mac_get_rate_flags_ht(struct
ath10k *ar, u32 rate, u8 nss, u8
*bw |=RATE_INFO_BW_40;
*flags |=RATE_INFO_FLAGS_SHORT_GI;
} else {
- ath10k_warn(ar, "invalid ht params rate %d 100kbps nss %d mcs %d",
- rate, nss, mcs);
+ dev_warn_once(ar->dev,
+ "invalid ht params rate %d 100kbps nss %d mcsQuoted 8 lines
Quoted 4 lines
%d",
+ rate, nss, mcs);
}
}The {7, {1300, 2700, 1444, 3000} } is a correct value. The 1440 is report from firmware, its a wrong value, it has fixed in firmware.
Quoted 2 lines
In what version?
Here is the info:
ath10k_pci0000:02:00.0: qca6174 hw3.2 target0x05030000chip_id0x00340affsub 17aa:0827
ath10k_pci0000:02:00.0: firmware ver WLAN.RM.4.4.1-00140-QCARMSWPZ-1 api 6 features wowlan,ignore-otp,mfp crc3229eb8ca1
ath10k_pci0000:02:00.0:board_fileapi 2bmi_idN/A crc324ac0889b
ath10k_pci0000:02:00.0: htt-ver 3.60 wmi-op 4 htt-op 3 cal otp max-sta 32 raw 0 hwcrypto 1Quoted 5 lines
Quoted 4 lines
If change it to
dev_warn_once, then it will have no chance to find the other wrong values which report by firmware, and it indicate a wrong value to mac80211/cfg80211 and lead "iw wlan0 station dump" get a wrong bitrate.Agreed.
Quoted 4 lines
I agree, we should keep this warning. If the firmware still keeps sending invalid rates we should add a specific check to ignore the known invalid values, but not all of them.
Would it be helpful to adjust the default rate limits and set the to a higher value instead. It might be difficult to account all possible invalid values?
Something like,
ath10k_warn_ratelimited()to adjust the
DEFAULT_RATELIMIT_INTERVALandDEFAULT_RATELIMIT_BURSTusingDEFINE_RATELIMIT_STATEQuoted 8 lines
Quoted 3 lines
Let me know if you like this idea. I can send a patch in to do this. I will hang on to this firmware version for a little but longer, so we have a test case. :)
I would rather first try to fix the root cause, which is the firmware sending invalid rates. Wen, you mentioned there's a fix in firmware. Do you know which firmware version (and branch) has the fix?
Picking this back up. Wen, which firmware version has this fix? I can test this on my system and get rid of the noisy messages. :)
Quoted 6 lines
Quoted 2 lines
thanks,
-- ShuahI know its been years, but reading through this Wen mentioned there is a fix in the firmware? I haven't tried all of the firmware binaries in Kalle's tree but the most recent definitely still spam the logs with this message. Is there a specific version I can use to get rid of these?
One thing to note is the older "firmware-4.bin" did not have this problem, but was met with worse problems like driver/firmware crashes.
_______________________________________________
ath10k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 5/5] ath10k: reduce invalid ht params rate message noise
Quoted 109 lines · Jeff Johnson
(just a resend with Wen's current e-mail address, no further comments)
Quoted 107 lines · James Prestwood
Quoted 97 lines · Shuah Khan
Quoted 87 lines · Kalle Valo
Quoted 85 lines · Shuah Khan
Quoted 46 lines · Kalle Valo
Quoted 35 lines · Wen Gong
Quoted 34 lines · Shuah Khan
ath10k_mac_get_rate_flags_ht()floods dmesg with the following messages,
when it fails to find a match for mcs=7 and rate=1440.
supported_ht_mcs_rate_nss2:
{7, {1300, 2700, 1444, 3000} }
ath10k_pci0000:02:00.0: invalid ht params rate 1440 100kbps nss 2 mcs 7
dev_warn_ratelimited()isn't helping the noise. Usedev_warn_once()instead.Signed-off-by Shuah Khan <[email protected]>---
drivers/net/wireless/ath/ath10k/mac.c| 5 +++--1 file changed, 3 insertions(+), 2 deletions(-)drivers/net/wireless/ath/ath10k/mac.c +0
-0@@ -8970,8 +8970,9 @@ static voidath10k_mac_get_rate_flags_ht(struct
ath10k *ar, u32 rate, u8 nss, u8
*bw |=RATE_INFO_BW_40;
*flags |=RATE_INFO_FLAGS_SHORT_GI;
} else {
- ath10k_warn(ar, "invalid ht params rate %d 100kbps nss %d mcs %d",
- rate, nss, mcs);
+ dev_warn_once(ar->dev,
+ "invalid ht params rate %d 100kbps nss %dQuoted 8 lines
Quoted 4 lines
mcs %d",
+ rate, nss, mcs);
}
}The {7, {1300, 2700, 1444, 3000} } is a correct value. The 1440 is report from firmware, its a wrong value, it has fixed in firmware.
Quoted 2 lines
In what version?
Here is the info:
ath10k_pci0000:02:00.0: qca6174 hw3.2 target0x05030000chip_id0x00340affsub 17aa:0827
ath10k_pci0000:02:00.0: firmware ver WLAN.RM.4.4.1-00140-QCARMSWPZ-1 api 6 features wowlan,ignore-otp,mfp crc3229eb8ca1
ath10k_pci0000:02:00.0:board_fileapi 2bmi_idN/A crc324ac0889b
ath10k_pci0000:02:00.0: htt-ver 3.60 wmi-op 4 htt-op 3 cal otp max-sta 32 raw 0 hwcrypto 1Quoted 5 lines
Quoted 4 lines
If change it to
dev_warn_once, then it will have no chance to find the other wrong values which report by firmware, and it indicate a wrong value to mac80211/cfg80211 and lead "iw wlan0 station dump" get a wrong bitrate.Agreed.
Quoted 4 lines
I agree, we should keep this warning. If the firmware still keeps sending invalid rates we should add a specific check to ignore the known invalid values, but not all of them.
Would it be helpful to adjust the default rate limits and set the to a higher value instead. It might be difficult to account all possible invalid values?
Something like,
ath10k_warn_ratelimited()to adjust the
DEFAULT_RATELIMIT_INTERVALandDEFAULT_RATELIMIT_BURSTusingDEFINE_RATELIMIT_STATEQuoted 8 lines
Quoted 3 lines
Let me know if you like this idea. I can send a patch in to do this. I will hang on to this firmware version for a little but longer, so we have a test case. :)
I would rather first try to fix the root cause, which is the firmware sending invalid rates. Wen, you mentioned there's a fix in firmware. Do you know which firmware version (and branch) has the fix?
Quoted 5 lines
Picking this back up. Wen, which firmware version has this fix? I can test this on my system and get rid of the noisy messages. :)
thanks,
-- ShuahI know its been years, but reading through this Wen mentioned there is a fix in the firmware? I haven't tried all of the firmware binaries in Kalle's tree but the most recent definitely still spam the logs with this message. Is there a specific version I can use to get rid of these?
One thing to note is the older "firmware-4.bin" did not have this problem, but was met with worse problems like driver/firmware crashes.
I hate to keep bringing this up, and if its a "won't fix" type of issue you don't have to tell me twice and I can deal with it out of tree. Any answer would be greatly appreciated so I know how to proceed, and if its something I can wait for on upstream or handle on my own. Since its apparently a firmware bug its not something I can fix, or I would try to myself.
(sorry, my last inquiry was mistakenly from a different email address).
Thanks,
James
Re: [PATCH 5/5] ath10k: reduce invalid ht params rate message noise
Quoted 6 lines · James Prestwood
I hate to keep bringing this up, and if its a "won't fix" type of issue you don't have to tell me twice and I can deal with it out of tree. Any answer would be greatly appreciated so I know how to proceed, and if its something I can wait for on upstream or handle on my own. Since its apparently a firmware bug its not something I can fix, or I would try to myself.
I've asked the development team for an update.
/jeff