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

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 [off-list ref]

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.

quoted

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

[ ... ]

quoted

@@ -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?

As pointed out before, according to my understanding, ndo_get_stats64()
callback can't run from software interrupt context (BH), so this is not a real
issue.

Regards,
Lorenzo

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