[RFC PATCH 0/1] Avoid per WMI message tb allocation

[RFC PATCH 0/1] Avoid per WMI message tb allocation

[Nicolas Escande]

So for a bit of context I have devices that have memory allocation
faillures when parsing WMI messages such as bellow: 

	[73277.177015] swapper/0: page allocation failure: order:2, mode:0x40920(GFP_ATOMIC|__GFP_COMP|__GFP_ZERO), nodemask=(null)
	[73277.177035] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted XXX #109
	[73277.177040] Hardware name: XXX
	[73277.177043] Call trace:
	[73277.177045]  show_stack+0x14/0x20 (C)
	[73277.177054]  dump_stack_lvl+0x58/0x74
	[73277.177058]  dump_stack+0x14/0x1c
	[73277.177061]  warn_alloc+0xd4/0x150
	[73277.177065]  __alloc_pages_noprof+0x688/0xa10
	[73277.177068]  ___kmalloc_large_node+0x64/0xf8
	[73277.177072]  __kmalloc_large_noprof+0x10/0x20
	[73277.177075]  ath12k_wmi_tlv_parse_alloc.constprop.0+0x24/0x90 [ath12k]
	[73277.177095]  ath12k_wmi_op_rx+0x658/0x2624 [ath12k]
	[73277.177111]  ath12k_htc_rx_completion_handler+0x464/0x660 [ath12k]
	[73277.177125]  ath12k_ce_per_engine_service+0x2dc/0x3ec [ath12k]
	[73277.177140]  ath12k_pci_ce_workqueue+0x30/0x50 [ath12k]
	[73277.177155]  process_one_work+0x154/0x2e0
	[73277.177160]  bh_worker+0x1dc/0x228
	[73277.177164]  workqueue_softirq_action+0x74/0x80
	[73277.177168]  tasklet_action+0x10/0x40
	[73277.177172]  handle_softirqs+0xfc/0x240
	[73277.177175]  __do_softirq+0x10/0x18
	[73277.177178]  ____do_softirq+0xc/0x20
	...

So it seems that parsing a a WMI TLV requires an array of WMI_TAG_MAX
(void *). This array is then populated with pointers of parsed structs
depending on the WMI type. This alloc happens on a per WMI message, in
softirq, using GFP_ATOMIC which puts pressure on the kernel allocator and
can fail.

I'm not very familliar but from my understanding of the system_bh_wq, we
can have at most one work item per CPU running this code path. So the
following patch aims changes the code to have a per cpu array instead to 
parse the WMI msg and thus avoiding dynamic allocation.

What do you think ? Is this the right way forward ?

Nicolas Escande (1):
  wifi: ath12k: avoid dynamic alloc when parsing wmi tb

 drivers/net/wireless/ath/ath12k/core.c |   7 +
 drivers/net/wireless/ath/ath12k/core.h |   2 +
 drivers/net/wireless/ath/ath12k/wmi.c  | 170 ++++++-------------------
 3 files changed, 49 insertions(+), 130 deletions(-)

-- 
2.53.0

[RFC PATCH 1/1] wifi: ath12k: avoid dynamic alloc when parsing wmi tb

[Nicolas Escande]

On each WMI message received from the hardware, we alloc a temporary array
of WMI_TAG_MAX entries of type void *. This array is then populated with
pointers of parsed structs depending on the WMI type, and then freed. This
alloc can fail when memory pressure in the system is high enough.

Given the fact that it is scheduled in softirq with the system_bh_wq, we
should not be able to parse more than one WMI message per CPU at any time

So instead lets move to a per cpu allocated array, stored in the struct
ath12k_base, that is reused accros calls. The ath12k_wmi_tlv_parse_alloc()
is also renamed into / merged with ath12k_wmi_tlv_parse() as it no longer
allocs memory but returns the existing per-cpu one.

Signed-off-by: Nicolas Escande <nico.escande@gmail.com>
---
 drivers/net/wireless/ath/ath12k/core.c |   7 +
 drivers/net/wireless/ath/ath12k/core.h |   2 +
 drivers/net/wireless/ath/ath12k/wmi.c  | 170 ++++++-------------------
 3 files changed, 49 insertions(+), 130 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 9d6c50a94e64..961c9df69aa1 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -2237,6 +2237,7 @@ void ath12k_core_free(struct ath12k_base *ab)
 	timer_delete_sync(&ab->rx_replenish_retry);
 	destroy_workqueue(ab->workqueue_aux);
 	destroy_workqueue(ab->workqueue);
+	free_percpu(ab->wmi_tb);
 	kfree(ab);
 }
 
@@ -2249,6 +2250,11 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev, size_t priv_size,
 	if (!ab)
 		return NULL;
 
+	ab->wmi_tb = __alloc_percpu(WMI_TAG_MAX * sizeof(void *),
+				    __alignof__(void *));
+	if (!ab->wmi_tb)
+		goto err_sc_free;
+
 	init_completion(&ab->driver_recovery);
 
 	ab->workqueue = create_singlethread_workqueue("ath12k_wq");
@@ -2296,6 +2302,7 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev, size_t priv_size,
 err_free_wq:
 	destroy_workqueue(ab->workqueue);
 err_sc_free:
+	free_percpu(ab->wmi_tb);
 	kfree(ab);
 	return NULL;
 }
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 990934ec92fc..378573a100e8 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -19,6 +19,7 @@
 #include <linux/average.h>
 #include <linux/of.h>
 #include <linux/rhashtable.h>
+#include <linux/percpu.h>
 #include "qmi.h"
 #include "htc.h"
 #include "wmi.h"
