Netdev List
 help / color / mirror / Atom feed
* [PATCH iwl-net v2] ice: add missing xa_destroy for sched_node_ids
@ 2026-07-06 23:31 Jacob Keller
  2026-07-07 13:31 ` [Intel-wired-lan] " Loktionov, Aleksandr
  0 siblings, 1 reply; 2+ messages in thread
From: Jacob Keller @ 2026-07-06 23:31 UTC (permalink / raw)
  To: Tony Nguyen, Przemek Kitszel; +Cc: intel-wired-lan, netdev, Jacob Keller

Commit 16dfa49406bc ("ice: Introduce new parameters in ice_sched_node")
added a sched_node_ids xarray to the port info structure, but never called
xa_destroy on it.

Since xarrays can allocate internal memory, this can result in a memory
leak even if every element in the xarray has been removed.

The xarray is currently embedded in the port_info structure. This appears
to have been done because its use is within functions that take the
port_info as a primary argument.

However, this complicates managing the lifecycle of the field. The
port_info structure is allocated in ice_init_hw() using devm, and it is
not released until the devm cleanup when the driver is unloaded.

The ice_init_hw() function is called in many places, including devlink
reload, and possibly during DDP load after updating the Tx scheduler
layout.

Adding a call of xa_destroy to the ice_deinit_hw() causes Sashiko to raise
multiple concerns due to potential ordering issues and possible ways that
port_info could be a dangling reference.

To handle this, move the sched_node_ids out of port_info and into the hw
structure. All users of the array already have a pointer to hw anyways, and
there is only one sched_node_ids per adapter. While here, remove the overly
verbose comment explaining the nature of the sched_node_ids xarray.

Add the missing xa_destroy to the cleanup path and to ice_deinit_hw(),
ensuring that we properly release the xarray memory.

This was caught by Sashiko during development of unrelated code.

Fixes: 16dfa49406bc ("ice: Introduce new parameters in ice_sched_node")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Changes in v2:
- Move sched_node_ids out of port_into into hw.
- Link to v1: https://patch.msgid.link/20260514-jk-fix-missing-xa-destroy-v1-1-de437bf52347@intel.com
---
 drivers/net/ethernet/intel/ice/ice_type.h   | 2 +-
 drivers/net/ethernet/intel/ice/ice_common.c | 9 ++++++---
 drivers/net/ethernet/intel/ice/ice_sched.c  | 4 ++--
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index d9a5c1aae7c2..cf147a212707 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -765,7 +765,6 @@ struct ice_port_info {
 	/* List contain profile ID(s) and other params per layer */
 	struct list_head rl_prof_list[ICE_AQC_TOPO_MAX_LEVEL_NUM];
 	struct ice_qos_cfg qos_cfg;
-	struct xarray sched_node_ids;
 	u8 is_vf:1;
 	u8 is_custom_tx_enabled:1;
 };
@@ -930,6 +929,7 @@ struct ice_hw {
 	u8 sw_entry_point_layer;
 	u16 max_children[ICE_AQC_TOPO_MAX_LEVEL_NUM];
 	struct list_head agg_list;	/* lists all aggregator */
+	struct xarray sched_node_ids;
 
 	struct ice_vsi_ctx *vsi_ctx[ICE_MAX_VSI];
 	u8 evb_veb;		/* true for VEB, false for VEPA */
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index ef1ce106f81b..04633103e3e6 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -1051,14 +1051,13 @@ int ice_init_hw(struct ice_hw *hw)
 
 	hw->evb_veb = true;
 
-	/* init xarray for identifying scheduling nodes uniquely */
-	xa_init_flags(&hw->port_info->sched_node_ids, XA_FLAGS_ALLOC);
+	xa_init_flags(&hw->sched_node_ids, XA_FLAGS_ALLOC);
 
 	/* Query the allocated resources for Tx scheduler */
 	status = ice_sched_query_res_alloc(hw);
 	if (status) {
 		ice_debug(hw, ICE_DBG_SCHED, "Failed to get scheduler allocated resources\n");
-		goto err_unroll_alloc;
+		goto err_unroll_xarray;
 	}
 	ice_sched_get_psm_clk_freq(hw);
 
@@ -1146,6 +1145,8 @@ int ice_init_hw(struct ice_hw *hw)
 	ice_cleanup_fltr_mgmt_struct(hw);
 err_unroll_sched:
 	ice_sched_cleanup_all(hw);
+err_unroll_xarray:
+	xa_destroy(&hw->sched_node_ids);
 err_unroll_alloc:
 	devm_kfree(ice_hw_to_dev(hw), hw->port_info);
 err_unroll_cqinit:
@@ -1186,6 +1187,8 @@ void ice_deinit_hw(struct ice_hw *hw)
 
 	/* Clear VSI contexts if not already cleared */
 	ice_clear_all_vsi_ctx(hw);
+
+	xa_destroy(&hw->sched_node_ids);
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/ice/ice_sched.c b/drivers/net/ethernet/intel/ice/ice_sched.c
index fff0c1afdb41..ffa18d86729a 100644
--- a/drivers/net/ethernet/intel/ice/ice_sched.c
+++ b/drivers/net/ethernet/intel/ice/ice_sched.c
@@ -371,7 +371,7 @@ void ice_free_sched_node(struct ice_port_info *pi, struct ice_sched_node *node)
 
 	devm_kfree(ice_hw_to_dev(hw), node->children);
 	kfree(node->name);
-	xa_erase(&pi->sched_node_ids, node->id);
+	xa_erase(&hw->sched_node_ids, node->id);
 	devm_kfree(ice_hw_to_dev(hw), node);
 }
 
@@ -977,7 +977,7 @@ ice_sched_add_elems(struct ice_port_info *pi, struct ice_sched_node *tc_node,
 		if (!new_node->name)
 			return -ENOMEM;
 
-		status = xa_alloc(&pi->sched_node_ids, &new_node->id, NULL, XA_LIMIT(0, UINT_MAX),
+		status = xa_alloc(&hw->sched_node_ids, &new_node->id, NULL, XA_LIMIT(0, UINT_MAX),
 				  GFP_KERNEL);
 		if (status) {
 			ice_debug(hw, ICE_DBG_SCHED, "xa_alloc failed for sched node status =%d\n",

---
base-commit: 9e05e91a9a847ed57926414bd7c2c5e54d6c56c6
change-id: 20260514-jk-fix-missing-xa-destroy-d3f90f3711be

Best regards,
--  
Jacob Keller <jacob.e.keller@intel.com>


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* RE: [Intel-wired-lan] [PATCH iwl-net v2] ice: add missing xa_destroy for sched_node_ids
  2026-07-06 23:31 [PATCH iwl-net v2] ice: add missing xa_destroy for sched_node_ids Jacob Keller
@ 2026-07-07 13:31 ` Loktionov, Aleksandr
  0 siblings, 0 replies; 2+ messages in thread
