[B.A.T.M.A.N.] [PATCH] batman-adv: Initialize lockdep class keys for hashes

[B.A.T.M.A.N.] [PATCH] batman-adv: Initialize lockdep class keys for hashes

[Sven Eckelmann]

The hash for claim and backbone hash in the bridge loop avoidance code receive
the same key because they are getting initialized by hash_new with the same
key. Lockdep will create a backtrace when they are used recursively. This can
be avoided by reinitializing the key directly after the hash_new.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 bridge_loop_avoidance.c |   11 +++++++++++
 hash.c                  |    9 +++++++++
 hash.h                  |    3 +++
 3 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c
index 8a17a78..c74d8a2 100644
--- a/bridge_loop_avoidance.c
+++ b/bridge_loop_avoidance.c
@@ -1124,6 +1124,14 @@ out:
 	bla_start_timer(bat_priv);
 }
 
+/* The hash for claim and backbone hash receive the same key because they
+ * are getting initialized by hash_new with the same key. Reinitializing
+ * them with to different keys to allow nested locking without generating
+ * lockdep warnings
+ */
+static struct lock_class_key claim_hash_lock_class_key;
+static struct lock_class_key backbone_hash_lock_class_key;
+
 /* initialize all bla structures */
 int bla_init(struct bat_priv *bat_priv)
 {
@@ -1158,6 +1166,9 @@ int bla_init(struct bat_priv *bat_priv)
 	bat_priv->claim_hash = hash_new(128);
 	bat_priv->backbone_hash = hash_new(32);
 
+	hash_set_lock_class(bat_priv->claim_hash, &claim_hash_lock_class_key);
+	hash_set_lock_class(bat_priv->backbone_hash, &backbone_hash_lock_class_key);
+
 	if (!bat_priv->claim_hash || !bat_priv->backbone_hash)
 		return -1;
 
diff --git a/hash.c b/hash.c
index 117687b..13d4597 100644
--- a/hash.c
+++ b/hash.c
@@ -69,3 +69,12 @@ free_hash:
 	kfree(hash);
 	return NULL;
 }
+
+void hash_set_lock_class(struct hashtable_t *hash, struct lock_class_key *key)
+{
+	uint32_t i;
+
+	for (i = 0 ; i < hash->size; i++) {
+		lockdep_set_class(&hash->list_locks[i], key);
+	}
+}
diff --git a/hash.h b/hash.h
index d4bd786..7bcb98f 100644
--- a/hash.h
+++ b/hash.h
@@ -45,6 +45,9 @@ struct hashtable_t {
 /* allocates and clears the hash */
 struct hashtable_t *hash_new(uint32_t size);
 
+/* set class key for all locks */
+void hash_set_lock_class(struct hashtable_t *hash, struct lock_class_key *key);
+
 /* free only the hashtable and the hash itself. */
 void hash_destroy(struct hashtable_t *hash);
 
-- 
1.7.9.1

[B.A.T.M.A.N.] [PATCHv2] batman-adv: Initialize lockdep class keys for hashes

[Sven Eckelmann]

The hash for claim and backbone hash in the bridge loop avoidance code receive
the same key because they are getting initialized by hash_new with the same
key. Lockdep will create a backtrace when they are used recursively. This can
be avoided by reinitializing the key directly after the hash_new.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
Sry, sent the wrong patch (the checkpatch-unclean one)

 bridge_loop_avoidance.c |   12 ++++++++++++
 hash.c                  |    8 ++++++++
 hash.h                  |    3 +++
 3 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c
index 8a17a78..66e2ce6 100644
--- a/bridge_loop_avoidance.c
+++ b/bridge_loop_avoidance.c
@@ -1124,6 +1124,14 @@ out:
 	bla_start_timer(bat_priv);
 }
 
+/* The hash for claim and backbone hash receive the same key because they
+ * are getting initialized by hash_new with the same key. Reinitializing
+ * them with to different keys to allow nested locking without generating
+ * lockdep warnings
+ */
+static struct lock_class_key claim_hash_lock_class_key;
+static struct lock_class_key backbone_hash_lock_class_key;
+
 /* initialize all bla structures */
 int bla_init(struct bat_priv *bat_priv)
 {
@@ -1158,6 +1166,10 @@ int bla_init(struct bat_priv *bat_priv)
 	bat_priv->claim_hash = hash_new(128);
 	bat_priv->backbone_hash = hash_new(32);
 
+	hash_set_lock_class(bat_priv->claim_hash, &claim_hash_lock_class_key);
+	hash_set_lock_class(bat_priv->backbone_hash,
+			    &backbone_hash_lock_class_key);
+
 	if (!bat_priv->claim_hash || !bat_priv->backbone_hash)
 		return -1;
 
diff --git a/hash.c b/hash.c
index 117687b..4578c20 100644
--- a/hash.c
+++ b/hash.c
@@ -69,3 +69,11 @@ free_hash:
 	kfree(hash);
 	return NULL;
 }
+
+void hash_set_lock_class(struct hashtable_t *hash, struct lock_class_key *key)
+{
+	uint32_t i;
+
+	for (i = 0 ; i < hash->size; i++)
+		lockdep_set_class(&hash->list_locks[i], key);
+}
diff --git a/hash.h b/hash.h
index d4bd786..7bcb98f 100644
--- a/hash.h
+++ b/hash.h
@@ -45,6 +45,9 @@ struct hashtable_t {
 /* allocates and clears the hash */
 struct hashtable_t *hash_new(uint32_t size);
 
+/* set class key for all locks */
+void hash_set_lock_class(struct hashtable_t *hash, struct lock_class_key *key);
+
 /* free only the hashtable and the hash itself. */
 void hash_destroy(struct hashtable_t *hash);
 
-- 
1.7.9.1

Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: Initialize lockdep class keys for hashes

[Marek Lindner]

On Thursday, March 29, 2012 12:38:20 Sven Eckelmann wrote:
> The hash for claim and backbone hash in the bridge loop avoidance code
> receive the same key because they are getting initialized by hash_new with
> the same key. Lockdep will create a backtrace when they are used
> recursively. This can be avoided by reinitializing the key directly after
> the hash_new.

Applied in revision 2c9aa3b.

Thanks,
Marek

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Initialize lockdep class keys for hashes

[Marek Lindner]

On Saturday, December 01, 2012 18:46:33 Linus Lüssing wrote:
> Tested-by: Linus Lüssing <linus.luessing@web.de>

Applied in revision 6ca1b35.

Thanks,
Marek

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Initialize lockdep class keys for hashes

[Linus Lüssing]

Tested-by: Linus Lüssing <linus.luessing@web.de>

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Initialize lockdep class keys for hashes

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 996 bytes --]

On Tue, Nov 13, 2012 at 09:06:22AM +0100, Antonio Quartulli wrote:
> On Sat, Nov 10, 2012 at 11:00:32AM +0100, Antonio Quartulli wrote:
> > Different hashes have the same class key key because they get
> > initialised with the same one. For this reason lockdep can create
> > false warning when they are used recursively.
> > 
> > Re-initialise the key for each hash after the invocation to hash_new()
> > to avoid this problem.
> > 
> > Signed-off-by: Antonio Quartulli <ordex@autistici.org>
> 
> I was thinking...instead of creating such "a free noise" in udev, we may want to
> add a configuration facility so that the user can choose which uevents to enable
> and which not..
> 
> If we don't do so batman-adv will start throwing uevents even if not needed.
> 
> Please, drop this patch. We need some more code here :-)

Sorry, the message above was meant for another patch -.-



-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Initialize lockdep class keys for hashes

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 841 bytes --]

On Sat, Nov 10, 2012 at 11:00:32AM +0100, Antonio Quartulli wrote:
> Different hashes have the same class key key because they get
> initialised with the same one. For this reason lockdep can create
> false warning when they are used recursively.
> 
> Re-initialise the key for each hash after the invocation to hash_new()
> to avoid this problem.
> 
> Signed-off-by: Antonio Quartulli <ordex@autistici.org>

I was thinking...instead of creating such "a free noise" in udev, we may want to
add a configuration facility so that the user can choose which uevents to enable
and which not..

If we don't do so batman-adv will start throwing uevents even if not needed.

Please, drop this patch. We need some more code here :-)

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[B.A.T.M.A.N.] [PATCH] batman-adv: Initialize lockdep class keys for hashes

[Antonio Quartulli]

Different hashes have the same class key key because they get
initialised with the same one. For this reason lockdep can create
false warning when they are used recursively.

Re-initialise the key for each hash after the invocation to hash_new()
to avoid this problem.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 originator.c        |  6 ++++++
 translation-table.c | 10 ++++++++++
 vis.c               |  6 ++++++
 3 files changed, 22 insertions(+)

diff --git a/originator.c b/originator.c
index 8c32cf1..109081c 100644
--- a/originator.c
+++ b/originator.c
@@ -29,6 +29,9 @@
 #include "soft-interface.h"
 #include "bridge_loop_avoidance.h"
 
+/* hash class keys */
+static struct lock_class_key batadv_orig_hash_lock_class_key;
+
 static void batadv_purge_orig(struct work_struct *work);
 
 static void batadv_start_purge_timer(struct batadv_priv *bat_priv)
@@ -57,6 +60,9 @@ int batadv_originator_init(struct batadv_priv *bat_priv)
 	if (!bat_priv->orig_hash)
 		goto err;
 
+	batadv_hash_set_lock_class(bat_priv->orig_hash,
+				   &batadv_orig_hash_lock_class_key);
+
 	batadv_start_purge_timer(bat_priv);
 	return 0;
 
diff --git a/translation-table.c b/translation-table.c
index 4657d9e..1bcb705 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -29,6 +29,10 @@
 
 #include <linux/crc16.h>
 
+/* hash class keys */
+static struct lock_class_key batadv_tt_local_hash_lock_class_key;
+static struct lock_class_key batadv_tt_global_hash_lock_class_key;
+
 static void batadv_send_roam_adv(struct batadv_priv *bat_priv, uint8_t *client,
 				 struct batadv_orig_node *orig_node);
 static void batadv_tt_purge(struct work_struct *work);
@@ -235,6 +239,9 @@ static int batadv_tt_local_init(struct batadv_priv *bat_priv)
 	if (!bat_priv->tt.local_hash)
 		return -ENOMEM;
 
+	batadv_hash_set_lock_class(bat_priv->tt.local_hash,
+				   &batadv_tt_local_hash_lock_class_key);
+
 	return 0;
 }
 
@@ -691,6 +698,9 @@ static int batadv_tt_global_init(struct batadv_priv *bat_priv)
 	if (!bat_priv->tt.global_hash)
 		return -ENOMEM;
 
+	batadv_hash_set_lock_class(bat_priv->tt.global_hash,
+				   &batadv_tt_global_hash_lock_class_key);
+
 	return 0;
 }
 
diff --git a/vis.c b/vis.c
index 0f65a9d..60eb9b7 100644
--- a/vis.c
+++ b/vis.c
@@ -28,6 +28,9 @@
 
 #define BATADV_MAX_VIS_PACKET_SIZE 1000
 
+/* hash class keys */
+static struct lock_class_key batadv_vis_hash_lock_class_key;
+
 static void batadv_start_vis_timer(struct batadv_priv *bat_priv);
 
 /* free the info */
@@ -852,6 +855,9 @@ int batadv_vis_init(struct batadv_priv *bat_priv)
 		goto err;
 	}
 
+	batadv_hash_set_lock_class(bat_priv->vis.hash,
+				   &batadv_vis_hash_lock_class_key);
+
 	bat_priv->vis.my_info = kmalloc(BATADV_MAX_VIS_PACKET_SIZE, GFP_ATOMIC);
 	if (!bat_priv->vis.my_info)
 		goto err;
-- 
1.8.0