The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Byungchul Park <byungchul.park@lge.com>
To: tj@kernel.org, johannes.berg@intel.com, peterz@infradead.org,
	mingo@kernel.org
Cc: tglx@linutronix.de, oleg@redhat.com, david@fromorbit.com,
	linux-kernel@vger.kernel.org, kernel-team@lge.com
Subject: [PATCH 2/3] lockdep: Introduce lock_acquire_might()
Date: Tue,  5 Sep 2017 11:29:13 +0900	[thread overview]
Message-ID: <1504578554-4137-3-git-send-email-byungchul.park@lge.com> (raw)
In-Reply-To: <1504578554-4137-1-git-send-email-byungchul.park@lge.com>

>From the point of view of crossrelease, we can never be aware of the
release context in advance, until we get to the lock_release().
However, this way we cannot report deadlocks occured at the time.

Sometimes, we want to report that kind of problems, taking a risk
generating false dependencies e.g. lock_acquire()s in workqueue code,
which inevitably generate false ones with all acquisitions in works.

It would be better to provide another primitive, lock_acquire_might()
for that purpose so that lockdep internal can be aware of what users
expect and get chances to enhance to avoid false ones.

The primitive should:

   1. work as if it's trylock, since links between lock_acquire_might()
      and later ones are only meaningful. Remind this should be used to
      do what crossrelease commit does, in advance.

   2. make acquisitions by lock_acquire_might() ignored on the commit.

TODO: Although it's worth reporting potential problems, we have to do
our best to avoid adding false dependencies into graph, while current
lockdep code(not only crossrelease code) does nothing to care it.

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

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index bfa8e0b..68c806a 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -679,6 +679,7 @@ static inline void print_irqtrace_events(struct task_struct *curr)
 #define lock_acquire_exclusive(l, s, t, n, i)		lock_acquire(l, s, t, 0, 1, n, i)
 #define lock_acquire_shared(l, s, t, n, i)		lock_acquire(l, s, t, 1, 1, n, i)
 #define lock_acquire_shared_recursive(l, s, t, n, i)	lock_acquire(l, s, t, 2, 1, n, i)
+#define lock_acquire_might(l, s, t, n, i)		lock_acquire(l, s, t, 3, 1, n, i)
 
 #define spin_acquire(l, s, t, i)		lock_acquire_exclusive(l, s, t, NULL, i)
 #define spin_acquire_nest(l, s, t, n, i)	lock_acquire_exclusive(l, s, t, n, i)
@@ -704,6 +705,7 @@ static inline void print_irqtrace_events(struct task_struct *curr)
 #define lock_map_acquire(l)			lock_acquire_exclusive(l, 0, 0, NULL, _THIS_IP_)
 #define lock_map_acquire_read(l)		lock_acquire_shared_recursive(l, 0, 0, NULL, _THIS_IP_)
 #define lock_map_acquire_tryread(l)		lock_acquire_shared_recursive(l, 0, 1, NULL, _THIS_IP_)
+#define lock_map_acquire_might(l)		lock_acquire_might(l, 0, 1, NULL, _THIS_IP_)
 #define lock_map_release(l)			lock_release(l, 1, _THIS_IP_)
 
 #ifdef CONFIG_PROVE_LOCKING
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 0ac2f70..963c176 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -80,9 +80,25 @@
 #define LT_READ_N 1 /* 1: read non-recursion type */
 #define LT_READ_R 2 /* 2: read recursion type */
 
+/*
+ * This is used to detect deadlocks at the real time for
+ * crosslocks. For example, workqueue code uses acquisitions
+ * manually for that purpose taking a risk creating false
+ * dependencies. It would be better for lockdep to be aware
+ * what users such as workqueue code want in that case and
+ * get chances to enhance lockdep internal code to avoid
+ * false dependencies as far as possible.
+ *
+ * TODO: For now, this flag is only used to skip this kind
+ * of acquisitions on commit. But, adding these false ones
+ * into graph itself should be avoided if possible.
+ */
+#define LT_MIGHT  3 /* 3: might acquire */
+
 #define is_write(a)  ((a) == LT_WRITE)
 #define is_read(a)   ((a) == LT_READ_N || (a) == LT_READ_R)
 #define is_read_r(a) ((a) == LT_READ_R)
+#define is_might(a)  ((a) == LT_MIGHT)
 
 /*
  * lockdep_lock: protects the lockdep graph, the hashes and the
@@ -4827,7 +4843,7 @@ static inline struct lock_class *xlock_class(struct cross_lock *xlock)
  */
 static inline int depend_before(struct held_lock *hlock)
 {
-	return !is_read_r(hlock->read) && hlock->check && !hlock->trylock;
+	return !is_read_r(hlock->read) && !is_might(hlock->read) && hlock->check && !hlock->trylock;
 }
 
 /*
@@ -4835,7 +4851,7 @@ static inline int depend_before(struct held_lock *hlock)
  */
 static inline int depend_after(struct held_lock *hlock)
 {
-	return !is_read_r(hlock->read) && hlock->check;
+	return !is_read_r(hlock->read) && !is_might(hlock->read) && hlock->check;
 }
 
 /*
-- 
1.9.1

  parent reply	other threads:[~2017-09-05  2:29 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-05  2:29 [PATCH 0/3] Fix the workqueue and lockdep annotation issue Byungchul Park
2017-09-05  2:29 ` [PATCH 1/3] lockdep: Use enum type on hlock->read instead of magic number Byungchul Park
2017-09-05  2:29 ` Byungchul Park [this message]
2017-09-05  7:22   ` [PATCH 2/3] lockdep: Introduce lock_acquire_might() Peter Zijlstra
2017-09-12  0:35     ` Byungchul Park
2017-09-26  0:58       ` Byungchul Park
2017-09-05  2:29 ` [PATCH 3/3] lockdep: Remove unnecessary acquisitions wrt workqueue flush Byungchul Park
2017-09-05  7:25   ` Peter Zijlstra
2017-09-05  7:36     ` 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com)
2017-09-05  8:16       ` Peter Zijlstra
2017-09-05  8:19       ` 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=1504578554-4137-3-git-send-email-byungchul.park@lge.com \
    --to=byungchul.park@lge.com \
    --cc=david@fromorbit.com \
    --cc=johannes.berg@intel.com \
    --cc=kernel-team@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=tj@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