public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: David Yang <mmyangfl@gmail.com>
To: netdev@vger.kernel.org
Cc: David Yang <mmyangfl@gmail.com>,
	Florian Fainelli <florian.fainelli@broadcom.com>,
	Broadcom internal kernel review list
	<bcm-kernel-feedback-list@broadcom.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: [RFC net-next] net: systemport: Use u64_stats_t with u64_stats_sync properly
Date: Fri, 23 Jan 2026 04:15:41 +0800	[thread overview]
Message-ID: <20260122201545.2792428-1-mmyangfl@gmail.com> (raw)

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>
---
RFC Comment:

I couldn't find the lock associated with u64_stats_sync. Should this be
considered an issue?
 drivers/net/ethernet/broadcom/bcmsysport.c | 56 ++++++++++++----------
 drivers/net/ethernet/broadcom/bcmsysport.h | 12 ++---
 2 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index bc4e1f3b3752..7017b2d2a1b5 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -435,8 +435,8 @@ static void bcm_sysport_update_tx_stats(struct bcm_sysport_priv *priv,
 		ring = &priv->tx_rings[q];
 		do {
 			start = u64_stats_fetch_begin(&priv->syncp);
-			bytes = ring->bytes;
-			packets = ring->packets;
+			bytes = u64_stats_read(&ring->bytes);
+			packets = u64_stats_read(&ring->packets);
 		} while (u64_stats_fetch_retry(&priv->syncp, start));
 
 		*tx_bytes += bytes;
@@ -458,8 +458,10 @@ static void bcm_sysport_get_stats(struct net_device *dev,
 	if (netif_running(dev)) {
 		bcm_sysport_update_mib_counters(priv);
 		bcm_sysport_update_tx_stats(priv, &tx_bytes, &tx_packets);
-		stats64->tx_bytes = tx_bytes;
-		stats64->tx_packets = tx_packets;
+		u64_stats_update_begin(&priv->syncp);
+		u64_stats_set(&stats64->tx_bytes, tx_bytes);
+		u64_stats_set(&stats64->tx_packets, tx_packets);
+		u64_stats_update_end(&priv->syncp);
 	}
 
 	for (i =  0, j = 0; i < BCM_SYSPORT_STATS_LEN; i++) {
@@ -482,28 +484,32 @@ static void bcm_sysport_get_stats(struct net_device *dev,
 		    s->type == BCM_SYSPORT_STAT_NETDEV64) {
 			do {
 				start = u64_stats_fetch_begin(syncp);
-				data[i] = *(u64 *)p;
+				data[i] = u64_stats_read((u64_stats_t *)p);
 			} while (u64_stats_fetch_retry(syncp, start));
 		} else
 			data[i] = *(u32 *)p;
 		j++;
 	}
 
-	/* For SYSTEMPORT Lite since we have holes in our statistics, j would
-	 * be equal to BCM_SYSPORT_STATS_LEN at the end of the loop, but it
-	 * needs to point to how many total statistics we have minus the
-	 * number of per TX queue statistics
-	 */
-	j = bcm_sysport_get_sset_count(dev, ETH_SS_STATS) -
-	    dev->num_tx_queues * NUM_SYSPORT_TXQ_STAT;
+	do {
+		/* For SYSTEMPORT Lite since we have holes in our statistics, j
+		 * would be equal to BCM_SYSPORT_STATS_LEN at the end of the
+		 * loop, but it needs to point to how many total statistics we
+		 * have minus the number of per TX queue statistics
+		 */
+		j = bcm_sysport_get_sset_count(dev, ETH_SS_STATS) -
+		    dev->num_tx_queues * NUM_SYSPORT_TXQ_STAT;
 
-	for (i = 0; i < dev->num_tx_queues; i++) {
-		ring = &priv->tx_rings[i];
-		data[j] = ring->packets;
-		j++;
-		data[j] = ring->bytes;
-		j++;
-	}
+		start = u64_stats_fetch_begin(syncp);
+
+		for (i = 0; i < dev->num_tx_queues; i++) {
+			ring = &priv->tx_rings[i];
+			data[j] = u64_stats_read(&ring->packets);
+			j++;
+			data[j] = u64_stats_read(&ring->bytes);
+			j++;
+		}
+	} while (u64_stats_fetch_retry(syncp, start));
 }
 
 static void bcm_sysport_get_wol(struct net_device *dev,
@@ -829,8 +835,8 @@ static unsigned int bcm_sysport_desc_rx(struct bcm_sysport_priv *priv,
 		ndev->stats.rx_packets++;
 		ndev->stats.rx_bytes += len;
 		u64_stats_update_begin(&priv->syncp);
-		stats64->rx_packets++;
-		stats64->rx_bytes += len;
+		u64_stats_inc(&stats64->rx_packets);
+		u64_stats_add(&stats64->rx_bytes, len);
 		u64_stats_update_end(&priv->syncp);
 
 		napi_gro_receive(&priv->napi, skb);
@@ -914,8 +920,8 @@ static unsigned int __bcm_sysport_tx_reclaim(struct bcm_sysport_priv *priv,
 	}
 
 	u64_stats_update_begin(&priv->syncp);
-	ring->packets += pkts_compl;
-	ring->bytes += bytes_compl;
+	u64_stats_add(&ring->packets, pkts_compl);
+	u64_stats_add(&ring->bytes, bytes_compl);
 	u64_stats_update_end(&priv->syncp);
 
 	ring->c_index = c_index;
@@ -1857,8 +1863,8 @@ static void bcm_sysport_get_stats64(struct net_device *dev,
 
 	do {
 		start = u64_stats_fetch_begin(&priv->syncp);
-		stats->rx_packets = stats64->rx_packets;
-		stats->rx_bytes = stats64->rx_bytes;
+		stats->rx_packets = u64_stats_read(&stats64->rx_packets);
+		stats->rx_bytes = u64_stats_read(&stats64->rx_bytes);
 	} while (u64_stats_fetch_retry(&priv->syncp, start));
 }
 
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
index a34296f989f1..91b1c8293a23 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -652,10 +652,10 @@ struct bcm_sysport_stats {
 
 struct bcm_sysport_stats64 {
 	/* 64bit stats on 32bit/64bit Machine */
-	u64	rx_packets;
-	u64	rx_bytes;
-	u64	tx_packets;
-	u64	tx_bytes;
+	u64_stats_t	rx_packets;
+	u64_stats_t	rx_bytes;
+	u64_stats_t	tx_packets;
+	u64_stats_t	tx_bytes;
 };
 
 /* Software house keeping helper structure */
@@ -698,8 +698,8 @@ struct bcm_sysport_tx_ring {
 	unsigned int	clean_index;	/* Current clean index */
 	struct bcm_sysport_cb *cbs;	/* Transmit control blocks */
 	struct bcm_sysport_priv *priv;	/* private context backpointer */
-	unsigned long	packets;	/* packets statistics */
-	unsigned long	bytes;		/* bytes statistics */
+	u64_stats_t	packets;	/* packets statistics */
+	u64_stats_t	bytes;		/* bytes statistics */
 	unsigned int	switch_queue;	/* switch port queue number */
 	unsigned int	switch_port;	/* switch port queue number */
 	bool		inspect;	/* inspect switch port and queue */
-- 
2.51.0


             reply	other threads:[~2026-01-22 20:16 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-22 20:15 David Yang [this message]
2026-01-26 19:17 ` [RFC net-next] net: systemport: Use u64_stats_t with u64_stats_sync properly Vadim Fedorenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260122201545.2792428-1-mmyangfl@gmail.com \
    --to=mmyangfl@gmail.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=florian.fainelli@broadcom.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox