Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 0/6] net: hold instance lock around NETDEV_DOWN and NETDEV_GOING_DOWN
@ 2026-06-30 18:21 Stanislav Fomichev
  2026-06-30 18:21 ` [PATCH net-next 1/6] net: hold instance lock around NETDEV_DOWN/GOING_DOWN Stanislav Fomichev
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2026-06-30 18:21 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, kuba, pabeni

NETDEV_UP and NETDEV_REGISTER already run under the per-device
instance lock. The teardown side does not. Make it symmetric so
ops-locked drivers can rely on the lock being held in both
directions.

Stanislav Fomichev (6):
  net: hold instance lock around NETDEV_DOWN/GOING_DOWN
  net: dsa: hold instance lock on close-on-shutdown paths
  net: mtk_eth_soc: hold instance lock around DMA-device-swap close
  net: rtnetlink: take instance lock inside rtnl_configure_link
  net: require instance lock for NETDEV_DOWN/GOING_DOWN notifiers
  net: document NETDEV_UNREGISTER unlocked rationale

 Documentation/networking/netdevices.rst     | 10 ++++++++++
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |  5 +++++
 net/core/dev.c                              |  5 +++++
 net/core/lock_debug.c                       |  4 ++--
 net/core/rtnetlink.c                        | 17 ++++++++++-------
 net/dsa/dsa.c                               | 20 +++++++++++++++++---
 net/dsa/user.c                              | 19 +++++++++++++++++--
 7 files changed, 66 insertions(+), 14 deletions(-)

-- 
2.53.0-Meta


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH net-next 1/6] net: hold instance lock around NETDEV_DOWN/GOING_DOWN
  2026-06-30 18:21 [PATCH net-next 0/6] net: hold instance lock around NETDEV_DOWN and NETDEV_GOING_DOWN Stanislav Fomichev
@ 2026-06-30 18:21 ` Stanislav Fomichev
  2026-06-30 18:21 ` [PATCH net-next 2/6] net: dsa: hold instance lock on close-on-shutdown paths Stanislav Fomichev
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2026-06-30 18:21 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, kuba, pabeni

Mirror what call_netdevice_register_net_notifiers does but for the
teardown. Cover only DOWN and GOING_DOWN. UNREGISTER is still unlocked
because of the SW devices using dev_xxx methods.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 net/core/dev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index 4b3d5cfdf6e0..9d49493f4fb5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1912,9 +1912,11 @@ static void call_netdevice_unregister_notifiers(struct notifier_block *nb,
 						struct net_device *dev)
 {
 	if (dev->flags & IFF_UP) {
+		netdev_lock_ops(dev);
 		call_netdevice_notifier(nb, NETDEV_GOING_DOWN,
 					dev);
 		call_netdevice_notifier(nb, NETDEV_DOWN, dev);
+		netdev_unlock_ops(dev);
 	}
 	call_netdevice_notifier(nb, NETDEV_UNREGISTER, dev);
 }
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 2/6] net: dsa: hold instance lock on close-on-shutdown paths
  2026-06-30 18:21 [PATCH net-next 0/6] net: hold instance lock around NETDEV_DOWN and NETDEV_GOING_DOWN Stanislav Fomichev
  2026-06-30 18:21 ` [PATCH net-next 1/6] net: hold instance lock around NETDEV_DOWN/GOING_DOWN Stanislav Fomichev
@ 2026-06-30 18:21 ` Stanislav Fomichev
  2026-06-30 18:21 ` [PATCH net-next 3/6] net: mtk_eth_soc: hold instance lock around DMA-device-swap close Stanislav Fomichev
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2026-06-30 18:21 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, kuba, pabeni

netif_close_many will soon assert ops lock (for locked DOWN/GOING_DOWN).
Update dsa_switch_shutdown to manually grab and release the ops lock.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 net/dsa/dsa.c  | 20 +++++++++++++++++---
 net/dsa/user.c | 19 +++++++++++++++++--
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 9cb732f6b1e3..da53a666d4b8 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -18,6 +18,7 @@
 #include <linux/of.h>
 #include <linux/of_net.h>
 #include <net/dsa_stubs.h>
+#include <net/netdev_lock.h>
 #include <net/sch_generic.h>
 
 #include "conduit.h"
@@ -1620,10 +1621,23 @@ void dsa_switch_shutdown(struct dsa_switch *ds)
 
 	rtnl_lock();
 
-	dsa_switch_for_each_cpu_port(dp, ds)
-		list_add(&dp->conduit->close_list, &close_list);
+	dsa_switch_for_each_cpu_port(dp, ds) {
+		if (!(dp->conduit->flags & IFF_UP))
+			continue;
+		list_add_tail(&dp->conduit->close_list, &close_list);
+		netdev_lock_ops(dp->conduit);
+	}
+
+	netif_close_many(&close_list, false);
 
-	netif_close_many(&close_list, true);
+	while (!list_empty(&close_list)) {
+		struct net_device *conduit;
+
+		conduit = list_first_entry(&close_list, struct net_device,
+					   close_list);
+		netdev_unlock_ops(conduit);
+		list_del_init(&conduit->close_list);
+	}
 
 	dsa_switch_for_each_user_port(dp, ds) {
 		conduit = dsa_port_to_conduit(dp);
diff --git a/net/dsa/user.c b/net/dsa/user.c
index 8704c1a3a5b7..8ea47444d6d5 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -13,6 +13,7 @@
 #include <linux/of_net.h>
 #include <linux/of_mdio.h>
 #include <linux/mdio.h>
+#include <net/netdev_lock.h>
 #include <net/rtnetlink.h>
 #include <net/pkt_cls.h>
 #include <net/selftests.h>
@@ -3600,10 +3601,24 @@ static int dsa_user_netdevice_event(struct notifier_block *nb,
 			if (dp->cpu_dp != cpu_dp)
 				continue;
 
-			list_add(&dp->user->close_list, &close_list);
+			if (!(dp->user->flags & IFF_UP))
+				continue;
+
+			list_add_tail(&dp->user->close_list, &close_list);
+			netdev_lock_ops(dp->user);
 		}
 
-		netif_close_many(&close_list, true);
+		netif_close_many(&close_list, false);
+
+		while (!list_empty(&close_list)) {
+			struct net_device *user_dev;
+
+			user_dev = list_first_entry(&close_list,
+						    struct net_device,
+						    close_list);
+			netdev_unlock_ops(user_dev);
+			list_del_init(&user_dev->close_list);
+		}
 
 		return NOTIFY_OK;
 	}
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 3/6] net: mtk_eth_soc: hold instance lock around DMA-device-swap close
  2026-06-30 18:21 [PATCH net-next 0/6] net: hold instance lock around NETDEV_DOWN and NETDEV_GOING_DOWN Stanislav Fomichev
  2026-06-30 18:21 ` [PATCH net-next 1/6] net: hold instance lock around NETDEV_DOWN/GOING_DOWN Stanislav Fomichev
  2026-06-30 18:21 ` [PATCH net-next 2/6] net: dsa: hold instance lock on close-on-shutdown paths Stanislav Fomichev
@ 2026-06-30 18:21 ` Stanislav Fomichev
  2026-06-30 18:21 ` [PATCH net-next 4/6] net: rtnetlink: take instance lock inside rtnl_configure_link Stanislav Fomichev
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2026-06-30 18:21 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, kuba, pabeni

netif_close_many will soon assert ops lock (for locked DOWN/GOING_DOWN).
Update mtk_eth_set_dma_device to manually grab and release the ops lock.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 5d291e50a47b..fe7610c42e5d 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -26,6 +26,7 @@
 #include <linux/bitfield.h>
 #include <net/dsa.h>
 #include <net/dst_metadata.h>
+#include <net/netdev_lock.h>
 #include <net/page_pool/helpers.h>
 #include <linux/genalloc.h>
 
@@ -5030,10 +5031,14 @@ void mtk_eth_set_dma_device(struct mtk_eth *eth, struct device *dma_dev)
 			continue;
 
 		list_add_tail(&dev->close_list, &dev_list);
+		netdev_lock_ops(dev);
 	}
 
 	netif_close_many(&dev_list, false);
 
+	list_for_each_entry(dev, &dev_list, close_list)
+		netdev_unlock_ops(dev);
+
 	eth->dma_dev = dma_dev;
 
 	list_for_each_entry_safe(dev, tmp, &dev_list, close_list) {
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 4/6] net: rtnetlink: take instance lock inside rtnl_configure_link
  2026-06-30 18:21 [PATCH net-next 0/6] net: hold instance lock around NETDEV_DOWN and NETDEV_GOING_DOWN Stanislav Fomichev
                   ` (2 preceding siblings ...)
  2026-06-30 18:21 ` [PATCH net-next 3/6] net: mtk_eth_soc: hold instance lock around DMA-device-swap close Stanislav Fomichev
@ 2026-06-30 18:21 ` Stanislav Fomichev
  2026-06-30 18:21 ` [PATCH net-next 5/6] net: require instance lock for NETDEV_DOWN/GOING_DOWN notifiers Stanislav Fomichev
  2026-06-30 18:21 ` [PATCH net-next 6/6] net: document NETDEV_UNREGISTER unlocked rationale Stanislav Fomichev
  5 siblings, 0 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2026-06-30 18:21 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, kuba, pabeni

rtnl_configure_link calls __dev_change_flags() and __dev_notify_flags,
both need the instance lock. rtnl_newlink_create grabs it but stacked
devices do not. Move the lock inside rtnl_configure_link.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 net/core/rtnetlink.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 12aa3aa1688b..1b7d6f6b8b68 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3660,14 +3660,16 @@ int rtnl_configure_link(struct net_device *dev, const struct ifinfomsg *ifm,
 			u32 portid, const struct nlmsghdr *nlh)
 {
 	unsigned int old_flags, changed;
-	int err;
+	int err = 0;
+
+	netdev_lock_ops(dev);
 
 	old_flags = dev->flags;
 	if (ifm && (ifm->ifi_flags || ifm->ifi_change)) {
 		err = __dev_change_flags(dev, rtnl_dev_combine_flags(dev, ifm),
 					 NULL);
 		if (err < 0)
-			return err;
+			goto out;
 	}
 
 	changed = old_flags ^ dev->flags;
@@ -3677,7 +3679,10 @@ int rtnl_configure_link(struct net_device *dev, const struct ifinfomsg *ifm,
 	}
 
 	__dev_notify_flags(dev, old_flags, changed, portid, nlh);
-	return 0;
+
+out:
+	netdev_unlock_ops(dev);
+	return err;
 }
 EXPORT_SYMBOL(rtnl_configure_link);
 
@@ -3918,22 +3923,20 @@ static int rtnl_newlink_create(struct sk_buff *skb, struct ifinfomsg *ifm,
 		goto out;
 	}
 
-	netdev_lock_ops(dev);
-
 	err = rtnl_configure_link(dev, ifm, portid, nlh);
 	if (err < 0)
 		goto out_unregister;
 	if (tb[IFLA_MASTER]) {
+		netdev_lock_ops(dev);
 		err = do_set_master(dev, nla_get_u32(tb[IFLA_MASTER]), extack);
+		netdev_unlock_ops(dev);
 		if (err)
 			goto out_unregister;
 	}
 
-	netdev_unlock_ops(dev);
 out:
 	return err;
 out_unregister:
-	netdev_unlock_ops(dev);
 	if (ops->newlink) {
 		LIST_HEAD(list_kill);
 
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 5/6] net: require instance lock for NETDEV_DOWN/GOING_DOWN notifiers
  2026-06-30 18:21 [PATCH net-next 0/6] net: hold instance lock around NETDEV_DOWN and NETDEV_GOING_DOWN Stanislav Fomichev
                   ` (3 preceding siblings ...)
  2026-06-30 18:21 ` [PATCH net-next 4/6] net: rtnetlink: take instance lock inside rtnl_configure_link Stanislav Fomichev
@ 2026-06-30 18:21 ` Stanislav Fomichev
  2026-06-30 18:21 ` [PATCH net-next 6/6] net: document NETDEV_UNREGISTER unlocked rationale Stanislav Fomichev
  5 siblings, 0 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2026-06-30 18:21 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, kuba, pabeni

Sprinkle a few asserts about ops lock: netif_close_many and __dev_notify_flags
should now consistently run under the lock

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 Documentation/networking/netdevices.rst | 2 ++
 net/core/dev.c                          | 3 +++
 net/core/lock_debug.c                   | 4 ++--
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/networking/netdevices.rst b/Documentation/networking/netdevices.rst
index d2a238f8cc8b..1bb68a73bb67 100644
--- a/Documentation/networking/netdevices.rst
+++ b/Documentation/networking/netdevices.rst
@@ -421,6 +421,8 @@ For devices with locked ops, currently only the following notifiers are
 * ``NETDEV_CHANGENAME``
 * ``NETDEV_REGISTER``
 * ``NETDEV_UP``
+* ``NETDEV_DOWN``
+* ``NETDEV_GOING_DOWN``
 
 The following notifiers are running without the lock:
 * ``NETDEV_UNREGISTER``
diff --git a/net/core/dev.c b/net/core/dev.c
index 9d49493f4fb5..714d05283500 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1802,6 +1802,7 @@ void netif_close_many(struct list_head *head, bool unlink)
 	__dev_close_many(head);
 
 	list_for_each_entry_safe(dev, tmp, head, close_list) {
+		netdev_assert_locked_ops_compat(dev);
 		rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP | IFF_RUNNING, GFP_KERNEL, 0, NULL);
 		call_netdevice_notifiers(NETDEV_DOWN, dev);
 		if (unlink)
@@ -9787,6 +9788,8 @@ void __dev_notify_flags(struct net_device *dev, unsigned int old_flags,
 {
 	unsigned int changes = dev->flags ^ old_flags;
 
+	netdev_assert_locked_ops_compat(dev);
+
 	if (gchanges)
 		rtmsg_ifinfo(RTM_NEWLINK, dev, gchanges, GFP_ATOMIC, portid, nlh);
 
diff --git a/net/core/lock_debug.c b/net/core/lock_debug.c
index 8a81c5430705..abc4c00728b1 100644
--- a/net/core/lock_debug.c
+++ b/net/core/lock_debug.c
@@ -24,15 +24,15 @@ int netdev_debug_event(struct notifier_block *nb, unsigned long event,
 	case NETDEV_CHANGE:
 	case NETDEV_REGISTER:
 	case NETDEV_UP:
+	case NETDEV_DOWN:
+	case NETDEV_GOING_DOWN:
 		netdev_assert_locked_ops_compat(dev);
 		fallthrough;
-	case NETDEV_DOWN:
 	case NETDEV_REBOOT:
 	case NETDEV_UNREGISTER:
 	case NETDEV_CHANGEMTU:
 	case NETDEV_CHANGEADDR:
 	case NETDEV_PRE_CHANGEADDR:
-	case NETDEV_GOING_DOWN:
 	case NETDEV_FEAT_CHANGE:
 	case NETDEV_BONDING_FAILOVER:
 	case NETDEV_PRE_UP:
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 6/6] net: document NETDEV_UNREGISTER unlocked rationale
  2026-06-30 18:21 [PATCH net-next 0/6] net: hold instance lock around NETDEV_DOWN and NETDEV_GOING_DOWN Stanislav Fomichev
                   ` (4 preceding siblings ...)
  2026-06-30 18:21 ` [PATCH net-next 5/6] net: require instance lock for NETDEV_DOWN/GOING_DOWN notifiers Stanislav Fomichev
@ 2026-06-30 18:21 ` Stanislav Fomichev
  2026-06-30 23:43   ` Jakub Kicinski
  5 siblings, 1 reply; 10+ messages in thread
From: Stanislav Fomichev @ 2026-06-30 18:21 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, kuba, pabeni

The lock-state table marks UNREGISTER as unlocked without saying
why. Add a short note that many handlers release the lowers via
dev_close().

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 Documentation/networking/netdevices.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/networking/netdevices.rst b/Documentation/networking/netdevices.rst
index 1bb68a73bb67..761cdb08bf0f 100644
--- a/Documentation/networking/netdevices.rst
+++ b/Documentation/networking/netdevices.rst
@@ -427,6 +427,14 @@ For devices with locked ops, currently only the following notifiers are
 The following notifiers are running without the lock:
 * ``NETDEV_UNREGISTER``
 
+Many ``NETDEV_UNREGISTER`` handlers release their lowers with
+``dev_close()``, which takes the instance lock itself. Holding
+the lock across UNREGISTER would deadlock.
+
+Moving UNREGISTER under the lock is mechanical: switch those
+callers to the ``netif_*()`` lock-held variants. Deferred to
+limit churn.
+
 There are no clear expectations for the remaining notifiers. Notifiers not on
 the list may run with or without the instance lock, potentially even invoking
 the same notifier type with and without the lock from different code paths.
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 6/6] net: document NETDEV_UNREGISTER unlocked rationale
  2026-06-30 18:21 ` [PATCH net-next 6/6] net: document NETDEV_UNREGISTER unlocked rationale Stanislav Fomichev
