[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20180906164350.GI24106@hirez.programming.kicks-ass.net>
Date: Thu, 6 Sep 2018 18:43:50 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Joel Fernandes <joel@...lfernandes.org>
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;
Powered by blists - more mailing lists