Re: hot busy loop inside ath11k

[Baochen Qiang <quic_bqiang@quicinc.com>]

On 11/7/2024 10:36 PM, Kalle Valo wrote:
> "Yury Vostrikov" <mon@unformed.ru> writes:
> 
>> Hi Kalle,
>>
>> It seems there is problem with busy wait inside
>> ath11k_debugfs_fw_stats_request. I have a laptop with QCNFA765 WiFi
>> controller. It is running vanilla v6.11.4 with the following fw:
>>
>>> [    3.934078] ath11k_pci 0000:01:00.0: wcn6855 hw2.1
>>> [    4.801624] ath11k_pci 0000:01:00.0: chip_id 0x12 chip_family 0xb board_id 0xff soc_id 0x400c1211
>>> [    4.802469] ath11k_pci 0000:01:00.0: fw_version 0x11088c35 fw_build_timestamp 2024-04-17 08:34 fw_build_id WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.41
>>
>> Sometimes after device wakes up, the system becomes
>> sluggish and burns CPU. A assume it is because of a firmware bug, but driver
>> should not waste CPU regardless.
>>
>> According to perf, the most time is spent in busy waitin inside  ath11k_debugfs_fw_stats_request:
>>
>> 94.60%     0.00%  i3status         [kernel.kallsyms]                 [k] do_syscall_64
>>         |
>>          --94.60%--do_syscall_64
>>                    |
>>                     --94.55%--__sys_sendmsg
>>                               ___sys_sendmsg
>>                               ____sys_sendmsg
>>                               netlink_sendmsg
>>                               netlink_unicast
>>                               genl_rcv
>>                               netlink_rcv_skb
>>                               genl_rcv_msg
>>                               |
>>                                --94.55%--genl_family_rcv_msg_dumpit
>>                                          __netlink_dump_start
>>                                          netlink_dump
>>                                          genl_dumpit
>>                                          nl80211_dump_station
>>                                          |
>>                                           --94.55%--ieee80211_dump_station
>>                                                     sta_set_sinfo
>>                                                     |
>>                                                      --94.55%--ath11k_mac_op_sta_statistics
>>                                                                ath11k_debugfs_get_fw_stats
>>                                                                |
>>                                                                 --94.55%--ath11k_debugfs_fw_stats_request
>>                                                                           |
>>                                                                           |--41.73%--_raw_spin_lock_bh
>>                                                                           |
>>                                                                           |--22.74%--__local_bh_enable_ip
>>                                                                           |
>>                                                                           |--9.22%--_raw_spin_unlock_bh
>>                                                                           |
>>                                                                            --6.66%--srso_alias_safe_ret
>>
>> If I'm reading the code correctly, then ath11k_debugfs_fw_stats_request has 3 second timeout:
>>
>>> timeout = jiffies + msecs_to_jiffies(3 * 1000);
>>
>> however, it only waits for 1 second:
>>
>>> time_left = wait_for_completion_timeout(&ar->fw_stats_complete, 1 * HZ);
>>
>> the rest (2 seconds) is spent inside busy loop
>>
>>> 	for (;;) {
>>> 		if (time_after(jiffies, timeout))
>>> 			break;
>>>
>>> 		spin_lock_bh(&ar->data_lock);
>>> 		if (ar->fw_stats_done) {
>>> 			spin_unlock_bh(&ar->data_lock);
>>> 			break;
>>> 		}
>>> 		spin_unlock_bh(&ar->data_lock);
>>> 	}
>>
>>
>> spinning  for 2 seconds seems excessive to me. What do you think?
> 
> Oh wow, excessive is an understatement :) That's horrible, I don't know
> how we missed that. And an unlimited loop like that is a big no-no in
> kernel, every loop should have a some kind of maximum limit. Can someone
> send a patch?
Hi Kalle, I already sent an internal patch to fix this.

> 
>> Also, if you happen to know how & where to report firmware bugs, I'd
>> appreciate any pointers.
> 
> I recommend filing to bugzilla:
> 
> https://wireless.docs.kernel.org/en/latest/en/users/drivers/ath11k/bugreport.html
> 
> Though we are overwhelmed with everything right now so don't expect a
> quick response :/
>