ice_vsi_realloc_stat_arrays() resized the ring stats arrays in place with krealloc_array(), sizing them from vsi->req_txq/req_rxq. That is not what ice_vsi_set_num_qs() computes later in ice_vsi_cfg_def(), so after a rebuild the arrays could end up shorter than vsi->alloc_txq / vsi->alloc_rxq, and ice_vsi_alloc_ring_stats() then walked past their end. Requesting fewer queues than the PF pool can hand out was enough to trigger it.
Replace it with ice_vsi_resize_stat_arrays(), which allocates a fresh struct ice_vsi_stats instead, sized with ice_vsi_get_num_qs() and the queues ice_vsi_decfg() is about to return to the PF pool, which is exactly what ice_vsi_set_num_qs() will compute once they are back there. Copy the surviving entry pointers over and install the new structure, all before ice_vsi_decfg() runs. The entries that did not fit are freed together with the old container, via __ice_vsi_free_stats() with @free_entries set to false. Doing the allocation up front also means the failure path is a plain unlock and return, with the VSI still fully configured, rather than a half-torn-down VSI to unwind. While at it, skip the stats handling for ICE_VSI_CHNL, which has no entry in pf->vsi_stats[] to begin with. Signed-off-by: Przemek Kitszel <[email protected]> --- this is an actual fix/improvement, that removes some of the tech debt, but there is no Fixes tag, as this was not directly caused by an user observable bug, rather as a pre-work / ai-review resolution for the next commit --- drivers/net/ethernet/intel/ice/ice_lib.c | 118 +++++++++++++---------- 1 file changed, 69 insertions(+), 49 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c index 998b9eb6e9fe..0cd54af40c37 100644 --- a/drivers/net/ethernet/intel/ice/ice_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_lib.c @@ -2953,6 +2953,51 @@ ice_vsi_rebuild_get_coalesce(struct ice_vsi *vsi, return vsi->num_q_vectors; } +static void ice_vsi_free_unused_stat_arrays(struct ice_vsi_stats *vsi_stat, + struct ice_vsi_stats *new_vsi_stat) +{ + int new_txq = new_vsi_stat->tx_ring_stats_len; + int new_rxq = new_vsi_stat->rx_ring_stats_len; + int prev_txq = vsi_stat->tx_ring_stats_len; + int prev_rxq = vsi_stat->rx_ring_stats_len; + + for (int i = new_txq; i < prev_txq; i++) { + if (vsi_stat->tx_ring_stats[i]) { + kfree_rcu(vsi_stat->tx_ring_stats[i], rcu); + WRITE_ONCE(vsi_stat->tx_ring_stats[i], NULL); + } + } + for (int i = new_rxq; i < prev_rxq; i++) { + if (vsi_stat->rx_ring_stats[i]) { + kfree_rcu(vsi_stat->rx_ring_stats[i], rcu); + WRITE_ONCE(vsi_stat->rx_ring_stats[i], NULL); + } + } +} + +static void ice_vsi_set_stat_arrays(struct ice_vsi *vsi, + struct ice_vsi_stats *new_vsi_stat) +{ + u16 new_txq, new_rxq, prev_txq, prev_rxq; + struct ice_vsi_stats *vsi_stat; + struct ice_pf *pf = vsi->back; + + new_txq = new_vsi_stat->tx_ring_stats_len; + new_rxq = new_vsi_stat->rx_ring_stats_len; + vsi_stat = pf->vsi_stats[vsi->idx]; + pf->vsi_stats[vsi->idx] = new_vsi_stat; + if (!vsi_stat) + return; /* don't copy if there is no source */ + + prev_txq = vsi_stat->tx_ring_stats_len; + prev_rxq = vsi_stat->rx_ring_stats_len; + + memcpy(new_vsi_stat->tx_ring_stats, vsi_stat->tx_ring_stats, + sizeof(*vsi_stat->tx_ring_stats) * min(prev_txq, new_txq)); + memcpy(new_vsi_stat->rx_ring_stats, vsi_stat->rx_ring_stats, + sizeof(*vsi_stat->rx_ring_stats) * min(prev_rxq, new_rxq)); +} + /** * ice_vsi_rebuild_set_coalesce - set coalesce from earlier saved arrays * @vsi: VSI connected with q_vectors @@ -3039,63 +3084,38 @@ ice_vsi_rebuild_set_coalesce(struct ice_vsi *vsi, } /** - * ice_vsi_realloc_stat_arrays - Frees unused stat structures or alloc new ones - * @vsi: VSI pointer + * ice_vsi_resize_stat_arrays - resize ring stats arrays for new queue count + * @vsi: VSI to swap the ring stats arrays of + * + * Call while @vsi still owns its queues and before ice_vsi_decfg() returns them + * to the PF pool, so that the new size is what ice_vsi_set_num_qs() will compute + * afterwards. Surviving entries are carried over, the rest is freed. + * + * Returns 0 on success and negative value on failure */ -static int -ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi) +static int ice_vsi_resize_stat_arrays(struct ice_vsi *vsi) { - u16 req_txq = vsi->req_txq ? vsi->req_txq : vsi->alloc_txq; - u16 req_rxq = vsi->req_rxq ? vsi->req_rxq : vsi->alloc_rxq; - struct ice_ring_stats **tx_ring_stats; - struct ice_ring_stats **rx_ring_stats; - struct ice_vsi_stats *vsi_stat; + struct ice_vsi_alloc_queues_params qs; + struct ice_vsi_stats *old_stat; + struct ice_vsi_stats *new_stat; struct ice_pf *pf = vsi->back; - u16 prev_txq = vsi->alloc_txq; - u16 prev_rxq = vsi->alloc_rxq; - int i; - vsi_stat = pf->vsi_stats[vsi->idx]; + if (vsi->type == ICE_VSI_CHNL) + return 0; - if (req_txq < prev_txq) { - for (i = req_txq; i < prev_txq; i++) { - if (vsi_stat->tx_ring_stats[i]) { - kfree_rcu(vsi_stat->tx_ring_stats[i], rcu); - WRITE_ONCE(vsi_stat->tx_ring_stats[i], NULL); - } - } - } + qs = ice_vsi_get_num_qs(vsi, vsi->alloc_txq + vsi->num_xdp_txq, + vsi->alloc_rxq); - tx_ring_stats = vsi_stat->tx_ring_stats; - vsi_stat->tx_ring_stats = - krealloc_array(vsi_stat->tx_ring_stats, req_txq, - sizeof(*vsi_stat->tx_ring_stats), - GFP_KERNEL | __GFP_ZERO); - if (!vsi_stat->tx_ring_stats) { - vsi_stat->tx_ring_stats = tx_ring_stats; + new_stat = ice_vsi_new_stat_arrays(qs.alloc_txq, qs.alloc_rxq); + if (!new_stat) return -ENOMEM; - } - vsi_stat->tx_ring_stats_len = req_txq; - if (req_rxq < prev_rxq) { - for (i = req_rxq; i < prev_rxq; i++) { - if (vsi_stat->rx_ring_stats[i]) { - kfree_rcu(vsi_stat->rx_ring_stats[i], rcu); - WRITE_ONCE(vsi_stat->rx_ring_stats[i], NULL); - } - } - } - - rx_ring_stats = vsi_stat->rx_ring_stats; - vsi_stat->rx_ring_stats = - krealloc_array(vsi_stat->rx_ring_stats, req_rxq, - sizeof(*vsi_stat->rx_ring_stats), - GFP_KERNEL | __GFP_ZERO); - if (!vsi_stat->rx_ring_stats) { - vsi_stat->rx_ring_stats = rx_ring_stats; - return -ENOMEM; + old_stat = pf->vsi_stats[vsi->idx]; + ice_vsi_set_stat_arrays(vsi, new_stat); + if (old_stat) { + ice_vsi_free_unused_stat_arrays(old_stat, new_stat); + __ice_vsi_free_stats(old_stat, false); } - vsi_stat->rx_ring_stats_len = req_rxq; return 0; } @@ -3127,7 +3147,7 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags) mutex_lock(&vsi->xdp_state_lock); - ret = ice_vsi_realloc_stat_arrays(vsi); + ret = ice_vsi_resize_stat_arrays(vsi); if (ret) goto unlock; -- 2.51.1
