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;

Reply via email to