@ 2026-06-30 23:43   ` Jakub Kicinski
  2026-07-01 16:02     ` Stanislav Fomichev
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2026-06-30 23:43 UTC (permalink / raw)
  To: Stanislav Fomichev; +Cc: netdev, davem, edumazet, pabeni

On Tue, 30 Jun 2026 11:21:29 -0700 Stanislav Fomichev wrote:
> +Many ``NETDEV_UNREGISTER`` handlers release their lowers with
> +``dev_close()``, which takes the instance lock itself. Holding
> +the lock across UNREGISTER would deadlock.
> +
> +Moving UNREGISTER under the lock is mechanical: switch those
> +callers to the ``netif_*()`` lock-held variants. Deferred to
> +limit churn.

Not following TBH. Let's say there's a UNREGISTER ntf for eth0.
Are you saying that eg. vlan which closes their own vlan0 devices
on top of eth0 needs to be switched to netif_ ? That wouldn't make
sense since the notification is holding netdev_lock(eth0) and
we're talking about netif_close(vlan0)?

Doing anything with the device that is sending the UNREGISTER
sounds odd, since it's going away..

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 6/6] net: document NETDEV_UNREGISTER unlocked rationale
  2026-06-30 23:43   ` Jakub Kicinski
@ 2026-07-01 16:02     ` Stanislav Fomichev
  2026-07-01 16:52       ` Jakub Kicinski
  0 siblings, 1 reply; 10+ messages in thread
From: Stanislav Fomichev @ 2026-07-01 16:02 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, davem, edumazet, pabeni

On 06/30, Jakub Kicinski wrote:
> On Tue, 30 Jun 2026 11:21:29 -0700 Stanislav Fomichev wrote:
> > +Many ``NETDEV_UNREGISTER`` handlers release their lowers with
> > +``dev_close()``, which takes the instance lock itself. Holding
> > +the lock across UNREGISTER would deadlock.
> > +
> > +Moving UNREGISTER under the lock is mechanical: switch those
> > +callers to the ``netif_*()`` lock-held variants. Deferred to
> > +limit churn.


> Doing anything with the device that is sending the UNREGISTER
> sounds odd, since it's going away..

Looking at __bond_release_one, it does try to restore a few original
settings, mostly mtu, I think? (And it does call dev_close unconditionally,
that's fixable).

bond_netdev_event
  bond_slave_netdev_event(slave_dev=event_dev)
    __bond_release_one
      {__netif,dev}_set_mtu(slave_dev, slave->original_mtu)

Or am I misreading this part?

> Not following TBH. Let's say there's a UNREGISTER ntf for eth0.
> Are you saying that eg. vlan which closes their own vlan0 devices
> on top of eth0 needs to be switched to netif_ ? That wouldn't make
> sense since the notification is holding netdev_lock(eth0) and
> we're talking about netif_close(vlan0)?

Maybe rewording it to `... UNREGISTER handlers interact with their
lowers using dev_xxx handlers which take the instance lock` ?

I'm mainly looking for an excuse/explanation on why UNREGISTER
is not converted in this series. Or should I bite the bullet and
add a few patches to make UNREGISTER ops locked as well?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 6/6] net: document NETDEV_UNREGISTER unlocked rationale
  2026-07-01 16:02     ` Stanislav Fomichev
