Messages in this thread Patch in this message |  | | From | NeilBrown <> | | Subject | Re: 回复: Re: [PATCH] bpf: disable lockdep while running BPF on lock_release | | Date | Thu, 06 Aug 2026 08:43:50 +1000 |
| |
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);
|  |