[PATCH v2] rhashtable: use private lockdep class for all locks.
NeilBrown <[email protected]>
| Newsgroups | org.kernel.vger.linux-crypto,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
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