@ 2026-07-01 16:52       ` Jakub Kicinski
  0 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2026-07-01 16:52 UTC (permalink / raw)
  To: Stanislav Fomichev; +Cc: netdev, davem, edumazet, pabeni

On Wed, 1 Jul 2026 09:02:27 -0700 Stanislav Fomichev wrote:
> On 06/30, Jakub Kicinski wrote:
> > On Tue, 30 Jun 2026 11:21:29 -0700 Stanislav Fomichev wrote:  
> > > +Many ``NETDEV_UNREGISTER`` handlers release their lowers with
> > > +``dev_close()``, which takes the instance lock itself. Holding
> > > +the lock across UNREGISTER would deadlock.
> > > +
> > > +Moving UNREGISTER under the lock is mechanical: switch those
> > > +callers to the ``netif_*()`` lock-held variants. Deferred to
> > > +limit churn.  
> 
> > Doing anything with the device that is sending the UNREGISTER
> > sounds odd, since it's going away..  
> 
> Looking at __bond_release_one, it does try to restore a few original
> settings, mostly mtu, I think? (And it does call dev_close unconditionally,
> that's fixable).
> 
> bond_netdev_event
>   bond_slave_netdev_event(slave_dev=event_dev)
>     __bond_release_one
>       {__netif,dev}_set_mtu(slave_dev, slave->original_mtu)
> 
> Or am I misreading this part?

Oh, I see. I guess it's pointless to restore on unregistered
device but I guess it's not illegal so maybe not worth the
complexity of a skip..

> > Not following TBH. Let's say there's a UNREGISTER ntf for eth0.
> > Are you saying that eg. vlan which closes their own vlan0 devices
> > on top of eth0 needs to be switched to netif_ ? That wouldn't make
> > sense since the notification is holding netdev_lock(eth0) and
> > we're talking about netif_close(vlan0)?  
> 
> Maybe rewording it to `... UNREGISTER handlers interact with their
> lowers using dev_xxx handlers which take the instance lock` ?
> 
> I'm mainly looking for an excuse/explanation on why UNREGISTER
> is not converted in this series. Or should I bite the bullet and
> add a few patches to make UNREGISTER ops locked as well?

The documentation is fine. Just need a better phrasing, I guess.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-07-01 16:52 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-30 18:21 [PATCH net-next 0/6] net: hold instance lock around NETDEV_DOWN and NETDEV_GOING_DOWN Stanislav Fomichev
2026-06-30 18:21 ` [PATCH net-next 1/6] net: hold instance lock around NETDEV_DOWN/GOING_DOWN Stanislav Fomichev
2026-06-30 18:21 ` [PATCH net-next 2/6] net: dsa: hold instance lock on close-on-shutdown paths Stanislav Fomichev
2026-06-30 18:21 ` [PATCH net-next 3/6] net: mtk_eth_soc: hold instance lock around DMA-device-swap close Stanislav Fomichev
2026-06-30 18:21 ` [PATCH net-next 4/6] net: rtnetlink: take instance lock inside rtnl_configure_link Stanislav Fomichev
2026-06-30 18:21 ` [PATCH net-next 5/6] net: require instance lock for NETDEV_DOWN/GOING_DOWN notifiers Stanislav Fomichev
2026-06-30 18:21 ` [PATCH net-next 6/6] net: document NETDEV_UNREGISTER unlocked rationale Stanislav Fomichev
2026-06-30 23:43   ` Jakub Kicinski
2026-07-01 16:02     ` Stanislav Fomichev
2026-07-01 16:52       ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox