[PATCH v2] rhashtable: use private lockdep class for all locks.

NeilBrown posted 1 patch 1 month, 2 weeks ago
include/linux/rhashtable-types.h |  3 +++
include/linux/rhashtable.h       | 22 +++++++++-------------
lib/rhashtable.c                 | 14 +++++++++++---
lib/test_rhashtable.c            |  2 +-
4 files changed, 24 insertions(+), 17 deletions(-)
[PATCH v2] rhashtable: use private lockdep class for all locks.
Posted by NeilBrown 1 month, 2 weeks ago

This patch builds on
 Commit: 060d4e94b8d4 ("rhashtable: give each instance its own lockdep class")

to allow lockdep to see each rhashtable as unique with respect to all of
the locks, not just the ht->mutex.

This is needed if rhashtable is to be used tracepoint BPF which could
run while an rhashtable lock (in a different table) is held - see Link
below.

Rather then adding extra keys for the other locks, we use distinct
sub-classes for the different locks.
rhashtable->lock uses class 0
rhashtable->mutex uses class 1
bucket bitlocks, which are the only ones that are ever nested, use
 classes 2 and 3.

Currently rht_lock() and rht_lock_nested() are quite separate code
despite the near-identical function.  This patch moves rht_lock() to
after rht_lock_nested(), and simply calls that other function with a
nesting level of zero.

Link: https://lore.kernel.org/all/20260801-fix-rhashtable-bucket-lockdep-v1-1-15a0f8ae094c@gmail.com/
Closes: https://syzkaller.appspot.com/bug?extid=ef8d17bae14efb960935
Tested-by: quanyeyang <quanyeyang@proton.me>
Signed-off-by: NeilBrown <neil@brown.name>
---

v2 includes a change to test_rhashtable.c as advised by sashiko.dev
NeilBrown


 include/linux/rhashtable-types.h |  3 +++
 include/linux/rhashtable.h       | 22 +++++++++-------------
 lib/rhashtable.c                 | 14 +++++++++++---
 lib/test_rhashtable.c            |  2 +-
 4 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/include/linux/rhashtable-types.h b/include/linux/rhashtable-types.h
index 57c11ec9dc64..0e1b172a4f6c 100644
--- a/include/linux/rhashtable-types.h
+++ b/include/linux/rhashtable-types.h
@@ -97,6 +97,9 @@ struct rhashtable {
 #ifdef CONFIG_MEM_ALLOC_PROFILING
 	struct alloc_tag		*alloc_tag;
 #endif
+#ifdef CONFIG_LOCKDEP
+	struct lock_class_key		*lockdep_key;
+#endif
 };
 
 /**
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 79f83b6eec27..f8358d43691b 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -320,18 +320,6 @@ static inline struct rhash_lock_head __rcu **rht_bucket_insert(
  * When we write to a bucket without unlocking, we use rht_assign_locked().
  */
 
-static inline unsigned long rht_lock(struct bucket_table *tbl,
-				     struct rhash_lock_head __rcu **bkt)
-	__acquires(__bitlock(0, bkt))
-{
-	unsigned long flags;
-
-	local_irq_save(flags);
-	bit_spin_lock(0, (unsigned long *)bkt);
-	lock_map_acquire(&tbl->dep_map);
-	return flags;
-}
-
 static inline unsigned long rht_lock_nested(struct bucket_table *tbl,
 					struct rhash_lock_head __rcu **bucket,
 					unsigned int subclass)
@@ -341,10 +329,18 @@ static inline unsigned long rht_lock_nested(struct bucket_table *tbl,
 
 	local_irq_save(flags);
 	bit_spin_lock(0, (unsigned long *)bucket);
-	lock_acquire_exclusive(&tbl->dep_map, subclass, 0, NULL, _THIS_IP_);
+	/* subclass 0 is used for ->lock and 1 for ->mutex. 2+ for bitlocks */
+	lock_acquire_exclusive(&tbl->dep_map, subclass+2, 0, NULL, _THIS_IP_);
 	return flags;
 }
 
