| Message ID | 1541709268-3766-8-git-send-email-longman@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | locking/lockdep: Add a new class of terminal locks | expand |
On Thu, Nov 08, 2018 at 03:34:23PM -0500, Waiman Long wrote: > There are use cases where we want to allow 2-level nesting of one > terminal lock underneath another one. So the terminal lock type is now > extended to support a new nested terminal lock where it can allow the > acquisition of another regular terminal lock underneath it. You're stretching things here... If you're allowing things under it, it is no longer a terminal lock. Why would you want to do such a thing?
On 11/10/2018 09:20 AM, Peter Zijlstra wrote: > On Thu, Nov 08, 2018 at 03:34:23PM -0500, Waiman Long wrote: >> There are use cases where we want to allow 2-level nesting of one >> terminal lock underneath another one. So the terminal lock type is now >> extended to support a new nested terminal lock where it can allow the >> acquisition of another regular terminal lock underneath it. > You're stretching things here... If you're allowing things under it, it > is no longer a terminal lock. > > Why would you want to do such a thing? A majority of the gain in debugobjects is to make the hash lock a kind of terminal lock. Yes, I may be stretching it a bit here. I will take back the nesting patch and consider doing that in a future patch. Cheers, Longman
On Sat, Nov 10, 2018 at 07:30:54PM -0500, Waiman Long wrote: > On 11/10/2018 09:20 AM, Peter Zijlstra wrote: > > On Thu, Nov 08, 2018 at 03:34:23PM -0500, Waiman Long wrote: > >> There are use cases where we want to allow 2-level nesting of one > >> terminal lock underneath another one. So the terminal lock type is now > >> extended to support a new nested terminal lock where it can allow the > >> acquisition of another regular terminal lock underneath it. > > You're stretching things here... If you're allowing things under it, it > > is no longer a terminal lock. > > > > Why would you want to do such a thing? > > A majority of the gain in debugobjects is to make the hash lock a kind > of terminal lock. Yes, I may be stretching it a bit here. I will take > back the nesting patch and consider doing that in a future patch. Maybe try and write a better changelog? I'm not following, but that could also be because I've been awake for almost 20 hours :/
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index c5ff8c5..ed4d177 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -147,12 +147,16 @@ struct lock_class_stats { * 1) LOCKDEP_FLAG_NOVALIDATE: No full validation, just simple checks. * 2) LOCKDEP_FLAG_TERMINAL: This is a terminal lock where lock/unlock on * another lock within its critical section is not allowed. + * 3) LOCKDEP_FLAG_TERMINAL_NESTED: This is a terminal lock that allows + * one more regular terminal lock to be nested underneath it. */ #define LOCKDEP_FLAG_NOVALIDATE (1 << 0) #define LOCKDEP_FLAG_TERMINAL (1 << 1) +#define LOCKDEP_FLAG_TERMINAL_NESTED (1 << 2) #define LOCKDEP_NOCHECK_FLAGS (LOCKDEP_FLAG_NOVALIDATE |\ - LOCKDEP_FLAG_TERMINAL) + LOCKDEP_FLAG_TERMINAL |\ + LOCKDEP_FLAG_TERMINAL_NESTED) /* * Map the lock object (the lock instance) to the lock-class object. @@ -314,6 +318,8 @@ extern void lockdep_init_map(struct lockdep_map *lock, const char *name, do { (lock)->dep_map.flags |= LOCKDEP_FLAG_NOVALIDATE; } while (0) #define lockdep_set_terminal_class(lock) \ do { (lock)->dep_map.flags |= LOCKDEP_FLAG_TERMINAL; } while (0) +#define lockdep_set_terminal_nested_class(lock) \ + do { (lock)->dep_map.flags |= LOCKDEP_FLAG_TERMINAL_NESTED; } while (0) /* * Compare locking classes @@ -431,6 +437,7 @@ static inline void lockdep_on(void) #define lockdep_set_novalidate_class(lock) do { } while (0) #define lockdep_set_terminal_class(lock) do { } while (0) +#define lockdep_set_terminal_nested_class(lock) do { } while (0) /* * We don't define lockdep_match_class() and lockdep_match_key() for !LOCKDEP diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 02631a0..2b75613 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -3268,13 +3268,24 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass, class_idx = class - lock_classes + 1; if (depth) { + int prev_type; + hlock = curr->held_locks + depth - 1; /* - * Warn if the previous lock is a terminal lock. + * Warn if the previous lock is a terminal lock or the + * previous lock is a nested terminal lock and the current + * one isn't a regular terminal lock. */ - if (DEBUG_LOCKS_WARN_ON(hlock_is_terminal(hlock))) + prev_type = hlock_is_terminal(hlock); + if (DEBUG_LOCKS_WARN_ON((prev_type == LOCKDEP_FLAG_TERMINAL) || + ((prev_type == LOCKDEP_FLAG_TERMINAL_NESTED) && + (flags_is_terminal(class->flags) != + LOCKDEP_FLAG_TERMINAL)))) { + pr_warn("Terminal lock error: prev lock = %s, curr lock = %s\n", + hlock->instance->name, class->name); return 0; + } if (hlock->class_idx == class_idx && nest_lock) { if (hlock->references) { diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h index 271fba8..abe646a 100644 --- a/kernel/locking/lockdep_internals.h +++ b/kernel/locking/lockdep_internals.h @@ -215,5 +215,5 @@ static inline unsigned long debug_class_ops_read(struct lock_class *class) static inline unsigned int flags_is_terminal(unsigned int flags) { - return flags & LOCKDEP_FLAG_TERMINAL; + return flags & (LOCKDEP_FLAG_TERMINAL|LOCKDEP_FLAG_TERMINAL_NESTED); }
There are use cases where we want to allow 2-level nesting of one terminal lock underneath another one. So the terminal lock type is now extended to support a new nested terminal lock where it can allow the acquisition of another regular terminal lock underneath it. Signed-off-by: Waiman Long <longman@redhat.com> --- include/linux/lockdep.h | 9 ++++++++- kernel/locking/lockdep.c | 15 +++++++++++++-- kernel/locking/lockdep_internals.h | 2 +- 3 files changed, 22 insertions(+), 4 deletions(-)