From: Yuyang Du <duyuyang@gmail.com>
To: peterz@infradead.org, will.deacon@arm.com, mingo@kernel.org
Cc: bvanassche@acm.org, ming.lei@redhat.com, frederic@kernel.org,
tglx@linutronix.de, linux-kernel@vger.kernel.org,
longman@redhat.com, paulmck@linux.vnet.ibm.com,
boqun.feng@gmail.com, Yuyang Du <duyuyang@gmail.com>
Subject: [PATCH v3 23/30] locking/lockdep: Define the two task model for lockdep checks formally
Date: Fri, 28 Jun 2019 17:15:21 +0800 [thread overview]
Message-ID: <20190628091528.17059-24-duyuyang@gmail.com> (raw)
In-Reply-To: <20190628091528.17059-1-duyuyang@gmail.com>
Lockdep effectively uses a two-task model to track workload's locking
behavior and do the checking to find inversion deadlock scenarios based
on such model. Lets explain it in detail.
When there is a new lock dependency L1 -> L2 (i.e., the current task
attempts to acquire L2 while holding L1), which is from a new lock
chain's latest two locks, lockdep's view of the world is composed of two
virtual tasks:
- Task1: the entire previous locking behavior depicted by the forward
lock dependency graph.
- Task2: the current new locking behavior, the L1 -> L2 dependency.
For these two tasks, lockdep tries to find whether there exists the
inverse locking order of L1 -> L2, namely L2 -> L1. If this inverse
locking order exists, then lockdep finds the typical inversion deadlock
scenario, a.k.a, ABBA deadlock. And if not, Task2 will be merged into
Task1 to become a new bigger Task1 with a bigger graph. Then this track
and check cycle goes on and on.
There is some nuances between this two-task model and the real workload
locking behavior in terms of equivalency which affects lockdep finding
true (not false positive) deadlock scenarios. Some examples:
(The following Tx denotes concrete tasks)
T1
--
L1
L2
(L1 and L2 released)
L2
L3
T2
--
L1
L2
L3
Both T1 and T2 will produce the same Task1 from the perspective of
lockdep's two-task model. However, this model does not actually reflect
the reality in full length. In T1, L1 -> L3 actually has no "real"
dependency while in T2 it is "real". A real X -> Y lock dependency is
defined as a task is attempting to acquire Y while holding X. This may
result in false positive deadlock scenarios. For example:
Case #1.1:
T1 T2
-- --
L1
L2 L3
L3 L1 [Deadlock]
Case #1.2 (T1 is a singular task):
T1 T2
-- --
L1
L2
(L1 L2 released)
L2 L3
L3 L1 [No deadlock]
Case #1.3:
T1a T1b T2
--- --- --
L1 L1
L2 L2
(L1 L2 released
in T1a and T1b)
L2 L2 L3
L3 L3 L1 [Deadlock]
From Case #1.2 (no deadlock) to Case #1.3 (deadlock), we can see that
lockdep is also assuming there can be multiple Task1's on top of the
two-task model, and from pragmatic point of view, this assumption is not
unrealistic to make.
However, with read locks that can be fully concurrent with read locks
and not be blocked by write locks (such as the rwlock). Lockdep's such
model is flawed. For example (the following read locks, denoted as RR,
and write locks are of type rwlock):
Case #2.1:
T1 T2
-- --
W1
RR2 W3
W3 W1 [Deadlock]
Case #2.2:
T1a T1b T2
--- --- --
W1 RR2 W3
RR2 W3 W1 [No deadlock]
Case #2.3:
T1a T1b T2
--- --- --
W1 W1
RR2 RR2
(W1 RR2 released
in T1a and T1b)
RR2 RR2 W3
W3 W3 W1 [No deadlock]
Lockdep cannot differentiate Case #2.1 from Case #2.2 and Case #2.3 or
vice versa. This is because when modeling Task1, it cannot tell whether
two neighboring direct dependencies in the graph are in the same real
task and hence create a "real" dependency.
To make up for this, the two-task model needs to be strengthened. We
previously mapped lock chains to lock dependency graph and added
read-write lock types into dependencies. This patch finally modifies the
graph traversing algorithm (__bfs()) to stop when two neighboring direct
dependencies are not in the same lock chain and the middle lock is a
recursive-read lock (rwlock).
Signed-off-by: Yuyang Du <duyuyang@gmail.com>
---
kernel/locking/lockdep.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 7d02b94..444eb62 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1545,6 +1545,71 @@ static inline void set_lock_type2(struct lock_list *lock, int read)
}
/*
+ * A lock stopper in the dependency graph is a read lock that stops the
+ * graph traversal passing through it even if the two dependencies are
+ * linked in a path. A stopper sits in such two lock dependencies:
+ *
+ * X -> RR (stopper) -> X (where RR is recursive-read lock)
+ *
+ * and these two dependencies are NOT from one lock chain.
+ */
+static inline bool
+read_lock_stopper(struct lock_list *parent, struct lock_list *child, int forward)
+{
+ struct lock_chain *chain1, *chain2;
+ struct lock_list *list[2] = { child, parent };
+ u64 key1, key2;
+ int distance, other = 1 - forward;
+
+ /* This is the source entry */
+ if (!get_lock_parent(parent))
+ return false;
+
+ if (!(get_lock_type1(list[other]) == LOCK_TYPE_RECURSIVE &&
+ get_lock_type2(list[forward]) == LOCK_TYPE_RECURSIVE))
+ return false;
+
+ distance = list[forward]->distance;
+
+ list_for_each_entry_rcu(chain1, &list[forward]->chains, chain_entry) {
+ key1 = chain1->chain_key;
+
+ list_for_each_entry_rcu(chain2, &list[other]->chains, chain_entry) {
+ /*
+ * If the two chains are in the same task lock stack,
+ * we should be able to calculate key2 from key1 if
+ * distance is 1, or calculate key1 from key2 if
+ * distance is larger than 1.
+ */
+ if (distance == 1) {
+ int class_idx = fw_dep_class(list[other]) - lock_classes;
+ key1 = iterate_chain_key(key1, class_idx,
+ get_lock_type2(list[other]));
+ key2 = chain2->chain_key;
+
+ if (key1 == key2)
+ return false;
+ }
+ else {
+ int i = chain1->base, j = chain2->base;
+
+ if (chain1->depth != chain2->depth - distance)
+ continue;
+
+ for (; i < chain1->depth - 1; i++, j++)
+ if (chain_hlocks[i] != chain_hlocks[j] ||
+ chain_hlocks_type[i] != chain_hlocks_type[i])
+ continue;
+
+ return false;
+ }
+ }
+ }
+
+ return true;
+}
+
+/*
* Forward- or backward-dependency search, used for both circular dependency
* checking and hardirq-unsafe/softirq-unsafe checking.
*/
@@ -1584,6 +1649,9 @@ static int __bfs(struct lock_list *source_entry,
if (!lock_accessed(entry, forward)) {
unsigned int cq_depth;
+ if (read_lock_stopper(lock, entry, forward))
+ continue;
+
mark_lock_accessed(entry, lock, forward);
if (__cq_enqueue(cq, entry)) {
--
1.8.3.1
next prev parent reply other threads:[~2019-06-28 9:17 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-28 9:14 [PATCH v3 00/30] Support recursive-read lock deadlock detection Yuyang Du
2019-06-28 9:14 ` [PATCH v3 01/30] locking/lockdep: Rename deadlock check functions Yuyang Du
2019-06-28 9:15 ` [PATCH v3 02/30] locking/lockdep: Change return type of add_chain_cache() Yuyang Du
2019-06-28 9:15 ` [PATCH v3 03/30] locking/lockdep: Change return type of lookup_chain_cache_add() Yuyang Du
2019-06-28 9:15 ` [PATCH v3 04/30] locking/lockdep: Pass lock chain from validate_chain() to check_prev_add() Yuyang Du
2019-06-28 9:15 ` [PATCH v3 05/30] locking/lockdep: Add lock chain list_head field in struct lock_list and lock_chain Yuyang Du
2019-06-28 9:15 ` [PATCH v3 06/30] locking/lockdep: Update comments in struct lock_list and held_lock Yuyang Du
2019-06-28 9:15 ` [PATCH v3 07/30] locking/lockdep: Remove indirect dependency redundancy check Yuyang Du
2019-06-28 9:15 ` [PATCH v3 08/30] locking/lockdep: Skip checks if direct dependency is already present Yuyang Du
2019-06-28 9:15 ` [PATCH v3 09/30] locking/lockdep: Remove chain_head argument in validate_chain() Yuyang Du
2019-06-28 9:15 ` [PATCH v3 10/30] locking/lockdep: Remove useless lock type assignment Yuyang Du
2019-06-28 9:15 ` [PATCH v3 11/30] locking/lockdep: Specify the depth of current lock stack in lookup_chain_cache_add() Yuyang Du
2019-06-28 9:15 ` [PATCH v3 12/30] locking/lockdep: Treat every lock dependency as in a new lock chain Yuyang Du
2019-06-28 9:15 ` [PATCH v3 13/30] locking/lockdep: Combine lock_lists in struct lock_class into an array Yuyang Du
2019-06-28 9:15 ` [PATCH v3 14/30] locking/lockdep: Consolidate forward and backward lock_lists into one Yuyang Du
2019-06-28 9:15 ` [PATCH v3 15/30] locking/lockdep: Add lock chains to direct lock dependency graph Yuyang Du
2019-06-28 9:15 ` [PATCH v3 16/30] locking/lockdep: Use lock type enum to explicitly specify read or write locks Yuyang Du
2019-06-28 9:15 ` [PATCH v3 17/30] locking/lockdep: Add read-write type for a lock dependency Yuyang Du
2019-07-10 5:18 ` Boqun Feng
2019-07-11 5:02 ` Yuyang Du
2019-06-28 9:15 ` [PATCH v3 18/30] locking/lockdep: Add helper functions to operate on the searched path Yuyang Du
2019-06-28 9:15 ` [PATCH v3 19/30] locking/lockdep: Update direct dependency's read-write type if it exists Yuyang Du
2019-06-28 9:15 ` [PATCH v3 20/30] locking/lockdep: Introduce chain_hlocks_type for held lock's read-write type Yuyang Du
2019-06-28 9:15 ` [PATCH v3 21/30] locking/lockdep: Hash held lock's read-write type into chain key Yuyang Du
2019-06-28 9:15 ` [PATCH v3 22/30] locking/lockdep: Adjust BFS algorithm to support multiple matches Yuyang Du
2019-06-28 9:15 ` Yuyang Du [this message]
2019-06-28 9:15 ` [PATCH v3 24/30] locking/lockdep: Introduce mark_lock_unaccessed() Yuyang Du
2019-06-28 9:15 ` [PATCH v3 25/30] locking/lockdep: Add nest lock type Yuyang Du
2019-06-28 9:15 ` [PATCH v3 26/30] locking/lockdep: Add lock exclusiveness table Yuyang Du
2019-06-28 9:15 ` [PATCH v3 27/30] locking/lockdep: Support read-write lock's deadlock detection Yuyang Du
2019-06-28 9:15 ` [PATCH v3 28/30] locking/lockdep: Adjust selftest case for recursive read lock Yuyang Du
2019-06-28 9:15 ` [PATCH v3 29/30] locking/lockdep: Add more lockdep selftest cases Yuyang Du
2019-06-28 9:15 ` [PATCH v3 30/30] locking/lockdep: Remove irq-safe to irq-unsafe read check Yuyang Du
2019-07-10 5:30 ` Boqun Feng
2019-07-10 6:30 ` Yuyang Du
2019-07-10 1:54 ` [PATCH v3 00/30] Support recursive-read lock deadlock detection Yuyang Du
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=20190628091528.17059-24-duyuyang@gmail.com \
--to=duyuyang@gmail.com \
--cc=boqun.feng@gmail.com \
--cc=bvanassche@acm.org \
--cc=frederic@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=longman@redhat.com \
--cc=ming.lei@redhat.com \
--cc=mingo@kernel.org \
--cc=paulmck@linux.vnet.ibm.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=will.deacon@arm.com \
/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