* [PATCH 0/2] locking/lockdep: A few cleanups @ 2018-12-28 5:01 Frederic Weisbecker 2018-12-28 5:02 ` [PATCH 1/2] locking/lockdep: Simplify mark_held_locks() Frederic Weisbecker 2018-12-28 5:02 ` [PATCH 2/2] locking/lockdep: Provide enum lock_usage_bit mask names Frederic Weisbecker 0 siblings, 2 replies; 5+ messages in thread From: Frederic Weisbecker @ 2018-12-28 5:01 UTC (permalink / raw) To: LKML; +Cc: Frederic Weisbecker, Ingo Molnar, Peter Zijlstra Just a few simplification and code cleanup. Frederic Weisbecker (2): locking/lockdep: Simplify mark_held_locks() locking/lockdep: Provide enum lock_usage_bit mask names kernel/locking/lockdep.c | 54 +++++++++++++------------------------- kernel/locking/lockdep_internals.h | 4 +++ 2 files changed, 22 insertions(+), 36 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] locking/lockdep: Simplify mark_held_locks() 2018-12-28 5:01 [PATCH 0/2] locking/lockdep: A few cleanups Frederic Weisbecker @ 2018-12-28 5:02 ` Frederic Weisbecker 2019-01-21 11:31 ` [tip:locking/core] " tip-bot for Frederic Weisbecker 2018-12-28 5:02 ` [PATCH 2/2] locking/lockdep: Provide enum lock_usage_bit mask names Frederic Weisbecker 1 sibling, 1 reply; 5+ messages in thread From: Frederic Weisbecker @ 2018-12-28 5:02 UTC (permalink / raw) To: LKML; +Cc: Frederic Weisbecker, Ingo Molnar, Peter Zijlstra The enum mark_type appears a bit artificial here. We can directly pass the base enum lock_usage_bit value to mark_held_locks(). All we need then is to add the read index for each lock if necessary. It makes the code clearer. Signed-off-by: Frederic Weisbecker <frederic@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ingo Molnar <mingo@kernel.org> --- kernel/locking/lockdep.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 5c837a5..118a554 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -2709,35 +2709,28 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this, return 1; } -enum mark_type { -#define LOCKDEP_STATE(__STATE) __STATE, -#include "lockdep_states.h" -#undef LOCKDEP_STATE -}; - /* * Mark all held locks with a usage bit: */ static int -mark_held_locks(struct task_struct *curr, enum mark_type mark) +mark_held_locks(struct task_struct *curr, enum lock_usage_bit base_bit) { - enum lock_usage_bit usage_bit; struct held_lock *hlock; int i; for (i = 0; i < curr->lockdep_depth; i++) { + enum lock_usage_bit hlock_bit = base_bit; hlock = curr->held_locks + i; - usage_bit = 2 + (mark << 2); /* ENABLED */ if (hlock->read) - usage_bit += 1; /* READ */ + hlock_bit += 1; /* READ */ - BUG_ON(usage_bit >= LOCK_USAGE_STATES); + BUG_ON(hlock_bit >= LOCK_USAGE_STATES); if (!hlock->check) continue; - if (!mark_lock(curr, hlock, usage_bit)) + if (!mark_lock(curr, hlock, hlock_bit)) return 0; } @@ -2758,7 +2751,7 @@ static void __trace_hardirqs_on_caller(unsigned long ip) * We are going to turn hardirqs on, so set the * usage bit for all held locks: */ - if (!mark_held_locks(curr, HARDIRQ)) + if (!mark_held_locks(curr, LOCK_ENABLED_HARDIRQ)) return; /* * If we have softirqs enabled, then set the usage @@ -2766,7 +2759,7 @@ static void __trace_hardirqs_on_caller(unsigned long ip) * this bit from being set before) */ if (curr->softirqs_enabled) - if (!mark_held_locks(curr, SOFTIRQ)) + if (!mark_held_locks(curr, LOCK_ENABLED_SOFTIRQ)) return; curr->hardirq_enable_ip = ip; @@ -2880,7 +2873,7 @@ void trace_softirqs_on(unsigned long ip) * enabled too: */ if (curr->hardirqs_enabled) - mark_held_locks(curr, SOFTIRQ); + mark_held_locks(curr, LOCK_ENABLED_SOFTIRQ); current->lockdep_recursion = 0; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [tip:locking/core] locking/lockdep: Simplify mark_held_locks() 2018-12-28 5:02 ` [PATCH 1/2] locking/lockdep: Simplify mark_held_locks() Frederic Weisbecker @ 2019-01-21 11:31 ` tip-bot for Frederic Weisbecker 0 siblings, 0 replies; 5+ messages in thread From: tip-bot for Frederic Weisbecker @ 2019-01-21 11:31 UTC (permalink / raw) To: linux-tip-commits Cc: peterz, frederic, akpm, torvalds, hpa, linux-kernel, tglx, will.deacon, mingo, paulmck Commit-ID: 436a49ae7b693161c4fdf98b575ef16243dc2dfa Gitweb: https://git.kernel.org/tip/436a49ae7b693161c4fdf98b575ef16243dc2dfa Author: Frederic Weisbecker <frederic@kernel.org> AuthorDate: Fri, 28 Dec 2018 06:02:00 +0100 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Mon, 21 Jan 2019 11:18:54 +0100 locking/lockdep: Simplify mark_held_locks() The enum mark_type appears a bit artificial here. We can directly pass the base enum lock_usage_bit value to mark_held_locks(). All we need then is to add the read index for each lock if necessary. It makes the code clearer. Signed-off-by: Frederic Weisbecker <frederic@kernel.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Will Deacon <will.deacon@arm.com> Link: https://lkml.kernel.org/r/1545973321-24422-2-git-send-email-frederic@kernel.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- kernel/locking/lockdep.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index e805fe3bf87f..1dcd8341e35b 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -2709,35 +2709,28 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this, return 1; } -enum mark_type { -#define LOCKDEP_STATE(__STATE) __STATE, -#include "lockdep_states.h" -#undef LOCKDEP_STATE -}; - /* * Mark all held locks with a usage bit: */ static int -mark_held_locks(struct task_struct *curr, enum mark_type mark) +mark_held_locks(struct task_struct *curr, enum lock_usage_bit base_bit) { - enum lock_usage_bit usage_bit; struct held_lock *hlock; int i; for (i = 0; i < curr->lockdep_depth; i++) { + enum lock_usage_bit hlock_bit = base_bit; hlock = curr->held_locks + i; - usage_bit = 2 + (mark << 2); /* ENABLED */ if (hlock->read) - usage_bit += 1; /* READ */ + hlock_bit += 1; /* READ */ - BUG_ON(usage_bit >= LOCK_USAGE_STATES); + BUG_ON(hlock_bit >= LOCK_USAGE_STATES); if (!hlock->check) continue; - if (!mark_lock(curr, hlock, usage_bit)) + if (!mark_lock(curr, hlock, hlock_bit)) return 0; } @@ -2758,7 +2751,7 @@ static void __trace_hardirqs_on_caller(unsigned long ip) * We are going to turn hardirqs on, so set the * usage bit for all held locks: */ - if (!mark_held_locks(curr, HARDIRQ)) + if (!mark_held_locks(curr, LOCK_ENABLED_HARDIRQ)) return; /* * If we have softirqs enabled, then set the usage @@ -2766,7 +2759,7 @@ static void __trace_hardirqs_on_caller(unsigned long ip) * this bit from being set before) */ if (curr->softirqs_enabled) - if (!mark_held_locks(curr, SOFTIRQ)) + if (!mark_held_locks(curr, LOCK_ENABLED_SOFTIRQ)) return; curr->hardirq_enable_ip = ip; @@ -2880,7 +2873,7 @@ void trace_softirqs_on(unsigned long ip) * enabled too: */ if (curr->hardirqs_enabled) - mark_held_locks(curr, SOFTIRQ); + mark_held_locks(curr, LOCK_ENABLED_SOFTIRQ); current->lockdep_recursion = 0; } ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] locking/lockdep: Provide enum lock_usage_bit mask names 2018-12-28 5:01 [PATCH 0/2] locking/lockdep: A few cleanups Frederic Weisbecker 2018-12-28 5:02 ` [PATCH 1/2] locking/lockdep: Simplify mark_held_locks() Frederic Weisbecker @ 2018-12-28 5:02 ` Frederic Weisbecker 2019-01-21 11:31 ` [tip:locking/core] " tip-bot for Frederic Weisbecker 1 sibling, 1 reply; 5+ messages in thread From: Frederic Weisbecker @ 2018-12-28 5:02 UTC (permalink / raw) To: LKML; +Cc: Frederic Weisbecker, Ingo Molnar, Peter Zijlstra It makes the code more self-explanatory and tells throughout the code what magic number refers to: * state (Hardirq/Softirq) * direction (used in or enabled above state) * read or write We can even remove some comments that were compensating for the lack of those constant names. Signed-off-by: Frederic Weisbecker <frederic@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ingo Molnar <mingo@kernel.org> --- kernel/locking/lockdep.c | 33 +++++++++++---------------------- kernel/locking/lockdep_internals.h | 4 ++++ 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 118a554..115d43e 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -1624,29 +1624,18 @@ static const char *state_rnames[] = { static inline const char *state_name(enum lock_usage_bit bit) { - return (bit & 1) ? state_rnames[bit >> 2] : state_names[bit >> 2]; + return (bit & LOCK_USAGE_READ_MASK) ? state_rnames[bit >> 2] : state_names[bit >> 2]; } static int exclusive_bit(int new_bit) { - /* - * USED_IN - * USED_IN_READ - * ENABLED - * ENABLED_READ - * - * bit 0 - write/read - * bit 1 - used_in/enabled - * bit 2+ state - */ - - int state = new_bit & ~3; - int dir = new_bit & 2; + int state = new_bit & LOCK_USAGE_STATE_MASK; + int dir = new_bit & LOCK_USAGE_DIR_MASK; /* * keep state, bit flip the direction and strip read. */ - return state | (dir ^ 2); + return state | (dir ^ LOCK_USAGE_DIR_MASK); } static int check_irq_usage(struct task_struct *curr, struct held_lock *prev, @@ -2662,8 +2651,8 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this, enum lock_usage_bit new_bit) { int excl_bit = exclusive_bit(new_bit); - int read = new_bit & 1; - int dir = new_bit & 2; + int read = new_bit & LOCK_USAGE_READ_MASK; + int dir = new_bit & LOCK_USAGE_DIR_MASK; /* * mark USED_IN has to look forwards -- to ensure no dependency @@ -2687,19 +2676,19 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this, * states. */ if ((!read || !dir || STRICT_READ_CHECKS) && - !usage(curr, this, excl_bit, state_name(new_bit & ~1))) + !usage(curr, this, excl_bit, state_name(new_bit & ~LOCK_USAGE_READ_MASK))) return 0; /* * Check for read in write conflicts */ if (!read) { - if (!valid_state(curr, this, new_bit, excl_bit + 1)) + if (!valid_state(curr, this, new_bit, excl_bit + LOCK_USAGE_READ_MASK)) return 0; if (STRICT_READ_CHECKS && - !usage(curr, this, excl_bit + 1, - state_name(new_bit + 1))) + !usage(curr, this, excl_bit + LOCK_USAGE_READ_MASK, + state_name(new_bit + LOCK_USAGE_READ_MASK))) return 0; } @@ -2723,7 +2712,7 @@ mark_held_locks(struct task_struct *curr, enum lock_usage_bit base_bit) hlock = curr->held_locks + i; if (hlock->read) - hlock_bit += 1; /* READ */ + hlock_bit += LOCK_USAGE_READ_MASK; BUG_ON(hlock_bit >= LOCK_USAGE_STATES); diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h index 88c847a..2ebb9d0 100644 --- a/kernel/locking/lockdep_internals.h +++ b/kernel/locking/lockdep_internals.h @@ -22,6 +22,10 @@ enum lock_usage_bit { LOCK_USAGE_STATES }; +#define LOCK_USAGE_READ_MASK 1 +#define LOCK_USAGE_DIR_MASK 2 +#define LOCK_USAGE_STATE_MASK (~(LOCK_USAGE_READ_MASK | LOCK_USAGE_DIR_MASK)) + /* * Usage-state bitmasks: */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [tip:locking/core] locking/lockdep: Provide enum lock_usage_bit mask names 2018-12-28 5:02 ` [PATCH 2/2] locking/lockdep: Provide enum lock_usage_bit mask names Frederic Weisbecker @ 2019-01-21 11:31 ` tip-bot for Frederic Weisbecker 0 siblings, 0 replies; 5+ messages in thread From: tip-bot for Frederic Weisbecker @ 2019-01-21 11:31 UTC (permalink / raw) To: linux-tip-commits Cc: mingo, will.deacon, paulmck, tglx, peterz, torvalds, linux-kernel, hpa, frederic, akpm Commit-ID: bba2a8f1f974a45ca6ceaf688b2be7bc1c418a2f Gitweb: https://git.kernel.org/tip/bba2a8f1f974a45ca6ceaf688b2be7bc1c418a2f Author: Frederic Weisbecker <frederic@kernel.org> AuthorDate: Fri, 28 Dec 2018 06:02:01 +0100 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Mon, 21 Jan 2019 11:18:56 +0100 locking/lockdep: Provide enum lock_usage_bit mask names It makes the code more self-explanatory and tells throughout the code what magic number refers to: - state (Hardirq/Softirq) - direction (used in or enabled above state) - read or write We can even remove some comments that were compensating for the lack of those constant names. Signed-off-by: Frederic Weisbecker <frederic@kernel.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Will Deacon <will.deacon@arm.com> Link: https://lkml.kernel.org/r/1545973321-24422-3-git-send-email-frederic@kernel.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- kernel/locking/lockdep.c | 33 +++++++++++---------------------- kernel/locking/lockdep_internals.h | 4 ++++ 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 1dcd8341e35b..608f74ed8bb9 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -1624,29 +1624,18 @@ static const char *state_rnames[] = { static inline const char *state_name(enum lock_usage_bit bit) { - return (bit & 1) ? state_rnames[bit >> 2] : state_names[bit >> 2]; + return (bit & LOCK_USAGE_READ_MASK) ? state_rnames[bit >> 2] : state_names[bit >> 2]; } static int exclusive_bit(int new_bit) { - /* - * USED_IN - * USED_IN_READ - * ENABLED - * ENABLED_READ - * - * bit 0 - write/read - * bit 1 - used_in/enabled - * bit 2+ state - */ - - int state = new_bit & ~3; - int dir = new_bit & 2; + int state = new_bit & LOCK_USAGE_STATE_MASK; + int dir = new_bit & LOCK_USAGE_DIR_MASK; /* * keep state, bit flip the direction and strip read. */ - return state | (dir ^ 2); + return state | (dir ^ LOCK_USAGE_DIR_MASK); } static int check_irq_usage(struct task_struct *curr, struct held_lock *prev, @@ -2662,8 +2651,8 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this, enum lock_usage_bit new_bit) { int excl_bit = exclusive_bit(new_bit); - int read = new_bit & 1; - int dir = new_bit & 2; + int read = new_bit & LOCK_USAGE_READ_MASK; + int dir = new_bit & LOCK_USAGE_DIR_MASK; /* * mark USED_IN has to look forwards -- to ensure no dependency @@ -2687,19 +2676,19 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this, * states. */ if ((!read || !dir || STRICT_READ_CHECKS) && - !usage(curr, this, excl_bit, state_name(new_bit & ~1))) + !usage(curr, this, excl_bit, state_name(new_bit & ~LOCK_USAGE_READ_MASK))) return 0; /* * Check for read in write conflicts */ if (!read) { - if (!valid_state(curr, this, new_bit, excl_bit + 1)) + if (!valid_state(curr, this, new_bit, excl_bit + LOCK_USAGE_READ_MASK)) return 0; if (STRICT_READ_CHECKS && - !usage(curr, this, excl_bit + 1, - state_name(new_bit + 1))) + !usage(curr, this, excl_bit + LOCK_USAGE_READ_MASK, + state_name(new_bit + LOCK_USAGE_READ_MASK))) return 0; } @@ -2723,7 +2712,7 @@ mark_held_locks(struct task_struct *curr, enum lock_usage_bit base_bit) hlock = curr->held_locks + i; if (hlock->read) - hlock_bit += 1; /* READ */ + hlock_bit += LOCK_USAGE_READ_MASK; BUG_ON(hlock_bit >= LOCK_USAGE_STATES); diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h index 88c847a41c8a..2ebb9d0ea91c 100644 --- a/kernel/locking/lockdep_internals.h +++ b/kernel/locking/lockdep_internals.h @@ -22,6 +22,10 @@ enum lock_usage_bit { LOCK_USAGE_STATES }; +#define LOCK_USAGE_READ_MASK 1 +#define LOCK_USAGE_DIR_MASK 2 +#define LOCK_USAGE_STATE_MASK (~(LOCK_USAGE_READ_MASK | LOCK_USAGE_DIR_MASK)) + /* * Usage-state bitmasks: */ ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-21 11:32 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-28 5:01 [PATCH 0/2] locking/lockdep: A few cleanups Frederic Weisbecker 2018-12-28 5:02 ` [PATCH 1/2] locking/lockdep: Simplify mark_held_locks() Frederic Weisbecker 2019-01-21 11:31 ` [tip:locking/core] " tip-bot for Frederic Weisbecker 2018-12-28 5:02 ` [PATCH 2/2] locking/lockdep: Provide enum lock_usage_bit mask names Frederic Weisbecker 2019-01-21 11:31 ` [tip:locking/core] " tip-bot for Frederic Weisbecker
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.