Skip to content

Commit 86ae8bd

Browse files
vladimirolteangregkh
authored andcommitted
net: dsa: link interfaces with the DSA master to get rid of lockdep warnings
[ Upstream commit 2f1e8ea ] Since commit 845e0eb ("net: change addr_list_lock back to static key"), cascaded DSA setups (DSA switch port as DSA master for another DSA switch port) are emitting this lockdep warning: ============================================ WARNING: possible recursive locking detected 5.8.0-rc1-00133-g923e4b5032dd-dirty torvalds#208 Not tainted -------------------------------------------- dhcpcd/323 is trying to acquire lock: ffff000066dd4268 (&dsa_master_addr_list_lock_key/1){+...}-{2:2}, at: dev_mc_sync+0x44/0x90 but task is already holding lock: ffff00006608c268 (&dsa_master_addr_list_lock_key/1){+...}-{2:2}, at: dev_mc_sync+0x44/0x90 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&dsa_master_addr_list_lock_key/1); lock(&dsa_master_addr_list_lock_key/1); *** DEADLOCK *** May be due to missing lock nesting notation 3 locks held by dhcpcd/323: #0: ffffdbd1381dda18 (rtnl_mutex){+.+.}-{3:3}, at: rtnl_lock+0x24/0x30 #1: ffff00006614b268 (_xmit_ETHER){+...}-{2:2}, at: dev_set_rx_mode+0x28/0x48 #2: ffff00006608c268 (&dsa_master_addr_list_lock_key/1){+...}-{2:2}, at: dev_mc_sync+0x44/0x90 stack backtrace: Call trace: dump_backtrace+0x0/0x1e0 show_stack+0x20/0x30 dump_stack+0xec/0x158 __lock_acquire+0xca0/0x2398 lock_acquire+0xe8/0x440 _raw_spin_lock_nested+0x64/0x90 dev_mc_sync+0x44/0x90 dsa_slave_set_rx_mode+0x34/0x50 __dev_set_rx_mode+0x60/0xa0 dev_mc_sync+0x84/0x90 dsa_slave_set_rx_mode+0x34/0x50 __dev_set_rx_mode+0x60/0xa0 dev_set_rx_mode+0x30/0x48 __dev_open+0x10c/0x180 __dev_change_flags+0x170/0x1c8 dev_change_flags+0x2c/0x70 devinet_ioctl+0x774/0x878 inet_ioctl+0x348/0x3b0 sock_do_ioctl+0x50/0x310 sock_ioctl+0x1f8/0x580 ksys_ioctl+0xb0/0xf0 __arm64_sys_ioctl+0x28/0x38 el0_svc_common.constprop.0+0x7c/0x180 do_el0_svc+0x2c/0x98 el0_sync_handler+0x9c/0x1b8 el0_sync+0x158/0x180 Since DSA never made use of the netdev API for describing links between upper devices and lower devices, the dev->lower_level value of a DSA switch interface would be 1, which would warn when it is a DSA master. We can use netdev_upper_dev_link() to describe the relationship between a DSA slave and a DSA master. To be precise, a DSA "slave" (switch port) is an "upper" to a DSA "master" (host port). The relationship is "many uppers to one lower", like in the case of VLAN. So, for that reason, we use the same function as VLAN uses. There might be a chance that somebody will try to take hold of this interface and use it immediately after register_netdev() and before netdev_upper_dev_link(). To avoid that, we do the registration and linkage while holding the RTNL, and we use the RTNL-locked cousin of register_netdev(), which is register_netdevice(). Since this warning was not there when lockdep was using dynamic keys for addr_list_lock, we are blaming the lockdep patch itself. The network stack _has_ been using static lockdep keys before, and it _is_ likely that stacked DSA setups have been triggering these lockdep warnings since forever, however I can't test very old kernels on this particular stacked DSA setup, to ensure I'm not in fact introducing regressions. Fixes: 845e0eb ("net: change addr_list_lock back to static key") Suggested-by: Cong Wang <xiyou.wangcong@gmail.com> Signed-off-by: Vladimir Oltean <olteanv@gmail.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 150feba commit 86ae8bd

File tree

1 file changed

+16
-2
lines changed

1 file changed

+16
-2
lines changed

net/dsa/slave.c

+16-2
Original file line numberDiff line numberDiff line change
@@ -1801,15 +1801,27 @@ int dsa_slave_create(struct dsa_port *port)
18011801

18021802
dsa_slave_notify(slave_dev, DSA_PORT_REGISTER);
18031803

1804-
ret = register_netdev(slave_dev);
1804+
rtnl_lock();
1805+
1806+
ret = register_netdevice(slave_dev);
18051807
if (ret) {
18061808
netdev_err(master, "error %d registering interface %s\n",
18071809
ret, slave_dev->name);
1810+
rtnl_unlock();
18081811
goto out_phy;
18091812
}
18101813

1814+
ret = netdev_upper_dev_link(master, slave_dev, NULL);
1815+
1816+
rtnl_unlock();
1817+
1818+
if (ret)
1819+
goto out_unregister;
1820+
18111821
return 0;
18121822

1823+
out_unregister:
1824+
unregister_netdev(slave_dev);
18131825
out_phy:
18141826
rtnl_lock();
18151827
phylink_disconnect_phy(p->dp->pl);
@@ -1826,16 +1838,18 @@ int dsa_slave_create(struct dsa_port *port)
18261838

18271839
void dsa_slave_destroy(struct net_device *slave_dev)
18281840
{
1841+
struct net_device *master = dsa_slave_to_master(slave_dev);
18291842
struct dsa_port *dp = dsa_slave_to_port(slave_dev);
18301843
struct dsa_slave_priv *p = netdev_priv(slave_dev);
18311844

18321845
netif_carrier_off(slave_dev);
18331846
rtnl_lock();
1847+
netdev_upper_dev_unlink(master, slave_dev);
1848+
unregister_netdevice(slave_dev);
18341849
phylink_disconnect_phy(dp->pl);
18351850
rtnl_unlock();
18361851

18371852
dsa_slave_notify(slave_dev, DSA_PORT_UNREGISTER);
1838-
unregister_netdev(slave_dev);
18391853
phylink_destroy(dp->pl);
18401854
gro_cells_destroy(&p->gcells);
18411855
free_percpu(p->stats64);

0 commit comments

Comments
 (0)