[PATCH net-next 0/3] net: dsa: yt921x: Use u64_stats_t for MIB stats

[PATCH net-next 0/3] net: dsa: yt921x: Use u64_stats_t for MIB stats

[David Yang]

Avoid load/store tearing of 64-bit values on 32-bit architectures.

v0:
  - Sending remaining part of
    https://lore.kernel.org/r/20260118013019.1078847-1-mmyangfl@gmail.com

David Yang (3):
  net: dsa: yt921x: Refactor port error message
  net: dsa: yt921x: Return early for failed MIB read
  net: dsa: yt921x: Use u64_stats_t for MIB stats

 drivers/net/dsa/yt921x.c | 270 +++++++++++++++++++++++----------------
 drivers/net/dsa/yt921x.h | 114 +++++++++--------
 2 files changed, 221 insertions(+), 163 deletions(-)

-- 
2.51.0

[PATCH net-next 1/3] net: dsa: yt921x: Refactor port error message

[David Yang]

Sometimes we might get error from the underlying bus, but the return
type of the function is void, so an error message is desired. There are
many instances of the same pattern, refactor it.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/net/dsa/yt921x.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
index 98e8915dd6c2..ce2878ef9b24 100644
--- a/drivers/net/dsa/yt921x.c
+++ b/drivers/net/dsa/yt921x.c
@@ -185,6 +185,9 @@ struct yt921x_reg_mdio {
 #define to_yt921x_priv(_ds) container_of_const(_ds, struct yt921x_priv, ds)
 #define to_device(priv) ((priv)->ds.dev)
 
+#define print_port_err(dev, port, res, action) dev_err( \
+	(dev), "Failed to %s port %d: %i\n", (action), (port), (res))
+
 static int yt921x_reg_read(struct yt921x_priv *priv, u32 reg, u32 *valp)
 {
 	WARN_ON(!mutex_is_locked(&priv->reg_lock));
@@ -721,8 +724,7 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
 			mib->tx_jumbo;
 
 	if (res)
-		dev_err(dev, "Failed to %s port %d: %i\n", "read stats for",
-			port, res);
+		print_port_err(dev, port, res, "read stats for");
 	return res;
 }
 
@@ -1102,8 +1104,7 @@ yt921x_dsa_port_mirror_del(struct dsa_switch *ds, int port,
 	mutex_unlock(&priv->reg_lock);
 
 	if (res)
-		dev_err(dev, "Failed to %s port %d: %i\n", "unmirror",
-			port, res);
+		print_port_err(dev, port, res, "unmirror");
 }
 
 static int
@@ -1690,8 +1691,7 @@ static void yt921x_dsa_port_fast_age(struct dsa_switch *ds, int port)
 	mutex_unlock(&priv->reg_lock);
 
 	if (res)
-		dev_err(dev, "Failed to %s port %d: %i\n", "clear FDB for",
-			port, res);
+		print_port_err(dev, port, res, "clear FDB for");
 }
 
 static int
@@ -2266,8 +2266,7 @@ yt921x_dsa_port_bridge_leave(struct dsa_switch *ds, int port,
 	mutex_unlock(&priv->reg_lock);
 
 	if (res)
-		dev_err(dev, "Failed to %s port %d: %i\n", "unbridge",
-			port, res);
+		print_port_err(dev, port, res, "unbridge");
 }
 
 static int
@@ -2399,8 +2398,7 @@ yt921x_dsa_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
 	mutex_unlock(&priv->reg_lock);
 
 	if (res)
-		dev_err(dev, "Failed to %s port %d: %i\n", "set STP state for",
-			port, res);
+		print_port_err(dev, port, res, "set STP state for");
 }
 
 static int __maybe_unused
@@ -2738,8 +2736,7 @@ yt921x_phylink_mac_link_down(struct phylink_config *config, unsigned int mode,
 	mutex_unlock(&priv->reg_lock);
 
 	if (res)
-		dev_err(dp->ds->dev, "Failed to %s port %d: %i\n", "bring down",
-			port, res);
+		print_port_err(dp->ds->dev, port, res, "bring down");
 }
 
 static void
@@ -2759,8 +2756,7 @@ yt921x_phylink_mac_link_up(struct phylink_config *config,
 	mutex_unlock(&priv->reg_lock);
 
 	if (res)
-		dev_err(dp->ds->dev, "Failed to %s port %d: %i\n", "bring up",
-			port, res);
+		print_port_err(dp->ds->dev, port, res, "bring up");
 
 	schedule_delayed_work(&priv->ports[port].mib_read, 0);
 }
@@ -2779,8 +2775,7 @@ yt921x_phylink_mac_config(struct phylink_config *config, unsigned int mode,
 	mutex_unlock(&priv->reg_lock);
 
 	if (res)
-		dev_err(dp->ds->dev, "Failed to %s port %d: %i\n", "config",
-			port, res);
+		print_port_err(dp->ds->dev, port, res, "config");
 }
 
 static void
-- 
2.51.0

Re: [PATCH net-next 1/3] net: dsa: yt921x: Refactor port error message

[Jakub Kicinski]

On Wed,  4 Feb 2026 01:12:41 +0800 David Yang wrote:
> +#define print_port_err(dev, port, res, action) dev_err( \
> +	(dev), "Failed to %s port %d: %i\n", (action), (port), (res))

functions are preferred over macros when possible per kernel coding
guide
 

[PATCH net-next 2/3] net: dsa: yt921x: Return early for failed MIB read

[David Yang]

This patch does not change anything effectively, but serves as a
prerequisite for another patch.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/net/dsa/yt921x.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
index ce2878ef9b24..b9f507b07d1f 100644
--- a/drivers/net/dsa/yt921x.c
+++ b/drivers/net/dsa/yt921x.c
@@ -714,6 +714,11 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
 		WRITE_ONCE(*valp, val);
 	}
 
+	if (res) {
+		print_port_err(dev, port, res, "read stats for");
+		return res;
+	}
+
 	pp->rx_frames = mib->rx_64byte + mib->rx_65_127byte +
 			mib->rx_128_255byte + mib->rx_256_511byte +
 			mib->rx_512_1023byte + mib->rx_1024_1518byte +
@@ -723,9 +728,7 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
 			mib->tx_512_1023byte + mib->tx_1024_1518byte +
 			mib->tx_jumbo;
 
-	if (res)
-		print_port_err(dev, port, res, "read stats for");
-	return res;
+	return 0;
 }
 
 static void yt921x_poll_mib(struct work_struct *work)
-- 
2.51.0

Re: [net-next,2/3] net: dsa: yt921x: Return early for failed MIB read

[Jakub Kicinski]

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: dsa: yt921x: Return early for failed MIB read

This patch refactors error handling in yt921x_read_mib() by checking
for read failures earlier and returning immediately, before attempting
to compute statistics from potentially incomplete data.

> net: dsa: yt921x: Return early for failed MIB read
>
> This patch does not change anything effectively, but serves as a
> prerequisite for another patch.

The claim "This patch does not change anything effectively" is not
accurate. The patch changes the behavior when MIB register reads fail.

Before the patch: If yt921x_reg_read() fails partway through the loop,
the function would still compute pp->rx_frames and pp->tx_frames using
the partially-read MIB values before returning the error code. The frame
counts would contain a mix of old values (for registers read before the
error) and new values (for registers read successfully).

After the patch: If yt921x_reg_read() fails, the function returns
immediately without updating pp->rx_frames and pp->tx_frames, leaving
them at their previous values from the last successful read.

This affects how five callers behave when reads fail:
  - yt921x_dsa_get_pause_stats()
  - yt921x_dsa_get_eth_mac_stats()
  - yt921x_dsa_get_rmon_stats()
  - yt921x_dsa_get_eth_ctrl_stats()
  - yt921x_dsa_get_ethtool_stats()

These functions don't check the return value, so they will now report
stale data from the last successful read instead of partially-updated
data from a failed read.

Should the commit message be updated to accurately describe this
behavior change?

> diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
> --- a/drivers/net/dsa/yt921x.c
> +++ b/drivers/net/dsa/yt921x.c
> @@ -714,6 +714,11 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
>  		WRITE_ONCE(*valp, val);
>  	}
>
> +	if (res) {
> +		print_port_err(dev, port, res, "read stats for");
> +		return res;
> +	}
> +
>  	pp->rx_frames = mib->rx_64byte + mib->rx_65_127byte +
>  			mib->rx_128_255byte + mib->rx_256_511byte +
>  			mib->rx_512_1023byte + mib->rx_1024_1518byte +
> @@ -723,9 +728,7 @@
>  			mib->tx_512_1023byte + mib->tx_1024_1518byte +
>  			mib->tx_jumbo;
>
> -	if (res)
> -		print_port_err(dev, port, res, "read stats for");
> -	return res;
> +	return 0;
>  }
-- 
pw-bot: cr

[PATCH net-next 3/3] net: dsa: yt921x: Use u64_stats_t for MIB stats

[David Yang]

64-bit variables might not be atomic on 32-bit architectures, thus
cannot be unconditionally made lock-free. Use u64_stats_t so it would
still be lock-free on 64-bit architectures.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/net/dsa/yt921x.c | 236 +++++++++++++++++++++++----------------
 drivers/net/dsa/yt921x.h | 114 ++++++++++---------
 2 files changed, 205 insertions(+), 145 deletions(-)

diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
index b9f507b07d1f..a7b1a21204f1 100644
--- a/drivers/net/dsa/yt921x.c
+++ b/drivers/net/dsa/yt921x.c
@@ -20,6 +20,7 @@
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
 #include <linux/sort.h>
+#include <linux/u64_stats_sync.h>
 
 #include <net/dsa.h>
 #include <net/dscp.h>
@@ -673,22 +674,20 @@ yt921x_mbus_ext_init(struct yt921x_priv *priv, struct device_node *mnp)
 static int yt921x_read_mib(struct yt921x_priv *priv, int port)
 {
 	struct yt921x_port *pp = &priv->ports[port];
+	struct yt921x_mib *mib_new = &pp->mib_new;
 	struct device *dev = to_device(priv);
 	struct yt921x_mib *mib = &pp->mib;
+	u64 rx_frames;
+	u64 tx_frames;
 	int res = 0;
 
-	/* Reading of yt921x_port::mib is not protected by a lock and it's vain
-	 * to keep its consistency, since we have to read registers one by one
-	 * and there is no way to make a snapshot of MIB stats.
-	 *
-	 * Writing (by this function only) is and should be protected by
-	 * reg_lock.
+	/* u64_stats_read/set is redundant for mib_new, but I don't want to
+	 * declare a plain u64 yt921x_mib variant.
 	 */
 
 	for (size_t i = 0; i < ARRAY_SIZE(yt921x_mib_descs); i++) {
 		const struct yt921x_mib_desc *desc = &yt921x_mib_descs[i];
 		u32 reg = YT921X_MIBn_DATA0(port) + desc->offset;
-		u64 *valp = &((u64 *)mib)[i];
 		u32 val0;
 		u64 val;
 
@@ -697,7 +696,7 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
 			break;
 
 		if (desc->size <= 1) {
-			u64 old_val = *valp;
+			u64 old_val = u64_stats_read(&mib->stats[i]);
 
 			val = (old_val & ~(u64)U32_MAX) | val0;
 			if (val < old_val)
@@ -711,7 +710,7 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
 			val = ((u64)val1 << 32) | val0;
 		}
 
-		WRITE_ONCE(*valp, val);
+		u64_stats_set(&mib_new->stats[i], val);
 	}
 
 	if (res) {
@@ -719,14 +718,29 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
 		return res;
 	}
 
-	pp->rx_frames = mib->rx_64byte + mib->rx_65_127byte +
-			mib->rx_128_255byte + mib->rx_256_511byte +
-			mib->rx_512_1023byte + mib->rx_1024_1518byte +
-			mib->rx_jumbo;
-	pp->tx_frames = mib->tx_64byte + mib->tx_65_127byte +
-			mib->tx_128_255byte + mib->tx_256_511byte +
-			mib->tx_512_1023byte + mib->tx_1024_1518byte +
-			mib->tx_jumbo;
+	rx_frames = u64_stats_read(&mib_new->rx_64byte) +
+		    u64_stats_read(&mib_new->rx_65_127byte) +
+		    u64_stats_read(&mib_new->rx_128_255byte) +
+		    u64_stats_read(&mib_new->rx_256_511byte) +
+		    u64_stats_read(&mib_new->rx_512_1023byte) +
+		    u64_stats_read(&mib_new->rx_1024_1518byte) +
+		    u64_stats_read(&mib_new->rx_jumbo);
+	tx_frames = u64_stats_read(&mib_new->tx_64byte) +
+		    u64_stats_read(&mib_new->tx_65_127byte) +
+		    u64_stats_read(&mib_new->tx_128_255byte) +
+		    u64_stats_read(&mib_new->tx_256_511byte) +
+		    u64_stats_read(&mib_new->tx_512_1023byte) +
+		    u64_stats_read(&mib_new->tx_1024_1518byte) +
+		    u64_stats_read(&mib_new->tx_jumbo);
+
+	u64_stats_update_begin(&pp->syncp);
+	for (size_t i = 0; i < ARRAY_SIZE(yt921x_mib_descs); i++) {
+		u64_stats_set(&mib->stats[i],
+			      u64_stats_read(&mib_new->stats[i]));
+	}
+	u64_stats_set(&pp->rx_frames, rx_frames);
+	u64_stats_set(&pp->tx_frames, tx_frames);
+	u64_stats_update_end(&pp->syncp);
 
 	return 0;
 }
@@ -771,22 +785,27 @@ yt921x_dsa_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data)
 	struct yt921x_priv *priv = to_yt921x_priv(ds);
 	struct yt921x_port *pp = &priv->ports[port];
 	struct yt921x_mib *mib = &pp->mib;
+	unsigned int start;
 	size_t j;
 
 	mutex_lock(&priv->reg_lock);
 	yt921x_read_mib(priv, port);
 	mutex_unlock(&priv->reg_lock);
 
-	j = 0;
-	for (size_t i = 0; i < ARRAY_SIZE(yt921x_mib_descs); i++) {
-		const struct yt921x_mib_desc *desc = &yt921x_mib_descs[i];
+	do {
+		start = u64_stats_fetch_begin(&pp->syncp);
 
-		if (!desc->name)
-			continue;
+		j = 0;
+		for (size_t i = 0; i < ARRAY_SIZE(yt921x_mib_descs); i++) {
+			const struct yt921x_mib_desc *desc = &yt921x_mib_descs[i];
 
-		data[j] = ((u64 *)mib)[i];
-		j++;
-	}
+			if (!desc->name)
+				continue;
+
+			data[j] = u64_stats_read(&((u64_stats_t *)mib)[i]);
+			j++;
+		}
+	} while (u64_stats_fetch_retry(&pp->syncp, start));
 }
 
 static int yt921x_dsa_get_sset_count(struct dsa_switch *ds, int port, int sset)
@@ -813,33 +832,38 @@ yt921x_dsa_get_eth_mac_stats(struct dsa_switch *ds, int port,
 	struct yt921x_priv *priv = to_yt921x_priv(ds);
 	struct yt921x_port *pp = &priv->ports[port];
 	struct yt921x_mib *mib = &pp->mib;
+	unsigned int start;
 
 	mutex_lock(&priv->reg_lock);
 	yt921x_read_mib(priv, port);
 	mutex_unlock(&priv->reg_lock);
 
-	mac_stats->FramesTransmittedOK = pp->tx_frames;
-	mac_stats->SingleCollisionFrames = mib->tx_single_collisions;
-	mac_stats->MultipleCollisionFrames = mib->tx_multiple_collisions;
-	mac_stats->FramesReceivedOK = pp->rx_frames;
-	mac_stats->FrameCheckSequenceErrors = mib->rx_crc_errors;
-	mac_stats->AlignmentErrors = mib->rx_alignment_errors;
-	mac_stats->OctetsTransmittedOK = mib->tx_good_bytes;
-	mac_stats->FramesWithDeferredXmissions = mib->tx_deferred;
-	mac_stats->LateCollisions = mib->tx_late_collisions;
-	mac_stats->FramesAbortedDueToXSColls = mib->tx_aborted_errors;
-	/* mac_stats->FramesLostDueToIntMACXmitError */
-	/* mac_stats->CarrierSenseErrors */
-	mac_stats->OctetsReceivedOK = mib->rx_good_bytes;
-	/* mac_stats->FramesLostDueToIntMACRcvError */
-	mac_stats->MulticastFramesXmittedOK = mib->tx_multicast;
-	mac_stats->BroadcastFramesXmittedOK = mib->tx_broadcast;
-	/* mac_stats->FramesWithExcessiveDeferral */
-	mac_stats->MulticastFramesReceivedOK = mib->rx_multicast;
-	mac_stats->BroadcastFramesReceivedOK = mib->rx_broadcast;
-	/* mac_stats->InRangeLengthErrors */
-	/* mac_stats->OutOfRangeLengthField */
-	mac_stats->FrameTooLongErrors = mib->rx_oversize_errors;
+	do {
+		start = u64_stats_fetch_begin(&pp->syncp);
+
+		mac_stats->FramesTransmittedOK = u64_stats_read(&pp->tx_frames);
+		mac_stats->SingleCollisionFrames = u64_stats_read(&mib->tx_single_collisions);
+		mac_stats->MultipleCollisionFrames = u64_stats_read(&mib->tx_multiple_collisions);
+		mac_stats->FramesReceivedOK = u64_stats_read(&pp->rx_frames);
+		mac_stats->FrameCheckSequenceErrors = u64_stats_read(&mib->rx_crc_errors);
+		mac_stats->AlignmentErrors = u64_stats_read(&mib->rx_alignment_errors);
+		mac_stats->OctetsTransmittedOK = u64_stats_read(&mib->tx_good_bytes);
+		mac_stats->FramesWithDeferredXmissions = u64_stats_read(&mib->tx_deferred);
+		mac_stats->LateCollisions = u64_stats_read(&mib->tx_late_collisions);
+		mac_stats->FramesAbortedDueToXSColls = u64_stats_read(&mib->tx_aborted_errors);
+		/* mac_stats->FramesLostDueToIntMACXmitError */
+		/* mac_stats->CarrierSenseErrors */
+		mac_stats->OctetsReceivedOK = u64_stats_read(&mib->rx_good_bytes);
+		/* mac_stats->FramesLostDueToIntMACRcvError */
+		mac_stats->MulticastFramesXmittedOK = u64_stats_read(&mib->tx_multicast);
+		mac_stats->BroadcastFramesXmittedOK = u64_stats_read(&mib->tx_broadcast);
+		/* mac_stats->FramesWithExcessiveDeferral */
+		mac_stats->MulticastFramesReceivedOK = u64_stats_read(&mib->rx_multicast);
+		mac_stats->BroadcastFramesReceivedOK = u64_stats_read(&mib->rx_broadcast);
+		/* mac_stats->InRangeLengthErrors */
+		/* mac_stats->OutOfRangeLengthField */
+		mac_stats->FrameTooLongErrors = u64_stats_read(&mib->rx_oversize_errors);
+	} while (u64_stats_fetch_retry(&pp->syncp, start));
 }
 
 static void
@@ -849,14 +873,19 @@ yt921x_dsa_get_eth_ctrl_stats(struct dsa_switch *ds, int port,
 	struct yt921x_priv *priv = to_yt921x_priv(ds);
 	struct yt921x_port *pp = &priv->ports[port];
 	struct yt921x_mib *mib = &pp->mib;
+	unsigned int start;
 
 	mutex_lock(&priv->reg_lock);
 	yt921x_read_mib(priv, port);
 	mutex_unlock(&priv->reg_lock);
 
-	ctrl_stats->MACControlFramesTransmitted = mib->tx_pause;
-	ctrl_stats->MACControlFramesReceived = mib->rx_pause;
-	/* ctrl_stats->UnsupportedOpcodesReceived */
+	do {
+		start = u64_stats_fetch_begin(&pp->syncp);
+
+		ctrl_stats->MACControlFramesTransmitted = u64_stats_read(&mib->tx_pause);
+		ctrl_stats->MACControlFramesReceived = u64_stats_read(&mib->rx_pause);
+		/* ctrl_stats->UnsupportedOpcodesReceived */
+	} while (u64_stats_fetch_retry(&pp->syncp, start));
 }
 
 static const struct ethtool_rmon_hist_range yt921x_rmon_ranges[] = {
@@ -878,6 +907,7 @@ yt921x_dsa_get_rmon_stats(struct dsa_switch *ds, int port,
 	struct yt921x_priv *priv = to_yt921x_priv(ds);
 	struct yt921x_port *pp = &priv->ports[port];
 	struct yt921x_mib *mib = &pp->mib;
+	unsigned int start;
 
 	mutex_lock(&priv->reg_lock);
 	yt921x_read_mib(priv, port);
@@ -885,26 +915,30 @@ yt921x_dsa_get_rmon_stats(struct dsa_switch *ds, int port,
 
 	*ranges = yt921x_rmon_ranges;
 
-	rmon_stats->undersize_pkts = mib->rx_undersize_errors;
-	rmon_stats->oversize_pkts = mib->rx_oversize_errors;
-	rmon_stats->fragments = mib->rx_alignment_errors;
-	/* rmon_stats->jabbers */
-
-	rmon_stats->hist[0] = mib->rx_64byte;
-	rmon_stats->hist[1] = mib->rx_65_127byte;
-	rmon_stats->hist[2] = mib->rx_128_255byte;
-	rmon_stats->hist[3] = mib->rx_256_511byte;
-	rmon_stats->hist[4] = mib->rx_512_1023byte;
-	rmon_stats->hist[5] = mib->rx_1024_1518byte;
-	rmon_stats->hist[6] = mib->rx_jumbo;
-
-	rmon_stats->hist_tx[0] = mib->tx_64byte;
-	rmon_stats->hist_tx[1] = mib->tx_65_127byte;
-	rmon_stats->hist_tx[2] = mib->tx_128_255byte;
-	rmon_stats->hist_tx[3] = mib->tx_256_511byte;
-	rmon_stats->hist_tx[4] = mib->tx_512_1023byte;
-	rmon_stats->hist_tx[5] = mib->tx_1024_1518byte;
-	rmon_stats->hist_tx[6] = mib->tx_jumbo;
+	do {
+		start = u64_stats_fetch_begin(&pp->syncp);
+
+		rmon_stats->undersize_pkts = u64_stats_read(&mib->rx_undersize_errors);
+		rmon_stats->oversize_pkts = u64_stats_read(&mib->rx_oversize_errors);
+		rmon_stats->fragments = u64_stats_read(&mib->rx_alignment_errors);
+		/* rmon_stats->jabbers */
+
+		rmon_stats->hist[0] = u64_stats_read(&mib->rx_64byte);
+		rmon_stats->hist[1] = u64_stats_read(&mib->rx_65_127byte);
+		rmon_stats->hist[2] = u64_stats_read(&mib->rx_128_255byte);
+		rmon_stats->hist[3] = u64_stats_read(&mib->rx_256_511byte);
+		rmon_stats->hist[4] = u64_stats_read(&mib->rx_512_1023byte);
+		rmon_stats->hist[5] = u64_stats_read(&mib->rx_1024_1518byte);
+		rmon_stats->hist[6] = u64_stats_read(&mib->rx_jumbo);
+
+		rmon_stats->hist_tx[0] = u64_stats_read(&mib->tx_64byte);
+		rmon_stats->hist_tx[1] = u64_stats_read(&mib->tx_65_127byte);
+		rmon_stats->hist_tx[2] = u64_stats_read(&mib->tx_128_255byte);
+		rmon_stats->hist_tx[3] = u64_stats_read(&mib->tx_256_511byte);
+		rmon_stats->hist_tx[4] = u64_stats_read(&mib->tx_512_1023byte);
+		rmon_stats->hist_tx[5] = u64_stats_read(&mib->tx_1024_1518byte);
+		rmon_stats->hist_tx[6] = u64_stats_read(&mib->tx_jumbo);
+	} while (u64_stats_fetch_retry(&pp->syncp, start));
 }
 
 static void
@@ -914,33 +948,41 @@ yt921x_dsa_get_stats64(struct dsa_switch *ds, int port,
 	struct yt921x_priv *priv = to_yt921x_priv(ds);
 	struct yt921x_port *pp = &priv->ports[port];
 	struct yt921x_mib *mib = &pp->mib;
+	unsigned int start;
+
+	do {
+		start = u64_stats_fetch_begin(&pp->syncp);
+
+		stats->rx_length_errors = u64_stats_read(&mib->rx_undersize_errors) +
+					  u64_stats_read(&mib->rx_fragment_errors);
+		stats->rx_over_errors = u64_stats_read(&mib->rx_oversize_errors);
+		stats->rx_crc_errors = u64_stats_read(&mib->rx_crc_errors);
+		stats->rx_frame_errors = u64_stats_read(&mib->rx_alignment_errors);
+		/* stats->rx_fifo_errors */
+		/* stats->rx_missed_errors */
+
+		stats->tx_aborted_errors = u64_stats_read(&mib->tx_aborted_errors);
+		/* stats->tx_carrier_errors */
+		stats->tx_fifo_errors = u64_stats_read(&mib->tx_undersize_errors);
+		/* stats->tx_heartbeat_errors */
+		stats->tx_window_errors = u64_stats_read(&mib->tx_late_collisions);
+
+		stats->rx_packets = u64_stats_read(&pp->rx_frames);
+		stats->tx_packets = u64_stats_read(&pp->tx_frames);
+		stats->rx_bytes = u64_stats_read(&mib->rx_good_bytes) -
+				  ETH_FCS_LEN * stats->rx_packets;
+		stats->tx_bytes = u64_stats_read(&mib->tx_good_bytes) -
+				  ETH_FCS_LEN * stats->tx_packets;
+		stats->rx_dropped = u64_stats_read(&mib->rx_dropped);
+		/* stats->tx_dropped */
+		stats->multicast = u64_stats_read(&mib->rx_multicast);
+		stats->collisions = u64_stats_read(&mib->tx_collisions);
+	} while (u64_stats_fetch_retry(&pp->syncp, start));
 
-	stats->rx_length_errors = mib->rx_undersize_errors +
-				  mib->rx_fragment_errors;
-	stats->rx_over_errors = mib->rx_oversize_errors;
-	stats->rx_crc_errors = mib->rx_crc_errors;
-	stats->rx_frame_errors = mib->rx_alignment_errors;
-	/* stats->rx_fifo_errors */
-	/* stats->rx_missed_errors */
-
-	stats->tx_aborted_errors = mib->tx_aborted_errors;
-	/* stats->tx_carrier_errors */
-	stats->tx_fifo_errors = mib->tx_undersize_errors;
-	/* stats->tx_heartbeat_errors */
-	stats->tx_window_errors = mib->tx_late_collisions;
-
-	stats->rx_packets = pp->rx_frames;
-	stats->tx_packets = pp->tx_frames;
-	stats->rx_bytes = mib->rx_good_bytes - ETH_FCS_LEN * stats->rx_packets;
-	stats->tx_bytes = mib->tx_good_bytes - ETH_FCS_LEN * stats->tx_packets;
 	stats->rx_errors = stats->rx_length_errors + stats->rx_over_errors +
 			   stats->rx_crc_errors + stats->rx_frame_errors;
 	stats->tx_errors = stats->tx_aborted_errors + stats->tx_fifo_errors +
 			   stats->tx_window_errors;
-	stats->rx_dropped = mib->rx_dropped;
-	/* stats->tx_dropped */
-	stats->multicast = mib->rx_multicast;
-	stats->collisions = mib->tx_collisions;
 }
 
 static void
@@ -950,13 +992,18 @@ yt921x_dsa_get_pause_stats(struct dsa_switch *ds, int port,
 	struct yt921x_priv *priv = to_yt921x_priv(ds);
 	struct yt921x_port *pp = &priv->ports[port];
 	struct yt921x_mib *mib = &pp->mib;
+	unsigned int start;
 
 	mutex_lock(&priv->reg_lock);
 	yt921x_read_mib(priv, port);
 	mutex_unlock(&priv->reg_lock);
 
-	pause_stats->tx_pause_frames = mib->tx_pause;
-	pause_stats->rx_pause_frames = mib->rx_pause;
+	do {
+		start = u64_stats_fetch_begin(&pp->syncp);
+
+		pause_stats->tx_pause_frames = u64_stats_read(&mib->tx_pause);
+		pause_stats->rx_pause_frames = u64_stats_read(&mib->rx_pause);
+	} while (u64_stats_fetch_retry(&pp->syncp, start));
 }
 
 static int
@@ -3418,6 +3465,7 @@ static int yt921x_mdio_probe(struct mdio_device *mdiodev)
 
 		pp->index = i;
 		INIT_DELAYED_WORK(&pp->mib_read, yt921x_poll_mib);
+		u64_stats_init(&pp->syncp);
 	}
 
 	ds = &priv->ds;
diff --git a/drivers/net/dsa/yt921x.h b/drivers/net/dsa/yt921x.h
index 3f129b8d403f..0f4231accf3e 100644
--- a/drivers/net/dsa/yt921x.h
+++ b/drivers/net/dsa/yt921x.h
@@ -6,6 +6,8 @@
 #ifndef __YT921X_H
 #define __YT921X_H
 
+#include <linux/u64_stats_sync.h>
+
 #include <net/dsa.h>
 
 #define YT921X_SMI_SWITCHID_M		GENMASK(3, 2)
@@ -530,55 +532,61 @@ enum yt921x_fdb_entry_status {
 #define yt921x_port_is_external(port) (8 <= (port) && (port) < 9)
 
 struct yt921x_mib {
-	u64 rx_broadcast;
-	u64 rx_pause;
-	u64 rx_multicast;
-	u64 rx_crc_errors;
-
-	u64 rx_alignment_errors;
-	u64 rx_undersize_errors;
-	u64 rx_fragment_errors;
-	u64 rx_64byte;
-
-	u64 rx_65_127byte;
-	u64 rx_128_255byte;
-	u64 rx_256_511byte;
-	u64 rx_512_1023byte;
-
-	u64 rx_1024_1518byte;
-	u64 rx_jumbo;
-	u64 rx_good_bytes;
-
-	u64 rx_bad_bytes;
-	u64 rx_oversize_errors;
-
-	u64 rx_dropped;
-	u64 tx_broadcast;
-	u64 tx_pause;
-	u64 tx_multicast;
-
-	u64 tx_undersize_errors;
-	u64 tx_64byte;
-	u64 tx_65_127byte;
-	u64 tx_128_255byte;
-
-	u64 tx_256_511byte;
-	u64 tx_512_1023byte;
-	u64 tx_1024_1518byte;
-	u64 tx_jumbo;
-
-	u64 tx_good_bytes;
-	u64 tx_collisions;
-
-	u64 tx_aborted_errors;
-	u64 tx_multiple_collisions;
-	u64 tx_single_collisions;
-	u64 tx_good;
-
-	u64 tx_deferred;
-	u64 tx_late_collisions;
-	u64 rx_oam;
-	u64 tx_oam;
+	union {
+		struct {
+			u64_stats_t rx_broadcast;
+			u64_stats_t rx_pause;
+			u64_stats_t rx_multicast;
+			u64_stats_t rx_crc_errors;
+
+			u64_stats_t rx_alignment_errors;
+			u64_stats_t rx_undersize_errors;
+			u64_stats_t rx_fragment_errors;
+			u64_stats_t rx_64byte;
+
+			u64_stats_t rx_65_127byte;
+			u64_stats_t rx_128_255byte;
+			u64_stats_t rx_256_511byte;
+			u64_stats_t rx_512_1023byte;
+
+			u64_stats_t rx_1024_1518byte;
+			u64_stats_t rx_jumbo;
+			u64_stats_t rx_good_bytes;
+
+			u64_stats_t rx_bad_bytes;
+			u64_stats_t rx_oversize_errors;
+
+			u64_stats_t rx_dropped;
+			u64_stats_t tx_broadcast;
+			u64_stats_t tx_pause;
+			u64_stats_t tx_multicast;
+
+			u64_stats_t tx_undersize_errors;
+			u64_stats_t tx_64byte;
+			u64_stats_t tx_65_127byte;
+			u64_stats_t tx_128_255byte;
+
+			u64_stats_t tx_256_511byte;
+			u64_stats_t tx_512_1023byte;
+			u64_stats_t tx_1024_1518byte;
+			u64_stats_t tx_jumbo;
+
+			u64_stats_t tx_good_bytes;
+			u64_stats_t tx_collisions;
+
+			u64_stats_t tx_aborted_errors;
+			u64_stats_t tx_multiple_collisions;
+			u64_stats_t tx_single_collisions;
+			u64_stats_t tx_good;
+
+			u64_stats_t tx_deferred;
+			u64_stats_t tx_late_collisions;
+			u64_stats_t rx_oam;
+			u64_stats_t tx_oam;
+		};
+
+		u64_stats_t stats[39];
+	};
 };
 
 struct yt921x_port {
@@ -588,9 +596,13 @@ struct yt921x_port {
 	bool isolated;
 
 	struct delayed_work mib_read;
+	struct u64_stats_sync syncp;
 	struct yt921x_mib mib;
-	u64 rx_frames;
-	u64 tx_frames;
+	u64_stats_t rx_frames;
+	u64_stats_t tx_frames;
+
+	/* only used by read routine to avoid huge allocations on the stack */
+	struct yt921x_mib mib_new;
 };
 
 struct yt921x_reg_ops {
-- 
2.51.0

Re: [PATCH net-next 3/3] net: dsa: yt921x: Use u64_stats_t for MIB stats

[Jakub Kicinski]

On Wed,  4 Feb 2026 01:12:43 +0800 David Yang wrote:
> 64-bit variables might not be atomic on 32-bit architectures, thus
> cannot be unconditionally made lock-free. Use u64_stats_t so it would
> still be lock-free on 64-bit architectures.

IDK why we need to worry about lock free reading of those 64b values...

> @@ -771,22 +785,27 @@ yt921x_dsa_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data)
>  	struct yt921x_priv *priv = to_yt921x_priv(ds);
>  	struct yt921x_port *pp = &priv->ports[port];
>  	struct yt921x_mib *mib = &pp->mib;
> +	unsigned int start;
>  	size_t j;
>  
>  	mutex_lock(&priv->reg_lock);
>  	yt921x_read_mib(priv, port);
>  	mutex_unlock(&priv->reg_lock);

..when in all(?) the readers converted by this patch we take the mutex
to refresh the stats anyway. And presumably do expensive IO/reg reads
under that mutex. So just extend the mutex over the read section and
save us the retry logic, please, it will make no difference to perf.

> -	j = 0;
> -	for (size_t i = 0; i < ARRAY_SIZE(yt921x_mib_descs); i++) {
> -		const struct yt921x_mib_desc *desc = &yt921x_mib_descs[i];
> +	do {
> +		start = u64_stats_fetch_begin(&pp->syncp);
>  
> -		if (!desc->name)
> -			continue;
> +		j = 0;
> +		for (size_t i = 0; i < ARRAY_SIZE(yt921x_mib_descs); i++) {
> +			const struct yt921x_mib_desc *desc = &yt921x_mib_descs[i];
>  
> -		data[j] = ((u64 *)mib)[i];
> -		j++;
> -	}
> +			if (!desc->name)
> +				continue;
> +
> +			data[j] = u64_stats_read(&((u64_stats_t *)mib)[i]);
> +			j++;
> +		}
> +	} while (u64_stats_fetch_retry(&pp->syncp, start));
>  }

Re: [PATCH net-next 3/3] net: dsa: yt921x: Use u64_stats_t for MIB stats

[David Yang]

On Thu, Feb 5, 2026 at 1:12 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed,  4 Feb 2026 01:12:43 +0800 David Yang wrote:
> > 64-bit variables might not be atomic on 32-bit architectures, thus
> > cannot be unconditionally made lock-free. Use u64_stats_t so it would
> > still be lock-free on 64-bit architectures.
>
> IDK why we need to worry about lock free reading of those 64b values...
>
> > @@ -771,22 +785,27 @@ yt921x_dsa_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data)
> >       struct yt921x_priv *priv = to_yt921x_priv(ds);
> >       struct yt921x_port *pp = &priv->ports[port];
> >       struct yt921x_mib *mib = &pp->mib;
> > +     unsigned int start;
> >       size_t j;
> >
> >       mutex_lock(&priv->reg_lock);
> >       yt921x_read_mib(priv, port);
> >       mutex_unlock(&priv->reg_lock);
>
> ..when in all(?) the readers converted by this patch we take the mutex
> to refresh the stats anyway. And presumably do expensive IO/reg reads
> under that mutex. So just extend the mutex over the read section and
> save us the retry logic, please, it will make no difference to perf.

.get_stats64() is atomic, that's why everyone (mostly) is using
u64_stats_t for DSA stats.

Re: [PATCH net-next 3/3] net: dsa: yt921x: Use u64_stats_t for MIB stats

[Jakub Kicinski]

On Thu, 5 Feb 2026 13:32:29 +0800 David Yang wrote:
> On Thu, Feb 5, 2026 at 1:12 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Wed,  4 Feb 2026 01:12:43 +0800 David Yang wrote:  
> > > 64-bit variables might not be atomic on 32-bit architectures, thus
> > > cannot be unconditionally made lock-free. Use u64_stats_t so it would
> > > still be lock-free on 64-bit architectures.  
> >
> > IDK why we need to worry about lock free reading of those 64b values...
> >  
> > > @@ -771,22 +785,27 @@ yt921x_dsa_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data)
> > >       struct yt921x_priv *priv = to_yt921x_priv(ds);
> > >       struct yt921x_port *pp = &priv->ports[port];
> > >       struct yt921x_mib *mib = &pp->mib;
> > > +     unsigned int start;
> > >       size_t j;
> > >
> > >       mutex_lock(&priv->reg_lock);
> > >       yt921x_read_mib(priv, port);
> > >       mutex_unlock(&priv->reg_lock);  
> >
> > ..when in all(?) the readers converted by this patch we take the mutex
> > to refresh the stats anyway. And presumably do expensive IO/reg reads
> > under that mutex. So just extend the mutex over the read section and
> > save us the retry logic, please, it will make no difference to perf.  
> 
> .get_stats64() is atomic, that's why everyone (mostly) is using
> u64_stats_t for DSA stats.

I understand, but only ndo_get_stats64() is, all the other cases like
ethtool stats can keep holding the lock and use u64 / u64_stats_read()
without a retry.

I had this argument with people before, I don't get why people try to
microoptimize device stats so much. The u64_stats* infra is really meant
for per-CPU/per-queue software stats which must have low overhead.