On Wed, 05 Aug 2026, NeilBrown wrote:
>
> One thing I didn't like about the original patch is that it seemed to add
> a lot of noise to the code, passing keys around in multiple places.
> Maybe the cleanest approach would be to embed the keys in the "struct
> rhashtable" so each table gets its own key.
> Then use lockdep_register_key() in rhashtable_init and use it as needed.
>
> You could even have just one key and use the different subclasses for
> the different locks.
> 0 for rhashtable.mutex
> 1 for rhashtable.lock
> 2 for the bit locks
>
> but maybe that is needless complexity.
>
So I thought about this some more and came up with this approach which
might be a good compromise. I builds but I haven't tested it.
What do you thing?
NeilBrown
diff --git a/include/linux/rhashtable-types.h b/include/linux/rhashtable-types.h
index 57c11ec9dc64..2029fba216a8 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
+o 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 d459bef245f4..17340433d983 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);
