linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Boqun Feng <boqun.feng@gmail.com>
To: linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Waiman Long <longman@redhat.com>,
	Boqun Feng <boqun.feng@gmail.com>
Subject: [RFC v7 10/19] lockdep: Adjust check_redundant() for recursive read change
Date: Fri,  7 Aug 2020 15:42:29 +0800	[thread overview]
Message-ID: <20200807074238.1632519-11-boqun.feng@gmail.com> (raw)
In-Reply-To: <20200807074238.1632519-1-boqun.feng@gmail.com>

check_redundant() will report redundancy if it finds a path could
replace the about-to-add dependency in the BFS search. With recursive
read lock changes, we certainly need to change the match function for
the check_redundant(), because the path needs to match not only the lock
class but also the dependency kinds. For example, if the about-to-add
dependency @prev -> @next is A -(SN)-> B, and we find a path A -(S*)->
.. -(*R)->B in the dependency graph with __bfs() (for simplicity, we can
also say we find an -(SR)-> path from A to B), we can not replace the
dependency with that path in the BFS search. Because the -(SN)->
dependency can make a strong path with a following -(S*)-> dependency,
however an -(SR)-> path cannot.

Further, we can replace an -(SN)-> dependency with a -(EN)-> path, that
means if we find a path which is stronger than or equal to the
about-to-add dependency, we can report the redundancy. By "stronger", it
means both the start and the end of the path are not weaker than the
start and the end of the dependency (E is "stronger" than S and N is
"stronger" than R), so that we can replace the dependency with that
path.

To make sure we find a path whose start point is not weaker than the
about-to-add dependency, we use a trick: the ->only_xr of the root
(start point) of __bfs() is initialized as @prev-> == 0, therefore if
@prev is E, __bfs() will pick only -(E*)-> for the first dependency,
otherwise, __bfs() can pick -(E*)-> or -(S*)-> for the first dependency.

To make sure we find a path whose end point is not weaker than the
about-to-add dependency, we replace the match function for __bfs()
check_redundant(), we check for the case that either @next is R
(anything is not weaker than it) or the end point of the path is N
(which is not weaker than anything).

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 kernel/locking/lockdep.c | 47 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index e5b2c1cf4286..85a4d3539faa 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1833,9 +1833,39 @@ print_circular_bug_header(struct lock_list *entry, unsigned int depth,
 	print_circular_bug_entry(entry, depth);
 }
 
-static inline bool class_equal(struct lock_list *entry, void *data)
+/*
+ * We are about to add A -> B into the dependency graph, and in __bfs() a
+ * strong dependency path A -> .. -> B is found: hlock_class equals
+ * entry->class.
+ *
+ * If A -> .. -> B can replace A -> B in any __bfs() search (means the former
+ * is _stronger_ than or equal to the latter), we consider A -> B as redundant.
+ * For example if A -> .. -> B is -(EN)-> (i.e. A -(E*)-> .. -(*N)-> B), and A
+ * -> B is -(ER)-> or -(EN)->, then we don't need to add A -> B into the
+ * dependency graph, as any strong path ..-> A -> B ->.. we can get with
+ * having dependency A -> B, we could already get a equivalent path ..-> A ->
+ * .. -> B -> .. with A -> .. -> B. Therefore A -> B is reduntant.
+ *
+ * We need to make sure both the start and the end of A -> .. -> B is not
+ * weaker than A -> B. For the start part, please see the comment in
+ * check_redundant(). For the end part, we need:
+ *
+ * Either
+ *
+ *     a) A -> B is -(*R)-> (everything is not weaker than that)
+ *
+ * or
+ *
+ *     b) A -> .. -> B is -(*N)-> (nothing is stronger than this)
+ *
+ */
+static inline bool hlock_equal(struct lock_list *entry, void *data)
 {
-	return entry->class == data;
+	struct held_lock *hlock = (struct held_lock *)data;
+
+	return hlock_class(hlock) == entry->class && /* Found A -> .. -> B */
+	       (hlock->read == 2 ||  /* A -> B is -(*R)-> */
+		!entry->only_xr); /* A -> .. -> B is -(*N)-> */
 }
 
 /*
@@ -2045,10 +2075,21 @@ check_redundant(struct held_lock *src, struct held_lock *target)
 	struct lock_list src_entry;
 
 	bfs_init_root(&src_entry, src);
+	/*
+	 * Special setup for check_redundant().
+	 *
+	 * To report redundant, we need to find a strong dependency path that
+	 * is equal to or stronger than <src> -> <target>. So if <src> is E,
+	 * we need to let __bfs() only search for a path starting at a -(E*)->,
+	 * we achieve this by setting the initial node's ->only_xr to true in
+	 * that case. And if <prev> is S, we set initial ->only_xr to false
+	 * because both -(S*)-> (equal) and -(E*)-> (stronger) are redundant.
+	 */
+	src_entry.only_xr = src->read == 0;
 
 	debug_atomic_inc(nr_redundant_checks);
 
