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

From: Jakub Kicinski

Date: Thu Feb 05 2026 - 00:13:09 EST


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));
> }