@@ -934,6 +935,7 @@ struct ath12k_base {
 	struct device *dev;
 	struct ath12k_qmi qmi;
 	struct ath12k_wmi_base wmi_ab;
+	void __percpu *wmi_tb;
 	struct completion fw_ready;
 	u8 device_id;
 	int num_radios;
diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
index 404f031a3c87..9ccc10c07c21 100644
--- a/drivers/net/wireless/ath/ath12k/wmi.c
+++ b/drivers/net/wireless/ath/ath12k/wmi.c
@@ -289,29 +289,19 @@ static int ath12k_wmi_tlv_iter_parse(struct ath12k_base *ab, u16 tag, u16 len,
 	return 0;
 }
 
-static int ath12k_wmi_tlv_parse(struct ath12k_base *ar, const void **tb,
-				const void *ptr, size_t len)
-{
-	return ath12k_wmi_tlv_iter(ar, ptr, len, ath12k_wmi_tlv_iter_parse,
-				   (void *)tb);
-}
-
 static const void **
-ath12k_wmi_tlv_parse_alloc(struct ath12k_base *ab,
-			   struct sk_buff *skb, gfp_t gfp)
+ath12k_wmi_tlv_parse(struct ath12k_base *ab, struct sk_buff *skb)
 {
 	const void **tb;
 	int ret;
 
-	tb = kcalloc(WMI_TAG_MAX, sizeof(*tb), gfp);
-	if (!tb)
-		return ERR_PTR(-ENOMEM);
+	tb = this_cpu_ptr(ab->wmi_tb);
+	memset(tb, 0, WMI_TAG_MAX * sizeof(void *));
 
-	ret = ath12k_wmi_tlv_parse(ab, tb, skb->data, skb->len);
-	if (ret) {
-		kfree(tb);
+	ret = ath12k_wmi_tlv_iter(ab, skb->data, skb->len,
+				  ath12k_wmi_tlv_iter_parse, (void *)tb);
+	if (ret)
 		return ERR_PTR(ret);
-	}
 
 	return tb;
 }
@@ -5714,7 +5704,7 @@ static int ath12k_pull_vdev_start_resp_tlv(struct ath12k_base *ab, struct sk_buf
 	const struct wmi_vdev_start_resp_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -5724,13 +5714,11 @@ static int ath12k_pull_vdev_start_resp_tlv(struct ath12k_base *ab, struct sk_buf
 	ev = tb[WMI_TAG_VDEV_START_RESPONSE_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch vdev start resp ev");
-		kfree(tb);
 		return -EPROTO;
 	}
 
 	*vdev_rsp = *ev;
 
-	kfree(tb);
 	return 0;
 }
 
@@ -5809,7 +5797,7 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
 
 	ath12k_dbg(ab, ATH12K_DBG_WMI, "processing regulatory ext channel list\n");
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -5819,7 +5807,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
 	ev = tb[WMI_TAG_REG_CHAN_LIST_CC_EXT_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch reg chan list ext update ev\n");
-		kfree(tb);
 		return -EPROTO;
 	}
 
@@ -5849,7 +5836,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
 	if (num_2g_reg_rules > MAX_REG_RULES || num_5g_reg_rules > MAX_REG_RULES) {
 		ath12k_warn(ab, "Num reg rules for 2G/5G exceeds max limit (num_2g_reg_rules: %d num_5g_reg_rules: %d max_rules: %d)\n",
 			    num_2g_reg_rules, num_5g_reg_rules, MAX_REG_RULES);
-		kfree(tb);
 		return -EINVAL;
 	}
 
@@ -5859,7 +5845,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
 		if (num_6g_reg_rules_ap[i] > MAX_6GHZ_REG_RULES) {
 			ath12k_warn(ab, "Num 6G reg rules for AP mode(%d) exceeds max limit (num_6g_reg_rules_ap: %d, max_rules: %d)\n",
 				    i, num_6g_reg_rules_ap[i], MAX_6GHZ_REG_RULES);
-			kfree(tb);
 			return -EINVAL;
 		}
 
@@ -5884,14 +5869,12 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
 		    num_6g_reg_rules_cl[WMI_REG_VLP_AP][i] >  MAX_6GHZ_REG_RULES) {
 			ath12k_warn(ab, "Num 6g client reg rules exceeds max limit, for client(type: %d)\n",
 				    i);
-			kfree(tb);
 			return -EINVAL;
 		}
 	}
 
 	if (!total_reg_rules) {
 		ath12k_warn(ab, "No reg rules available\n");
-		kfree(tb);
 		return -EINVAL;
 	}
 
@@ -5993,7 +5976,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
 						      ext_wmi_reg_rule);
 
 		if (!reg_info->reg_rules_2g_ptr) {
-			kfree(tb);
 			ath12k_warn(ab, "Unable to Allocate memory for 2g rules\n");
 			return -ENOMEM;
 		}
@@ -6027,7 +6009,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
 						      ext_wmi_reg_rule);
 
 		if (!reg_info->reg_rules_5g_ptr) {
-			kfree(tb);
 			ath12k_warn(ab, "Unable to Allocate memory for 5g rules\n");
 			return -ENOMEM;
 		}
@@ -6046,7 +6027,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
 						      ext_wmi_reg_rule);
 
 		if (!reg_info->reg_rules_6g_ap_ptr[i]) {
-			kfree(tb);
 			ath12k_warn(ab, "Unable to Allocate memory for 6g ap rules\n");
 			return -ENOMEM;
 		}
@@ -6061,7 +6041,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
 							      ext_wmi_reg_rule);
 
 			if (!reg_info->reg_rules_6g_client_ptr[j][i]) {
-				kfree(tb);
 				ath12k_warn(ab, "Unable to Allocate memory for 6g client rules\n");
 				return -ENOMEM;
 			}
@@ -6096,7 +6075,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
 
 	ath12k_dbg(ab, ATH12K_DBG_WMI, "processed regulatory ext channel list\n");
 
-	kfree(tb);
 	return 0;
 }
 
