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(-)
--
2.27.0
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
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
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
@@ -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;+gotoexit;}arsta=(structath10k_sta*)sta->drv_priv;
From: Kalle Valo <hidden> Date: 2021-02-10 08:15:05
Shuah Khan [off-list ref] wrote:
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
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
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
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 <skhan@linuxfoundation.org>
---
drivers/net/wireless/ath/ath10k/mac.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
From: Kalle Valo <hidden> Date: 2021-02-11 06:52:23
Shuah Khan [off-list ref] wrote:
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 <skhan@linuxfoundation.org>
Signed-off-by: Kalle Valo <redacted>
ath10k_drain_tx() must not be called with conf_mutex held as workers can
use that also. Add check to detect conf_mutex held calls.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
drivers/net/wireless/ath/ath10k/mac.c | 1 +
1 file changed, 1 insertion(+)
@@ -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. */voidath10k_drain_tx(structath10k*ar){+WARN_ON(lockdep_is_held(&ar->conf_mutex));/* make sure rcu-protected mac80211 tx path itself is drained */synchronize_net();
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.