Re: [PATCH net-next v8 02/10] net: airoha: Introduce airoha_gdm_dev struct

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Potential deadlock in `airoha_update_hw_stats()` due to using `spin_lock()` instead of `spin_lock_bh()`.
--

commit 6823c137925a562390270eb1ce7967caaa011b7b
Author: Lorenzo Bianconi <lorenzo@kernel.org>

net: airoha: Introduce airoha_gdm_dev struct

This commit introduces the airoha_gdm_dev struct to collect net_device related
info, accommodating future support for connecting multiple external SerDes to
GDM3 or GDM4 ports via a hardware arbiter.

> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 5a027cc7ffcb1..5335271ea8654 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c

[ ... ]

> @@ -1877,10 +1888,11 @@ static int airoha_dev_init(struct net_device *dev)
>  	return 0;
>  }
>  
> -static void airoha_dev_get_stats64(struct net_device *dev,
> +static void airoha_dev_get_stats64(struct net_device *netdev,
>  				   struct rtnl_link_stats64 *storage)
>  {
> -	struct airoha_gdm_port *port = netdev_priv(dev);
> +	struct airoha_gdm_dev *dev = netdev_priv(netdev);
> +	struct airoha_gdm_port *port = dev->port;
>  	unsigned int start;
>  
>  	airoha_update_hw_stats(port);

This isn't a bug introduced by this patch, but could this lead to a deadlock?

Because airoha_dev_get_stats64() is registered as the ndo_get_stats64 callback,
it can be called from both process context (e.g., via user-space netlink dumps)
and software interrupt context (BH) (e.g., via bonding or teaming drivers).

Looking at airoha_update_hw_stats(), it uses spin_lock() to protect the
hardware statistics update:

drivers/net/ethernet/airoha/airoha_eth.c:airoha_update_hw_stats() {
	...
	spin_lock(&port->stats.lock);
	u64_stats_update_begin(&port->stats.syncp);
	...
}

Because spin_lock() disables preemption but not softirqs, if a process context
call is interrupted by a BH call on the same CPU, the BH handler will attempt
to acquire the already-held lock.

Should this be upgraded to spin_lock_bh() to prevent a potential deadlock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-airoha-eth-multi-serdes-v8-0-6bd70e329df6@kernel.org?part=2