[patch net-next 1/2] net: correct lock name in dev_[uc/mc]_sync documentations.

Subsystems: networking [general], the rest

STALE5336d

7 messages, 3 authors, 2012-01-09 · open the first message on its own page

[patch net-next 1/2] net: correct lock name in dev_[uc/mc]_sync documentations.

From: Jiri Pirko <hidden>
Date: 2012-01-09 16:18:41

Signed-off-by: Jiri Pirko <redacted>
---
 net/core/dev_addr_lists.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index febba51..c34ce9f 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c

@@ -427,7 +427,7 @@ EXPORT_SYMBOL(dev_uc_del);
  *
  *	Add newly added addresses to the destination device and release
  *	addresses that have no users left. The source device must be
- *	locked by netif_tx_lock_bh.
+ *	locked by netif_addr_lock_bh.
  *
  *	This function is intended to be called from the dev->set_rx_mode
  *	function of layered software devices.

@@ -590,7 +590,7 @@ EXPORT_SYMBOL(dev_mc_del_global);
  *
  *	Add newly added addresses to the destination device and release
  *	addresses that have no users left. The source device must be
- *	locked by netif_tx_lock_bh.
+ *	locked by netif_addr_lock_bh.
  *
  *	This function is intended to be called from the ndo_set_rx_mode
  *	function of layered software devices.

-- 
1.7.6

[patch net-next 2/2] net: introduce netif_addr_lock_nested() and call if when appropriate

From: Jiri Pirko <hidden>
Date: 2012-01-09 16:18:42

dev_uc_sync() and dev_mc_sync() are acquiring netif_addr_lock for
destination device of synchronization. Since netif_addr_lock is already
held at the time for source device, this triggers depmod deathlock
warning.

There's no way this deathlock can happen so use spin_lock_nested() to
silence the warning.

Signed-off-by: Jiri Pirko <redacted>
---
 include/linux/netdevice.h |    5 +++++
 net/core/dev_addr_lists.c |   12 ++++++------
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index a1d1095..656fade 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h

@@ -2450,6 +2450,11 @@ static inline void netif_addr_lock(struct net_device *dev)
 	spin_lock(&dev->addr_list_lock);
 }
 
+static inline void netif_addr_lock_nested(struct net_device *dev)
+{
+	spin_lock_nested(&dev->addr_list_lock, DENTRY_D_LOCK_NESTED);
+}
+
 static inline void netif_addr_lock_bh(struct net_device *dev)
 {
 	spin_lock_bh(&dev->addr_list_lock);

diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index c34ce9f..29c07fe 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c

@@ -439,11 +439,11 @@ int dev_uc_sync(struct net_device *to, struct net_device *from)
 	if (to->addr_len != from->addr_len)
 		return -EINVAL;
 
-	netif_addr_lock_bh(to);
+	netif_addr_lock_nested(to);
 	err = __hw_addr_sync(&to->uc, &from->uc, to->addr_len);
 	if (!err)
 		__dev_set_rx_mode(to);
-	netif_addr_unlock_bh(to);
+	netif_addr_unlock(to);
 	return err;
 }
 EXPORT_SYMBOL(dev_uc_sync);

@@ -463,7 +463,7 @@ void dev_uc_unsync(struct net_device *to, struct net_device *from)
 		return;
 
 	netif_addr_lock_bh(from);
-	netif_addr_lock(to);
+	netif_addr_lock_nested(to);
 	__hw_addr_unsync(&to->uc, &from->uc, to->addr_len);
 	__dev_set_rx_mode(to);
 	netif_addr_unlock(to);

@@ -602,11 +602,11 @@ int dev_mc_sync(struct net_device *to, struct net_device *from)
 	if (to->addr_len != from->addr_len)
 		return -EINVAL;
 
-	netif_addr_lock_bh(to);
+	netif_addr_lock_nested(to);
 	err = __hw_addr_sync(&to->mc, &from->mc, to->addr_len);
 	if (!err)
 		__dev_set_rx_mode(to);
-	netif_addr_unlock_bh(to);
+	netif_addr_unlock(to);
 	return err;
 }
 EXPORT_SYMBOL(dev_mc_sync);

@@ -626,7 +626,7 @@ void dev_mc_unsync(struct net_device *to, struct net_device *from)
 		return;
 
 	netif_addr_lock_bh(from);
-	netif_addr_lock(to);
+	netif_addr_lock_nested(to);
 	__hw_addr_unsync(&to->mc, &from->mc, to->addr_len);
 	__dev_set_rx_mode(to);
 	netif_addr_unlock(to);

-- 
1.7.6

[patch net-next v2 2/2] net: introduce netif_addr_lock_nested() and call if when appropriate

From: Jiri Pirko <hidden>
Date: 2012-01-09 16:36:57

dev_uc_sync() and dev_mc_sync() are acquiring netif_addr_lock for
destination device of synchronization. Since netif_addr_lock is already
held at the time for source device, this triggers depmod deathlock
warning.

There's no way this deathlock can happen so use spin_lock_nested() to
silence the warning.

Signed-off-by: Jiri Pirko <redacted>

v1->v2:
	use SINGLE_DEPTH_NESTING instead of wrongly c&p'ed DENTRY_D_LOCK_NESTED
---
 include/linux/netdevice.h |    5 +++++
 net/core/dev_addr_lists.c |   12 ++++++------
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index a1d1095..d0522bb 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h

@@ -2450,6 +2450,11 @@ static inline void netif_addr_lock(struct net_device *dev)
 	spin_lock(&dev->addr_list_lock);
 }
 
+static inline void netif_addr_lock_nested(struct net_device *dev)
+{
+	spin_lock_nested(&dev->addr_list_lock, SINGLE_DEPTH_NESTING);
+}
+
 static inline void netif_addr_lock_bh(struct net_device *dev)
 {
 	spin_lock_bh(&dev->addr_list_lock);

diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index c34ce9f..29c07fe 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c

@@ -439,11 +439,11 @@ int dev_uc_sync(struct net_device *to, struct net_device *from)
 	if (to->addr_len != from->addr_len)
 		return -EINVAL;
 
-	netif_addr_lock_bh(to);
+	netif_addr_lock_nested(to);
 	err = __hw_addr_sync(&to->uc, &from->uc, to->addr_len);
 	if (!err)
 		__dev_set_rx_mode(to);
-	netif_addr_unlock_bh(to);
+	netif_addr_unlock(to);
 	return err;
 }
 EXPORT_SYMBOL(dev_uc_sync);

@@ -463,7 +463,7 @@ void dev_uc_unsync(struct net_device *to, struct net_device *from)
 		return;
 
 	netif_addr_lock_bh(from);
-	netif_addr_lock(to);
+	netif_addr_lock_nested(to);
 	__hw_addr_unsync(&to->uc, &from->uc, to->addr_len);
 	__dev_set_rx_mode(to);
 	netif_addr_unlock(to);

@@ -602,11 +602,11 @@ int dev_mc_sync(struct net_device *to, struct net_device *from)
 	if (to->addr_len != from->addr_len)
 		return -EINVAL;
 
-	netif_addr_lock_bh(to);
+	netif_addr_lock_nested(to);
 	err = __hw_addr_sync(&to->mc, &from->mc, to->addr_len);
 	if (!err)
 		__dev_set_rx_mode(to);
-	netif_addr_unlock_bh(to);
+	netif_addr_unlock(to);
 	return err;
 }
 EXPORT_SYMBOL(dev_mc_sync);

@@ -626,7 +626,7 @@ void dev_mc_unsync(struct net_device *to, struct net_device *from)
 		return;
 
 	netif_addr_lock_bh(from);
-	netif_addr_lock(to);
+	netif_addr_lock_nested(to);
 	__hw_addr_unsync(&to->mc, &from->mc, to->addr_len);
 	__dev_set_rx_mode(to);
 	netif_addr_unlock(to);

-- 
1.7.6

Re: [patch net-next v2 2/2] net: introduce netif_addr_lock_nested() and call if when appropriate

From: Ben Hutchings <hidden>
Date: 2012-01-09 19:07:14

On Mon, 2012-01-09 at 17:36 +0100, Jiri Pirko wrote:

dev_uc_sync() and dev_mc_sync() are acquiring netif_addr_lock for
destination device of synchronization. Since netif_addr_lock is already
held at the time for source device, this triggers depmod deathlock
warning.

[...]

I think you mean '...lockdep deadlock warning'?

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [patch net-next 1/2] net: correct lock name in dev_[uc/mc]_sync documentations.

From: David Miller <davem@davemloft.net>
Date: 2012-01-09 20:47:16

From: Jiri Pirko <redacted>
Date: Mon,  9 Jan 2012 17:18:34 +0100

Signed-off-by: Jiri Pirko <redacted>

Applied.

Re: [patch net-next v2 2/2] net: introduce netif_addr_lock_nested() and call if when appropriate

From: David Miller <davem@davemloft.net>
Date: 2012-01-09 20:47:27

From: Ben Hutchings <redacted>
Date: Mon, 9 Jan 2012 19:07:08 +0000

On Mon, 2012-01-09 at 17:36 +0100, Jiri Pirko wrote:

quoted

dev_uc_sync() and dev_mc_sync() are acquiring netif_addr_lock for
destination device of synchronization. Since netif_addr_lock is already
held at the time for source device, this triggers depmod deathlock
warning.

[...]

I think you mean '...lockdep deadlock warning'?

Applied with this wording fixed.

Re: [patch net-next v2 2/2] net: introduce netif_addr_lock_nested() and call if when appropriate

From: Jiri Pirko <hidden>
Date: 2012-01-09 23:47:48

Mon, Jan 09, 2012 at 08:07:08PM CET, bhutchings@solarflare.com wrote:

On Mon, 2012-01-09 at 17:36 +0100, Jiri Pirko wrote:

quoted

dev_uc_sync() and dev_mc_sync() are acquiring netif_addr_lock for
destination device of synchronization. Since netif_addr_lock is already
held at the time for source device, this triggers depmod deathlock
warning.

[...]

I think you mean '...lockdep deadlock warning'?

That's him :) I mixed that up a bit. Sorry :(

Jirka

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help