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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Potential spinlock deadlock due to using `spin_lock()` instead of `spin_lock_bh()` in process context, while the same lock is acquired in atomic (softirq) context.
--

commit 38990de22bba61840a039693f5868e79590d8503
Author: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>

net: dsa: lan9645x: add port statistics

Add statistics support for the port counters. Chip registers are 32 bit,
so this unit is responsible maintaining a 64bit software cache, and
updating it frequently to handle overflows in hardware.

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

[Severity: High]
Can this deadlock if a softirq interrupts the process-context lock holder?

The periodic workqueue (which calls __lan9645x_stats_view_idx_update) and
ethtool callbacks acquire sw_lock using plain spin_lock(). Since plain
spin_lock() does not disable softirqs, the process can be preempted while
holding the lock.

> +		__lan9645x_stats_view_idx_transfer(lan9645x, vtype, idx);
> +		spin_unlock(&s->sw_lock);
> +	}
> +}

[ ... ]

> +/* 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);

[Severity: High]
If lan9645x_stats_get_stats64() runs in softirq context as the comment
implies, would it spin forever attempting to acquire sw_lock if it
interrupted the process-context thread holding the same lock?

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

> +
> +	stats->rx_bytes = c[SCNT_RX_OCT] + c[SCNT_RX_PMAC_OCT];

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-dsa_lan9645x_switch_driver_base-v8-0-90228d8bba58@microchip.com?part=9