[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20251103-jk-refactor-queue-stats-v1-8-164d2ed859b6@intel.com>
Date: Mon, 03 Nov 2025 17:06:53 -0800
From: Jacob Keller <jacob.e.keller@...el.com>
To: Aleksandr Loktionov <aleksandr.loktionov@...el.com>,
Alexander Lobakin <aleksander.lobakin@...el.com>,
Tony Nguyen <anthony.l.nguyen@...el.com>,
Przemek Kitszel <przemyslaw.kitszel@...el.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
Jacob Keller <jacob.e.keller@...el.com>
Subject: [PATCH iwl-next 8/9] ice: shorten ring stat names and add
accessors
The ice Tx/Rx hotpath has a few statistics counters for tracking unexpected
events. These values are stored as u64 but are not accumulated using the
u64_stats API. This could result in load/tear stores on some architectures.
Even some 64-bit architectures could have issues since the fields are not
read or written using ACCESS_ONCE or READ_ONCE.
A following change is going to refactor the stats accumulator code to use
the u64_stats API for all of these stats, and to use u64_stats_read and
u64_stats_inc properly to prevent load/store tears on all architectures.
Using u64_stats_inc and the syncp pointer is slightly verbose and would be
duplicated in a number of places in the Tx and Rx hot path. Add accessor
macros for the cases where only a single stat value is touched at once. To
keep lines short, also shorten the stats names and convert ice_txq_stats
and ice_rxq_stats to struct_group.
This will ease the transition to properly using the u64_stats API in the
following change.
Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
---
drivers/net/ethernet/intel/ice/ice_txrx.h | 52 +++++++++++++++++++--------
drivers/net/ethernet/intel/ice/ice_txrx_lib.h | 2 +-
drivers/net/ethernet/intel/ice/ice_main.c | 12 +++----
drivers/net/ethernet/intel/ice/ice_txrx.c | 14 ++++----
drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 2 +-
drivers/net/ethernet/intel/ice/ice_xsk.c | 4 +--
6 files changed, 55 insertions(+), 31 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h b/drivers/net/ethernet/intel/ice/ice_txrx.h
index 8586d5bebac7..cf3656dc560c 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.h
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.h
@@ -129,18 +129,6 @@ struct ice_tx_offload_params {
u8 header_len;
};
-struct ice_txq_stats {
- u64 restart_q;
- u64 tx_busy;
- u64 tx_linearize;
-};
-
-struct ice_rxq_stats {
- u64 non_eop_descs;
- u64 alloc_page_failed;
- u64 alloc_buf_failed;
-};
-
struct ice_ring_stats {
struct rcu_head rcu; /* to avoid race on free */
struct u64_stats_sync syncp;
@@ -148,12 +136,48 @@ struct ice_ring_stats {
u64 pkts;
u64 bytes;
union {
- struct ice_txq_stats tx_stats;
- struct ice_rxq_stats rx_stats;
+ struct_group(tx,
+ u64 tx_restart_q;
+ u64 tx_busy;
+ u64 tx_linearize;
+ );
+ struct_group(rx,
+ u64 rx_non_eop_descs;
+ u64 rx_page_failed;
+ u64 rx_buf_failed;
+ );
};
);
};
+/**
+ * ice_stats_read - Read a single ring stat value
+ * @stats: pointer to ring_stats structure for a queue
+ * @member: the ice_ring_stats member to read
+ *
+ * Shorthand for reading a single 64-bit stat value from struct
+ * ice_ring_stats.
+ *
+ * Return: the value of the requested stat.
+ */
+#define ice_stats_read(stats, member) ({ \
+ struct ice_ring_stats *__stats = (stats); \
+ __stats->member; \
+})
+
+/**
+ * ice_stats_inc - Increment a single ring stat value
+ * @stats: pointer to the ring_stats structure for a queue
+ * @member: the ice_string_stats member to increment
+ *
+ * Shorthand for incrementing a single 64-bit stat value in struct
+ * ice_ring_stats.
+ */
+#define ice_stats_inc(stats, member) do { \
+ struct ice_ring_stats *__stats = (stats); \
+ __stats->member++; \
+} while (0)
+
enum ice_ring_state_t {
ICE_TX_XPS_INIT_DONE,
ICE_TX_NBITS,
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.h b/drivers/net/ethernet/intel/ice/ice_txrx_lib.h
index 6a3f10f7a53f..f17990b68b62 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.h
@@ -38,7 +38,7 @@ ice_is_non_eop(const struct ice_rx_ring *rx_ring,
if (likely(ice_test_staterr(rx_desc->wb.status_error0, ICE_RXD_EOF)))
return false;
- rx_ring->ring_stats->rx_stats.non_eop_descs++;
+ ice_stats_inc(rx_ring->ring_stats, rx_non_eop_descs);
return true;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 5a3bcbb5f63c..885e85f478d8 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -159,7 +159,7 @@ static void ice_check_for_hang_subtask(struct ice_pf *pf)
* prev_pkt would be negative if there was no
* pending work.
*/
- packets = ring_stats->stats.pkts & INT_MAX;
+ packets = ice_stats_read(ring_stats, pkts) & INT_MAX;
if (tx_ring->prev_pkt == packets) {
/* Trigger sw interrupt to revive the queue */
ice_trigger_sw_intr(hw, tx_ring->q_vector);
@@ -6869,9 +6869,9 @@ ice_update_vsi_tx_ring_stats(struct ice_vsi *vsi,
ice_fetch_u64_stats_per_ring(ring->ring_stats, &pkts, &bytes);
vsi_stats->tx_packets += pkts;
vsi_stats->tx_bytes += bytes;
- vsi->tx_restart += ring->ring_stats->tx_stats.restart_q;
- vsi->tx_busy += ring->ring_stats->tx_stats.tx_busy;
- vsi->tx_linearize += ring->ring_stats->tx_stats.tx_linearize;
+ vsi->tx_restart += ring->ring_stats->tx_restart_q;
+ vsi->tx_busy += ring->ring_stats->tx_busy;
+ vsi->tx_linearize += ring->ring_stats->tx_linearize;
}
}
@@ -6913,8 +6913,8 @@ static void ice_update_vsi_ring_stats(struct ice_vsi *vsi)
ice_fetch_u64_stats_per_ring(ring_stats, &pkts, &bytes);
vsi_stats->rx_packets += pkts;
vsi_stats->rx_bytes += bytes;
- vsi->rx_buf_failed += ring_stats->rx_stats.alloc_buf_failed;
- vsi->rx_page_failed += ring_stats->rx_stats.alloc_page_failed;
+ vsi->rx_buf_failed += ring_stats->rx_buf_failed;
+ vsi->rx_page_failed += ring_stats->rx_page_failed;
}
/* update XDP Tx rings counters */
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
index f0f5133c389f..ddd40c87772a 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -379,7 +379,7 @@ static bool ice_clean_tx_irq(struct ice_tx_ring *tx_ring, int napi_budget)
if (netif_tx_queue_stopped(txring_txq(tx_ring)) &&
!test_bit(ICE_VSI_DOWN, vsi->state)) {
netif_tx_wake_queue(txring_txq(tx_ring));
- ++tx_ring->ring_stats->tx_stats.restart_q;
+ ice_stats_inc(tx_ring->ring_stats, tx_restart_q);
}
}
@@ -849,7 +849,7 @@ bool ice_alloc_rx_bufs(struct ice_rx_ring *rx_ring, unsigned int cleaned_count)
addr = libeth_rx_alloc(&fq, ntu);
if (addr == DMA_MAPPING_ERROR) {
- rx_ring->ring_stats->rx_stats.alloc_page_failed++;
+ ice_stats_inc(rx_ring->ring_stats, rx_page_failed);
break;
}
@@ -863,7 +863,7 @@ bool ice_alloc_rx_bufs(struct ice_rx_ring *rx_ring, unsigned int cleaned_count)
addr = libeth_rx_alloc(&hdr_fq, ntu);
if (addr == DMA_MAPPING_ERROR) {
- rx_ring->ring_stats->rx_stats.alloc_page_failed++;
+ ice_stats_inc(rx_ring->ring_stats, rx_page_failed);
libeth_rx_recycle_slow(fq.fqes[ntu].netmem);
break;
@@ -1045,7 +1045,7 @@ static int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int budget)
/* exit if we failed to retrieve a buffer */
if (!skb) {
libeth_xdp_return_buff_slow(xdp);
- rx_ring->ring_stats->rx_stats.alloc_buf_failed++;
+ ice_stats_inc(rx_ring->ring_stats, rx_buf_failed);
continue;
}
@@ -1363,7 +1363,7 @@ static int __ice_maybe_stop_tx(struct ice_tx_ring *tx_ring, unsigned int size)
/* A reprieve! - use start_queue because it doesn't call schedule */
netif_tx_start_queue(txring_txq(tx_ring));
- ++tx_ring->ring_stats->tx_stats.restart_q;
+ ice_stats_inc(tx_ring->ring_stats, tx_restart_q);
return 0;
}
@@ -2165,7 +2165,7 @@ ice_xmit_frame_ring(struct sk_buff *skb, struct ice_tx_ring *tx_ring)
if (__skb_linearize(skb))
goto out_drop;
count = ice_txd_use_count(skb->len);
- tx_ring->ring_stats->tx_stats.tx_linearize++;
+ ice_stats_inc(tx_ring->ring_stats, tx_linearize);
}
/* need: 1 descriptor per page * PAGE_SIZE/ICE_MAX_DATA_PER_TXD,
@@ -2176,7 +2176,7 @@ ice_xmit_frame_ring(struct sk_buff *skb, struct ice_tx_ring *tx_ring)
*/
if (ice_maybe_stop_tx(tx_ring, count + ICE_DESCS_PER_CACHE_LINE +
ICE_DESCS_FOR_CTX_DESC)) {
- tx_ring->ring_stats->tx_stats.tx_busy++;
+ ice_stats_inc(tx_ring->ring_stats, tx_busy);
return NETDEV_TX_BUSY;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
index 956da38d63b0..e68f3e5d35b4 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
@@ -480,7 +480,7 @@ int __ice_xmit_xdp_ring(struct xdp_buff *xdp, struct ice_tx_ring *xdp_ring,
return ICE_XDP_CONSUMED;
busy:
- xdp_ring->ring_stats->tx_stats.tx_busy++;
+ ice_stats_inc(xdp_ring->ring_stats, tx_busy);
return ICE_XDP_CONSUMED;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index 989ff1fd9110..953e68ed0f9a 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -497,7 +497,7 @@ static int ice_xmit_xdp_tx_zc(struct xdp_buff *xdp,
return ICE_XDP_TX;
busy:
- xdp_ring->ring_stats->tx_stats.tx_busy++;
+ ice_stats_inc(xdp_ring->ring_stats, tx_busy);
return ICE_XDP_CONSUMED;
}
@@ -659,7 +659,7 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring,
xsk_buff_free(first);
first = NULL;
- rx_ring->ring_stats->rx_stats.alloc_buf_failed++;
+ ice_stats_inc(rx_ring->ring_stats, rx_buf_failed);
continue;
}
--
2.51.0.rc1.197.g6d975e95c9d7
Powered by blists - more mailing lists