-	ret = check_path(target, &src_entry, class_equal, &target_entry);
+	ret = check_path(target, &src_entry, hlock_equal, &target_entry);
 
 	if (ret == BFS_RMATCH)
 		debug_atomic_inc(nr_redundant);
-- 
2.28.0


  parent reply	other threads:[~2020-08-07  7:44 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-07  7:42 [RFC v7 00/19] lockdep: Support deadlock detection for recursive read locks Boqun Feng
2020-08-07  7:42 ` [RFC v7 01/19] locking: More accurate annotations for read_lock() Boqun Feng
2020-08-07  7:42 ` [RFC v7 02/19] lockdep/Documention: Recursive read lock detection reasoning Boqun Feng
2020-08-07  7:42 ` [RFC v7 03/19] lockdep: Demagic the return value of BFS Boqun Feng
2020-08-07  7:42 ` [RFC v7 04/19] lockdep: Make __bfs() visit every dependency until a match Boqun Feng
2020-08-07  7:42 ` [RFC v7 05/19] lockdep: Reduce the size of lock_list::distance Boqun Feng
2020-08-07  7:42 ` [RFC v7 06/19] lockdep: Introduce lock_list::dep Boqun Feng
2020-08-07  7:42 ` [RFC v7 07/19] lockdep: Extend __bfs() to work with multiple types of dependencies Boqun Feng
2020-08-07  7:42 ` [RFC v7 08/19] lockdep: Make __bfs(.match) return bool Boqun Feng
2020-08-07  7:42 ` [RFC v7 09/19] lockdep: Support deadlock detection for recursive read locks in check_noncircular() Boqun Feng
2020-08-07  7:42 ` Boqun Feng [this message]
2020-08-07  7:42 ` [RFC v7 11/19] lockdep: Fix recursive read lock related safe->unsafe detection Boqun Feng
2020-09-15 18:32   ` Qian Cai
2020-09-16  8:10     ` Boqun Feng
2020-09-16 16:14       ` Boqun Feng
2020-09-16 21:11         ` Qian Cai
2020-09-17  1:53           ` Boqun Feng
2020-08-07  7:42 ` [RFC v7 12/19] lockdep: Add recursive read locks into dependency graph Boqun Feng
2020-09-14 18:16   ` Qian Cai
2020-09-14 22:04     ` Qian Cai
2020-08-07  7:42 ` [RFC v7 13/19] lockdep/selftest: Add a R-L/L-W test case specific to chain cache behavior Boqun Feng
2020-08-07  7:42 ` [RFC v7 14/19] lockdep: Take read/write status in consideration when generate chainkey Boqun Feng
2020-08-21 17:41   ` Peter Zijlstra
2020-08-22  2:52     ` boqun.feng
2020-08-07  7:42 ` [RFC v7 15/19] lockdep/selftest: Unleash irq_read_recursion2 and add more Boqun Feng
2020-08-07  7:42 ` [RFC v7 16/19] lockdep/selftest: Add more recursive read related test cases Boqun Feng
2020-08-07  7:42 ` [RFC v7 17/19] Revert "locking/lockdep/selftests: Fix mixed read-write ABBA tests" Boqun Feng
2020-08-07  7:42 ` [RFC v7 18/19] locking/selftest: Add test cases for queued_read_lock() Boqun Feng
2020-08-07  7:42 ` [RFC v7 19/19] lockdep/selftest: Introduce recursion3 Boqun Feng
2020-08-21 19:56 ` [RFC v7 00/19] lockdep: Support deadlock detection for recursive read locks Peter Zijlstra
2020-08-23  1:12   ` boqun.feng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200807074238.1632519-11-boqun.feng@gmail.com \
    --to=boqun.feng@gmail.com \
    --cc=corbet@lwn.net \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).