Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Derek Basehore <dbasehore@chromium.org>
To:     peterz@infradead.org
Cc:     mingo@redhat.com, will.deacon@arm.com,
        linux-kernel@vger.kernel.org,
        Derek Basehore <dbasehore@chromium.org>
Subject: RFC [PATCH 1/1] locking/lockdep: Fix nest lock warning on unlock
Date:   Tue, 11 Dec 2018 18:25:06 -0800
Message-Id: <20181212022506.171936-2-dbasehore@chromium.org>
In-Reply-To: <20181212022506.171936-1-dbasehore@chromium.org>
References: <20181212022506.171936-1-dbasehore@chromium.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

The function __lock_acquire checks that the nest lock is held passed
in as an argument. The issue with this is that __lock_acquire is used
for internal bookkeeping on lock_release. This produces a false
positive lockdep warning on unlock. Since you explicitly don't need to
hold the nest lock on unlock, this is an issue.

This fixes the problem by only checking the nest lock on the actual
lock acquire step.

Signed-off-by: Derek Basehore <dbasehore@chromium.org>
---
 kernel/locking/lockdep.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 1efada2dd9dd..2e7297ee6596 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -3155,15 +3155,15 @@ EXPORT_SYMBOL_GPL(lockdep_init_map);
 struct lock_class_key __lockdep_no_validate__;
 EXPORT_SYMBOL_GPL(__lockdep_no_validate__);
 
-static int
-print_lock_nested_lock_not_held(struct task_struct *curr,
-				struct held_lock *hlock,
+static void
+print_lock_nested_lock_not_held(struct lockdep_map *lock,
+				struct lockdep_map *nest_lock,
 				unsigned long ip)
 {
 	if (!debug_locks_off())
-		return 0;
+		return;
 	if (debug_locks_silent)
-		return 0;
+		return;
 
 	pr_warn("\n");
 	pr_warn("==================================\n");
@@ -3171,22 +3171,21 @@ print_lock_nested_lock_not_held(struct task_struct *curr,
 	print_kernel_ident();
 	pr_warn("----------------------------------\n");
 
-	pr_warn("%s/%d is trying to lock:\n", curr->comm, task_pid_nr(curr));
-	print_lock(hlock);
+	pr_warn("%s/%d is trying to lock:\n", current->comm,
+			task_pid_nr(current));
+	pr_warn("%s\n", lock->name);
 
 	pr_warn("\nbut this task is not holding:\n");
-	pr_warn("%s\n", hlock->nest_lock->name);
+	pr_warn("%s\n", nest_lock->name);
 
 	pr_warn("\nstack backtrace:\n");
 	dump_stack();
 
 	pr_warn("\nother info that might help us debug this:\n");
-	lockdep_print_held_locks(curr);
+	lockdep_print_held_locks(current);
 
 	pr_warn("\nstack backtrace:\n");
 	dump_stack();
-
-	return 0;
 }
 
 static int __lock_is_held(const struct lockdep_map *lock, int read);
@@ -3335,9 +3334,6 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
 	}
 	chain_key = iterate_chain_key(chain_key, class_idx);
 
-	if (nest_lock && !__lock_is_held(nest_lock, -1))
-		return print_lock_nested_lock_not_held(curr, hlock, ip);
-
 	if (!validate_chain(curr, lock, hlock, chain_head, chain_key))
 		return 0;
 
@@ -3843,6 +3839,9 @@ void lock_acquire(struct lockdep_map *lock, unsigned int subclass,
 	trace_lock_acquire(lock, subclass, trylock, read, check, nest_lock, ip);
 	__lock_acquire(lock, subclass, trylock, read, check,
 		       irqs_disabled_flags(flags), nest_lock, ip, 0, 0);
+	if (nest_lock && !__lock_is_held(nest_lock, -1))
+		print_lock_nested_lock_not_held(lock, nest_lock, ip);
+
 	current->lockdep_recursion = 0;
 	raw_local_irq_restore(flags);
 }
-- 
2.20.0.rc2.403.gdbc3b29805-goog