From: Abel Vesa <abel.vesa@linaro.org>
To: bjorn.andersson@oss.qualcomm.com
Cc: Jeff Johnson <jjohnson@kernel.org>,
Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>,
Mahendran P <quic_mahep@quicinc.com>,
Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>,
linux-arm-msm@vger.kernel.org,
Jeff Johnson <jeff.johnson@oss.qualcomm.com>,
linux-wireless@vger.kernel.org, ath12k@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] wifi: ath12k: Avoid CPU busy-wait by handling VDEV_STAT and BCN_STAT
Date: Tue, 10 Jun 2025 11:16:40 +0300 [thread overview]
Message-ID: <aEfp6B97gwAeeOs3@linaro.org> (raw)
In-Reply-To: <20250609-ath12k-fw-stats-done-v1-1-2b3624656697@oss.qualcomm.com>
On 25-06-09 22:06:22, Bjorn Andersson via B4 Relay wrote:
> From: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
>
> When the ath12k driver is built without CONFIG_ATH12K_DEBUG, the
> recently refactored stats code can cause any user space application
> (such at NetworkManager) to consume 100% CPU for 3 seconds, every time
> stats are read.
>
> Commit 'b8a0d83fe4c7 ("wifi: ath12k: move firmware stats out of
> debugfs")' moved ath12k_debugfs_fw_stats_request() out of debugfs, by
> merging the additional logic into ath12k_mac_get_fw_stats().
>
> Among the added responsibility of ath12k_mac_get_fw_stats() was the
> busy-wait for `fw_stats_done`.
>
> Signalling of `fw_stats_done` happens when one of the
> WMI_REQUEST_PDEV_STAT, WMI_REQUEST_VDEV_STAT, and WMI_REQUEST_BCN_STAT
> messages are received, but the handling of the latter two commands remained
> in the debugfs code. As `fw_stats_done` isn't signalled, the calling
> processes will spin until the timeout (3 seconds) is reached.
>
> Moving the handling of these two additional responses out of debugfs
> resolves the issue.
>
> Fixes: b8a0d83fe4c7 ("wifi: ath12k: move firmware stats out of debugfs")
> Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Tested on Dell XPS 13 9345.
Tested-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> drivers/net/wireless/ath/ath12k/debugfs.c | 58 --------------------------
> drivers/net/wireless/ath/ath12k/debugfs.h | 7 ----
> drivers/net/wireless/ath/ath12k/wmi.c | 67 +++++++++++++++++++++++++++----
> 3 files changed, 60 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/debugfs.c b/drivers/net/wireless/ath/ath12k/debugfs.c
> index dd624d73b8b2714e77c9d89b5a52f7b3fcb02951..23da93afaa5c25e806c9859dbbdd796afd23d78a 100644
> --- a/drivers/net/wireless/ath/ath12k/debugfs.c
> +++ b/drivers/net/wireless/ath/ath12k/debugfs.c
> @@ -1251,64 +1251,6 @@ void ath12k_debugfs_soc_destroy(struct ath12k_base *ab)
> */
> }
>
> -void
> -ath12k_debugfs_fw_stats_process(struct ath12k *ar,
> - struct ath12k_fw_stats *stats)
> -{
> - struct ath12k_base *ab = ar->ab;
> - struct ath12k_pdev *pdev;
> - bool is_end;
> - static unsigned int num_vdev, num_bcn;
> - size_t total_vdevs_started = 0;
> - int i;
> -
> - if (stats->stats_id == WMI_REQUEST_VDEV_STAT) {
> - if (list_empty(&stats->vdevs)) {
> - ath12k_warn(ab, "empty vdev stats");
> - return;
> - }
> - /* FW sends all the active VDEV stats irrespective of PDEV,
> - * hence limit until the count of all VDEVs started
> - */
> - rcu_read_lock();
> - for (i = 0; i < ab->num_radios; i++) {
> - pdev = rcu_dereference(ab->pdevs_active[i]);
> - if (pdev && pdev->ar)
> - total_vdevs_started += pdev->ar->num_started_vdevs;
> - }
> - rcu_read_unlock();
> -
> - is_end = ((++num_vdev) == total_vdevs_started);
> -
> - list_splice_tail_init(&stats->vdevs,
> - &ar->fw_stats.vdevs);
> -
> - if (is_end) {
> - ar->fw_stats.fw_stats_done = true;
> - num_vdev = 0;
> - }
> - return;
> - }
> - if (stats->stats_id == WMI_REQUEST_BCN_STAT) {
> - if (list_empty(&stats->bcn)) {
> - ath12k_warn(ab, "empty beacon stats");
> - return;
> - }
> - /* Mark end until we reached the count of all started VDEVs
> - * within the PDEV
> - */
> - is_end = ((++num_bcn) == ar->num_started_vdevs);
> -
> - list_splice_tail_init(&stats->bcn,
> - &ar->fw_stats.bcn);
> -
> - if (is_end) {
> - ar->fw_stats.fw_stats_done = true;
> - num_bcn = 0;
> - }
> - }
> -}
> -
> static int ath12k_open_vdev_stats(struct inode *inode, struct file *file)
> {
> struct ath12k *ar = inode->i_private;
> diff --git a/drivers/net/wireless/ath/ath12k/debugfs.h b/drivers/net/wireless/ath/ath12k/debugfs.h
> index ebef7dace3448e4bdf2d6cb155d089267315172c..21641a8a03460c6cc1b34929a353e5605bb834ce 100644
> --- a/drivers/net/wireless/ath/ath12k/debugfs.h
> +++ b/drivers/net/wireless/ath/ath12k/debugfs.h
> @@ -12,8 +12,6 @@ void ath12k_debugfs_soc_create(struct ath12k_base *ab);
> void ath12k_debugfs_soc_destroy(struct ath12k_base *ab);
> void ath12k_debugfs_register(struct ath12k *ar);
> void ath12k_debugfs_unregister(struct ath12k *ar);
> -void ath12k_debugfs_fw_stats_process(struct ath12k *ar,
> - struct ath12k_fw_stats *stats);
> void ath12k_debugfs_op_vif_add(struct ieee80211_hw *hw,
> struct ieee80211_vif *vif);
> void ath12k_debugfs_pdev_create(struct ath12k_base *ab);
> @@ -126,11 +124,6 @@ static inline void ath12k_debugfs_unregister(struct ath12k *ar)
> {
> }
>
> -static inline void ath12k_debugfs_fw_stats_process(struct ath12k *ar,
> - struct ath12k_fw_stats *stats)
> -{
> -}
> -
> static inline bool ath12k_debugfs_is_extd_rx_stats_enabled(struct ath12k *ar)
> {
> return false;
> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
> index 60e2444fe08cefa39ae218d07eb9736d2a0c982b..2d2444417e2b2d9281754d113f2b073034e27739 100644
> --- a/drivers/net/wireless/ath/ath12k/wmi.c
> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
> @@ -7626,6 +7626,63 @@ static int ath12k_wmi_pull_fw_stats(struct ath12k_base *ab, struct sk_buff *skb,
> &parse);
> }
>
> +static void ath12k_wmi_fw_stats_process(struct ath12k *ar,
> + struct ath12k_fw_stats *stats)
> +{
> + struct ath12k_base *ab = ar->ab;
> + struct ath12k_pdev *pdev;
> + bool is_end;
> + static unsigned int num_vdev, num_bcn;
> + size_t total_vdevs_started = 0;
> + int i;
> +
> + if (stats->stats_id == WMI_REQUEST_VDEV_STAT) {
> + if (list_empty(&stats->vdevs)) {
> + ath12k_warn(ab, "empty vdev stats");
> + return;
> + }
> + /* FW sends all the active VDEV stats irrespective of PDEV,
> + * hence limit until the count of all VDEVs started
> + */
> + rcu_read_lock();
> + for (i = 0; i < ab->num_radios; i++) {
> + pdev = rcu_dereference(ab->pdevs_active[i]);
> + if (pdev && pdev->ar)
> + total_vdevs_started += pdev->ar->num_started_vdevs;
> + }
> + rcu_read_unlock();
> +
> + is_end = ((++num_vdev) == total_vdevs_started);
> +
> + list_splice_tail_init(&stats->vdevs,
> + &ar->fw_stats.vdevs);
> +
> + if (is_end) {
> + ar->fw_stats.fw_stats_done = true;
> + num_vdev = 0;
> + }
> + return;
> + }
> + if (stats->stats_id == WMI_REQUEST_BCN_STAT) {
> + if (list_empty(&stats->bcn)) {
> + ath12k_warn(ab, "empty beacon stats");
> + return;
> + }
> + /* Mark end until we reached the count of all started VDEVs
> + * within the PDEV
> + */
> + is_end = ((++num_bcn) == ar->num_started_vdevs);
> +
> + list_splice_tail_init(&stats->bcn,
> + &ar->fw_stats.bcn);
> +
> + if (is_end) {
> + ar->fw_stats.fw_stats_done = true;
> + num_bcn = 0;
> + }
> + }
> +}
> +
> static void ath12k_update_stats_event(struct ath12k_base *ab, struct sk_buff *skb)
> {
> struct ath12k_fw_stats stats = {};
> @@ -7655,19 +7712,15 @@ static void ath12k_update_stats_event(struct ath12k_base *ab, struct sk_buff *sk
>
> spin_lock_bh(&ar->data_lock);
>
> - /* WMI_REQUEST_PDEV_STAT can be requested via .get_txpower mac ops or via
> - * debugfs fw stats. Therefore, processing it separately.
> - */
> + /* Handle WMI_REQUEST_PDEV_STAT status update */
> if (stats.stats_id == WMI_REQUEST_PDEV_STAT) {
> list_splice_tail_init(&stats.pdevs, &ar->fw_stats.pdevs);
> ar->fw_stats.fw_stats_done = true;
> goto complete;
> }
>
> - /* WMI_REQUEST_VDEV_STAT and WMI_REQUEST_BCN_STAT are currently requested only
> - * via debugfs fw stats. Hence, processing these in debugfs context.
> - */
> - ath12k_debugfs_fw_stats_process(ar, &stats);
> + /* Handle WMI_REQUEST_VDEV_STAT and WMI_REQUEST_BCN_STAT updates. */
> + ath12k_wmi_fw_stats_process(ar, &stats);
>
> complete:
> complete(&ar->fw_stats_complete);
>
> ---
> base-commit: 4f27f06ec12190c7c62c722e99ab6243dea81a94
> change-id: 20250609-ath12k-fw-stats-done-dca8bf77a7da
>
> Best regards,
> --
> Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
>
>
>
next prev parent reply other threads:[~2025-06-10 8:16 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-10 3:06 [PATCH] wifi: ath12k: Avoid CPU busy-wait by handling VDEV_STAT and BCN_STAT Bjorn Andersson via B4 Relay
2025-06-10 7:46 ` Rameshkumar Sundaram
2025-06-10 17:41 ` Bjorn Andersson
2025-06-11 3:04 ` Baochen Qiang
2025-06-10 8:16 ` Abel Vesa [this message]
2025-06-17 23:32 ` Jeff Johnson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aEfp6B97gwAeeOs3@linaro.org \
--to=abel.vesa@linaro.org \
--cc=aditya.kumar.singh@oss.qualcomm.com \
--cc=ath12k@lists.infradead.org \
--cc=bjorn.andersson@oss.qualcomm.com \
--cc=jeff.johnson@oss.qualcomm.com \
--cc=jjohnson@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=quic_mahep@quicinc.com \
--cc=rameshkumar.sundaram@oss.qualcomm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox