[B.A.T.M.A.N.] [RFC] batman-adv: Set special lockdep classes to avoid lockdep warning

Subsystems: the rest

6 messages, 3 authors, 2012-08-24 · open the first message on its own page

[B.A.T.M.A.N.] [RFC] batman-adv: Set special lockdep classes to avoid lockdep warning

From: Sven Eckelmann <sven@narfation.org>
Date: 2012-08-19 07:29:53

[text will be added after I am awake... maybe]

[Should be fix the problem found in bug 162... formerly known as 161. code
stolen from macvlan... this is just a reimplementation of a solution proposed
by Simon Wunderlich]

Reported-by: Linus Lüssing <redacted>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 soft-interface.c |   29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/soft-interface.c b/soft-interface.c
index 1aee7db..588c6f6 100644
--- a/soft-interface.c
+++ b/soft-interface.c

@@ -344,7 +344,36 @@ out:
 	return;
 }
 
+/*
+ * batman-adv network devices have devices nesting below it and are a special
+ * "super class" of normal network devices; split their locks off into a
+ * separate class since they always nest.
+ */
+static struct lock_class_key batadv_netdev_xmit_lock_key;
+static struct lock_class_key batadv_netdev_addr_lock_key;
+
+static void batadv_set_lockdep_class_one(struct net_device *dev,
+					 struct netdev_queue *txq,
+					 void *_unused)
+{
+	lockdep_set_class(&txq->_xmit_lock, &batadv_netdev_xmit_lock_key);
+}
+
+static void batadv_set_lockdep_class(struct net_device *dev)
+{
+	lockdep_set_class(&dev->addr_list_lock, &batadv_netdev_addr_lock_key);
+	netdev_for_each_tx_queue(dev, batadv_set_lockdep_class_one, NULL);
+}
+
+static int batadv_softif_init(struct net_device *dev)
+{
+	batadv_set_lockdep_class(dev);
+
+	return 0;
+}
+
 static const struct net_device_ops batadv_netdev_ops = {
+	.ndo_init = batadv_softif_init,
 	.ndo_open = batadv_interface_open,
 	.ndo_stop = batadv_interface_release,
 	.ndo_get_stats = batadv_interface_stats,

-- 
1.7.10.4

Re: [B.A.T.M.A.N.] [RFC] batman-adv: Set special lockdep classes to avoid lockdep warning

From: Simon Wunderlich <hidden>
Date: 2012-08-19 07:57:48

Hey Sven,

it works in my QEMU machines (and I could reproduce the bug report from 162).

Acked-by: Simon Wunderlich <redacted>
(provided you add some sane commit message. ;] )


Cheers,
	Simon


On Sun, Aug 19, 2012 at 09:29:53AM +0200, Sven Eckelmann wrote:

quoted hunk

[text will be added after I am awake... maybe]

[Should be fix the problem found in bug 162... formerly known as 161. code
stolen from macvlan... this is just a reimplementation of a solution proposed
by Simon Wunderlich]

Reported-by: Linus Lüssing <redacted>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 soft-interface.c |   29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/soft-interface.c b/soft-interface.c
index 1aee7db..588c6f6 100644
--- a/soft-interface.c
+++ b/soft-interface.c

@@ -344,7 +344,36 @@ out:
 	return;
 }
 
+/*
+ * batman-adv network devices have devices nesting below it and are a special
+ * "super class" of normal network devices; split their locks off into a
+ * separate class since they always nest.
+ */
+static struct lock_class_key batadv_netdev_xmit_lock_key;
+static struct lock_class_key batadv_netdev_addr_lock_key;
+
+static void batadv_set_lockdep_class_one(struct net_device *dev,
+					 struct netdev_queue *txq,
+					 void *_unused)
+{
+	lockdep_set_class(&txq->_xmit_lock, &batadv_netdev_xmit_lock_key);
+}
+
+static void batadv_set_lockdep_class(struct net_device *dev)
+{
+	lockdep_set_class(&dev->addr_list_lock, &batadv_netdev_addr_lock_key);
+	netdev_for_each_tx_queue(dev, batadv_set_lockdep_class_one, NULL);
+}
+
+static int batadv_softif_init(struct net_device *dev)
+{
+	batadv_set_lockdep_class(dev);
+
+	return 0;
+}
+
 static const struct net_device_ops batadv_netdev_ops = {
+	.ndo_init = batadv_softif_init,
 	.ndo_open = batadv_interface_open,
 	.ndo_stop = batadv_interface_release,
 	.ndo_get_stats = batadv_interface_stats,

-- 
1.7.10.4

[B.A.T.M.A.N.] [PATCH] batman-adv: Set special lockdep classes to avoid lockdep warning

From: Sven Eckelmann <sven@narfation.org>
Date: 2012-08-19 23:22:39

Transmissions over batman-adv devices always start another nested transmission
over devices attached to the batman-adv interface. These devices usually use
the ethernet lockdep class for the tx_queue lock which is also set by default
for all batman-adv devices. Lockdep will detect a nested locking attempt of two
locks with the same class and warn about a possible deadlock.

This is the default and expected behavior and should not alarm the locking
correctness prove mechanism. Therefore, the locks for all netdevice specific
lock get a special batman-adv lock class to avoid a false positive for each
transmission.

Reported-by: Linus Lüssing <redacted>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 soft-interface.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/soft-interface.c b/soft-interface.c
index 1aee7db..513d50a 100644
--- a/soft-interface.c
+++ b/soft-interface.c

@@ -354,6 +354,27 @@ static const struct net_device_ops batadv_netdev_ops = {
 	.ndo_validate_addr = eth_validate_addr
 };
 
+/*
+ * batman-adv network devices have devices nesting below it and are a special
+ * "super class" of normal network devices; split their locks off into a
+ * separate class since they always nest.
+ */
+static struct lock_class_key batadv_netdev_xmit_lock_key;
+static struct lock_class_key batadv_netdev_addr_lock_key;
+
+static void batadv_set_lockdep_class_one(struct net_device *dev,
+					 struct netdev_queue *txq,
+					 void *_unused)
+{
+	lockdep_set_class(&txq->_xmit_lock, &batadv_netdev_xmit_lock_key);
+}
+
+static void batadv_set_lockdep_class(struct net_device *dev)
+{
+	lockdep_set_class(&dev->addr_list_lock, &batadv_netdev_addr_lock_key);
+	netdev_for_each_tx_queue(dev, batadv_set_lockdep_class_one, NULL);
+}
+
 static void batadv_interface_setup(struct net_device *dev)
 {
 	struct batadv_priv *priv = netdev_priv(dev);

@@ -363,6 +384,7 @@ static void batadv_interface_setup(struct net_device *dev)
 	dev->netdev_ops = &batadv_netdev_ops;
 	dev->destructor = free_netdev;
 	dev->tx_queue_len = 0;
+	batadv_set_lockdep_class(dev);
 
 	/* can't call min_mtu, because the needed variables
 	 * have not been initialized yet

-- 
1.7.10.4

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Set special lockdep classes to avoid lockdep warning

From: Sven Eckelmann <sven@narfation.org>
Date: 2012-08-20 07:00:14

On Monday 20 August 2012 01:22:39 Sven Eckelmann wrote:

lock get a special batman-adv lock class to avoid a false positive for each

The first "lock" should have been "queues"

Kind regards,
	Sven

[B.A.T.M.A.N.] [PATCHv2] batman-adv: Set special lockdep classes to avoid lockdep warning

From: Sven Eckelmann <sven@narfation.org>
Date: 2012-08-20 07:03:59

Transmissions over batman-adv devices always start another nested transmission
over devices attached to the batman-adv interface. These devices usually use
the ethernet lockdep class for the tx_queue lock which is also set by default
for all batman-adv devices. Lockdep will detect a nested locking attempt of two
locks with the same class and warn about a possible deadlock.

This is the default and expected behavior and should not alarm the locking
correctness prove mechanism. Therefore, the locks for all netdevice specific tx
queues get a special batman-adv lock class to avoid a false positive for each
transmission.

Reported-by: Linus Lüssing <redacted>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
* fixed comment style
* fixed commit message

 soft-interface.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/soft-interface.c b/soft-interface.c
index 1aee7db..909ab54 100644
--- a/soft-interface.c
+++ b/soft-interface.c

@@ -354,6 +354,26 @@ static const struct net_device_ops batadv_netdev_ops = {
 	.ndo_validate_addr = eth_validate_addr
 };
 
+/* batman-adv network devices have devices nesting below it and are a special
+ * "super class" of normal network devices; split their locks off into a
+ * separate class since they always nest.
+ */
+static struct lock_class_key batadv_netdev_xmit_lock_key;
+static struct lock_class_key batadv_netdev_addr_lock_key;
+
+static void batadv_set_lockdep_class_one(struct net_device *dev,
+					 struct netdev_queue *txq,
+					 void *_unused)
+{
+	lockdep_set_class(&txq->_xmit_lock, &batadv_netdev_xmit_lock_key);
+}
+
+static void batadv_set_lockdep_class(struct net_device *dev)
+{
+	lockdep_set_class(&dev->addr_list_lock, &batadv_netdev_addr_lock_key);
+	netdev_for_each_tx_queue(dev, batadv_set_lockdep_class_one, NULL);
+}
+
 static void batadv_interface_setup(struct net_device *dev)
 {
 	struct batadv_priv *priv = netdev_priv(dev);

@@ -363,6 +383,7 @@ static void batadv_interface_setup(struct net_device *dev)
 	dev->netdev_ops = &batadv_netdev_ops;
 	dev->destructor = free_netdev;
 	dev->tx_queue_len = 0;
+	batadv_set_lockdep_class(dev);
 
 	/* can't call min_mtu, because the needed variables
 	 * have not been initialized yet

-- 
1.7.10.4

Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: Set special lockdep classes to avoid lockdep warning

From: Marek Lindner <hidden>
Date: 2012-08-24 21:42:46

On Monday, August 20, 2012 09:03:59 Sven Eckelmann wrote:

Transmissions over batman-adv devices always start another nested
transmission over devices attached to the batman-adv interface. These
devices usually use the ethernet lockdep class for the tx_queue lock which
is also set by default for all batman-adv devices. Lockdep will detect a
nested locking attempt of two locks with the same class and warn about a
possible deadlock.

This is the default and expected behavior and should not alarm the locking
correctness prove mechanism. Therefore, the locks for all netdevice
specific tx queues get a special batman-adv lock class to avoid a false
positive for each transmission.

Reported-by: Linus Lüssing <redacted>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
* fixed comment style
* fixed commit message

 soft-interface.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Applied in revision e27bb6f.

Thanks,
Marek

`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