Messages in this thread Patch in this message | | | Date | Thu, 6 Sep 2018 18:43:50 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH] lockdep: Have assert functions test for actual interrupts disabled |
| |
On Thu, Sep 06, 2018 at 10:17:01AM -0400, Steven Rostedt wrote: > I still think checking if IRQS are really disabled or not when lockdep > thinks it is (or not) is valuable and doesn't cause any other problems.
Since check_flags() is a relatively cheap thing I would rather do something like so..
--- include/linux/lockdep.h | 6 ++++-- kernel/locking/lockdep.c | 36 +++++++++++++++++++++++------------- 2 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index b0d0b51c4d85..24ff6302c04b 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -596,15 +596,17 @@ do { \ lock_release(&(lock)->dep_map, 0, _THIS_IP_); \ } while (0) +extern bool lockdep_check_flags(void); + #define lockdep_assert_irqs_enabled() do { \ WARN_ONCE(debug_locks && !current->lockdep_recursion && \ - !current->hardirqs_enabled, \ + !current->hardirqs_enabled && lockdep_check_flags(), \ "IRQs not enabled as expected\n"); \ } while (0) #define lockdep_assert_irqs_disabled() do { \ WARN_ONCE(debug_locks && !current->lockdep_recursion && \ - current->hardirqs_enabled, \ + current->hardirqs_enabled && lockdep_check_flags(), \ "IRQs not disabled as expected\n"); \ } while (0) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index e406c5fdb41e..6cc58a16f2fe 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -3807,10 +3807,9 @@ static void __lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie cookie /* * Check whether we follow the irq-flags state precisely: */ -static void check_flags(unsigned long flags) +void __lockdep_check_flags(unsigned long flags) { -#if defined(CONFIG_PROVE_LOCKING) && defined(CONFIG_DEBUG_LOCKDEP) && \ - defined(CONFIG_TRACE_IRQFLAGS) +#if defined(CONFIG_PROVE_LOCKING) && defined(CONFIG_TRACE_IRQFLAGS) if (!debug_locks) return; @@ -3844,6 +3843,17 @@ static void check_flags(unsigned long flags) #endif } +bool lockdep_check_flags(void) +{ + unsigned long flags; + + raw_local_irq_save(flags); + __lockdep_check_flags(flags); + raw_local_irq_restore(flags); + + return true; +} + void lock_set_class(struct lockdep_map *lock, const char *name, struct lock_class_key *key, unsigned int subclass, unsigned long ip) @@ -3855,7 +3865,7 @@ void lock_set_class(struct lockdep_map *lock, const char *name, raw_local_irq_save(flags); current->lockdep_recursion = 1; - check_flags(flags); + __lockdep_check_flags(flags); if (__lock_set_class(lock, name, key, subclass, ip)) check_chain_key(current); current->lockdep_recursion = 0; @@ -3872,7 +3882,7 @@ void lock_downgrade(struct lockdep_map *lock, unsigned long ip) raw_local_irq_save(flags); current->lockdep_recursion = 1; - check_flags(flags); + __lockdep_check_flags(flags); if (__lock_downgrade(lock, ip)) check_chain_key(current); current->lockdep_recursion = 0; @@ -3894,7 +3904,7 @@ void lock_acquire(struct lockdep_map *lock, unsigned int subclass, return; raw_local_irq_save(flags); - check_flags(flags); + __lockdep_check_flags(flags); current->lockdep_recursion = 1; trace_lock_acquire(lock, subclass, trylock, read, check, nest_lock, ip); @@ -3914,7 +3924,7 @@ void lock_release(struct lockdep_map *lock, int nested, return; raw_local_irq_save(flags); - check_flags(flags); + __lockdep_check_flags(flags); current->lockdep_recursion = 1; trace_lock_release(lock, ip); if (__lock_release(lock, nested, ip)) @@ -3933,7 +3943,7 @@ int lock_is_held_type(const struct lockdep_map *lock, int read) return 1; /* avoid false negative lockdep_assert_held() */ raw_local_irq_save(flags); - check_flags(flags); + __lockdep_check_flags(flags); current->lockdep_recursion = 1; ret = __lock_is_held(lock, read); @@ -3953,7 +3963,7 @@ struct pin_cookie lock_pin_lock(struct lockdep_map *lock) return cookie; raw_local_irq_save(flags); - check_flags(flags); + __lockdep_check_flags(flags); current->lockdep_recursion = 1; cookie = __lock_pin_lock(lock); @@ -3972,7 +3982,7 @@ void lock_repin_lock(struct lockdep_map *lock, struct pin_cookie cookie) return; raw_local_irq_save(flags); - check_flags(flags); + __lockdep_check_flags(flags); current->lockdep_recursion = 1; __lock_repin_lock(lock, cookie); @@ -3989,7 +3999,7 @@ void lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie cookie) return; raw_local_irq_save(flags); - check_flags(flags); + __lockdep_check_flags(flags); current->lockdep_recursion = 1; __lock_unpin_lock(lock, cookie); @@ -4130,7 +4140,7 @@ void lock_contended(struct lockdep_map *lock, unsigned long ip) return; raw_local_irq_save(flags); - check_flags(flags); + __lockdep_check_flags(flags); current->lockdep_recursion = 1; trace_lock_contended(lock, ip); __lock_contended(lock, ip); @@ -4150,7 +4160,7 @@ void lock_acquired(struct lockdep_map *lock, unsigned long ip) return; raw_local_irq_save(flags); - check_flags(flags); + __lockdep_check_flags(flags); current->lockdep_recursion = 1; __lock_acquired(lock, ip); current->lockdep_recursion = 0;
| |