| Message ID | 20230714114721.335526-1-przemyslaw.kitszel@intel.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [iwl-net] ice: Reset stats on queues num change | expand |
On Fri, Jul 14, 2023 at 07:47:21AM -0400, Przemek Kitszel wrote: > Reset VSI stats on queues number change. > > Commit 288ecf491b16 ("ice: Accumulate ring statistics over reset") > implemented functionality for interface statistics to persist over reset, > but it left stats persisting over queue count reconfiguration. > > Following scenario is fixed here: > # Observe statistics for Tx/Rx queues > ethtool -S ethX > # change number of queues > ethtool -L ethX combined 10 > # Observe statistics for Tx/Rx queues (after reset) > ethtool -S ethX > > Ben has left a note where to place the VSI stats reset, > what made this fix much easier to do. > > Note that newly allocated structs (case of num_txq > prev_txq) don't > need zeroing. > > Fixes: 288ecf491b16 ("ice: Accumulate ring statistics over reset") > Suggested-by: Benjamin Mikailenko <benjamin.mikailenko@intel.com> > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> Reviewed-by: Simon Horman <simon.horman@corigine.com>
From: Przemek Kitszel <przemyslaw.kitszel@intel.com> Date: Fri, 14 Jul 2023 07:47:21 -0400 > Reset VSI stats on queues number change. Please don't. > > Commit 288ecf491b16 ("ice: Accumulate ring statistics over reset") > implemented functionality for interface statistics to persist over reset, > but it left stats persisting over queue count reconfiguration. And? Why do you guys think we need to zero stats for the turned off queues? Let the stats have the same lifetime as pci_dev has. > > Following scenario is fixed here: > # Observe statistics for Tx/Rx queues > ethtool -S ethX > # change number of queues > ethtool -L ethX combined 10 > # Observe statistics for Tx/Rx queues (after reset) > ethtool -S ethX > > Ben has left a note where to place the VSI stats reset, > what made this fix much easier to do. > > Note that newly allocated structs (case of num_txq > prev_txq) don't > need zeroing. > > Fixes: 288ecf491b16 ("ice: Accumulate ring statistics over reset") > Suggested-by: Benjamin Mikailenko <benjamin.mikailenko@intel.com> > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> [...] Thanks, Olek
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c index 00e3afd507a4..09942bdea25d 100644 --- a/drivers/net/ethernet/intel/ice/ice_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_lib.c @@ -3130,13 +3130,15 @@ ice_vsi_rebuild_set_coalesce(struct ice_vsi *vsi, } /** - * ice_vsi_realloc_stat_arrays - Frees unused stat structures + * ice_vsi_adjust_stat_arrays - Adjust VSI stat structures * @vsi: VSI pointer * @prev_txq: Number of Tx rings before ring reallocation * @prev_rxq: Number of Rx rings before ring reallocation + * + * Zero stat structures before reuse, free redundant ones. */ static void -ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi, int prev_txq, int prev_rxq) +ice_vsi_adjust_stat_arrays(struct ice_vsi *vsi, int prev_txq, int prev_rxq) { struct ice_vsi_stats *vsi_stat; struct ice_pf *pf = vsi->back; @@ -3149,7 +3151,17 @@ ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi, int prev_txq, int prev_rxq) vsi_stat = pf->vsi_stats[vsi->idx]; - if (vsi->num_txq < prev_txq) { + if (vsi->num_txq != prev_txq) { + /* first, reset structs that we will reuse */ + int reuse_q_cnt = min_t(int, vsi->num_txq, prev_txq); + + for (i = 0; i < reuse_q_cnt; i++) { + struct ice_ring_stats *rs = vsi_stat->tx_ring_stats[i]; + + if (rs) + memset(rs, 0, sizeof(*rs)); + } + /* second, free redundant ones */ for (i = vsi->num_txq; i < prev_txq; i++) { if (vsi_stat->tx_ring_stats[i]) { kfree_rcu(vsi_stat->tx_ring_stats[i], rcu); @@ -3158,7 +3170,16 @@ ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi, int prev_txq, int prev_rxq) } } - if (vsi->num_rxq < prev_rxq) { + /* apply very same logic as for tx */ + if (vsi->num_rxq != prev_rxq) { + int reuse_q_cnt = min_t(int, vsi->num_rxq, prev_rxq); + + for (i = 0; i < reuse_q_cnt; i++) { + struct ice_ring_stats *rs = vsi_stat->rx_ring_stats[i]; + + if (rs) + memset(rs, 0, sizeof(*rs)); + } for (i = vsi->num_rxq; i < prev_rxq; i++) { if (vsi_stat->rx_ring_stats[i]) { kfree_rcu(vsi_stat->rx_ring_stats[i], rcu); @@ -3222,7 +3243,9 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags) return ice_schedule_reset(pf, ICE_RESET_PFR); } - ice_vsi_realloc_stat_arrays(vsi, prev_txq, prev_rxq); + ice_vsi_adjust_stat_arrays(vsi, prev_txq, prev_rxq); + if (vsi->num_txq != prev_txq || vsi->num_rxq != prev_rxq) + vsi->stat_offsets_loaded = false; ice_vsi_rebuild_set_coalesce(vsi, coalesce, prev_num_q_vectors); kfree(coalesce);
Reset VSI stats on queues number change. Commit 288ecf491b16 ("ice: Accumulate ring statistics over reset") implemented functionality for interface statistics to persist over reset, but it left stats persisting over queue count reconfiguration. Following scenario is fixed here: # Observe statistics for Tx/Rx queues ethtool -S ethX # change number of queues ethtool -L ethX combined 10 # Observe statistics for Tx/Rx queues (after reset) ethtool -S ethX Ben has left a note where to place the VSI stats reset, what made this fix much easier to do. Note that newly allocated structs (case of num_txq > prev_txq) don't need zeroing. Fixes: 288ecf491b16 ("ice: Accumulate ring statistics over reset") Suggested-by: Benjamin Mikailenko <benjamin.mikailenko@intel.com> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> --- drivers/net/ethernet/intel/ice/ice_lib.c | 33 ++++++++++++++++++++---- 1 file changed, 28 insertions(+), 5 deletions(-) base-commit: 9d23aac8a85f69239e585c8656c6fdb21be65695