Messages in this thread Patch in this message | | | Date | Tue, 20 Oct 2015 14:18:53 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH] locking/lockdep: Fix expected depth value in __lock_release() |
| |
On Mon, Oct 19, 2015 at 03:30:28PM -0400, j.glisse@gmail.com wrote: > From: Jérôme Glisse <jglisse@redhat.com> > > In __lock_release() we are removing one entry from the stack and > rebuilding the hash chain by re-adding entry above the entry we > just removed. If the entry removed was between 2 entry of same > class then this 2 entry might be coalesced into one single entry > which in turns means that the lockdep_depth value will not be > incremented and thus the expected lockdep_depth value after this > operation will be wrong triggering an unjustified WARN_ONCE() at > the end of __lock_release().
This is the nest_lock stuff, right? Where it checks:
if (hlock->class_idx == class_idx && nest_lock) { ... return 1; }
What code did you find that triggered this? That is, what code is taking nested locks with other locks in the middle? (Not wrong per-se, just curious how that would come about).
> This patch adjust the expect depth value by decrementing it if > what was previously 2 entry inside the stack are coalesced into > only one entry.
Would it not make more sense to scan the entire hlock stack on __lock_acquire() and avoid getting collapsible entries in the first place?
Something like so...
--- kernel/locking/lockdep.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 4e49cc4c9952..6fcd98b86e7b 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -3125,15 +3125,21 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass, class_idx = class - lock_classes + 1; - if (depth) { - hlock = curr->held_locks + depth - 1; - if (hlock->class_idx == class_idx && nest_lock) { - if (hlock->references) - hlock->references++; - else - hlock->references = 2; + if (depth && nest_lock) { + int i; - return 1; + for (i = depth; i; --i) { + hlock = curr->held_locks + i - 1; + if (hlock->class_idx == class_idx && + hlock->nest_lock == nest_lock) { + + if (hlock->references) + hlock->references++; + else + hlock->references = 2; + + return 1; + } } }
| |