Re: [PATCH net-next v5 9/9] net: dsa: lan9645x: add port statistics

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Deadlock due to `spin_lock()` being used in both process and softirq contexts without disabling bottom halves.
- [Medium] Duplication of standard kernel statistics in the driver-private `ethtool -S` interface.
--

commit 59f4819b8342d3e4580ddccbc972e0d8f002441f
Author: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
Date:   Mon May 18 12:25:04 2026 +0000

net: dsa: lan9645x: add port statistics

This commit adds support for maintaining and updating a 64-bit software
cache of 32-bit hardware port counters. It introduces multiple ethtool
and netlink statistics callbacks to expose these counters.

> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_stats.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_stats.c
> new file mode 100644
> index 0000000000000..e38f510eb8f66
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_stats.c
[ ... ]
> +static const struct lan9645x_ethtool_stat lan9645x_port_ethtool_stats[] = {
> +	{ "rx_cat_drop",        SCNT_RX_CAT_DROP },
[ ... ]
> +	{ "rx_green_prio_7",    SCNT_RX_GREEN_PRIO_7 },
> +	{ "tx_drop",            SCNT_TX_DROP },

Is it necessary to include tx_drop in the private ethtool strings?

The networking subsystem guidelines indicate that counters with a standard
uAPI should use that standard interface instead of the driver-private ethtool
output. Since tx_drop is already aggregated and exposed via the standard
netlink interface in lan9645x_stats_get_stats64(), could this be removed
from the private stats?

[ ... ]
> +static void __lan9645x_stats_view_idx_update(struct lan9645x *lan9645x,
> +					     enum lan9645x_view_stat_type vtype,
> +					     int idx)
> +{
> +	struct lan9645x_stats *s = lan9645x->stats;
> +
> +	lockdep_assert_held(&s->hw_lock);
> +
> +	if (!__lan9645x_stats_view_idx_hw_read(lan9645x, vtype, idx)) {
> +		spin_lock(&s->sw_lock);

Can a deadlock occur here if the CPU is interrupted by a softirq?

The sw_lock is acquired with spin_lock() in process-context functions like
this one (which is called via a periodic workqueue), but it is also acquired
in lan9645x_stats_get_stats64() below:

[ ... ]
> +/* Called in atomic context */
> +void lan9645x_stats_get_stats64(struct lan9645x *lan9645x, int port,
> +				struct rtnl_link_stats64 *stats)
> +{
> +	struct lan9645x_stats *s = lan9645x->stats;
> +	u64 *c;
> +
> +	c = STAT_COUNTERS(lan9645x, LAN9645X_STAT_PORTS, port);
> +
> +	spin_lock(&s->sw_lock);

Since lan9645x_stats_get_stats64() can be called from atomic or softirq
context during packet processing, if a softirq interrupts a process holding
the lock on the same CPU, it seems it would spin forever.

Should the process-context paths use spin_lock_bh() instead to prevent this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-dsa_lan9645x_switch_driver_base-v5-0-968fbf34ffa3@microchip.com?part=9