1. Patchew
  2. linux
  3. View series

[PATCH net-next] net: airoha: Use u64_stats_t with u64_stats_sync properly

David Yang posted 1 patch 2 weeks, 2 days ago
There is a newer version of this series
drivers/net/ethernet/airoha/airoha_eth.c | 136 +++++++++++------------
drivers/net/ethernet/airoha/airoha_eth.h |  34 +++---
2 files changed, 85 insertions(+), 85 deletions(-)
[PATCH net-next] net: airoha: Use u64_stats_t with u64_stats_sync properly
Posted by David Yang 2 weeks, 2 days ago 
On 64bit arches, struct u64_stats_sync is empty and provides no help
against load/store tearing. Convert to u64_stats_t to ensure atomic
operations.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/net/ethernet/airoha/airoha_eth.c | 136 +++++++++++------------
 drivers/net/ethernet/airoha/airoha_eth.h |  34 +++---
 2 files changed, 85 insertions(+), 85 deletions(-)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index 62bcbbbe2a95..6ed220e5a094 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -1472,131 +1472,131 @@ static void airoha_update_hw_stats(struct airoha_gdm_port *port)
 
 	/* TX */
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_H(port->id));
-	port->stats.tx_ok_pkts += ((u64)val << 32);
+	u64_stats_add(&port->stats.tx_ok_pkts, (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_L(port->id));
-	port->stats.tx_ok_pkts += val;
+	u64_stats_add(&port->stats.tx_ok_pkts, val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_BYTE_CNT_H(port->id));
-	port->stats.tx_ok_bytes += ((u64)val << 32);
+	u64_stats_add(&port->stats.tx_ok_bytes, (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_BYTE_CNT_L(port->id));
-	port->stats.tx_ok_bytes += val;
+	u64_stats_add(&port->stats.tx_ok_bytes, val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_DROP_CNT(port->id));
-	port->stats.tx_drops += val;
+	u64_stats_add(&port->stats.tx_drops, val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_BC_CNT(port->id));
-	port->stats.tx_broadcast += val;
+	u64_stats_add(&port->stats.tx_broadcast, val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_MC_CNT(port->id));
-	port->stats.tx_multicast += val;
+	u64_stats_add(&port->stats.tx_multicast, val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_RUNT_CNT(port->id));
-	port->stats.tx_len[i] += val;
+	u64_stats_add(&port->stats.tx_len[i], val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_E64_CNT_H(port->id));
-	port->stats.tx_len[i] += ((u64)val << 32);
+	u64_stats_add(&port->stats.tx_len[i], (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_E64_CNT_L(port->id));
-	port->stats.tx_len[i++] += val;
+	u64_stats_add(&port->stats.tx_len[i++], val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L64_CNT_H(port->id));
-	port->stats.tx_len[i] += ((u64)val << 32);
+	u64_stats_add(&port->stats.tx_len[i], (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L64_CNT_L(port->id));
-	port->stats.tx_len[i++] += val;
+	u64_stats_add(&port->stats.tx_len[i++], val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L127_CNT_H(port->id));
-	port->stats.tx_len[i] += ((u64)val << 32);
+	u64_stats_add(&port->stats.tx_len[i], (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L127_CNT_L(port->id));
-	port->stats.tx_len[i++] += val;
+	u64_stats_add(&port->stats.tx_len[i++], val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L255_CNT_H(port->id));
-	port->stats.tx_len[i] += ((u64)val << 32);
+	u64_stats_add(&port->stats.tx_len[i], (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L255_CNT_L(port->id));
-	port->stats.tx_len[i++] += val;
+	u64_stats_add(&port->stats.tx_len[i++], val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L511_CNT_H(port->id));
-	port->stats.tx_len[i] += ((u64)val << 32);
+	u64_stats_add(&port->stats.tx_len[i], (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L511_CNT_L(port->id));
-	port->stats.tx_len[i++] += val;
+	u64_stats_add(&port->stats.tx_len[i++], val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L1023_CNT_H(port->id));
-	port->stats.tx_len[i] += ((u64)val << 32);
+	u64_stats_add(&port->stats.tx_len[i], (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L1023_CNT_L(port->id));
-	port->stats.tx_len[i++] += val;
+	u64_stats_add(&port->stats.tx_len[i++], val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_LONG_CNT(port->id));
-	port->stats.tx_len[i++] += val;
+	u64_stats_add(&port->stats.tx_len[i++], val);
 
 	/* RX */
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_OK_PKT_CNT_H(port->id));
-	port->stats.rx_ok_pkts += ((u64)val << 32);
+	u64_stats_add(&port->stats.rx_ok_pkts, (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_OK_PKT_CNT_L(port->id));
-	port->stats.rx_ok_pkts += val;
+	u64_stats_add(&port->stats.rx_ok_pkts, val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_OK_BYTE_CNT_H(port->id));
-	port->stats.rx_ok_bytes += ((u64)val << 32);
+	u64_stats_add(&port->stats.rx_ok_bytes, (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_OK_BYTE_CNT_L(port->id));
-	port->stats.rx_ok_bytes += val;
+	u64_stats_add(&port->stats.rx_ok_bytes, val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_DROP_CNT(port->id));
-	port->stats.rx_drops += val;
+	u64_stats_add(&port->stats.rx_drops, val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_BC_CNT(port->id));
-	port->stats.rx_broadcast += val;
+	u64_stats_add(&port->stats.rx_broadcast, val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_MC_CNT(port->id));
-	port->stats.rx_multicast += val;
+	u64_stats_add(&port->stats.rx_multicast, val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ERROR_DROP_CNT(port->id));
-	port->stats.rx_errors += val;
+	u64_stats_add(&port->stats.rx_errors, val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_CRC_ERR_CNT(port->id));
-	port->stats.rx_crc_error += val;
+	u64_stats_add(&port->stats.rx_crc_error, val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_OVERFLOW_DROP_CNT(port->id));
-	port->stats.rx_over_errors += val;
+	u64_stats_add(&port->stats.rx_over_errors, val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_FRAG_CNT(port->id));
-	port->stats.rx_fragment += val;
+	u64_stats_add(&port->stats.rx_fragment, val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_JABBER_CNT(port->id));
-	port->stats.rx_jabber += val;
+	u64_stats_add(&port->stats.rx_jabber, val);
 
 	i = 0;
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_RUNT_CNT(port->id));
-	port->stats.rx_len[i] += val;
+	u64_stats_add(&port->stats.rx_len[i], val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_E64_CNT_H(port->id));
-	port->stats.rx_len[i] += ((u64)val << 32);
+	u64_stats_add(&port->stats.rx_len[i], (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_E64_CNT_L(port->id));
-	port->stats.rx_len[i++] += val;
+	u64_stats_add(&port->stats.rx_len[i++], val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L64_CNT_H(port->id));
-	port->stats.rx_len[i] += ((u64)val << 32);
+	u64_stats_add(&port->stats.rx_len[i], (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L64_CNT_L(port->id));
-	port->stats.rx_len[i++] += val;
+	u64_stats_add(&port->stats.rx_len[i++], val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L127_CNT_H(port->id));
-	port->stats.rx_len[i] += ((u64)val << 32);
+	u64_stats_add(&port->stats.rx_len[i], (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L127_CNT_L(port->id));
-	port->stats.rx_len[i++] += val;
+	u64_stats_add(&port->stats.rx_len[i++], val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L255_CNT_H(port->id));
-	port->stats.rx_len[i] += ((u64)val << 32);
+	u64_stats_add(&port->stats.rx_len[i], (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L255_CNT_L(port->id));
-	port->stats.rx_len[i++] += val;
+	u64_stats_add(&port->stats.rx_len[i++], val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L511_CNT_H(port->id));
-	port->stats.rx_len[i] += ((u64)val << 32);
+	u64_stats_add(&port->stats.rx_len[i], (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L511_CNT_L(port->id));
-	port->stats.rx_len[i++] += val;
+	u64_stats_add(&port->stats.rx_len[i++], val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L1023_CNT_H(port->id));
-	port->stats.rx_len[i] += ((u64)val << 32);
+	u64_stats_add(&port->stats.rx_len[i], (u64)val << 32);
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L1023_CNT_L(port->id));
-	port->stats.rx_len[i++] += val;
+	u64_stats_add(&port->stats.rx_len[i++], val);
 
 	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_LONG_CNT(port->id));
-	port->stats.rx_len[i++] += val;
+	u64_stats_add(&port->stats.rx_len[i++], val);
 
 	/* reset mib counters */
 	airoha_fe_set(eth, REG_FE_GDM_MIB_CLEAR(port->id),
@@ -1795,16 +1795,16 @@ static void airoha_dev_get_stats64(struct net_device *dev,
 	airoha_update_hw_stats(port);
 	do {
 		start = u64_stats_fetch_begin(&port->stats.syncp);
-		storage->rx_packets = port->stats.rx_ok_pkts;
-		storage->tx_packets = port->stats.tx_ok_pkts;
-		storage->rx_bytes = port->stats.rx_ok_bytes;
-		storage->tx_bytes = port->stats.tx_ok_bytes;
-		storage->multicast = port->stats.rx_multicast;
-		storage->rx_errors = port->stats.rx_errors;
-		storage->rx_dropped = port->stats.rx_drops;
-		storage->tx_dropped = port->stats.tx_drops;
-		storage->rx_crc_errors = port->stats.rx_crc_error;
-		storage->rx_over_errors = port->stats.rx_over_errors;
+		storage->rx_packets = u64_stats_read(&port->stats.rx_ok_pkts);
+		storage->tx_packets = u64_stats_read(&port->stats.tx_ok_pkts);
+		storage->rx_bytes = u64_stats_read(&port->stats.rx_ok_bytes);
+		storage->tx_bytes = u64_stats_read(&port->stats.tx_ok_bytes);
+		storage->multicast = u64_stats_read(&port->stats.rx_multicast);
+		storage->rx_errors = u64_stats_read(&port->stats.rx_errors);
+		storage->rx_dropped = u64_stats_read(&port->stats.rx_drops);
+		storage->tx_dropped = u64_stats_read(&port->stats.tx_drops);
+		storage->rx_crc_errors = u64_stats_read(&port->stats.rx_crc_error);
+		storage->rx_over_errors = u64_stats_read(&port->stats.rx_over_errors);
 	} while (u64_stats_fetch_retry(&port->stats.syncp, start));
 }
 
@@ -2057,13 +2057,13 @@ static void airoha_ethtool_get_mac_stats(struct net_device *dev,
 	airoha_update_hw_stats(port);
 	do {
 		start = u64_stats_fetch_begin(&port->stats.syncp);
-		stats->FramesTransmittedOK = port->stats.tx_ok_pkts;
-		stats->OctetsTransmittedOK = port->stats.tx_ok_bytes;
-		stats->MulticastFramesXmittedOK = port->stats.tx_multicast;
-		stats->BroadcastFramesXmittedOK = port->stats.tx_broadcast;
-		stats->FramesReceivedOK = port->stats.rx_ok_pkts;
-		stats->OctetsReceivedOK = port->stats.rx_ok_bytes;
-		stats->BroadcastFramesReceivedOK = port->stats.rx_broadcast;
+		stats->FramesTransmittedOK = u64_stats_read(&port->stats.tx_ok_pkts);
+		stats->OctetsTransmittedOK = u64_stats_read(&port->stats.tx_ok_bytes);
+		stats->MulticastFramesXmittedOK = u64_stats_read(&port->stats.tx_multicast);
+		stats->BroadcastFramesXmittedOK = u64_stats_read(&port->stats.tx_broadcast);
+		stats->FramesReceivedOK = u64_stats_read(&port->stats.rx_ok_pkts);
+		stats->OctetsReceivedOK = u64_stats_read(&port->stats.rx_ok_bytes);
+		stats->BroadcastFramesReceivedOK = u64_stats_read(&port->stats.rx_broadcast);
 	} while (u64_stats_fetch_retry(&port->stats.syncp, start));
 }
 
@@ -2098,12 +2098,12 @@ airoha_ethtool_get_rmon_stats(struct net_device *dev,
 		int i;
 
 		start = u64_stats_fetch_begin(&port->stats.syncp);
-		stats->fragments = hw_stats->rx_fragment;
-		stats->jabbers = hw_stats->rx_jabber;
+		stats->fragments = u64_stats_read(&hw_stats->rx_fragment);
+		stats->jabbers = u64_stats_read(&hw_stats->rx_jabber);
 		for (i = 0; i < ARRAY_SIZE(airoha_ethtool_rmon_ranges) - 1;
 		     i++) {
-			stats->hist[i] = hw_stats->rx_len[i];
-			stats->hist_tx[i] = hw_stats->tx_len[i];
+			stats->hist[i] = u64_stats_read(&hw_stats->rx_len[i]);
+			stats->hist_tx[i] = u64_stats_read(&hw_stats->tx_len[i]);
 		}
 	} while (u64_stats_fetch_retry(&port->stats.syncp, start));
 }
diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
index 20e602d61e61..b2e9bd849f8b 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.h
+++ b/drivers/net/ethernet/airoha/airoha_eth.h
@@ -215,24 +215,24 @@ struct airoha_hw_stats {
 	struct u64_stats_sync syncp;
 
 	/* get_stats64 */
-	u64 rx_ok_pkts;
-	u64 tx_ok_pkts;
-	u64 rx_ok_bytes;
-	u64 tx_ok_bytes;
-	u64 rx_multicast;
-	u64 rx_errors;
-	u64 rx_drops;
-	u64 tx_drops;
-	u64 rx_crc_error;
-	u64 rx_over_errors;
+	u64_stats_t rx_ok_pkts;
+	u64_stats_t tx_ok_pkts;
+	u64_stats_t rx_ok_bytes;
+	u64_stats_t tx_ok_bytes;
+	u64_stats_t rx_multicast;
+	u64_stats_t rx_errors;
+	u64_stats_t rx_drops;
+	u64_stats_t tx_drops;
+	u64_stats_t rx_crc_error;
+	u64_stats_t rx_over_errors;
 	/* ethtool stats */
-	u64 tx_broadcast;
-	u64 tx_multicast;
-	u64 tx_len[7];
-	u64 rx_broadcast;
-	u64 rx_fragment;
-	u64 rx_jabber;
-	u64 rx_len[7];
+	u64_stats_t tx_broadcast;
+	u64_stats_t tx_multicast;
+	u64_stats_t tx_len[7];
+	u64_stats_t rx_broadcast;
+	u64_stats_t rx_fragment;
+	u64_stats_t rx_jabber;
+	u64_stats_t rx_len[7];
 };
 
 enum {
-- 
2.51.0
Re: [PATCH net-next] net: airoha: Use u64_stats_t with u64_stats_sync properly
Posted by David Laight 1 week, 6 days ago 
On Fri, 23 Jan 2026 02:52:51 +0800
David Yang <mmyangfl@gmail.com> wrote:

> On 64bit arches, struct u64_stats_sync is empty and provides no help
> against load/store tearing. Convert to u64_stats_t to ensure atomic
> operations.
> 
> Signed-off-by: David Yang <mmyangfl@gmail.com>
> ---
>  drivers/net/ethernet/airoha/airoha_eth.c | 136 +++++++++++------------
>  drivers/net/ethernet/airoha/airoha_eth.h |  34 +++---
>  2 files changed, 85 insertions(+), 85 deletions(-)
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 62bcbbbe2a95..6ed220e5a094 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -1472,131 +1472,131 @@ static void airoha_update_hw_stats(struct airoha_gdm_port *port)
>  
>  	/* TX */
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_H(port->id));
> -	port->stats.tx_ok_pkts += ((u64)val << 32);
> +	u64_stats_add(&port->stats.tx_ok_pkts, (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_L(port->id));
> -	port->stats.tx_ok_pkts += val;
> +	u64_stats_add(&port->stats.tx_ok_pkts, val);

Wouldn't that be better written as:
	u64 val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_H(port->id));
	val = val << 32 + airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_L(port->id));
	port->stats.tx_ok_pkts += val;
(Assuming there something has stopped the hardware increment the register
between the two accesses, and there is an associated atomic zero.)

Otherwise you are generating 'tearing' on 64bit systems by adding the high
and low halves separately - regardless of how the stats are read.
I think that works for 32bit as well.

Making the code completely unreadable with 'special' types and 'special'
copy routines really doesn't seem worth while.
The compiler won't generate code that does 'data tearing' for aligned word
accesses, and even if it did nothing would really break.
The most you want is a memcpy_in_words() function that guarantees to use
'word' size tranfsers for aligned buffers - and is probably an alias for
normal memcpy() on all current systems.

OTOH data tearing can be seen for 64 bit adds on 32bit systems.

It is worth nothing that on 32bits the 'packet count' and 'byte count'
increments get seen as a pair, this doesn't happen on 64bit - one happens
before the other.

	David
Re: [PATCH net-next] net: airoha: Use u64_stats_t with u64_stats_sync properly
Posted by Yangfl 1 week, 6 days ago 
On Mon, Jan 26, 2026 at 6:35 AM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Fri, 23 Jan 2026 02:52:51 +0800
> David Yang <mmyangfl@gmail.com> wrote:
>
> > On 64bit arches, struct u64_stats_sync is empty and provides no help
> > against load/store tearing. Convert to u64_stats_t to ensure atomic
> > operations.
> >
> > Signed-off-by: David Yang <mmyangfl@gmail.com>
> > ---
> >  drivers/net/ethernet/airoha/airoha_eth.c | 136 +++++++++++------------
> >  drivers/net/ethernet/airoha/airoha_eth.h |  34 +++---
> >  2 files changed, 85 insertions(+), 85 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> > index 62bcbbbe2a95..6ed220e5a094 100644
> > --- a/drivers/net/ethernet/airoha/airoha_eth.c
> > +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> > @@ -1472,131 +1472,131 @@ static void airoha_update_hw_stats(struct airoha_gdm_port *port)
> >
> >       /* TX */
> >       val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_H(port->id));
> > -     port->stats.tx_ok_pkts += ((u64)val << 32);
> > +     u64_stats_add(&port->stats.tx_ok_pkts, (u64)val << 32);
> >       val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_L(port->id));
> > -     port->stats.tx_ok_pkts += val;
> > +     u64_stats_add(&port->stats.tx_ok_pkts, val);
>
> Wouldn't that be better written as:
>         u64 val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_H(port->id));
>         val = val << 32 + airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_L(port->id));
>         port->stats.tx_ok_pkts += val;
> (Assuming there something has stopped the hardware increment the register
> between the two accesses, and there is an associated atomic zero.)
>
> Otherwise you are generating 'tearing' on 64bit systems by adding the high
> and low halves separately - regardless of how the stats are read.
> I think that works for 32bit as well.
>
> Making the code completely unreadable with 'special' types and 'special'
> copy routines really doesn't seem worth while.
> The compiler won't generate code that does 'data tearing' for aligned word
> accesses, and even if it did nothing would really break.

Quoting 316580b69d0a ("u64_stats: provide u64_stats_t type"):

  On 64bit arches, struct u64_stats_sync is empty and provides no help
against load/store tearing.

  Using READ_ONCE()/WRITE_ONCE() would be needed.

> The most you want is a memcpy_in_words() function that guarantees to use
> 'word' size tranfsers for aligned buffers - and is probably an alias for
> normal memcpy() on all current systems.

See https://lore.kernel.org/r/20260120092137.2161162-1-mmyangfl@gmail.com

And if you want a memcpy, you need a buffer. If you want to fold the
buffer, that's basically WRITE_ONCE().

>
> OTOH data tearing can be seen for 64 bit adds on 32bit systems.
>
> It is worth nothing that on 32bits the 'packet count' and 'byte count'
> increments get seen as a pair, this doesn't happen on 64bit - one happens
> before the other.
>
>         David
Re: [net-next] net: airoha: Use u64_stats_t with u64_stats_sync properly
Posted by Jakub Kicinski 1 week, 6 days ago 
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: airoha: Use u64_stats_t with u64_stats_sync properly

This commit converts statistics fields from u64 to u64_stats_t and changes
direct arithmetic operations to u64_stats_add()/u64_stats_read() calls
to ensure atomic operations on 64-bit architectures.

> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 62bcbbbe2a95..6ed220e5a094 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -1472,131 +1472,131 @@ static void airoha_update_hw_stats(struct airoha_gdm_port *port)
>
>  	/* TX */
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_H(port->id));
> -	port->stats.tx_ok_pkts += ((u64)val << 32);
> +	u64_stats_add(&port->stats.tx_ok_pkts, (u64)val << 32);

On 32-bit systems (ARCH_AIROHA is supported on ARM32 per
arch/arm/Kconfig.platforms), does this lose the upper 32 bits of the
counter?

The u64_stats_add() signature is:

  static inline void u64_stats_add(u64_stats_t *p, unsigned long val)

On 32-bit architectures, unsigned long is 32 bits. When (u64)val << 32
is passed, the resulting 64-bit value is truncated to unsigned long.
Since all the non-zero bits are in the upper half, the truncation yields
zero.

The original code worked because the destination was u64:

  port->stats.tx_ok_pkts += ((u64)val << 32);

This pattern appears 16 times in airoha_update_hw_stats() for all the
_CNT_H registers (tx_ok_pkts, tx_ok_bytes, tx_len[], rx_ok_pkts,
rx_ok_bytes, rx_len[]). On 32-bit builds, these counters would only
accumulate the low 32 bits from the hardware.

[ ... ]
-- 
pw-bot: cr
Re: [PATCH net-next] net: airoha: Use u64_stats_t with u64_stats_sync properly
Posted by Lorenzo Bianconi 2 weeks, 2 days ago 
> On 64bit arches, struct u64_stats_sync is empty and provides no help
> against load/store tearing. Convert to u64_stats_t to ensure atomic
> operations.

I guess we need a Fixes tag here.

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

> 
> Signed-off-by: David Yang <mmyangfl@gmail.com>
> ---
>  drivers/net/ethernet/airoha/airoha_eth.c | 136 +++++++++++------------
>  drivers/net/ethernet/airoha/airoha_eth.h |  34 +++---
>  2 files changed, 85 insertions(+), 85 deletions(-)
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 62bcbbbe2a95..6ed220e5a094 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -1472,131 +1472,131 @@ static void airoha_update_hw_stats(struct airoha_gdm_port *port)
>  
>  	/* TX */
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_H(port->id));
> -	port->stats.tx_ok_pkts += ((u64)val << 32);
> +	u64_stats_add(&port->stats.tx_ok_pkts, (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_PKT_CNT_L(port->id));
> -	port->stats.tx_ok_pkts += val;
> +	u64_stats_add(&port->stats.tx_ok_pkts, val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_BYTE_CNT_H(port->id));
> -	port->stats.tx_ok_bytes += ((u64)val << 32);
> +	u64_stats_add(&port->stats.tx_ok_bytes, (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_OK_BYTE_CNT_L(port->id));
> -	port->stats.tx_ok_bytes += val;
> +	u64_stats_add(&port->stats.tx_ok_bytes, val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_DROP_CNT(port->id));
> -	port->stats.tx_drops += val;
> +	u64_stats_add(&port->stats.tx_drops, val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_BC_CNT(port->id));
> -	port->stats.tx_broadcast += val;
> +	u64_stats_add(&port->stats.tx_broadcast, val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_MC_CNT(port->id));
> -	port->stats.tx_multicast += val;
> +	u64_stats_add(&port->stats.tx_multicast, val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_RUNT_CNT(port->id));
> -	port->stats.tx_len[i] += val;
> +	u64_stats_add(&port->stats.tx_len[i], val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_E64_CNT_H(port->id));
> -	port->stats.tx_len[i] += ((u64)val << 32);
> +	u64_stats_add(&port->stats.tx_len[i], (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_E64_CNT_L(port->id));
> -	port->stats.tx_len[i++] += val;
> +	u64_stats_add(&port->stats.tx_len[i++], val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L64_CNT_H(port->id));
> -	port->stats.tx_len[i] += ((u64)val << 32);
> +	u64_stats_add(&port->stats.tx_len[i], (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L64_CNT_L(port->id));
> -	port->stats.tx_len[i++] += val;
> +	u64_stats_add(&port->stats.tx_len[i++], val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L127_CNT_H(port->id));
> -	port->stats.tx_len[i] += ((u64)val << 32);
> +	u64_stats_add(&port->stats.tx_len[i], (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L127_CNT_L(port->id));
> -	port->stats.tx_len[i++] += val;
> +	u64_stats_add(&port->stats.tx_len[i++], val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L255_CNT_H(port->id));
> -	port->stats.tx_len[i] += ((u64)val << 32);
> +	u64_stats_add(&port->stats.tx_len[i], (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L255_CNT_L(port->id));
> -	port->stats.tx_len[i++] += val;
> +	u64_stats_add(&port->stats.tx_len[i++], val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L511_CNT_H(port->id));
> -	port->stats.tx_len[i] += ((u64)val << 32);
> +	u64_stats_add(&port->stats.tx_len[i], (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L511_CNT_L(port->id));
> -	port->stats.tx_len[i++] += val;
> +	u64_stats_add(&port->stats.tx_len[i++], val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L1023_CNT_H(port->id));
> -	port->stats.tx_len[i] += ((u64)val << 32);
> +	u64_stats_add(&port->stats.tx_len[i], (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_L1023_CNT_L(port->id));
> -	port->stats.tx_len[i++] += val;
> +	u64_stats_add(&port->stats.tx_len[i++], val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_TX_ETH_LONG_CNT(port->id));
> -	port->stats.tx_len[i++] += val;
> +	u64_stats_add(&port->stats.tx_len[i++], val);
>  
>  	/* RX */
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_OK_PKT_CNT_H(port->id));
> -	port->stats.rx_ok_pkts += ((u64)val << 32);
> +	u64_stats_add(&port->stats.rx_ok_pkts, (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_OK_PKT_CNT_L(port->id));
> -	port->stats.rx_ok_pkts += val;
> +	u64_stats_add(&port->stats.rx_ok_pkts, val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_OK_BYTE_CNT_H(port->id));
> -	port->stats.rx_ok_bytes += ((u64)val << 32);
> +	u64_stats_add(&port->stats.rx_ok_bytes, (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_OK_BYTE_CNT_L(port->id));
> -	port->stats.rx_ok_bytes += val;
> +	u64_stats_add(&port->stats.rx_ok_bytes, val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_DROP_CNT(port->id));
> -	port->stats.rx_drops += val;
> +	u64_stats_add(&port->stats.rx_drops, val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_BC_CNT(port->id));
> -	port->stats.rx_broadcast += val;
> +	u64_stats_add(&port->stats.rx_broadcast, val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_MC_CNT(port->id));
> -	port->stats.rx_multicast += val;
> +	u64_stats_add(&port->stats.rx_multicast, val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ERROR_DROP_CNT(port->id));
> -	port->stats.rx_errors += val;
> +	u64_stats_add(&port->stats.rx_errors, val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_CRC_ERR_CNT(port->id));
> -	port->stats.rx_crc_error += val;
> +	u64_stats_add(&port->stats.rx_crc_error, val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_OVERFLOW_DROP_CNT(port->id));
> -	port->stats.rx_over_errors += val;
> +	u64_stats_add(&port->stats.rx_over_errors, val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_FRAG_CNT(port->id));
> -	port->stats.rx_fragment += val;
> +	u64_stats_add(&port->stats.rx_fragment, val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_JABBER_CNT(port->id));
> -	port->stats.rx_jabber += val;
> +	u64_stats_add(&port->stats.rx_jabber, val);
>  
>  	i = 0;
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_RUNT_CNT(port->id));
> -	port->stats.rx_len[i] += val;
> +	u64_stats_add(&port->stats.rx_len[i], val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_E64_CNT_H(port->id));
> -	port->stats.rx_len[i] += ((u64)val << 32);
> +	u64_stats_add(&port->stats.rx_len[i], (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_E64_CNT_L(port->id));
> -	port->stats.rx_len[i++] += val;
> +	u64_stats_add(&port->stats.rx_len[i++], val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L64_CNT_H(port->id));
> -	port->stats.rx_len[i] += ((u64)val << 32);
> +	u64_stats_add(&port->stats.rx_len[i], (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L64_CNT_L(port->id));
> -	port->stats.rx_len[i++] += val;
> +	u64_stats_add(&port->stats.rx_len[i++], val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L127_CNT_H(port->id));
> -	port->stats.rx_len[i] += ((u64)val << 32);
> +	u64_stats_add(&port->stats.rx_len[i], (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L127_CNT_L(port->id));
> -	port->stats.rx_len[i++] += val;
> +	u64_stats_add(&port->stats.rx_len[i++], val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L255_CNT_H(port->id));
> -	port->stats.rx_len[i] += ((u64)val << 32);
> +	u64_stats_add(&port->stats.rx_len[i], (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L255_CNT_L(port->id));
> -	port->stats.rx_len[i++] += val;
> +	u64_stats_add(&port->stats.rx_len[i++], val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L511_CNT_H(port->id));
> -	port->stats.rx_len[i] += ((u64)val << 32);
> +	u64_stats_add(&port->stats.rx_len[i], (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L511_CNT_L(port->id));
> -	port->stats.rx_len[i++] += val;
> +	u64_stats_add(&port->stats.rx_len[i++], val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L1023_CNT_H(port->id));
> -	port->stats.rx_len[i] += ((u64)val << 32);
> +	u64_stats_add(&port->stats.rx_len[i], (u64)val << 32);
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_L1023_CNT_L(port->id));
> -	port->stats.rx_len[i++] += val;
> +	u64_stats_add(&port->stats.rx_len[i++], val);
>  
>  	val = airoha_fe_rr(eth, REG_FE_GDM_RX_ETH_LONG_CNT(port->id));
> -	port->stats.rx_len[i++] += val;
> +	u64_stats_add(&port->stats.rx_len[i++], val);
>  
>  	/* reset mib counters */
>  	airoha_fe_set(eth, REG_FE_GDM_MIB_CLEAR(port->id),
> @@ -1795,16 +1795,16 @@ static void airoha_dev_get_stats64(struct net_device *dev,
>  	airoha_update_hw_stats(port);
>  	do {
>  		start = u64_stats_fetch_begin(&port->stats.syncp);
> -		storage->rx_packets = port->stats.rx_ok_pkts;
> -		storage->tx_packets = port->stats.tx_ok_pkts;
> -		storage->rx_bytes = port->stats.rx_ok_bytes;
> -		storage->tx_bytes = port->stats.tx_ok_bytes;
> -		storage->multicast = port->stats.rx_multicast;
> -		storage->rx_errors = port->stats.rx_errors;
> -		storage->rx_dropped = port->stats.rx_drops;
> -		storage->tx_dropped = port->stats.tx_drops;
> -		storage->rx_crc_errors = port->stats.rx_crc_error;
> -		storage->rx_over_errors = port->stats.rx_over_errors;
> +		storage->rx_packets = u64_stats_read(&port->stats.rx_ok_pkts);
> +		storage->tx_packets = u64_stats_read(&port->stats.tx_ok_pkts);
> +		storage->rx_bytes = u64_stats_read(&port->stats.rx_ok_bytes);
> +		storage->tx_bytes = u64_stats_read(&port->stats.tx_ok_bytes);
> +		storage->multicast = u64_stats_read(&port->stats.rx_multicast);
> +		storage->rx_errors = u64_stats_read(&port->stats.rx_errors);
> +		storage->rx_dropped = u64_stats_read(&port->stats.rx_drops);
> +		storage->tx_dropped = u64_stats_read(&port->stats.tx_drops);
> +		storage->rx_crc_errors = u64_stats_read(&port->stats.rx_crc_error);
> +		storage->rx_over_errors = u64_stats_read(&port->stats.rx_over_errors);
>  	} while (u64_stats_fetch_retry(&port->stats.syncp, start));
>  }
>  
> @@ -2057,13 +2057,13 @@ static void airoha_ethtool_get_mac_stats(struct net_device *dev,
>  	airoha_update_hw_stats(port);
>  	do {
>  		start = u64_stats_fetch_begin(&port->stats.syncp);
> -		stats->FramesTransmittedOK = port->stats.tx_ok_pkts;
> -		stats->OctetsTransmittedOK = port->stats.tx_ok_bytes;
> -		stats->MulticastFramesXmittedOK = port->stats.tx_multicast;
> -		stats->BroadcastFramesXmittedOK = port->stats.tx_broadcast;
> -		stats->FramesReceivedOK = port->stats.rx_ok_pkts;
> -		stats->OctetsReceivedOK = port->stats.rx_ok_bytes;
> -		stats->BroadcastFramesReceivedOK = port->stats.rx_broadcast;
> +		stats->FramesTransmittedOK = u64_stats_read(&port->stats.tx_ok_pkts);
> +		stats->OctetsTransmittedOK = u64_stats_read(&port->stats.tx_ok_bytes);
> +		stats->MulticastFramesXmittedOK = u64_stats_read(&port->stats.tx_multicast);
> +		stats->BroadcastFramesXmittedOK = u64_stats_read(&port->stats.tx_broadcast);
> +		stats->FramesReceivedOK = u64_stats_read(&port->stats.rx_ok_pkts);
> +		stats->OctetsReceivedOK = u64_stats_read(&port->stats.rx_ok_bytes);
> +		stats->BroadcastFramesReceivedOK = u64_stats_read(&port->stats.rx_broadcast);
>  	} while (u64_stats_fetch_retry(&port->stats.syncp, start));
>  }
>  
> @@ -2098,12 +2098,12 @@ airoha_ethtool_get_rmon_stats(struct net_device *dev,
>  		int i;
>  
>  		start = u64_stats_fetch_begin(&port->stats.syncp);
> -		stats->fragments = hw_stats->rx_fragment;
> -		stats->jabbers = hw_stats->rx_jabber;
> +		stats->fragments = u64_stats_read(&hw_stats->rx_fragment);
> +		stats->jabbers = u64_stats_read(&hw_stats->rx_jabber);
>  		for (i = 0; i < ARRAY_SIZE(airoha_ethtool_rmon_ranges) - 1;
>  		     i++) {
> -			stats->hist[i] = hw_stats->rx_len[i];
> -			stats->hist_tx[i] = hw_stats->tx_len[i];
> +			stats->hist[i] = u64_stats_read(&hw_stats->rx_len[i]);
> +			stats->hist_tx[i] = u64_stats_read(&hw_stats->tx_len[i]);
>  		}
>  	} while (u64_stats_fetch_retry(&port->stats.syncp, start));
>  }
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
> index 20e602d61e61..b2e9bd849f8b 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.h
> +++ b/drivers/net/ethernet/airoha/airoha_eth.h
> @@ -215,24 +215,24 @@ struct airoha_hw_stats {
>  	struct u64_stats_sync syncp;
>  
>  	/* get_stats64 */
> -	u64 rx_ok_pkts;
> -	u64 tx_ok_pkts;
> -	u64 rx_ok_bytes;
> -	u64 tx_ok_bytes;
> -	u64 rx_multicast;
> -	u64 rx_errors;
> -	u64 rx_drops;
> -	u64 tx_drops;
> -	u64 rx_crc_error;
> -	u64 rx_over_errors;
> +	u64_stats_t rx_ok_pkts;
> +	u64_stats_t tx_ok_pkts;
> +	u64_stats_t rx_ok_bytes;
> +	u64_stats_t tx_ok_bytes;
> +	u64_stats_t rx_multicast;
> +	u64_stats_t rx_errors;
> +	u64_stats_t rx_drops;
> +	u64_stats_t tx_drops;
> +	u64_stats_t rx_crc_error;
> +	u64_stats_t rx_over_errors;
>  	/* ethtool stats */
> -	u64 tx_broadcast;
> -	u64 tx_multicast;
> -	u64 tx_len[7];
> -	u64 rx_broadcast;
> -	u64 rx_fragment;
> -	u64 rx_jabber;
> -	u64 rx_len[7];
> +	u64_stats_t tx_broadcast;
> +	u64_stats_t tx_multicast;
> +	u64_stats_t tx_len[7];
> +	u64_stats_t rx_broadcast;
> +	u64_stats_t rx_fragment;
> +	u64_stats_t rx_jabber;
> +	u64_stats_t rx_len[7];
>  };
>  
>  enum {
> -- 
> 2.51.0
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.