With mlx5 netdev instance locking enabled, registering an IPoIB netdev calls the WEXT notifier while holding the instance mutex. Its call to wireless_nlevent_flush() takes net_rwsem to walk every namespace, completing the following lockdep dependency cycle: netdev instance mutex -> net_rwsem (read) -> GID table mutex -> event_handler_rwsem (read) -> netdev instance mutex RoCE GID enumeration takes the GID table mutex under net_rwsem. GID updates dispatch events under the table mutex, taking event_handler_rwsem. The IPoIB event handler takes the netdev instance mutex in ipoib_queue_work() to traverse child interfaces. The all-namespace walk originated in the single global worker servicing per-namespace WEXT queues. The ordering fixes later reused that routine in netdevice notifiers to prevent delayed WEXT RTM_NEWLINK messages from following RTM_DELLINK or reporting stale interface flags. A notifier only needs to drain the affected device's namespace; there is no reason for it to walk and flush all namespaces. Pass dev_net(dev) from both notifier callers and make the flush helper operate on that namespace. Keep the all-namespace walk and net_rwsem in the existing worker. This removes the instance-mutex -> net_rwsem dependency while retaining synchronous flushing from the notifiers. It may seem strange that the fix for a lock cycle across multiple subsystems lies in a completely unrelated part of the code, but this seemed the most justified lock cycle edge to break. Issue: 5331015 Fixes: 8f7b00307bf1 ("net/mlx5e: Convert mlx5 netdevs to instance locking") Signed-off-by: Cosmin Ratiu Change-Id: I98d7e77ffc9fa0ce72296d93c200da8fe1db378e --- include/net/iw_handler.h | 8 +++++--- net/wireless/core.c | 2 +- net/wireless/wext-core.c | 33 +++++++++++++++++---------------- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/include/net/iw_handler.h b/include/net/iw_handler.h index b80e474cb0aa..632618d9ff93 100644 --- a/include/net/iw_handler.h +++ b/include/net/iw_handler.h @@ -201,6 +201,8 @@ #include /* IOCTL user space API */ #include +struct net; + /***************************** VERSION *****************************/ /* * This constant is used to know which version of the driver API is @@ -411,10 +413,10 @@ struct iw_spy_data { void wireless_send_event(struct net_device *dev, unsigned int cmd, union iwreq_data *wrqu, const char *extra); #ifdef CONFIG_WEXT_CORE -/* flush all previous wext events - if work is done from netdev notifiers */ -void wireless_nlevent_flush(void); +/* Flush previous wext events in this netns from netdev notifiers. */ +void wireless_nlevent_flush(struct net *net); #else -static inline void wireless_nlevent_flush(void) {} +static inline void wireless_nlevent_flush(struct net *net) {} #endif /* We may need a function to send a stream of events to user space. diff --git a/net/wireless/core.c b/net/wireless/core.c index cde3ca85494d..2f49ed14b26e 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -1878,7 +1878,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb, return NOTIFY_DONE; } - wireless_nlevent_flush(); + wireless_nlevent_flush(dev_net(dev)); return NOTIFY_OK; } diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c index db77912b3994..f09c589ff94c 100644 --- a/net/wireless/wext-core.c +++ b/net/wireless/wext-core.c @@ -342,32 +342,28 @@ static const int compat_event_type_size[] = { /* IW event code */ -void wireless_nlevent_flush(void) +void wireless_nlevent_flush(struct net *net) { struct sk_buff *skb; - struct net *net; - down_read(&net_rwsem); - for_each_net(net) { - while ((skb = skb_dequeue(&net->wext_nlevents))) - rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, - GFP_KERNEL); - } - up_read(&net_rwsem); + while ((skb = skb_dequeue(&net->wext_nlevents))) + rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, GFP_KERNEL); } EXPORT_SYMBOL_GPL(wireless_nlevent_flush); static int wext_netdev_notifier_call(struct notifier_block *nb, unsigned long state, void *ptr) { + struct net_device *dev = netdev_notifier_info_to_dev(ptr); + /* - * When a netdev changes state in any way, flush all pending messages - * to avoid them going out in a strange order, e.g. RTM_NEWLINK after - * RTM_DELLINK, or with IFF_UP after without IFF_UP during dev_close() - * or similar - all of which could otherwise happen due to delays from - * schedule_work(). + * When a netdev changes state in any way, flush all pending messages in + * its network namespace to avoid them going out in a strange order, + * e.g. RTM_NEWLINK after RTM_DELLINK, or with IFF_UP after without + * IFF_UP during dev_close() or similar - all of which could otherwise + * happen due to delays from schedule_work(). */ - wireless_nlevent_flush(); + wireless_nlevent_flush(dev_net(dev)); return NOTIFY_OK; } @@ -410,7 +406,12 @@ subsys_initcall(wireless_nlevent_init); /* Process events generated by the wireless layer or the driver. */ static void wireless_nlevent_process(struct work_struct *work) { - wireless_nlevent_flush(); + struct net *net; + + down_read(&net_rwsem); + for_each_net(net) + wireless_nlevent_flush(net); + up_read(&net_rwsem); } static DECLARE_WORK(wireless_nlevent_work, wireless_nlevent_process); -- 2.53.0