Thread (3 messages) 3 messages, 2 authors, 2026-05-02

Re: [PATCH v2] net: net_failover: Fix the deadlock in slave register

From: faicker mo <hidden>
Date: 2026-05-02 02:46:22
Also in: lkml

quoted
@@ -512,11 +512,11 @@ static int net_failover_slave_register(struct net_device *slave_dev,
      dev_hold(slave_dev);

      if (netif_running(failover_dev)) {
-             err = dev_open(slave_dev, NULL);
+             err = netif_open(slave_dev, NULL);
Same question for this call.  netif_open() -> __dev_open() also does
netdev_ops_assert_locked(dev), and on the failover_existing_slave_register()
path the slave's ops lock is not held.
Yes.
Before this change, dev_set_mtu()/dev_open()/dev_close() acquired the ops
lock internally via netdev_lock_ops(), so the pre-existing-slave path was
covered.  Does switching unconditionally to the non-locking variants
regress that path for slaves whose drivers use the per-device ops lock,
both as a lockdep splat when CONFIG_PROVE_LOCKING is set and as a loss of
ops-lock serialization against concurrent ethtool, queue and netdev-genl
operations on the slave?
quoted
@@ -562,10 +562,10 @@ static int net_failover_slave_register(struct net_device *slave_dev,
 err_vlan_add:
      dev_uc_unsync(slave_dev, failover_dev);
      dev_mc_unsync(slave_dev, failover_dev);
-     dev_close(slave_dev);
-err_dev_open:
+     netif_close(slave_dev);
+err_netif_open:
      dev_put(slave_dev);
-     dev_set_mtu(slave_dev, orig_mtu);
+     netif_set_mtu(slave_dev, orig_mtu);
Would it be more appropriate to acquire netdev_lock_ops(slave_dev) in
net_failover_slave_register() when the caller is not the NETDEV_REGISTER
notifier (where the ops lock is already held), instead of dropping the
locking unconditionally?
Yes, we should add netdev_lock_ops in failover_existing_slave_register
before calling failover_slave_register.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help