From: tip-bot for Boqun Feng <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: jiangshanlai@gmail.com, torvalds@linux-foundation.org,
byungchul.park@lge.com, peterz@infradead.org,
linux-kernel@vger.kernel.org, tglx@linutronix.de,
mingo@kernel.org, hpa@zytor.com, tj@kernel.org,
boqun.feng@gmail.com
Subject: [tip:locking/core] locking/lockdep: Explicitly initialize wq_barrier::done::map
Date: Thu, 17 Aug 2017 03:22:48 -0700 [thread overview]
Message-ID: <tip-52fa5bc5cbba089f09bc2c372e3432f3f3e48051@git.kernel.org> (raw)
In-Reply-To: <20170817094622.12915-1-boqun.feng@gmail.com>
Commit-ID: 52fa5bc5cbba089f09bc2c372e3432f3f3e48051
Gitweb: http://git.kernel.org/tip/52fa5bc5cbba089f09bc2c372e3432f3f3e48051
Author: Boqun Feng <boqun.feng@gmail.com>
AuthorDate: Thu, 17 Aug 2017 17:46:12 +0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 17 Aug 2017 12:12:33 +0200
locking/lockdep: Explicitly initialize wq_barrier::done::map
With the new lockdep crossrelease feature, which checks completions usage,
a false positive is reported in the workqueue code:
> Worker A : acquired of wfc.work -> wait for cpu_hotplug_lock to be released
> Task B : acquired of cpu_hotplug_lock -> wait for lock#3 to be released
> Task C : acquired of lock#3 -> wait for completion of barr->done
> (Task C is in lru_add_drain_all_cpuslocked())
> Worker D : wait for wfc.work to be released -> will complete barr->done
Such a dead lock can not happen because Task C's barr->done and Worker D's
barr->done can not be the same instance.
The reason of this false positive is we initialize all wq_barrier::done
at insert_wq_barrier() via init_completion(), which makes them belong to
the same lock class, therefore, impossible circles are reported.
To fix this, explicitly initialize the lockdep map for wq_barrier::done
in insert_wq_barrier(), so that the lock class key of wq_barrier::done
is a subkey of the corresponding work_struct, as a result we won't build
a dependency between a wq_barrier with a unrelated work, and we can
differ wq barriers based on the related works, so the false positive
above is avoided.
Also define the empty lockdep_init_map_crosslock() for !CROSSRELEASE
to make the code simple and away from unnecessary #ifdefs.
Reported-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Cc: Byungchul Park <byungchul.park@lge.com>
Cc: Lai Jiangshan <jiangshanlai@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20170817094622.12915-1-boqun.feng@gmail.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/lockdep.h | 1 +
kernel/workqueue.c | 11 ++++++++++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 651cc61..fc827ca 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -583,6 +583,7 @@ extern void crossrelease_hist_end(enum xhlock_context_t c);
extern void lockdep_init_task(struct task_struct *task);
extern void lockdep_free_task(struct task_struct *task);
#else
+#define lockdep_init_map_crosslock(m, n, k, s) do {} while (0)
/*
* To initialize a lockdep_map statically use this macro.
* Note that _name must not be NULL.
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index e86733a..f128b3b 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2476,7 +2476,16 @@ static void insert_wq_barrier(struct pool_workqueue *pwq,
*/
INIT_WORK_ONSTACK(&barr->work, wq_barrier_func);
__set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(&barr->work));
- init_completion(&barr->done);
+
+ /*
+ * Explicitly init the crosslock for wq_barrier::done, make its lock
+ * key a subkey of the corresponding work. As a result we won't
+ * build a dependency between wq_barrier::done and unrelated work.
+ */
+ lockdep_init_map_crosslock((struct lockdep_map *)&barr->done.map,
+ "(complete)wq_barr::done",
+ target->lockdep_map.key, 1);
+ __init_completion(&barr->done);
barr->task = current;
/*
next prev parent reply other threads:[~2017-08-17 10:27 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-17 9:46 [PATCH] workqueue/lockdep: Explicitly initialize wq_barrier::done::map Boqun Feng
2017-08-17 10:22 ` tip-bot for Boqun Feng [this message]
2017-08-17 13:26 ` Tejun Heo
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=tip-52fa5bc5cbba089f09bc2c372e3432f3f3e48051@git.kernel.org \
--to=tipbot@zytor.com \
--cc=boqun.feng@gmail.com \
--cc=byungchul.park@lge.com \
--cc=hpa@zytor.com \
--cc=jiangshanlai@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tip-commits@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=tj@kernel.org \
--cc=torvalds@linux-foundation.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.