[PATCH 0/3] wifi: ath11k: Fix clang+KASAN stack frame size warnings

[PATCH 0/3] wifi: ath11k: Fix clang+KASAN stack frame size warnings

[Jeff Johnson]

Fix the following stack size warnings observed when using clang with
KASAN enabled:

drivers/net/wireless/ath/ath11k/qmi.c:3199:13: warning: stack frame size (1560) exceeds limit (1024) in 'ath11k_qmi_driver_event_work' [-Wframe-larger-than]
drivers/net/wireless/ath/ath11k/dp_rx.c:5244:5: warning: stack frame size (1304) exceeds limit (1024) in 'ath11k_dp_rx_process_mon_status' [-Wframe-larger-than]
drivers/net/wireless/ath/ath11k/wow.c:672:5: warning: stack frame size (1336) exceeds limit (1024) in 'ath11k_wow_op_suspend' [-Wframe-larger-than]

---
Jeff Johnson (3):
      wifi: ath11k: mark some QMI driver event helpers as noinline
      wifi: ath11k: mark ath11k_dp_rx_mon_mpdu_pop() as noinline
      wifi: ath11k: mark ath11k_wow_convert_8023_to_80211() as noinline

 drivers/net/wireless/ath/ath11k/dp_rx.c | 11 ++++++-----
 drivers/net/wireless/ath/ath11k/qmi.c   |  8 ++++++--
 drivers/net/wireless/ath/ath11k/wow.c   |  6 ++++--
 3 files changed, 16 insertions(+), 9 deletions(-)
---
base-commit: 9ee80e341c4e473f6c13af3b529fed4ab073234c
change-id: 20241108-ath11k-noinline-74d8e50152c9

[PATCH 1/3] wifi: ath11k: mark some QMI driver event helpers as noinline

[Jeff Johnson]

When compiling the ath11k driver using clang with KASAN enabled, the
following warning is observed:

drivers/net/wireless/ath/ath11k/qmi.c:3199:13: warning: stack frame size (1560) exceeds limit (1024) in 'ath11k_qmi_driver_event_work' [-Wframe-larger-than]

This is similar to the issue found in ath12k/qmi.c that was discussed
in [1] and fixed with [2]. The issue is that clang inlining can
explode stack usage.

Just as in ath12k, ath11k_qmi_driver_event_work() itself is a pretty
lightweight function, but it dispatches to several other functions
which do the real work:

ath11k_qmi_driver_event_work()
	ath11k_qmi_event_server_arrive()
		ath11k_qmi_fw_ind_register_send()
		ath11k_qmi_host_cap_send() *
		ath11k_qmi_event_load_bdf()
	ath11k_qmi_event_mem_request()
		ath11k_qmi_respond_fw_mem_request()
	ath11k_qmi_event_load_bdf()
	ath11k_qmi_wlanfw_m3_info_send() *
		ath11k_qmi_m3_load()
	ath11k_qmi_process_coldboot_calibration()

Of these, the two marked with * have non-trivial stack usage. Mark
those functions as 'noinline_for_stack' to prevent them from being
inlined in ath12k_qmi_driver_event_work(), thereby eliminating the
excessive stack usage.

Note that this approach is a bit more "surgical" than the ath12k
approach as only the two functions with the largest stack usage are
modified.

Link: https://msgid.link/bc214795-1c51-4cb7-922f-67d6ef98bff2@quicinc.com # [1]
Link: https://patch.msgid.link/20241028-ath12k_qmi_driver_event_work-v1-1-0d532eb593fa@quicinc.com # [2]
Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com>
---
 drivers/net/wireless/ath/ath11k/qmi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index 7a22483b35cd..5759fc521316 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -1704,7 +1704,9 @@ static const struct qmi_elem_info qmi_wlfw_fw_init_done_ind_msg_v01_ei[] = {
 	},
 };
 
-static int ath11k_qmi_host_cap_send(struct ath11k_base *ab)
+/* clang stack usage explodes if this is inlined */
+static noinline_for_stack
+int ath11k_qmi_host_cap_send(struct ath11k_base *ab)
 {
 	struct qmi_wlanfw_host_cap_req_msg_v01 req;
 	struct qmi_wlanfw_host_cap_resp_msg_v01 resp;
@@ -2570,7 +2572,9 @@ static void ath11k_qmi_m3_free(struct ath11k_base *ab)
 	m3_mem->size = 0;
 }
 
-static int ath11k_qmi_wlanfw_m3_info_send(struct ath11k_base *ab)
+/* clang stack usage explodes if this is inlined */
+static noinline_for_stack
+int ath11k_qmi_wlanfw_m3_info_send(struct ath11k_base *ab)
 {
 	struct m3_mem_region *m3_mem = &ab->qmi.m3_mem;
 	struct qmi_wlanfw_m3_info_req_msg_v01 req;

-- 
2.42.0

[PATCH 2/3] wifi: ath11k: mark ath11k_dp_rx_mon_mpdu_pop() as noinline

[Jeff Johnson]

When compiling the ath11k driver using clang with KASAN enabled, the
following warning is observed:

drivers/net/wireless/ath/ath11k/dp_rx.c:5244:5: warning: stack frame size (1304) exceeds limit (1024) in 'ath11k_dp_rx_process_mon_status' [-Wframe-larger-than]

This is similar to the issue found in ath12k/qmi.c that was discussed
in [1] and fixed with [2]. The issue is that clang inlining can
explode stack usage.

ath11k_dp_rx_process_mon_status() itself is a pretty
lightweight function, but it dispatches to several other functions
which do the real work:

ath11k_dp_rx_process_mon_status()
	ath11k_dp_rx_reap_mon_status_ring()
	ath11k_dp_rx_mon_dest_process()
		ath11k_dp_rx_mon_mpdu_pop() *
		ath11k_dp_rx_mon_deliver()
			ath11k_dp_rx_mon_merg_msdus()
			ath11k_dp_rx_deliver_msdu()
	ath11k_dp_rx_update_peer_stats()

Of these, only ath11k_dp_rx_mon_mpdu_pop() has non-trivial stack
usage, so mark that function as 'noinline_for_stack' to prevent it
from being inlined in ath11k_dp_rx_process_mon_status(), thereby
eliminating the excessive stack usage.

Link: https://msgid.link/bc214795-1c51-4cb7-922f-67d6ef98bff2@quicinc.com # [1]
Link: https://patch.msgid.link/20241028-ath12k_qmi_driver_event_work-v1-1-0d532eb593fa@quicinc.com # [2]
Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com>
---
 drivers/net/wireless/ath/ath11k/dp_rx.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index 40088e62572e..f253fac116bc 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -4690,11 +4690,12 @@ static void ath11k_dp_mon_get_buf_len(struct hal_rx_msdu_desc_info *info,
 	}
 }
 
-static u32
-ath11k_dp_rx_mon_mpdu_pop(struct ath11k *ar, int mac_id,
-			  void *ring_entry, struct sk_buff **head_msdu,
-			  struct sk_buff **tail_msdu, u32 *npackets,
-			  u32 *ppdu_id)
+/* clang stack usage explodes if this is inlined */
+static noinline_for_stack
+u32 ath11k_dp_rx_mon_mpdu_pop(struct ath11k *ar, int mac_id,
+			      void *ring_entry, struct sk_buff **head_msdu,
+			      struct sk_buff **tail_msdu, u32 *npackets,
+			      u32 *ppdu_id)
 {
 	struct ath11k_pdev_dp *dp = &ar->dp;
 	struct ath11k_mon_data *pmon = (struct ath11k_mon_data *)&dp->mon_data;

-- 
2.42.0

[PATCH 3/3] wifi: ath11k: mark ath11k_wow_convert_8023_to_80211() as noinline

[Jeff Johnson]

When compiling the ath11k driver using clang with KASAN enabled, the
following warning is observed:

drivers/net/wireless/ath/ath11k/wow.c:672:5: warning: stack frame size (1336) exceeds limit (1024) in 'ath11k_wow_op_suspend' [-Wframe-larger-than]

This is similar to the issue found in ath12k/qmi.c that was discussed
in [1] and fixed with [2]. The issue is that clang inlining can
explode stack usage.

ath11k_wow_op_suspend() itself is a pretty lightweight function, but
it dispatches to several other functions which do the real work. One
path in particular is:

ath11k_wow_op_suspend()
	ath11k_wow_set_wakeups()
		ath11k_vif_wow_set_wakeups()
			ath11k_wow_convert_8023_to_80211()

Of these, ath11k_wow_convert_8023_to_80211() has non-trivial stack
usage, so mark it as 'noinline_for_stack' to prevent it from being
inlined in ath11k_wow_op_suspend(), thereby eliminating the excessive
stack usage.

Link: https://msgid.link/bc214795-1c51-4cb7-922f-67d6ef98bff2@quicinc.com # [1]
Link: https://patch.msgid.link/20241028-ath12k_qmi_driver_event_work-v1-1-0d532eb593fa@quicinc.com # [2]
Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com>
---
 drivers/net/wireless/ath/ath11k/wow.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/wow.c b/drivers/net/wireless/ath/ath11k/wow.c
index 827085a926b2..b6f08755129f 100644
--- a/drivers/net/wireless/ath/ath11k/wow.c
+++ b/drivers/net/wireless/ath/ath11k/wow.c
@@ -148,8 +148,10 @@ static int ath11k_wow_cleanup(struct ath11k *ar)
  * 802.11: |4B|dest mac(6B)| 6B |src mac(6B)|  8B  |type(2B)|  body...  |
  *         +--+------------+----+-----------+---------------+-----------+
  */
-static void ath11k_wow_convert_8023_to_80211(struct cfg80211_pkt_pattern *new,
-					     const struct cfg80211_pkt_pattern *old)
+/* clang stack usage explodes if this is inlined */
+static noinline_for_stack
+void ath11k_wow_convert_8023_to_80211(struct cfg80211_pkt_pattern *new,
+				      const struct cfg80211_pkt_pattern *old)
 {
 	u8 hdr_8023_pattern[ETH_HLEN] = {};
 	u8 hdr_8023_bit_mask[ETH_HLEN] = {};

-- 
2.42.0

Re: [PATCH 1/3] wifi: ath11k: mark some QMI driver event helpers as noinline

[Kalle Valo]

Jeff Johnson <quic_jjohnson@quicinc.com> writes:

> When compiling the ath11k driver using clang with KASAN enabled, the
> following warning is observed:
>
> drivers/net/wireless/ath/ath11k/qmi.c:3199:13: warning: stack frame size (1560) exceeds limit (1024) in 'ath11k_qmi_driver_event_work' [-Wframe-larger-than]
>
> This is similar to the issue found in ath12k/qmi.c that was discussed
> in [1] and fixed with [2]. The issue is that clang inlining can
> explode stack usage.
>
> Just as in ath12k, ath11k_qmi_driver_event_work() itself is a pretty
> lightweight function, but it dispatches to several other functions
> which do the real work:
>
> ath11k_qmi_driver_event_work()
> 	ath11k_qmi_event_server_arrive()
> 		ath11k_qmi_fw_ind_register_send()
> 		ath11k_qmi_host_cap_send() *
> 		ath11k_qmi_event_load_bdf()
> 	ath11k_qmi_event_mem_request()
> 		ath11k_qmi_respond_fw_mem_request()
> 	ath11k_qmi_event_load_bdf()
> 	ath11k_qmi_wlanfw_m3_info_send() *
> 		ath11k_qmi_m3_load()
> 	ath11k_qmi_process_coldboot_calibration()
>
> Of these, the two marked with * have non-trivial stack usage. Mark
> those functions as 'noinline_for_stack' to prevent them from being
> inlined in ath12k_qmi_driver_event_work(), thereby eliminating the
> excessive stack usage.
>
> Note that this approach is a bit more "surgical" than the ath12k
> approach as only the two functions with the largest stack usage are
> modified.
>
> Link: https://msgid.link/bc214795-1c51-4cb7-922f-67d6ef98bff2@quicinc.com # [1]
> Link: https://patch.msgid.link/20241028-ath12k_qmi_driver_event_work-v1-1-0d532eb593fa@quicinc.com # [2]
> Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com>

Acked-by: Kalle Valo <kvalo@kernel.org>

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 2/3] wifi: ath11k: mark ath11k_dp_rx_mon_mpdu_pop() as noinline

[Kalle Valo]

Jeff Johnson <quic_jjohnson@quicinc.com> writes:

> When compiling the ath11k driver using clang with KASAN enabled, the
> following warning is observed:
>
> drivers/net/wireless/ath/ath11k/dp_rx.c:5244:5: warning: stack frame size (1304) exceeds limit (1024) in 'ath11k_dp_rx_process_mon_status' [-Wframe-larger-than]
>
> This is similar to the issue found in ath12k/qmi.c that was discussed
> in [1] and fixed with [2]. The issue is that clang inlining can
> explode stack usage.
>
> ath11k_dp_rx_process_mon_status() itself is a pretty
> lightweight function, but it dispatches to several other functions
> which do the real work:
>
> ath11k_dp_rx_process_mon_status()
> 	ath11k_dp_rx_reap_mon_status_ring()
> 	ath11k_dp_rx_mon_dest_process()
> 		ath11k_dp_rx_mon_mpdu_pop() *
> 		ath11k_dp_rx_mon_deliver()
> 			ath11k_dp_rx_mon_merg_msdus()
> 			ath11k_dp_rx_deliver_msdu()
> 	ath11k_dp_rx_update_peer_stats()
>
> Of these, only ath11k_dp_rx_mon_mpdu_pop() has non-trivial stack
> usage, so mark that function as 'noinline_for_stack' to prevent it
> from being inlined in ath11k_dp_rx_process_mon_status(), thereby
> eliminating the excessive stack usage.
>
> Link: https://msgid.link/bc214795-1c51-4cb7-922f-67d6ef98bff2@quicinc.com # [1]
> Link: https://patch.msgid.link/20241028-ath12k_qmi_driver_event_work-v1-1-0d532eb593fa@quicinc.com # [2]
> Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com>

Acked-by: Kalle Valo <kvalo@kernel.org>

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 3/3] wifi: ath11k: mark ath11k_wow_convert_8023_to_80211() as noinline