From: Loktionov, Aleksandr @ 2026-07-07 13:31 UTC (permalink / raw)
  To: Keller, Jacob E, Nguyen, Anthony L, Kitszel, Przemyslaw
  Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	Keller, Jacob E



> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Jacob Keller
> Sent: Tuesday, July 7, 2026 1:31 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Keller,
> Jacob E <jacob.e.keller@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-net v2] ice: add missing
> xa_destroy for sched_node_ids
> 
> Commit 16dfa49406bc ("ice: Introduce new parameters in
> ice_sched_node") added a sched_node_ids xarray to the port info
> structure, but never called xa_destroy on it.
> 
> Since xarrays can allocate internal memory, this can result in a
> memory leak even if every element in the xarray has been removed.
> 
> The xarray is currently embedded in the port_info structure. This
> appears to have been done because its use is within functions that
> take the port_info as a primary argument.
> 
> However, this complicates managing the lifecycle of the field. The
> port_info structure is allocated in ice_init_hw() using devm, and it
> is not released until the devm cleanup when the driver is unloaded.
> 
> The ice_init_hw() function is called in many places, including devlink
> reload, and possibly during DDP load after updating the Tx scheduler
> layout.
> 
> Adding a call of xa_destroy to the ice_deinit_hw() causes Sashiko to
> raise multiple concerns due to potential ordering issues and possible
> ways that port_info could be a dangling reference.
> 
> To handle this, move the sched_node_ids out of port_info and into the
> hw structure. All users of the array already have a pointer to hw
> anyways, and there is only one sched_node_ids per adapter. While here,
> remove the overly verbose comment explaining the nature of the
> sched_node_ids xarray.
> 
> Add the missing xa_destroy to the cleanup path and to ice_deinit_hw(),
> ensuring that we properly release the xarray memory.
> 
> This was caught by Sashiko during development of unrelated code.
> 
> Fixes: 16dfa49406bc ("ice: Introduce new parameters in
> ice_sched_node")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

> ---
> Changes in v2:
> - Move sched_node_ids out of port_into into hw.
> - Link to v1: https://patch.msgid.link/20260514-jk-fix-missing-xa-
> destroy-v1-1-de437bf52347@intel.com
> ---
>  drivers/net/ethernet/intel/ice/ice_type.h   | 2 +-
>  drivers/net/ethernet/intel/ice/ice_common.c | 9 ++++++---
> drivers/net/ethernet/intel/ice/ice_sched.c  | 4 ++--
>  3 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_type.h
> b/drivers/net/ethernet/intel/ice/ice_type.h
> index d9a5c1aae7c2..cf147a212707 100644
> --- a/drivers/net/ethernet/intel/ice/ice_type.h
> +++ b/drivers/net/ethernet/intel/ice/ice_type.h
> @@ -765,7 +765,6 @@ struct ice_port_info {
>  	/* List contain profile ID(s) and other params per layer */
>  	struct list_head rl_prof_list[ICE_AQC_TOPO_MAX_LEVEL_NUM];
>  	struct ice_qos_cfg qos_cfg;

...

>  				  GFP_KERNEL);
>  		if (status) {
>  			ice_debug(hw, ICE_DBG_SCHED, "xa_alloc failed for
> sched node status =%d\n",
> 
> ---
> base-commit: 9e05e91a9a847ed57926414bd7c2c5e54d6c56c6
> change-id: 20260514-jk-fix-missing-xa-destroy-d3f90f3711be
> 
> Best regards,
> --
> Jacob Keller <jacob.e.keller@intel.com>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-07 13:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-06 23:31 [PATCH iwl-net v2] ice: add missing xa_destroy for sched_node_ids Jacob Keller
2026-07-07 13:31 ` [Intel-wired-lan] " Loktionov, Aleksandr

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox