Re: [net-next,v4,2/2] net: dsa: yt921x: Use u64_stats_t for MIB stats

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: dsa: yt921x: Use u64_stats_t for MIB stats

This patch converts the MIB statistics from plain u64 to u64_stats_t and
adds a u64_stats_sync seqcount to provide atomicity on 32-bit architectures
while remaining lock-free on 64-bit architectures.

> diff --git a/drivers/net/dsa/yt921x.h b/drivers/net/dsa/yt921x.h
> index 61bb0ab3b09a..17fee83952c2 100644
> --- a/drivers/net/dsa/yt921x.h
> +++ b/drivers/net/dsa/yt921x.h

[ ... ]

> @@ -533,9 +535,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;
>  };

The new syncp field requires u64_stats_init() to be called before use.
Looking at yt921x_mdio_probe(), the port initialization loop only sets
pp->index and calls INIT_DELAYED_WORK() but does not call
u64_stats_init(&pp->syncp).

On 32-bit architectures, u64_stats_init() calls seqcount_init() which
initializes the lockdep tracking. Without this initialization, builds
with CONFIG_DEBUG_LOCK_ALLOC may have lockdep issues.

For reference, the xrs700x DSA driver does this correctly in
xrs700x_alloc_port_mib():

    mutex_init(&p->mib_mutex);
    u64_stats_init(&p->syncp);

Should u64_stats_init(&pp->syncp) be added to the port initialization
loop in yt921x_mdio_probe()?