@@ -6107,7 +6085,7 @@ static int ath12k_pull_peer_del_resp_ev(struct ath12k_base *ab, struct sk_buff *
 	const struct wmi_peer_delete_resp_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -6117,7 +6095,6 @@ static int ath12k_pull_peer_del_resp_ev(struct ath12k_base *ab, struct sk_buff *
 	ev = tb[WMI_TAG_PEER_DELETE_RESP_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch peer delete resp ev");
-		kfree(tb);
 		return -EPROTO;
 	}
 
@@ -6127,7 +6104,6 @@ static int ath12k_pull_peer_del_resp_ev(struct ath12k_base *ab, struct sk_buff *
 	ether_addr_copy(peer_del_resp->peer_macaddr.addr,
 			ev->peer_macaddr.addr);
 
-	kfree(tb);
 	return 0;
 }
 
@@ -6139,7 +6115,7 @@ static int ath12k_pull_vdev_del_resp_ev(struct ath12k_base *ab,
 	const struct wmi_vdev_delete_resp_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -6149,13 +6125,11 @@ static int ath12k_pull_vdev_del_resp_ev(struct ath12k_base *ab,
 	ev = tb[WMI_TAG_VDEV_DELETE_RESP_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch vdev delete resp ev");
-		kfree(tb);
 		return -EPROTO;
 	}
 
 	*vdev_id = le32_to_cpu(ev->vdev_id);
 
-	kfree(tb);
 	return 0;
 }
 
@@ -6167,7 +6141,7 @@ static int ath12k_pull_bcn_tx_status_ev(struct ath12k_base *ab,
 	const struct wmi_bcn_tx_status_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -6177,14 +6151,12 @@ static int ath12k_pull_bcn_tx_status_ev(struct ath12k_base *ab,
 	ev = tb[WMI_TAG_OFFLOAD_BCN_TX_STATUS_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch bcn tx status ev");
-		kfree(tb);
 		return -EPROTO;
 	}
 
 	*vdev_id = le32_to_cpu(ev->vdev_id);
 	*tx_status = le32_to_cpu(ev->tx_status);
 
-	kfree(tb);
 	return 0;
 }
 
@@ -6195,7 +6167,7 @@ static int ath12k_pull_vdev_stopped_param_tlv(struct ath12k_base *ab, struct sk_
 	const struct wmi_vdev_stopped_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -6205,13 +6177,11 @@ static int ath12k_pull_vdev_stopped_param_tlv(struct ath12k_base *ab, struct sk_
 	ev = tb[WMI_TAG_VDEV_STOPPED_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch vdev stop ev");
-		kfree(tb);
 		return -EPROTO;
 	}
 
 	*vdev_id = le32_to_cpu(ev->vdev_id);
 
-	kfree(tb);
 	return 0;
 }
 
@@ -6350,7 +6320,7 @@ static int ath12k_pull_mgmt_tx_compl_param_tlv(struct ath12k_base *ab,
 	const struct wmi_mgmt_tx_compl_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -6360,7 +6330,6 @@ static int ath12k_pull_mgmt_tx_compl_param_tlv(struct ath12k_base *ab,
 	ev = tb[WMI_TAG_MGMT_TX_COMPL_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch mgmt tx compl ev");
-		kfree(tb);
 		return -EPROTO;
 	}
 
@@ -6370,7 +6339,6 @@ static int ath12k_pull_mgmt_tx_compl_param_tlv(struct ath12k_base *ab,
 	param->ppdu_id = ev->ppdu_id;
 	param->ack_rssi = ev->ack_rssi;
 
-	kfree(tb);
 	return 0;
 }
 
@@ -6533,7 +6501,7 @@ static int ath12k_pull_scan_ev(struct ath12k_base *ab, struct sk_buff *skb,
 	const struct wmi_scan_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -6543,7 +6511,6 @@ static int ath12k_pull_scan_ev(struct ath12k_base *ab, struct sk_buff *skb,
 	ev = tb[WMI_TAG_SCAN_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch scan ev");
-		kfree(tb);
 		return -EPROTO;
 	}
 
@@ -6555,7 +6522,6 @@ static int ath12k_pull_scan_ev(struct ath12k_base *ab, struct sk_buff *skb,
 	scan_evt_param->vdev_id = ev->vdev_id;
 	scan_evt_param->tsf_timestamp = ev->tsf_timestamp;
 
-	kfree(tb);
 	return 0;
 }
 
@@ -6566,7 +6532,7 @@ static int ath12k_pull_peer_sta_kickout_ev(struct ath12k_base *ab, struct sk_buf
 	const struct wmi_peer_sta_kickout_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -6576,7 +6542,6 @@ static int ath12k_pull_peer_sta_kickout_ev(struct ath12k_base *ab, struct sk_buf
 	ev = tb[WMI_TAG_PEER_STA_KICKOUT_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch peer sta kickout ev");
-		kfree(tb);
 		return -EPROTO;
 	}
 
@@ -6584,7 +6549,6 @@ static int ath12k_pull_peer_sta_kickout_ev(struct ath12k_base *ab, struct sk_buf
 	arg->reason = le32_to_cpu(ev->reason);
 	arg->rssi = le32_to_cpu(ev->rssi);
 
-	kfree(tb);
 	return 0;
 }
 
@@ -6595,7 +6559,7 @@ static int ath12k_pull_roam_ev(struct ath12k_base *ab, struct sk_buff *skb,
 	const struct wmi_roam_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -6605,7 +6569,6 @@ static int ath12k_pull_roam_ev(struct ath12k_base *ab, struct sk_buff *skb,
 	ev = tb[WMI_TAG_ROAM_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch roam ev");
-		kfree(tb);
 		return -EPROTO;
 	}
 
@@ -6613,7 +6576,6 @@ static int ath12k_pull_roam_ev(struct ath12k_base *ab, struct sk_buff *skb,
 	roam_ev->reason = ev->reason;
 	roam_ev->rssi = ev->rssi;
 
-	kfree(tb);
 	return 0;
 }
 
@@ -6647,7 +6609,7 @@ static int ath12k_pull_chan_info_ev(struct ath12k_base *ab, struct sk_buff *skb,
 	const struct wmi_chan_info_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -6657,7 +6619,6 @@ static int ath12k_pull_chan_info_ev(struct ath12k_base *ab, struct sk_buff *skb,
 	ev = tb[WMI_TAG_CHAN_INFO_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch chan info ev");
-		kfree(tb);
 		return -EPROTO;
 	}
 
@@ -6674,7 +6635,6 @@ static int ath12k_pull_chan_info_ev(struct ath12k_base *ab, struct sk_buff *skb,
 	ch_info_ev->mac_clk_mhz = ev->mac_clk_mhz;
 	ch_info_ev->vdev_id = ev->vdev_id;
 
-	kfree(tb);
 	return 0;
 }
 
@@ -6686,7 +6646,7 @@ ath12k_pull_pdev_bss_chan_info_ev(struct ath12k_base *ab, struct sk_buff *skb,
 	const struct wmi_pdev_bss_chan_info_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -6696,7 +6656,6 @@ ath12k_pull_pdev_bss_chan_info_ev(struct ath12k_base *ab, struct sk_buff *skb,
 	ev = tb[WMI_TAG_PDEV_BSS_CHAN_INFO_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch pdev bss chan info ev");
-		kfree(tb);
 		return -EPROTO;
 	}
 
@@ -6714,7 +6673,6 @@ ath12k_pull_pdev_bss_chan_info_ev(struct ath12k_base *ab, struct sk_buff *skb,
 	bss_ch_info_ev->rx_bss_cycle_count_low = ev->rx_bss_cycle_count_low;
 	bss_ch_info_ev->rx_bss_cycle_count_high = ev->rx_bss_cycle_count_high;
 
-	kfree(tb);
 	return 0;
 }
 
@@ -6726,7 +6684,7 @@ ath12k_pull_vdev_install_key_compl_ev(struct ath12k_base *ab, struct sk_buff *sk
 	const struct wmi_vdev_install_key_compl_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -6736,7 +6694,6 @@ ath12k_pull_vdev_install_key_compl_ev(struct ath12k_base *ab, struct sk_buff *sk
 	ev = tb[WMI_TAG_VDEV_INSTALL_KEY_COMPLETE_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch vdev install key compl ev");
-		kfree(tb);
 		return -EPROTO;
 	}
 
@@ -6746,7 +6703,6 @@ ath12k_pull_vdev_install_key_compl_ev(struct ath12k_base *ab, struct sk_buff *sk
 	arg->key_flags = le32_to_cpu(ev->key_flags);
 	arg->status = le32_to_cpu(ev->status);
 
-	kfree(tb);
 	return 0;
 }
 
@@ -6757,7 +6713,7 @@ static int ath12k_pull_peer_assoc_conf_ev(struct ath12k_base *ab, struct sk_buff
 	const struct wmi_peer_assoc_conf_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -6767,14 +6723,12 @@ static int ath12k_pull_peer_assoc_conf_ev(struct ath12k_base *ab, struct sk_buff
 	ev = tb[WMI_TAG_PEER_ASSOC_CONF_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch peer assoc conf ev");
-		kfree(tb);
 		return -EPROTO;
 	}
 
 	peer_assoc_conf->vdev_id = le32_to_cpu(ev->vdev_id);
 	peer_assoc_conf->macaddr = ev->peer_macaddr.addr;
 
-	kfree(tb);
 	return 0;
 }
 
@@ -6785,7 +6739,7 @@ ath12k_pull_pdev_temp_ev(struct ath12k_base *ab, struct sk_buff *skb,
 	const void **tb;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -6795,11 +6749,9 @@ ath12k_pull_pdev_temp_ev(struct ath12k_base *ab, struct sk_buff *skb,
 	ev = tb[WMI_TAG_PDEV_TEMPERATURE_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch pdev temp ev");
-		kfree(tb);
 		return -EPROTO;
 	}
 
-	kfree(tb);
 	return 0;
 }
 
@@ -8582,7 +8534,7 @@ static void ath12k_pdev_ctl_failsafe_check_event(struct ath12k_base *ab,
 	const struct wmi_pdev_ctl_failsafe_chk_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -8592,7 +8544,6 @@ static void ath12k_pdev_ctl_failsafe_check_event(struct ath12k_base *ab,
 	ev = tb[WMI_TAG_PDEV_CTL_FAILSAFE_CHECK_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch pdev ctl failsafe check ev");
-		kfree(tb);
 		return;
 	}
 
@@ -8606,8 +8557,6 @@ static void ath12k_pdev_ctl_failsafe_check_event(struct ath12k_base *ab,
 	if (ev->ctl_failsafe_status != 0)
 		ath12k_warn(ab, "pdev ctl failsafe failure status %d",
 			    ev->ctl_failsafe_status);
-
-	kfree(tb);
 }
 
 static void
@@ -8679,7 +8628,7 @@ ath12k_wmi_pdev_csa_switch_count_status_event(struct ath12k_base *ab,
 	const u32 *vdev_ids;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -8691,7 +8640,6 @@ ath12k_wmi_pdev_csa_switch_count_status_event(struct ath12k_base *ab,
 
 	if (!ev || !vdev_ids) {
 		ath12k_warn(ab, "failed to fetch pdev csa switch count ev");
-		kfree(tb);
 		return;
 	}
 
@@ -8701,8 +8649,6 @@ ath12k_wmi_pdev_csa_switch_count_status_event(struct ath12k_base *ab,
 		   ev->num_vdevs);
 
 	ath12k_wmi_process_csa_switch_count_event(ab, ev, vdev_ids);
-
-	kfree(tb);
 }
 
 static void
@@ -8714,7 +8660,7 @@ ath12k_wmi_pdev_dfs_radar_detected_event(struct ath12k_base *ab, struct sk_buff
 	struct ath12k *ar;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -8725,7 +8671,6 @@ ath12k_wmi_pdev_dfs_radar_detected_event(struct ath12k_base *ab, struct sk_buff
 
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch pdev dfs radar detected ev");
-		kfree(tb);
 		return;
 	}
 
@@ -8764,8 +8709,6 @@ ath12k_wmi_pdev_dfs_radar_detected_event(struct ath12k_base *ab, struct sk_buff
 
 exit:
 	rcu_read_unlock();
-
-	kfree(tb);
 }
 
 static void ath12k_tm_wmi_event_segmented(struct ath12k_base *ab, u32 cmd_id,
@@ -8776,7 +8719,7 @@ static void ath12k_tm_wmi_event_segmented(struct ath12k_base *ab, u32 cmd_id,
 	int ret;
 	u16 length;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
@@ -8787,14 +8730,11 @@ static void ath12k_tm_wmi_event_segmented(struct ath12k_base *ab, u32 cmd_id,
 	ev = tb[WMI_TAG_ARRAY_BYTE];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch ftm msg\n");
-		kfree(tb);
 		return;
 	}
 
 	length = skb->len - TLV_HDR_SIZE;
 	ath12k_tm_process_event(ab, cmd_id, ev, length);
-	kfree(tb);
-	tb = NULL;
 }
 
 static void
@@ -8831,7 +8771,7 @@ static void ath12k_fils_discovery_event(struct ath12k_base *ab,
 	const struct wmi_fils_discovery_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab,
@@ -8843,15 +8783,12 @@ static void ath12k_fils_discovery_event(struct ath12k_base *ab,
 	ev = tb[WMI_TAG_HOST_SWFDA_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch FILS discovery event\n");
-		kfree(tb);
 		return;
 	}
 
 	ath12k_warn(ab,
 		    "FILS discovery frame expected from host for vdev_id: %u, transmission scheduled at %u, next TBTT: %u\n",
 		    ev->vdev_id, ev->fils_tt, ev->tbtt);
-
-	kfree(tb);
 }
 
 static void ath12k_probe_resp_tx_status_event(struct ath12k_base *ab,
@@ -8861,7 +8798,7 @@ static void ath12k_probe_resp_tx_status_event(struct ath12k_base *ab,
 	const struct wmi_probe_resp_tx_status_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab,
@@ -8874,7 +8811,6 @@ static void ath12k_probe_resp_tx_status_event(struct ath12k_base *ab,
 	if (!ev) {
 		ath12k_warn(ab,
 			    "failed to fetch probe response transmission status event");
-		kfree(tb);
 		return;
 	}
 
@@ -8882,8 +8818,6 @@ static void ath12k_probe_resp_tx_status_event(struct ath12k_base *ab,
 		ath12k_warn(ab,
 			    "Probe response transmission failed for vdev_id %u, status %u\n",
 			    ev->vdev_id, ev->tx_status);
-
-	kfree(tb);
 }
 
 static int ath12k_wmi_p2p_noa_event(struct ath12k_base *ab,
@@ -8895,7 +8829,7 @@ static int ath12k_wmi_p2p_noa_event(struct ath12k_base *ab,
 	struct ath12k *ar;
 	int ret, vdev_id;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse P2P NoA TLV: %d\n", ret);
@@ -8905,10 +8839,8 @@ static int ath12k_wmi_p2p_noa_event(struct ath12k_base *ab,
 	ev = tb[WMI_TAG_P2P_NOA_EVENT];
 	noa = tb[WMI_TAG_P2P_NOA_INFO];
 
-	if (!ev || !noa) {
-		ret = -EPROTO;
-		goto out;
-	}
+	if (!ev || !noa)
+		return -EPROTO;
 
 	vdev_id = __le32_to_cpu(ev->vdev_id);
 
@@ -8931,8 +8863,6 @@ static int ath12k_wmi_p2p_noa_event(struct ath12k_base *ab,
 
 unlock:
 	rcu_read_unlock();
-out:
-	kfree(tb);
 	return ret;
 }
 
@@ -8943,7 +8873,7 @@ static void ath12k_rfkill_state_change_event(struct ath12k_base *ab,
 	const void **tb;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -8951,10 +8881,8 @@ static void ath12k_rfkill_state_change_event(struct ath12k_base *ab,
 	}
 
 	ev = tb[WMI_TAG_RFKILL_EVENT];
-	if (!ev) {
-		kfree(tb);
+	if (!ev)
 		return;
-	}
 
 	ath12k_dbg(ab, ATH12K_DBG_MAC,
 		   "wmi tlv rfkill state change gpio %d type %d radio_state %d\n",
@@ -8967,7 +8895,6 @@ static void ath12k_rfkill_state_change_event(struct ath12k_base *ab,
 	spin_unlock_bh(&ab->base_lock);
 
 	queue_work(ab->workqueue, &ab->rfkill_work);
-	kfree(tb);
 }
 
 static void
@@ -8983,7 +8910,7 @@ static void ath12k_wmi_twt_enable_event(struct ath12k_base *ab,
 	const struct wmi_twt_enable_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse wmi twt enable status event tlv: %d\n",
@@ -8994,15 +8921,12 @@ static void ath12k_wmi_twt_enable_event(struct ath12k_base *ab,
 	ev = tb[WMI_TAG_TWT_ENABLE_COMPLETE_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch twt enable wmi event\n");
-		goto exit;
+		return;
 	}
 
 	ath12k_dbg(ab, ATH12K_DBG_MAC, "wmi twt enable event pdev id %u status %u\n",
 		   le32_to_cpu(ev->pdev_id),
 		   le32_to_cpu(ev->status));
-
-exit:
-	kfree(tb);
 }
 
 static void ath12k_wmi_twt_disable_event(struct ath12k_base *ab,
@@ -9012,7 +8936,7 @@ static void ath12k_wmi_twt_disable_event(struct ath12k_base *ab,
 	const struct wmi_twt_disable_event *ev;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse wmi twt disable status event tlv: %d\n",
@@ -9023,15 +8947,12 @@ static void ath12k_wmi_twt_disable_event(struct ath12k_base *ab,
 	ev = tb[WMI_TAG_TWT_DISABLE_COMPLETE_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch twt disable wmi event\n");
-		goto exit;
+		return;
 	}
 
 	ath12k_dbg(ab, ATH12K_DBG_MAC, "wmi twt disable event pdev id %d status %u\n",
 		   le32_to_cpu(ev->pdev_id),
 		   le32_to_cpu(ev->status));
-
-exit:
-	kfree(tb);
 }
 
 static int ath12k_wmi_wow_wakeup_host_parse(struct ath12k_base *ab,
@@ -9104,7 +9025,7 @@ static void ath12k_wmi_gtk_offload_status_event(struct ath12k_base *ab,
 	const void **tb;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
@@ -9114,7 +9035,6 @@ static void ath12k_wmi_gtk_offload_status_event(struct ath12k_base *ab,
 	ev = tb[WMI_TAG_GTK_OFFLOAD_STATUS_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch gtk offload status ev");
-		kfree(tb);
 		return;
 	}
 
@@ -9124,7 +9044,6 @@ static void ath12k_wmi_gtk_offload_status_event(struct ath12k_base *ab,
 		rcu_read_unlock();
 		ath12k_warn(ab, "failed to get arvif for vdev_id:%d\n",
 			    le32_to_cpu(ev->vdev_id));
-		kfree(tb);
 		return;
 	}
 
@@ -9140,8 +9059,6 @@ static void ath12k_wmi_gtk_offload_status_event(struct ath12k_base *ab,
 				   (void *)&replay_ctr_be, GFP_ATOMIC);
 
 	rcu_read_unlock();
-
-	kfree(tb);
 }
 
 static void ath12k_wmi_event_mlo_setup_complete(struct ath12k_base *ab,
@@ -9153,7 +9070,7 @@ static void ath12k_wmi_event_mlo_setup_complete(struct ath12k_base *ab,
 	const void **tb;
 	int ret, i;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse mlo setup complete event tlv: %d\n",
@@ -9164,7 +9081,6 @@ static void ath12k_wmi_event_mlo_setup_complete(struct ath12k_base *ab,
 	ev = tb[WMI_TAG_MLO_SETUP_COMPLETE_EVENT];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch mlo setup complete event\n");
-		kfree(tb);
 		return;
 	}
 
@@ -9183,14 +9099,11 @@ static void ath12k_wmi_event_mlo_setup_complete(struct ath12k_base *ab,
 	if (!ar) {
 		ath12k_warn(ab, "invalid pdev_id %d status %u in setup complete event\n",
 			    ev->pdev_id, ev->status);
-		goto out;
+		return;
 	}
 
 	ar->mlo_setup_status = le32_to_cpu(ev->status);
 	complete(&ar->mlo_setup_done);
-
-out:
-	kfree(tb);
 }
 
 static void ath12k_wmi_event_teardown_complete(struct ath12k_base *ab,
@@ -9200,7 +9113,7 @@ static void ath12k_wmi_event_teardown_complete(struct ath12k_base *ab,
 	const void **tb;
 	int ret;
 
-	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
+	tb = ath12k_wmi_tlv_parse(ab, skb);
 	if (IS_ERR(tb)) {
 		ret = PTR_ERR(tb);
 		ath12k_warn(ab, "failed to parse teardown complete event tlv: %d\n", ret);
@@ -9210,11 +9123,8 @@ static void ath12k_wmi_event_teardown_complete(struct ath12k_base *ab,
 	ev = tb[WMI_TAG_MLO_TEARDOWN_COMPLETE];
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch teardown complete event\n");
-		kfree(tb);
 		return;
 	}
-
-	kfree(tb);
 }
 
 #ifdef CONFIG_ATH12K_DEBUGFS
-- 
2.53.0

Re: [RFC PATCH 1/1] wifi: ath12k: avoid dynamic alloc when parsing wmi tb

[Pablo MARTIN-GOMEZ]

Hello,

On 26/02/2026 17:55, Nicolas Escande wrote:
> On each WMI message received from the hardware, we alloc a temporary array
> of WMI_TAG_MAX entries of type void *. This array is then populated with
> pointers of parsed structs depending on the WMI type, and then freed. This
> alloc can fail when memory pressure in the system is high enough.
> 
> Given the fact that it is scheduled in softirq with the system_bh_wq, we
> should not be able to parse more than one WMI message per CPU at any time
> 
> So instead lets move to a per cpu allocated array, stored in the struct
> ath12k_base, that is reused accros calls. The ath12k_wmi_tlv_parse_alloc()
> is also renamed into / merged with ath12k_wmi_tlv_parse() as it no longer
> allocs memory but returns the existing per-cpu one.
> 
> Signed-off-by: Nicolas Escande <nico.escande@gmail.com>
> ---
>  drivers/net/wireless/ath/ath12k/core.c |   7 +
>  drivers/net/wireless/ath/ath12k/core.h |   2 +
>  drivers/net/wireless/ath/ath12k/wmi.c  | 170 ++++++-------------------
>  3 files changed, 49 insertions(+), 130 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
> index 9d6c50a94e64..961c9df69aa1 100644
> --- a/drivers/net/wireless/ath/ath12k/core.c
> +++ b/drivers/net/wireless/ath/ath12k/core.c
> @@ -2237,6 +2237,7 @@ void ath12k_core_free(struct ath12k_base *ab)
>  	timer_delete_sync(&ab->rx_replenish_retry);
>  	destroy_workqueue(ab->workqueue_aux);
>  	destroy_workqueue(ab->workqueue);
> +	free_percpu(ab->wmi_tb);
>  	kfree(ab);
>  }
>  
> @@ -2249,6 +2250,11 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev, size_t priv_size,
>  	if (!ab)
>  		return NULL;
>  
> +	ab->wmi_tb = __alloc_percpu(WMI_TAG_MAX * sizeof(void *),
> +				    __alignof__(void *));
> +	if (!ab->wmi_tb)
> +		goto err_sc_free;
If `!ab->wmi_tb`, you're going to `free_percpu(ab->wmi_tb)`; it works but it's not super pretty.
> +
>  	init_completion(&ab->driver_recovery);
>  
>  	ab->workqueue = create_singlethread_workqueue("ath12k_wq");
> @@ -2296,6 +2302,7 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev, size_t priv_size,
>  err_free_wq:
>  	destroy_workqueue(ab->workqueue);
>  err_sc_free:
> +	free_percpu(ab->wmi_tb);
>  	kfree(ab);
>  	return NULL;
>  }
[...]
> @@ -6795,11 +6749,9 @@ ath12k_pull_pdev_temp_ev(struct ath12k_base *ab, struct sk_buff *skb,
>  	ev = tb[WMI_TAG_PDEV_TEMPERATURE_EVENT];
>  	if (!ev) {
>  		ath12k_warn(ab, "failed to fetch pdev temp ev");
> -		kfree(tb);
>  		return -EPROTO;
>  	}
>  
> -	kfree(tb);
>  	return 0;
>  }
You're missing a change on `ath12k_reg_11d_new_cc_event` (was added by 591de41d7008585f2e7c35dbcf5922fcb4d79e39)

[...]

Best regards,
Pablo MG

Re: [RFC PATCH 1/1] wifi: ath12k: avoid dynamic alloc when parsing wmi tb

[Nicolas Escande]

On Thu Feb 26, 2026 at 8:48 PM CET, Pablo MARTIN-GOMEZ wrote:
[...]
>> @@ -2249,6 +2250,11 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev, size_t priv_size,
>>  	if (!ab)
>>  		return NULL;
>>  
>> +	ab->wmi_tb = __alloc_percpu(WMI_TAG_MAX * sizeof(void *),
>> +				    __alignof__(void *));
>> +	if (!ab->wmi_tb)
>> +		goto err_sc_free;
> If `!ab->wmi_tb`, you're going to `free_percpu(ab->wmi_tb)`; it works but it's not super pretty.
Yes it's by design, we do not need to null check before free() and it avoids
adding a new label that would cause more code chrun
>> +
>>  	init_completion(&ab->driver_recovery);
>>  
>>  	ab->workqueue = create_singlethread_workqueue("ath12k_wq");
>> @@ -2296,6 +2302,7 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev, size_t priv_size,
>>  err_free_wq:
>>  	destroy_workqueue(ab->workqueue);
>>  err_sc_free:
>> +	free_percpu(ab->wmi_tb);
>>  	kfree(ab);
>>  	return NULL;
>>  }
> [...]
>> @@ -6795,11 +6749,9 @@ ath12k_pull_pdev_temp_ev(struct ath12k_base *ab, struct sk_buff *skb,
>>  	ev = tb[WMI_TAG_PDEV_TEMPERATURE_EVENT];
>>  	if (!ev) {
>>  		ath12k_warn(ab, "failed to fetch pdev temp ev");
>> -		kfree(tb);
>>  		return -EPROTO;
>>  	}
>>  
>> -	kfree(tb);
>>  	return 0;
>>  }
> You're missing a change on `ath12k_reg_11d_new_cc_event` (was added by 591de41d7008585f2e7c35dbcf5922fcb4d79e39)
Indeed, this was ported from an older kernel, I'll fix before non RFC submission
if there is interrest from QCA around this feature.
> [...]
>
> Best regards,
> Pablo MG

Re: [RFC PATCH 1/1] wifi: ath12k: avoid dynamic alloc when parsing wmi tb

[Baochen Qiang]

On 2/27/2026 12:55 AM, Nicolas Escande wrote:
> On each WMI message received from the hardware, we alloc a temporary array
> of WMI_TAG_MAX entries of type void *. This array is then populated with
> pointers of parsed structs depending on the WMI type, and then freed. This
> alloc can fail when memory pressure in the system is high enough.
> 
> Given the fact that it is scheduled in softirq with the system_bh_wq, we
> should not be able to parse more than one WMI message per CPU at any time
> 
> So instead lets move to a per cpu allocated array, stored in the struct
> ath12k_base, that is reused accros calls. The ath12k_wmi_tlv_parse_alloc()
> is also renamed into / merged with ath12k_wmi_tlv_parse() as it no longer
> allocs memory but returns the existing per-cpu one.
> 
> Signed-off-by: Nicolas Escande <nico.escande@gmail.com>
> ---
>  drivers/net/wireless/ath/ath12k/core.c |   7 +
>  drivers/net/wireless/ath/ath12k/core.h |   2 +
>  drivers/net/wireless/ath/ath12k/wmi.c  | 170 ++++++-------------------
>  3 files changed, 49 insertions(+), 130 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
> index 9d6c50a94e64..961c9df69aa1 100644
> --- a/drivers/net/wireless/ath/ath12k/core.c
> +++ b/drivers/net/wireless/ath/ath12k/core.c
> @@ -2237,6 +2237,7 @@ void ath12k_core_free(struct ath12k_base *ab)
>  	timer_delete_sync(&ab->rx_replenish_retry);
>  	destroy_workqueue(ab->workqueue_aux);
>  	destroy_workqueue(ab->workqueue);
> +	free_percpu(ab->wmi_tb);
>  	kfree(ab);
>  }
>  
> @@ -2249,6 +2250,11 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev, size_t priv_size,
>  	if (!ab)
>  		return NULL;
>  
> +	ab->wmi_tb = __alloc_percpu(WMI_TAG_MAX * sizeof(void *),
> +				    __alignof__(void *));
> +	if (!ab->wmi_tb)
> +		goto err_sc_free;
> +
>  	init_completion(&ab->driver_recovery);
>  
>  	ab->workqueue = create_singlethread_workqueue("ath12k_wq");
> @@ -2296,6 +2302,7 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev, size_t priv_size,
>  err_free_wq:
>  	destroy_workqueue(ab->workqueue);
>  err_sc_free:
> +	free_percpu(ab->wmi_tb);
>  	kfree(ab);
>  	return NULL;
>  }
> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
> index 990934ec92fc..378573a100e8 100644
> --- a/drivers/net/wireless/ath/ath12k/core.h
> +++ b/drivers/net/wireless/ath/ath12k/core.h
> @@ -19,6 +19,7 @@
>  #include <linux/average.h>
>  #include <linux/of.h>
>  #include <linux/rhashtable.h>
> +#include <linux/percpu.h>
>  #include "qmi.h"
>  #include "htc.h"
>  #include "wmi.h"
> @@ -934,6 +935,7 @@ struct ath12k_base {
>  	struct device *dev;
>  	struct ath12k_qmi qmi;
>  	struct ath12k_wmi_base wmi_ab;
> +	void __percpu *wmi_tb;

instead of allocating it per device, how about making it global and allocating once when
loading driver. This way we may save some memory in case where more than one devices get
probed?

Re: [RFC PATCH 1/1] wifi: ath12k: avoid dynamic alloc when parsing wmi tb

[Nicolas Escande]

On Fri Feb 27, 2026 at 4:18 AM CET, Baochen Qiang wrote:
[...]
>> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
>> index 990934ec92fc..378573a100e8 100644
>> --- a/drivers/net/wireless/ath/ath12k/core.h
>> +++ b/drivers/net/wireless/ath/ath12k/core.h
>> @@ -19,6 +19,7 @@
>>  #include <linux/average.h>
>>  #include <linux/of.h>
>>  #include <linux/rhashtable.h>
>> +#include <linux/percpu.h>
>>  #include "qmi.h"
>>  #include "htc.h"
>>  #include "wmi.h"
>> @@ -934,6 +935,7 @@ struct ath12k_base {
>>  	struct device *dev;
>>  	struct ath12k_qmi qmi;
>>  	struct ath12k_wmi_base wmi_ab;
>> +	void __percpu *wmi_tb;
>
> instead of allocating it per device, how about making it global and allocating once when
> loading driver. This way we may save some memory in case where more than one devices get
> probed?
Yes I can do that.

I'll wait a few days to see if there's more feedback from you guys and I post
something more polished then. And I'll probably post something for ATH11K, it
seems to have the same pattern.

Thx for the review

Re: [RFC PATCH 1/1] wifi: ath12k: avoid dynamic alloc when parsing wmi tb

[Rameshkumar Sundaram]

On 2/26/2026 10:25 PM, Nicolas Escande wrote:
> On each WMI message received from the hardware, we alloc a temporary array
> of WMI_TAG_MAX entries of type void *. This array is then populated with
> pointers of parsed structs depending on the WMI type, and then freed. This
> alloc can fail when memory pressure in the system is high enough.
> 
> Given the fact that it is scheduled in softirq with the system_bh_wq, we
> should not be able to parse more than one WMI message per CPU at any time
> 
> So instead lets move to a per cpu allocated array, stored in the struct
> ath12k_base, that is reused accros calls. The ath12k_wmi_tlv_parse_alloc()
> is also renamed into / merged with ath12k_wmi_tlv_parse() as it no longer
> allocs memory but returns the existing per-cpu one.
> 
> Signed-off-by: Nicolas Escande <nico.escande@gmail.com>
> ---
>   drivers/net/wireless/ath/ath12k/core.c |   7 +
>   drivers/net/wireless/ath/ath12k/core.h |   2 +
>   drivers/net/wireless/ath/ath12k/wmi.c  | 170 ++++++-------------------
>   3 files changed, 49 insertions(+), 130 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
> index 9d6c50a94e64..961c9df69aa1 100644
> --- a/drivers/net/wireless/ath/ath12k/core.c
> +++ b/drivers/net/wireless/ath/ath12k/core.c
> @@ -2237,6 +2237,7 @@ void ath12k_core_free(struct ath12k_base *ab)
>   	timer_delete_sync(&ab->rx_replenish_retry);
>   	destroy_workqueue(ab->workqueue_aux);
>   	destroy_workqueue(ab->workqueue);
> +	free_percpu(ab->wmi_tb);
>   	kfree(ab);
>   }
>   
> @@ -2249,6 +2250,11 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev, size_t priv_size,
>   	if (!ab)
>   		return NULL;
>   
> +	ab->wmi_tb = __alloc_percpu(WMI_TAG_MAX * sizeof(void *),
> +				    __alignof__(void *));
> +	if (!ab->wmi_tb)
> +		goto err_sc_free;
> +


we could move this allocation after workqueue_aux alloc and avoid 
free_percpu(ab->wmi_tb); in the label

>   	init_completion(&ab->driver_recovery);
>   
>   	ab->workqueue = create_singlethread_workqueue("ath12k_wq");
> @@ -2296,6 +2302,7 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev, size_t priv_size,
>   err_free_wq:
>   	destroy_workqueue(ab->workqueue);
>   err_sc_free:
> +	free_percpu(ab->wmi_tb);
>   	kfree(ab);
>   	return NULL;
>   }
> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
> index 990934ec92fc..378573a100e8 100644
> --- a/drivers/net/wireless/ath/ath12k/core.h
> +++ b/drivers/net/wireless/ath/ath12k/core.h
> @@ -19,6 +19,7 @@
>   #include <linux/average.h>
>   #include <linux/of.h>
>   #include <linux/rhashtable.h>
> +#include <linux/percpu.h>
>   #include "qmi.h"
>   #include "htc.h"
>   #include "wmi.h"
> @@ -934,6 +935,7 @@ struct ath12k_base {
>   	struct device *dev;
>   	struct ath12k_qmi qmi;
>   	struct ath12k_wmi_base wmi_ab;
> +	void __percpu *wmi_tb;
>   	struct completion fw_ready;
>   	u8 device_id;
>   	int num_radios;
> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
> index 404f031a3c87..9ccc10c07c21 100644
> --- a/drivers/net/wireless/ath/ath12k/wmi.c
> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
> @@ -289,29 +289,19 @@ static int ath12k_wmi_tlv_iter_parse(struct ath12k_base *ab, u16 tag, u16 len,
>   	return 0;
>   }
>   
> -static int ath12k_wmi_tlv_parse(struct ath12k_base *ar, const void **tb,
> -				const void *ptr, size_t len)
> -{
> -	return ath12k_wmi_tlv_iter(ar, ptr, len, ath12k_wmi_tlv_iter_parse,
> -				   (void *)tb);
> -}
> -
>   static const void **
> -ath12k_wmi_tlv_parse_alloc(struct ath12k_base *ab,
> -			   struct sk_buff *skb, gfp_t gfp)
> +ath12k_wmi_tlv_parse(struct ath12k_base *ab, struct sk_buff *skb)
>   {
>   	const void **tb;
>   	int ret;
>   
> -	tb = kcalloc(WMI_TAG_MAX, sizeof(*tb), gfp);
> -	if (!tb)
> -		return ERR_PTR(-ENOMEM);
> +	tb = this_cpu_ptr(ab->wmi_tb);
> +	memset(tb, 0, WMI_TAG_MAX * sizeof(void *));

prefer sizeof(*tb) over sizeof(void *)

>   
> -	ret = ath12k_wmi_tlv_parse(ab, tb, skb->data, skb->len);
> -	if (ret) {
> -		kfree(tb);
> +	ret = ath12k_wmi_tlv_iter(ab, skb->data, skb->len,
> +				  ath12k_wmi_tlv_iter_parse, (void *)tb);
> +	if (ret)
>   		return ERR_PTR(ret);
> -	}
>   
>   	return tb;
>   }
> @@ -5714,7 +5704,7 @@ static int ath12k_pull_vdev_start_resp_tlv(struct ath12k_base *ab, struct sk_buf
>   	const struct wmi_vdev_start_resp_event *ev;
>   	int ret;
>   
> -	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
> +	tb = ath12k_wmi_tlv_parse(ab, skb);
>   	if (IS_ERR(tb)) {
>   		ret = PTR_ERR(tb);
>   		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
> @@ -5724,13 +5714,11 @@ static int ath12k_pull_vdev_start_resp_tlv(struct ath12k_base *ab, struct sk_buf
>   	ev = tb[WMI_TAG_VDEV_START_RESPONSE_EVENT];
>   	if (!ev) {
>   		ath12k_warn(ab, "failed to fetch vdev start resp ev");
> -		kfree(tb);
>   		return -EPROTO;
>   	}
>   
>   	*vdev_rsp = *ev;
>   
> -	kfree(tb);
>   	return 0;
>   }
>   
> @@ -5809,7 +5797,7 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
>   
>   	ath12k_dbg(ab, ATH12K_DBG_WMI, "processing regulatory ext channel list\n");
>   
> -	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
> +	tb = ath12k_wmi_tlv_parse(ab, skb);
>   	if (IS_ERR(tb)) {
>   		ret = PTR_ERR(tb);
>   		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
> @@ -5819,7 +5807,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
>   	ev = tb[WMI_TAG_REG_CHAN_LIST_CC_EXT_EVENT];
>   	if (!ev) {
>   		ath12k_warn(ab, "failed to fetch reg chan list ext update ev\n");
> -		kfree(tb);
>   		return -EPROTO;
>   	}
>   
> @@ -5849,7 +5836,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
>   	if (num_2g_reg_rules > MAX_REG_RULES || num_5g_reg_rules > MAX_REG_RULES) {
>   		ath12k_warn(ab, "Num reg rules for 2G/5G exceeds max limit (num_2g_reg_rules: %d num_5g_reg_rules: %d max_rules: %d)\n",
>   			    num_2g_reg_rules, num_5g_reg_rules, MAX_REG_RULES);
> -		kfree(tb);
>   		return -EINVAL;
>   	}
>   
> @@ -5859,7 +5845,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
>   		if (num_6g_reg_rules_ap[i] > MAX_6GHZ_REG_RULES) {
>   			ath12k_warn(ab, "Num 6G reg rules for AP mode(%d) exceeds max limit (num_6g_reg_rules_ap: %d, max_rules: %d)\n",
>   				    i, num_6g_reg_rules_ap[i], MAX_6GHZ_REG_RULES);
> -			kfree(tb);
>   			return -EINVAL;
>   		}
>   
> @@ -5884,14 +5869,12 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
>   		    num_6g_reg_rules_cl[WMI_REG_VLP_AP][i] >  MAX_6GHZ_REG_RULES) {
>   			ath12k_warn(ab, "Num 6g client reg rules exceeds max limit, for client(type: %d)\n",
>   				    i);
> -			kfree(tb);
>   			return -EINVAL;
>   		}
>   	}
>   
>   	if (!total_reg_rules) {
>   		ath12k_warn(ab, "No reg rules available\n");
> -		kfree(tb);
>   		return -EINVAL;
>   	}
>   
> @@ -5993,7 +5976,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
>   						      ext_wmi_reg_rule);
>   
>   		if (!reg_info->reg_rules_2g_ptr) {
> -			kfree(tb);
>   			ath12k_warn(ab, "Unable to Allocate memory for 2g rules\n");
>   			return -ENOMEM;
>   		}
> @@ -6027,7 +6009,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
>   						      ext_wmi_reg_rule);
>   
>   		if (!reg_info->reg_rules_5g_ptr) {
> -			kfree(tb);
>   			ath12k_warn(ab, "Unable to Allocate memory for 5g rules\n");
>   			return -ENOMEM;
>   		}
> @@ -6046,7 +6027,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
>   						      ext_wmi_reg_rule);
>   
>   		if (!reg_info->reg_rules_6g_ap_ptr[i]) {
> -			kfree(tb);
>   			ath12k_warn(ab, "Unable to Allocate memory for 6g ap rules\n");
>   			return -ENOMEM;
>   		}
> @@ -6061,7 +6041,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
>   							      ext_wmi_reg_rule);
>   
>   			if (!reg_info->reg_rules_6g_client_ptr[j][i]) {
> -				kfree(tb);
>   				ath12k_warn(ab, "Unable to Allocate memory for 6g client rules\n");
>   				return -ENOMEM;
>   			}
> @@ -6096,7 +6075,6 @@ static int ath12k_pull_reg_chan_list_ext_update_ev(struct ath12k_base *ab,
>   
>   	ath12k_dbg(ab, ATH12K_DBG_WMI, "processed regulatory ext channel list\n");
>   
> -	kfree(tb);
>   	return 0;
>   }
>   
> @@ -6107,7 +6085,7 @@ static int ath12k_pull_peer_del_resp_ev(struct ath12k_base *ab, struct sk_buff *
>   	const struct wmi_peer_delete_resp_event *ev;
>   	int ret;
>   
> -	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
> +	tb = ath12k_wmi_tlv_parse(ab, skb);
>   	if (IS_ERR(tb)) {
>   		ret = PTR_ERR(tb);
>   		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
> @@ -6117,7 +6095,6 @@ static int ath12k_pull_peer_del_resp_ev(struct ath12k_base *ab, struct sk_buff *
>   	ev = tb[WMI_TAG_PEER_DELETE_RESP_EVENT];
>   	if (!ev) {
>   		ath12k_warn(ab, "failed to fetch peer delete resp ev");
> -		kfree(tb);
>   		return -EPROTO;
>   	}
>   
> @@ -6127,7 +6104,6 @@ static int ath12k_pull_peer_del_resp_ev(struct ath12k_base *ab, struct sk_buff *
>   	ether_addr_copy(peer_del_resp->peer_macaddr.addr,
>   			ev->peer_macaddr.addr);
>   
> -	kfree(tb);
>   	return 0;
>   }
>   
> @@ -6139,7 +6115,7 @@ static int ath12k_pull_vdev_del_resp_ev(struct ath12k_base *ab,
>   	const struct wmi_vdev_delete_resp_event *ev;
>   	int ret;
>   
> -	tb = ath12k_wmi_tlv_parse_alloc(ab, skb, GFP_ATOMIC);
> +	tb = ath12k_wmi_tlv_parse(ab, skb);
>   	if (IS_ERR(tb)) {
>   		ret = PTR_ERR(tb);
>   		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
> @@ -6149,13 +6125,11 @@ static int ath12k_pull_vdev_del_resp_ev(struct ath12k_base *ab,
>   	ev = tb[WMI_TAG_VDEV_DELETE_RESP_EVENT];
>   	if (!ev) {
>   		ath12k_warn(ab, "failed to fetch vdev delete resp ev");
> -		kfree(tb);
>   		return -EPROTO;
>   	}
>   
>   	*vdev_id = le32_to_cpu(ev->vdev_id);
>   
> -	kfree(tb);
>   	return 0;
>   }
>   


and one more case to be handled,
ath12k_wmi_obss_color_collision_event()
	const void **tb __free(kfree) = ath12k_wmi_tlv_parse_alloc(ab, skb, 
GFP_ATOMIC);


Apart from this, the approach looks like a reasonable fix for the issue.

Another possible direction could be to define WMI event‑specific 
structures (or possibly a union of structs) and then leverage 
ath12k_wmi_tlv_parse() to extract only the required tags in a typed manner.

That said, such an approach would require a fair amount of refactoring, 
so sticking with the per‑CPU scratch buffer for now seems pragmatic.
Moving towards a typed‑parse model incrementally could be something we 
consider over time.


--
Ramesh

Re: [RFC PATCH 1/1] wifi: ath12k: avoid dynamic alloc when parsing wmi tb

[Nicolas Escande]

On Tue Mar 3, 2026 at 10:53 AM CET, Rameshkumar Sundaram wrote:
[...]
>>   
>> @@ -2249,6 +2250,11 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev, size_t priv_size,
>>   	if (!ab)
>>   		return NULL;
>>   
>> +	ab->wmi_tb = __alloc_percpu(WMI_TAG_MAX * sizeof(void *),
>> +				    __alignof__(void *));
>> +	if (!ab->wmi_tb)
>> +		goto err_sc_free;
>> +
>
>
> we could move this allocation after workqueue_aux alloc and avoid 
> free_percpu(ab->wmi_tb); in the label
>

I'll look into it.

>>   	init_completion(&ab->driver_recovery);
>>   
>>   	ab->workqueue = create_singlethread_workqueue("ath12k_wq");

[...]

>> -	tb = kcalloc(WMI_TAG_MAX, sizeof(*tb), gfp);
>> -	if (!tb)
>> -		return ERR_PTR(-ENOMEM);
>> +	tb = this_cpu_ptr(ab->wmi_tb);
>> +	memset(tb, 0, WMI_TAG_MAX * sizeof(void *));
>
> prefer sizeof(*tb) over sizeof(void *)

sure

>
>>   
>> -	ret = ath12k_wmi_tlv_parse(ab, tb, skb->data, skb->len);
>> -	if (ret) {
>> -		kfree(tb);
>> +	ret = ath12k_wmi_tlv_iter(ab, skb->data, skb->len,
>> +				  ath12k_wmi_tlv_iter_parse, (void *)tb);
>> +	if (ret)
>>   		return ERR_PTR(ret);
>> -	}
>>   
>>   	return tb;
>>   }

[...]

>
> and one more case to be handled,
> ath12k_wmi_obss_color_collision_event()
> 	const void **tb __free(kfree) = ath12k_wmi_tlv_parse_alloc(ab, skb, 
> GFP_ATOMIC);
>

Sure as I said, this was whipped up from an older kernel to see if there was
interrest. I'll fix all usages for the proper submission 

>
> Apart from this, the approach looks like a reasonable fix for the issue.
>

Good

> Another possible direction could be to define WMI event‑specific 
> structures (or possibly a union of structs) and then leverage 
> ath12k_wmi_tlv_parse() to extract only the required tags in a typed manner.
>

I looked briefly into this as it seems it was in fact done that way in ath10k.
But as in some rare occasion there are more than one single struct that gets
parsed from one wmi message, I guess thats the reason it was changed to this
pattern in ath11k.

> That said, such an approach would require a fair amount of refactoring, 
> so sticking with the per‑CPU scratch buffer for now seems pragmatic.
> Moving towards a typed‑parse model incrementally could be something we 
> consider over time.
>

Indeed you would gain back type safety but this would mean a fair amount of code
change.

>
> --
> Ramesh