linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Byungchul Park <byungchul.park@lge.com>
To: peterz@infradead.org, mingo@kernel.org
Cc: tglx@linutronix.de, walken@google.com, boqun.feng@gmail.com,
	kirill@shutemov.name, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, akpm@linux-foundation.org,
	willy@infradead.org, npiggin@gmail.com, kernel-team@lge.com
Subject: [PATCH v7 08/16] lockdep: Avoid adding redundant direct links of crosslocks
Date: Wed, 24 May 2017 17:59:41 +0900	[thread overview]
Message-ID: <1495616389-29772-9-git-send-email-byungchul.park@lge.com> (raw)
In-Reply-To: <1495616389-29772-1-git-send-email-byungchul.park@lge.com>

We can skip adding a dependency 'AX -> B', in case that we ensure 'AX ->
the previous of B in hlocks' to be created, where AX is a crosslock and
B is a typical lock. Remember that two adjacent locks in hlocks generate
a dependency like 'prev -> next', that is, 'the previous of B in hlocks
-> B' in this case.

For example:

             in hlocks[]
             ------------
          ^  A (gen_id: 4) --+
          |                  | previous gen_id
          |  B (gen_id: 3) <-+
          |  C (gen_id: 3)
          |  D (gen_id: 2)
   oldest |  E (gen_id: 1)

             in xhlocks[]
             ------------
          ^  A (gen_id: 4, prev_gen_id: 3(B's gen id))
          |  B (gen_id: 3, prev_gen_id: 3(C's gen id))
          |  C (gen_id: 3, prev_gen_id: 2(D's gen id))
          |  D (gen_id: 2, prev_gen_id: 1(E's gen id))
   oldest |  E (gen_id: 1, prev_gen_id: NA)

On commit for a crosslock AX(gen_id = 3), it's engough to add 'AX -> C',
but adding 'AX -> B' and 'AX -> A' is unnecessary since 'AX -> C', 'C ->
B' and 'B -> A' cover them, which are guaranteed to be generated.

This patch intoduces a variable, prev_gen_id, to avoid adding this kind
of redundant dependencies. In other words, the previous in hlocks will
anyway handle it if the previous's gen_id >= the crosslock's gen_id.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 include/linux/lockdep.h  | 11 +++++++++++
 kernel/locking/lockdep.c | 32 ++++++++++++++++++++++++++++++--
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index f7c730a..e5c5cc4 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -284,6 +284,17 @@ struct held_lock {
  */
 struct hist_lock {
 	/*
+	 * We can skip adding a dependency 'a target crosslock -> this
+	 * lock', in case that we ensure 'the target crosslock -> the
+	 * previous lock in held_locks' to be created. Remember that
+	 * 'the previous lock in held_locks -> this lock' is guaranteed
+	 * to be created, and 'A -> B' and 'B -> C' cover 'A -> C'.
+	 *
+	 * Keep the previous's gen_id to make the decision.
+	 */
+	unsigned int		prev_gen_id;
+
+	/*
 	 * Id for each entry in the ring buffer. This is used to
 	 * decide whether the ring buffer was overwritten or not.
 	 *
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 09f5eec..a14d2ca 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4778,7 +4778,7 @@ static inline int xhlock_valid(struct hist_lock *xhlock)
  *
  * Irq disable is only required.
  */
-static void add_xhlock(struct held_lock *hlock)
+static void add_xhlock(struct held_lock *hlock, unsigned int prev_gen_id)
 {
 	unsigned int idx = ++current->xhlock_idx;
 	struct hist_lock *xhlock = &xhlock(idx);
@@ -4793,6 +4793,11 @@ static void add_xhlock(struct held_lock *hlock)
 
 	/* Initialize hist_lock's members */
 	xhlock->hlock = *hlock;
+	/*
+	 * prev_gen_id is used to skip adding redundant dependencies,
+	 * which can be covered by the previous lock in held_locks.
+	 */
+	xhlock->prev_gen_id = prev_gen_id;
 	xhlock->hist_id = current->hist_id++;
 
 	xhlock->trace.nr_entries = 0;
@@ -4813,6 +4818,11 @@ static inline int same_context_xhlock(struct hist_lock *xhlock)
  */
 static void check_add_xhlock(struct held_lock *hlock)
 {
+	struct held_lock *prev;
+	struct held_lock *start;
+	unsigned int gen_id;
+	unsigned int gen_id_invalid;
+
 	/*
 	 * Record a hist_lock, only in case that acquisitions ahead
 	 * could depend on the held_lock. For example, if the held_lock
@@ -4822,7 +4832,22 @@ static void check_add_xhlock(struct held_lock *hlock)
 	if (!current->xhlocks || !depend_before(hlock))
 		return;
 
-	add_xhlock(hlock);
+	gen_id = (unsigned int)atomic_read(&cross_gen_id);
+	/*
+	 * gen_id_invalid should be old enough to be invalid.
+	 * Current gen_id - (UINIT_MAX / 4) would be a good
+	 * value to meet it.
+	 */
+	gen_id_invalid = gen_id - (UINT_MAX / 4);
+	start = current->held_locks;
+
+	for (prev = hlock - 1; prev >= start &&
+			!depend_before(prev); prev--);
+
+	if (prev < start)
+		add_xhlock(hlock, gen_id_invalid);
+	else if (prev->gen_id != gen_id)
+		add_xhlock(hlock, prev->gen_id);
 }
 
 /*
@@ -4979,6 +5004,9 @@ static void commit_xhlocks(struct cross_lock *xlock)
 
 			prev_hist_id = xhlock->hist_id;
 
+			if (!before(xhlock->prev_gen_id, xlock->hlock.gen_id))
+				continue;
+
 			/*
 			 * commit_xhlock() returns 0 with graph_lock already
 			 * released if fail.
-- 
1.9.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2017-05-24  9:00 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-24  8:59 [PATCH v7 00/16] lockdep: Implement crossrelease feature Byungchul Park
2017-05-24  8:59 ` [PATCH v7 01/16] lockdep: Refactor lookup_chain_cache() Byungchul Park
2017-05-24  8:59 ` [PATCH v7 02/16] lockdep: Add a function building a chain between two classes Byungchul Park
2017-05-24  8:59 ` [PATCH v7 03/16] lockdep: Change the meaning of check_prev_add()'s return value Byungchul Park
2017-05-24  8:59 ` [PATCH v7 04/16] lockdep: Make check_prev_add() able to handle external stack_trace Byungchul Park
2017-05-24  8:59 ` [PATCH v7 05/16] lockdep: Implement crossrelease feature Byungchul Park
2017-06-13  0:33   ` Byungchul Park
2017-06-22 23:27     ` Byungchul Park
2017-07-11 16:04   ` Peter Zijlstra
2017-07-12  2:24     ` Byungchul Park
2017-05-24  8:59 ` [PATCH v7 06/16] lockdep: Detect and handle hist_lock ring buffer overwrite Byungchul Park
2017-07-11 16:12   ` Peter Zijlstra
2017-07-12  2:00     ` Byungchul Park
2017-07-12  7:56       ` Peter Zijlstra
2017-07-13  2:07         ` Byungchul Park
2017-07-13  8:14           ` Peter Zijlstra
2017-07-13  8:57             ` Byungchul Park
2017-07-13  9:50               ` Peter Zijlstra
2017-07-13 10:09                 ` Byungchul Park
2017-07-13 10:29                   ` Peter Zijlstra
2017-07-13 11:12                     ` Peter Zijlstra
2017-07-13 11:23                       ` Byungchul Park
2017-07-14  1:41                         ` Byungchul Park
2017-07-14  6:42                         ` Byungchul Park
2017-07-21 13:54                           ` Peter Zijlstra
2017-07-25  6:29                             ` Byungchul Park
2017-07-25  8:45                               ` Peter Zijlstra
2017-07-13 11:19                     ` Byungchul Park
2017-07-18  1:25             ` Byungchul Park
2017-05-24  8:59 ` [PATCH v7 07/16] lockdep: Handle non(or multi)-acquisition of a crosslock Byungchul Park
2017-05-24  8:59 ` Byungchul Park [this message]
2017-07-25 15:41   ` [PATCH v7 08/16] lockdep: Avoid adding redundant direct links of crosslocks Peter Zijlstra
2017-07-26  7:16     ` Byungchul Park
2017-05-24  8:59 ` [PATCH v7 09/16] lockdep: Fix incorrect condition to print bug msgs for MAX_LOCKDEP_CHAIN_HLOCKS Byungchul Park
2017-05-24  8:59 ` [PATCH v7 10/16] lockdep: Make print_circular_bug() aware of crossrelease Byungchul Park
2017-05-24  8:59 ` [PATCH v7 11/16] lockdep: Apply crossrelease to completions Byungchul Park
2017-05-24  8:59 ` [PATCH v7 12/16] pagemap.h: Remove trailing white space Byungchul Park
2017-05-24  8:59 ` [PATCH v7 13/16] lockdep: Apply crossrelease to PG_locked locks Byungchul Park
2017-05-24  8:59 ` [PATCH v7 14/16] lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked Byungchul Park
2017-05-24  8:59 ` [PATCH v7 15/16] lockdep: Move data of CONFIG_LOCKDEP_PAGELOCK from page to page_ext Byungchul Park
2017-05-24  8:59 ` [PATCH v7 16/16] lockdep: Crossrelease feature documentation Byungchul Park

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=1495616389-29772-9-git-send-email-byungchul.park@lge.com \
    --to=byungchul.park@lge.com \
    --cc=akpm@linux-foundation.org \
    --cc=boqun.feng@gmail.com \
    --cc=kernel-team@lge.com \
    --cc=kirill@shutemov.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@kernel.org \
    --cc=npiggin@gmail.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=walken@google.com \
    --cc=willy@infradead.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