Messages in this thread | | | Date | Fri, 17 Jun 2022 10:57:11 -0400 | Subject | Re: [PATCH] locking/lockdep: Fix lockdep_init_map_*() confusion | From | Waiman Long <> |
| |
On 6/17/22 09:39, Peter Zijlstra wrote: > Commit dfd5e3f5fe27 ("locking/lockdep: Mark local_lock_t") added yet > another lockdep_init_map_*() variant, but forgot to update all the > existing users of the most complicated version. > > This could lead to a loss of lock_type and hence an incorrect report. > Given the relative rarity of both local_lock and these annotations, > this is unlikely to happen in practise, still, best fix things. > > Fixes: dfd5e3f5fe27 ("locking/lockdep: Mark local_lock_t") > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > include/linux/lockdep.h | 30 +++++++++++++++++------------- > kernel/locking/lockdep.c | 7 ++++--- > 2 files changed, 21 insertions(+), 16 deletions(-) > > --- a/include/linux/lockdep.h > +++ b/include/linux/lockdep.h > @@ -188,7 +188,7 @@ static inline void > lockdep_init_map_waits(struct lockdep_map *lock, const char *name, > struct lock_class_key *key, int subclass, u8 inner, u8 outer) > { > - lockdep_init_map_type(lock, name, key, subclass, inner, LD_WAIT_INV, LD_LOCK_NORMAL); > + lockdep_init_map_type(lock, name, key, subclass, inner, outer, LD_LOCK_NORMAL); > } > > static inline void > @@ -211,24 +211,28 @@ static inline void lockdep_init_map(stru > * or they are too narrow (they suffer from a false class-split): > */ > #define lockdep_set_class(lock, key) \ > - lockdep_init_map_waits(&(lock)->dep_map, #key, key, 0, \ > - (lock)->dep_map.wait_type_inner, \ > - (lock)->dep_map.wait_type_outer) > + lockdep_init_map_type(&(lock)->dep_map, #key, key, 0, \ > + (lock)->dep_map.wait_type_inner, \ > + (lock)->dep_map.wait_type_outer, \ > + (lock)->dep_map.lock_type) > > #define lockdep_set_class_and_name(lock, key, name) \ > - lockdep_init_map_waits(&(lock)->dep_map, name, key, 0, \ > - (lock)->dep_map.wait_type_inner, \ > - (lock)->dep_map.wait_type_outer) > + lockdep_init_map_type(&(lock)->dep_map, name, key, 0, \ > + (lock)->dep_map.wait_type_inner, \ > + (lock)->dep_map.wait_type_outer, \ > + (lock)->dep_map.lock_type) > > #define lockdep_set_class_and_subclass(lock, key, sub) \ > - lockdep_init_map_waits(&(lock)->dep_map, #key, key, sub,\ > - (lock)->dep_map.wait_type_inner, \ > - (lock)->dep_map.wait_type_outer) > + lockdep_init_map_type(&(lock)->dep_map, #key, key, sub, \ > + (lock)->dep_map.wait_type_inner, \ > + (lock)->dep_map.wait_type_outer, \ > + (lock)->dep_map.lock_type) > > #define lockdep_set_subclass(lock, sub) \ > - lockdep_init_map_waits(&(lock)->dep_map, #lock, (lock)->dep_map.key, sub,\ > - (lock)->dep_map.wait_type_inner, \ > - (lock)->dep_map.wait_type_outer) > + lockdep_init_map_type(&(lock)->dep_map, #lock, (lock)->dep_map.key, sub,\ > + (lock)->dep_map.wait_type_inner, \ > + (lock)->dep_map.wait_type_outer, \ > + (lock)->dep_map.lock_type) > > #define lockdep_set_novalidate_class(lock) \ > lockdep_set_class_and_name(lock, &__lockdep_no_validate__, #lock) > --- a/kernel/locking/lockdep.c > +++ b/kernel/locking/lockdep.c > @@ -5238,9 +5238,10 @@ __lock_set_class(struct lockdep_map *loc > return 0; > } > > - lockdep_init_map_waits(lock, name, key, 0, > - lock->wait_type_inner, > - lock->wait_type_outer); > + lockdep_init_map_type(lock, name, key, 0, > + lock->wait_type_inner, > + lock->wait_type_outer, > + lock->lock_type); > class = register_lock_class(lock, subclass, 0); > hlock->class_idx = class - lock_classes; > > You have almost eliminated all usage of lockdep_init_map_waits() except in lockdep_init_map_wait(). Should we just kill lockdep_init_map_waits() and make lockdep_init_map_wait() call lockdep_init_map_type() directly?
Cheers, Longman
| |