Re: [Intel-wired-lan] [PATCH iwl-next v4 6/6] ice: convert all ring stats to u64_stats_t
> -Original Message- > From: Intel-wired-lan On Behalf Of Jacob > Keller > Sent: 21 November 2025 01:51 > To: Loktionov, Aleksandr ; Lobakin, Aleksander > ; Nguyen, Anthony L > ; Kitszel, Przemyslaw > > Cc: Simon Horman ; [email protected]; > [email protected]; Keller, Jacob E ; > Loktionov, Aleksandr > Subject: [Intel-wired-lan] [PATCH iwl-next v4 6/6] ice: convert all ring > stats to u64_stats_t > > After several cleanups, the ice driver is now finally ready to convert all Tx > and Rx ring stats to the u64_stats_t and proper use of the u64 stats APIs. > > The final remaining part to cleanup is the VSI stats accumulation logic in > ice_update_vsi_ring_stats(). > > Refactor the function and its helpers so that all stat values (and not just > pkts and bytes) use the u64_stats APIs. The ice_fetch_u64_(tx|rx)_stats > functions read the stat values using u64_stats_read and then copy them into > local ice_vsi_(tx|rx)_stats structures. This does require making a new struct > with the stat fields as u64. > > The ice_update_vsi_(tx|rx)_ring_stats functions call the fetch functions per > ring and accumulate the result into one copy of the struct. This accumulated > total is then used to update the relevant VSI fields. > > Since these are relatively small, the contents are all stored on the stack > rather than allocating and freeing memory. > > Once the accumulator side is updated, the helper ice_stats_read and > ice_stats_inc and other related helper functions all easily translate to use > of u64_stats_read and u64_stats_inc. This completes the refactor and ensures > that all stats accesses now make proper use of the API. > > Reviewed-by: Aleksandr Loktionov > Signed-off-by: Jacob Keller > --- > drivers/net/ethernet/intel/ice/ice_txrx.h | 28 +++-- > drivers/net/ethernet/intel/ice/ice_lib.c | 29 ++--- > drivers/net/ethernet/intel/ice/ice_main.c | 180 -- > 3 files changed, 147 insertions(+), 90 deletions(-) > Tested-by: Rinitha S (A Contingent worker at Intel)
Re: [Intel-wired-lan] [PATCH iwl-next v4 6/6] ice: convert all ring stats to u64_stats_t
On 11/25/2025 2:17 AM, Simon Horman wrote: > On Thu, Nov 20, 2025 at 12:20:46PM -0800, Jacob Keller wrote: >> After several cleanups, the ice driver is now finally ready to convert all >> Tx and Rx ring stats to the u64_stats_t and proper use of the u64 stats >> APIs. >> >> The final remaining part to cleanup is the VSI stats accumulation logic in >> ice_update_vsi_ring_stats(). >> >> Refactor the function and its helpers so that all stat values (and not >> just pkts and bytes) use the u64_stats APIs. The >> ice_fetch_u64_(tx|rx)_stats functions read the stat values using >> u64_stats_read and then copy them into local ice_vsi_(tx|rx)_stats >> structures. This does require making a new struct with the stat fields as >> u64. >> >> The ice_update_vsi_(tx|rx)_ring_stats functions call the fetch functions >> per ring and accumulate the result into one copy of the struct. This >> accumulated total is then used to update the relevant VSI fields. >> >> Since these are relatively small, the contents are all stored on the stack >> rather than allocating and freeing memory. >> >> Once the accumulator side is updated, the helper ice_stats_read and >> ice_stats_inc and other related helper functions all easily translate to >> use of u64_stats_read and u64_stats_inc. This completes the refactor and >> ensures that all stats accesses now make proper use of the API. >> >> Reviewed-by: Aleksandr Loktionov >> Signed-off-by: Jacob Keller > > Thanks. > > I do notice in the cover that you solicit alternate approaches to > lead to a yet cleaner solution. But I think that the approach you have > taken does significantly improve both the cleanliness and correctness > of the code. So even if we think of something better later, I think > this is a good step to take now. > Thanks. In particular, I was wondering if there is a way to help improve or extend the overall u64_stats API to make some of this non-driver specific. It does seem like quite a lot of boilerplate code is needed to make correct use of the APIs, especially with the u64_stat_t type existing now which is necessary even on some 64-bit arch. Perhaps some of the macro wrappers could migrate into u64_stats.h header somehow. Though I'm not sure we can keep them quite as short without being in the driver. > Thanks for breaking out the series into bite-sized chunks, especially > the last few patches. It really helped me in my review. > Yep. I originally had this all munged together but it became impossible to follow the logic of how I got to this point, and also obscured several of the outright fixes. > Reviewed-by: Simon Horman > OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [Intel-wired-lan] [PATCH iwl-next v4 6/6] ice: convert all ring stats to u64_stats_t
On Thu, Nov 20, 2025 at 12:20:46PM -0800, Jacob Keller wrote: > After several cleanups, the ice driver is now finally ready to convert all > Tx and Rx ring stats to the u64_stats_t and proper use of the u64 stats > APIs. > > The final remaining part to cleanup is the VSI stats accumulation logic in > ice_update_vsi_ring_stats(). > > Refactor the function and its helpers so that all stat values (and not > just pkts and bytes) use the u64_stats APIs. The > ice_fetch_u64_(tx|rx)_stats functions read the stat values using > u64_stats_read and then copy them into local ice_vsi_(tx|rx)_stats > structures. This does require making a new struct with the stat fields as > u64. > > The ice_update_vsi_(tx|rx)_ring_stats functions call the fetch functions > per ring and accumulate the result into one copy of the struct. This > accumulated total is then used to update the relevant VSI fields. > > Since these are relatively small, the contents are all stored on the stack > rather than allocating and freeing memory. > > Once the accumulator side is updated, the helper ice_stats_read and > ice_stats_inc and other related helper functions all easily translate to > use of u64_stats_read and u64_stats_inc. This completes the refactor and > ensures that all stats accesses now make proper use of the API. > > Reviewed-by: Aleksandr Loktionov > Signed-off-by: Jacob Keller Thanks. I do notice in the cover that you solicit alternate approaches to lead to a yet cleaner solution. But I think that the approach you have taken does significantly improve both the cleanliness and correctness of the code. So even if we think of something better later, I think this is a good step to take now. Thanks for breaking out the series into bite-sized chunks, especially the last few patches. It really helped me in my review. Reviewed-by: Simon Horman
[Intel-wired-lan] [PATCH iwl-next v4 6/6] ice: convert all ring stats to u64_stats_t
After several cleanups, the ice driver is now finally ready to convert all
Tx and Rx ring stats to the u64_stats_t and proper use of the u64 stats
APIs.
The final remaining part to cleanup is the VSI stats accumulation logic in
ice_update_vsi_ring_stats().
Refactor the function and its helpers so that all stat values (and not
just pkts and bytes) use the u64_stats APIs. The
ice_fetch_u64_(tx|rx)_stats functions read the stat values using
u64_stats_read and then copy them into local ice_vsi_(tx|rx)_stats
structures. This does require making a new struct with the stat fields as
u64.
The ice_update_vsi_(tx|rx)_ring_stats functions call the fetch functions
per ring and accumulate the result into one copy of the struct. This
accumulated total is then used to update the relevant VSI fields.
Since these are relatively small, the contents are all stored on the stack
rather than allocating and freeing memory.
Once the accumulator side is updated, the helper ice_stats_read and
ice_stats_inc and other related helper functions all easily translate to
use of u64_stats_read and u64_stats_inc. This completes the refactor and
ensures that all stats accesses now make proper use of the API.
Reviewed-by: Aleksandr Loktionov
Signed-off-by: Jacob Keller
---
drivers/net/ethernet/intel/ice/ice_txrx.h | 28 +++--
drivers/net/ethernet/intel/ice/ice_lib.c | 29 ++---
drivers/net/ethernet/intel/ice/ice_main.c | 180 --
3 files changed, 147 insertions(+), 90 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h
b/drivers/net/ethernet/intel/ice/ice_txrx.h
index e7e4991c0934..c51b1e60f717 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.h
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.h
@@ -133,20 +133,20 @@ struct ice_ring_stats {
struct rcu_head rcu;/* to avoid race on free */
struct u64_stats_sync syncp;
struct_group(stats,
- u64 pkts;
- u64 bytes;
+ u64_stats_t pkts;
+ u64_stats_t bytes;
union {
struct_group(tx,
- u64 tx_restart_q;
- u64 tx_busy;
- u64 tx_linearize;
+ u64_stats_t tx_restart_q;
+ u64_stats_t tx_busy;
+ u64_stats_t tx_linearize;
/* negative if no pending Tx descriptors */
int prev_pkt;
);
struct_group(rx,
- u64 rx_non_eop_descs;
- u64 rx_page_failed;
- u64 rx_buf_failed;
+ u64_stats_t rx_non_eop_descs;
+ u64_stats_t rx_page_failed;
+ u64_stats_t rx_buf_failed;
);
};
);
@@ -164,7 +164,13 @@ struct ice_ring_stats {
*/
#define ice_stats_read(stats, member) ({ \
struct ice_ring_stats *__stats = (stats); \
- __stats->member;\
+ unsigned int start; \
+ u64 val;\
+ do {\
+ start = u64_stats_fetch_begin(&__stats->syncp); \
+ val = u64_stats_read(&__stats->member); \
+ } while (u64_stats_fetch_retry(&__stats->syncp, start));\
+ val;\
})
/**
@@ -177,7 +183,9 @@ struct ice_ring_stats {
*/
#define ice_stats_inc(stats, member) do { \
struct ice_ring_stats *__stats = (stats); \
- __stats->member++; \
+ u64_stats_update_begin(&__stats->syncp);\
+ u64_stats_inc(&__stats->member);\
+ u64_stats_update_end(&__stats->syncp); \
} while (0)
enum ice_ring_state_t {
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
b/drivers/net/ethernet/intel/ice/ice_lib.c
index 1e3946a1dd36..1d77e993ceb5 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -3433,21 +3433,6 @@ int ice_vsi_cfg_tc(struct ice_vsi *vsi, u8 ena_tc)
return ret;
}
-/**
- * ice_update_ring_stats - Update ring statistics
- * @stats: stats to be updated
- * @pkts: number of processed packets
- * @bytes: number of processed bytes
- *
- * This function assumes that caller has acquired a u64_stats_sync lock.
- */
-static void ice_update_ring_stats(struct ice_ring_stats *stats,
-