[Kalle Valo]

Jeff Johnson <quic_jjohnson@quicinc.com> writes:

> When compiling the ath11k driver using clang with KASAN enabled, the
> following warning is observed:
>
> drivers/net/wireless/ath/ath11k/wow.c:672:5: warning: stack frame size (1336) exceeds limit (1024) in 'ath11k_wow_op_suspend' [-Wframe-larger-than]
>
> This is similar to the issue found in ath12k/qmi.c that was discussed
> in [1] and fixed with [2]. The issue is that clang inlining can
> explode stack usage.
>
> ath11k_wow_op_suspend() itself is a pretty lightweight function, but
> it dispatches to several other functions which do the real work. One
> path in particular is:
>
> ath11k_wow_op_suspend()
> 	ath11k_wow_set_wakeups()
> 		ath11k_vif_wow_set_wakeups()
> 			ath11k_wow_convert_8023_to_80211()
>
> Of these, ath11k_wow_convert_8023_to_80211() has non-trivial stack
> usage, so mark it as 'noinline_for_stack' to prevent it from being
> inlined in ath11k_wow_op_suspend(), thereby eliminating the excessive
> stack usage.
>
> Link: https://msgid.link/bc214795-1c51-4cb7-922f-67d6ef98bff2@quicinc.com # [1]
> Link: https://patch.msgid.link/20241028-ath12k_qmi_driver_event_work-v1-1-0d532eb593fa@quicinc.com # [2]
> Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com>

Acked-by: Kalle Valo <kvalo@kernel.org>

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 0/3] wifi: ath11k: Fix clang+KASAN stack frame size warnings

[Jeff Johnson]

On Tue, 19 Nov 2024 07:47:37 -0800, Jeff Johnson wrote:
> Fix the following stack size warnings observed when using clang with
> KASAN enabled:
> 
> 

Applied, thanks!

[1/3] wifi: ath11k: mark some QMI driver event helpers as noinline
      commit: 8ea1d2072ad1a9c24b326b50ebdc2c810c4b2cce
[2/3] wifi: ath11k: mark ath11k_dp_rx_mon_mpdu_pop() as noinline
      commit: 500d7ec88652ba7316e7fba334754e39e3177e4a
[3/3] wifi: ath11k: mark ath11k_wow_convert_8023_to_80211() as noinline
      commit: 4ba72ff2919cad90e1963b708ce23b92120613ff

Best regards,
-- 
Jeff Johnson <quic_jjohnson@quicinc.com>