+static inline unsigned long rht_lock(struct bucket_table *tbl,
+				     struct rhash_lock_head __rcu **bkt)
+	__acquires(__bitlock(0, bkt))
+{
+	return rht_lock_nested(tbl, bkt, 0);
+}
+
 static inline void rht_unlock(struct bucket_table *tbl,
 			      struct rhash_lock_head __rcu **bkt,
 			      unsigned long flags)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 8b2c405e7a66..a99283a887ea 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -189,7 +189,6 @@ static struct bucket_table *bucket_table_alloc(struct rhashtable *ht,
 	struct bucket_table *tbl = NULL;
 	size_t size;
 	int i;
-	static struct lock_class_key __key;
 
 	tbl = alloc_hooks_tag(ht->alloc_tag,
 			kvmalloc_node_align_noprof(struct_size(tbl, buckets, nbuckets),
@@ -205,7 +204,10 @@ static struct bucket_table *bucket_table_alloc(struct rhashtable *ht,
 	if (tbl == NULL)
 		return NULL;
 
-	lockdep_init_map(&tbl->dep_map, "rhashtable_bucket", &__key, 0);
+#ifdef CONFIG_LOCKDEP
+	/* bitlocks must use nesting level 2 or more */
+	lockdep_init_map(&tbl->dep_map, "rhashtable_bucket", ht->lockdep_key, 0);
+#endif
 
 	tbl->size = size;
 
@@ -428,7 +430,7 @@ static void rht_deferred_worker(struct work_struct *work)
 	int err = 0;
 
 	ht = container_of(work, struct rhashtable, run_work);
-	mutex_lock(&ht->mutex);
+	mutex_lock_nested(&ht->mutex, 1);
 
 	tbl = rht_dereference(ht->tbl, ht);
 	tbl = rhashtable_last_table(ht, tbl);
@@ -1172,8 +1174,14 @@ int __rhashtable_init_noprof(struct rhashtable *ht,
 		return -EINVAL;
 
 	memset(ht, 0, sizeof(*ht));
+	/* mutex_lock must use nesting level 1 */
 	mutex_init_with_key(&ht->mutex, key);
 	spin_lock_init(&ht->lock);
+	/* spin_lock can use nesting level 0 */
+	lockdep_set_class(&ht->lock, key);
+#ifdef CONFIG_LOCKDEP
+	ht->lockdep_key = key;
+#endif
 	memcpy(&ht->p, params, sizeof(*params));
 
 	alloc_tag_record(ht->alloc_tag);
diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c
index b767a38a74f9..85a615e74591 100644
--- a/lib/test_rhashtable.c
+++ b/lib/test_rhashtable.c
@@ -477,7 +477,7 @@ static unsigned int __init print_ht(struct rhltable *rhlt)
 
 	ht = &rhlt->ht;
 	/* Take the mutex to avoid RCU warning */
-	mutex_lock(&ht->mutex);
+	mutex_lock_nested(&ht->mutex, 1);
 	tbl = rht_dereference(ht->tbl, ht);
 	for (i = 0; i < tbl->size; i++) {
 		struct rhash_head *pos, *next;

base-commit: dac3e89a2c90c2feeb471e1f22a2512ad424b792
-- 
2.50.0.107.gf914562f5916.dirty
Re: [PATCH v2] rhashtable: use private lockdep class for all locks.
Posted by Herbert Xu 3 weeks, 3 days ago
On Sat, Aug 15, 2026 at 04:24:07PM +1000, NeilBrown wrote:
> 
> This patch builds on
>  Commit: 060d4e94b8d4 ("rhashtable: give each instance its own lockdep class")
> 
> to allow lockdep to see each rhashtable as unique with respect to all of
> the locks, not just the ht->mutex.
> 
> This is needed if rhashtable is to be used tracepoint BPF which could
> run while an rhashtable lock (in a different table) is held - see Link
> below.
> 
> Rather then adding extra keys for the other locks, we use distinct
> sub-classes for the different locks.
> rhashtable->lock uses class 0
> rhashtable->mutex uses class 1
> bucket bitlocks, which are the only ones that are ever nested, use
>  classes 2 and 3.
> 
> Currently rht_lock() and rht_lock_nested() are quite separate code
> despite the near-identical function.  This patch moves rht_lock() to
> after rht_lock_nested(), and simply calls that other function with a
> nesting level of zero.
> 
> Link: https://lore.kernel.org/all/20260801-fix-rhashtable-bucket-lockdep-v1-1-15a0f8ae094c@gmail.com/
> Closes: https://syzkaller.appspot.com/bug?extid=ef8d17bae14efb960935
> Tested-by: quanyeyang <quanyeyang@proton.me>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> 
> v2 includes a change to test_rhashtable.c as advised by sashiko.dev
> NeilBrown
> 
> 
>  include/linux/rhashtable-types.h |  3 +++
>  include/linux/rhashtable.h       | 22 +++++++++-------------
>  lib/rhashtable.c                 | 14 +++++++++++---
>  lib/test_rhashtable.c            |  2 +-
>  4 files changed, 24 insertions(+), 17 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt