Re: [Intel-wired-lan] [PATCH iwl-next v4 2/6] ice: pass pointer to ice_fetch_u64_stats_per_ring
> -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 2/6] ice: pass pointer to > ice_fetch_u64_stats_per_ring > > The ice_fetch_u64_stats_per_ring function takes a pointer to the syncp from > the ring stats to synchronize reading of the packet stats. It also takes a *copy* of the ice_q_stats fields instead of a pointer to the stats. This completely defeats the point of using the u64_stats API. We pass the stats by value, so they are static at the point of reading within the u64_stats_fetch_retry loop. > > Simplify the function to take a pointer to the ice_ring_stats instead of two > separate parameters. Additionally, since we never call this outside of > ice_main.c, make it a static function. > > Reviewed-by: Aleksandr Loktionov > Signed-off-by: Jacob Keller > --- > drivers/net/ethernet/intel/ice/ice.h | 3 --- > drivers/net/ethernet/intel/ice/ice_main.c | 24 +--- > 2 files changed, 9 insertions(+), 18 deletions(-) > Tested-by: Rinitha S (A Contingent worker at Intel)
Re: [Intel-wired-lan] [PATCH iwl-next v4 2/6] ice: pass pointer to ice_fetch_u64_stats_per_ring
On 11/25/2025 2:16 AM, Simon Horman wrote: > On Thu, Nov 20, 2025 at 12:20:42PM -0800, Jacob Keller wrote: >> The ice_fetch_u64_stats_per_ring function takes a pointer to the syncp from >> the ring stats to synchronize reading of the packet stats. It also takes a >> *copy* of the ice_q_stats fields instead of a pointer to the stats. This >> completely defeats the point of using the u64_stats API. We pass the stats >> by value, so they are static at the point of reading within the >> u64_stats_fetch_retry loop. >> >> Simplify the function to take a pointer to the ice_ring_stats instead of >> two separate parameters. Additionally, since we never call this outside of >> ice_main.c, make it a static function. >> >> Reviewed-by: Aleksandr Loktionov >> Signed-off-by: Jacob Keller > > The *copy* was certainly working against us here. > But TBH, C syntax led me to read the code more than > once before seeing it. > Yes, I had the exact same issue. It took me a while while refactoring to notice this particular bug... > Reviewed-by: Simon Horman OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [Intel-wired-lan] [PATCH iwl-next v4 2/6] ice: pass pointer to ice_fetch_u64_stats_per_ring
On Thu, Nov 20, 2025 at 12:20:42PM -0800, Jacob Keller wrote: > The ice_fetch_u64_stats_per_ring function takes a pointer to the syncp from > the ring stats to synchronize reading of the packet stats. It also takes a > *copy* of the ice_q_stats fields instead of a pointer to the stats. This > completely defeats the point of using the u64_stats API. We pass the stats > by value, so they are static at the point of reading within the > u64_stats_fetch_retry loop. > > Simplify the function to take a pointer to the ice_ring_stats instead of > two separate parameters. Additionally, since we never call this outside of > ice_main.c, make it a static function. > > Reviewed-by: Aleksandr Loktionov > Signed-off-by: Jacob Keller The *copy* was certainly working against us here. But TBH, C syntax led me to read the code more than once before seeing it. Reviewed-by: Simon Horman
