From: Daniel Wagner <daniel.wagner@bmw-carit.de>
To: linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org
Cc: linux-kbuild@vger.kernel.org,
Marcelo Tosatti <mtosatti@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>,
"Paul E . McKenney" <paulmck@linux.vnet.ibm.com>,
Paul Gortmaker <paul.gortmaker@windriver.com>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
Steven Rostedt <rostedt@goodmis.org>,
Boqun Feng <boqun.feng@gmail.com>,
Daniel Wagner <daniel.wagner@bmw-carit.de>
Subject: [PATCH tip v6 4/5] rcu: Do not call rcu_nocb_gp_cleanup() while holding rnp->lock
Date: Thu, 28 Jan 2016 15:44:29 +0100 [thread overview]
Message-ID: <1453992270-4688-5-git-send-email-daniel.wagner@bmw-carit.de> (raw)
In-Reply-To: <1453992270-4688-1-git-send-email-daniel.wagner@bmw-carit.de>
rcu_nocb_gp_cleanup() is called while holding rnp->lock. Currently,
this is okay because the wake_up_all() in rcu_nocb_gp_cleanup() will
not enable the IRQs. lockdep is happy.
By switching over using swait this is not true anymore. swake_up_all()
enables the IRQs while processing the waiters. __do_softirq() can now
run and will eventually call rcu_process_callbacks() which wants to
grap nrp->lock.
Let's move the rcu_nocb_gp_cleanup() call outside the lock before we
switch over to swait.
If we would hold the rnp->lock and use swait, lockdep reports
following:
=================================
[ INFO: inconsistent lock state ]
4.2.0-rc5-00025-g9a73ba0 #136 Not tainted
---------------------------------
inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
rcu_preempt/8 [HC0[0]:SC0[0]:HE1:SE1] takes:
(rcu_node_1){+.?...}, at: [<ffffffff811387c7>] rcu_gp_kthread+0xb97/0xeb0
{IN-SOFTIRQ-W} state was registered at:
[<ffffffff81109b9f>] __lock_acquire+0xd5f/0x21e0
[<ffffffff8110be0f>] lock_acquire+0xdf/0x2b0
[<ffffffff81841cc9>] _raw_spin_lock_irqsave+0x59/0xa0
[<ffffffff81136991>] rcu_process_callbacks+0x141/0x3c0
[<ffffffff810b1a9d>] __do_softirq+0x14d/0x670
[<ffffffff810b2214>] irq_exit+0x104/0x110
[<ffffffff81844e96>] smp_apic_timer_interrupt+0x46/0x60
[<ffffffff81842e70>] apic_timer_interrupt+0x70/0x80
[<ffffffff810dba66>] rq_attach_root+0xa6/0x100
[<ffffffff810dbc2d>] cpu_attach_domain+0x16d/0x650
[<ffffffff810e4b42>] build_sched_domains+0x942/0xb00
[<ffffffff821777c2>] sched_init_smp+0x509/0x5c1
[<ffffffff821551e3>] kernel_init_freeable+0x172/0x28f
[<ffffffff8182cdce>] kernel_init+0xe/0xe0
[<ffffffff8184231f>] ret_from_fork+0x3f/0x70
irq event stamp: 76
hardirqs last enabled at (75): [<ffffffff81841330>] _raw_spin_unlock_irq+0x30/0x60
hardirqs last disabled at (76): [<ffffffff8184116f>] _raw_spin_lock_irq+0x1f/0x90
softirqs last enabled at (0): [<ffffffff810a8df2>] copy_process.part.26+0x602/0x1cf0
softirqs last disabled at (0): [< (null)>] (null)
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(rcu_node_1);
<Interrupt>
lock(rcu_node_1);
*** DEADLOCK ***
1 lock held by rcu_preempt/8:
#0: (rcu_node_1){+.?...}, at: [<ffffffff811387c7>] rcu_gp_kthread+0xb97/0xeb0
stack backtrace:
CPU: 0 PID: 8 Comm: rcu_preempt Not tainted 4.2.0-rc5-00025-g9a73ba0 #136
Hardware name: Dell Inc. PowerEdge R820/066N7P, BIOS 2.0.20 01/16/2014
0000000000000000 000000006d7e67d8 ffff881fb081fbd8 ffffffff818379e0
0000000000000000 ffff881fb0812a00 ffff881fb081fc38 ffffffff8110813b
0000000000000000 0000000000000001 ffff881f00000001 ffffffff8102fa4f
Call Trace:
[<ffffffff818379e0>] dump_stack+0x4f/0x7b
[<ffffffff8110813b>] print_usage_bug+0x1db/0x1e0
[<ffffffff8102fa4f>] ? save_stack_trace+0x2f/0x50
[<ffffffff811087ad>] mark_lock+0x66d/0x6e0
[<ffffffff81107790>] ? check_usage_forwards+0x150/0x150
[<ffffffff81108898>] mark_held_locks+0x78/0xa0
[<ffffffff81841330>] ? _raw_spin_unlock_irq+0x30/0x60
[<ffffffff81108a28>] trace_hardirqs_on_caller+0x168/0x220
[<ffffffff81108aed>] trace_hardirqs_on+0xd/0x10
[<ffffffff81841330>] _raw_spin_unlock_irq+0x30/0x60
[<ffffffff810fd1c7>] swake_up_all+0xb7/0xe0
[<ffffffff811386e1>] rcu_gp_kthread+0xab1/0xeb0
[<ffffffff811089bf>] ? trace_hardirqs_on_caller+0xff/0x220
[<ffffffff81841341>] ? _raw_spin_unlock_irq+0x41/0x60
[<ffffffff81137c30>] ? rcu_barrier+0x20/0x20
[<ffffffff810d2014>] kthread+0x104/0x120
[<ffffffff81841330>] ? _raw_spin_unlock_irq+0x30/0x60
[<ffffffff810d1f10>] ? kthread_create_on_node+0x260/0x260
[<ffffffff8184231f>] ret_from_fork+0x3f/0x70
[<ffffffff810d1f10>] ? kthread_create_on_node+0x260/0x260
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
kernel/rcu/tree.c | 4 +++-
kernel/rcu/tree.h | 3 ++-
kernel/rcu/tree_plugin.h | 16 +++++++++++++---
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index f07343b..baf6d09 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1590,7 +1590,6 @@ static int rcu_future_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
int needmore;
struct rcu_data *rdp = this_cpu_ptr(rsp->rda);
- rcu_nocb_gp_cleanup(rsp, rnp);
rnp->need_future_gp[c & 0x1] = 0;
needmore = rnp->need_future_gp[(c + 1) & 0x1];
trace_rcu_future_gp(rnp, rdp, c,
@@ -1991,6 +1990,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
int nocb = 0;
struct rcu_data *rdp;
struct rcu_node *rnp = rcu_get_root(rsp);
+ wait_queue_head_t *sq;
WRITE_ONCE(rsp->gp_activity, jiffies);
raw_spin_lock_irq(&rnp->lock);
@@ -2029,7 +2029,9 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
needgp = __note_gp_changes(rsp, rnp, rdp) || needgp;
/* smp_mb() provided by prior unlock-lock pair. */
nocb += rcu_future_gp_cleanup(rsp, rnp);
+ sq = rcu_nocb_gp_get(rnp);
raw_spin_unlock_irq(&rnp->lock);
+ rcu_nocb_gp_cleanup(sq);
cond_resched_rcu_qs();
WRITE_ONCE(rsp->gp_activity, jiffies);
rcu_gp_slow(rsp, gp_cleanup_delay);
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 9fb4e23..aa47e2c 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -607,7 +607,8 @@ static void zero_cpu_stall_ticks(struct rcu_data *rdp);
static void increment_cpu_stall_ticks(void);
static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu);
static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq);
-static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp);
+static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp);
+static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq);
static void rcu_init_one_nocb(struct rcu_node *rnp);
static bool __call_rcu_nocb(struct rcu_data *rdp, struct rcu_head *rhp,
bool lazy, unsigned long flags);
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 630c197..99b1ce6 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1822,9 +1822,9 @@ early_param("rcu_nocb_poll", parse_rcu_nocb_poll);
* Wake up any no-CBs CPUs' kthreads that were waiting on the just-ended
* grace period.
*/
-static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
+static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq)
{
- wake_up_all(&rnp->nocb_gp_wq[rnp->completed & 0x1]);
+ wake_up_all(sq);
}
/*
@@ -1840,6 +1840,11 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
rnp->need_future_gp[(rnp->completed + 1) & 0x1] += nrq;
}
+static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp)
+{
+ return &rnp->nocb_gp_wq[rnp->completed & 0x1];
+}
+
static void rcu_init_one_nocb(struct rcu_node *rnp)
{
init_waitqueue_head(&rnp->nocb_gp_wq[0]);
@@ -2514,7 +2519,7 @@ static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu)
return false;
}
-static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
+static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq)
{
}
@@ -2522,6 +2527,11 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
{
}
+static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp)
+{
+ return NULL;
+}
+
static void rcu_init_one_nocb(struct rcu_node *rnp)
{
}
--
2.5.0
next prev parent reply other threads:[~2016-01-28 14:51 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-28 14:44 [PATCH tip v6 0/5] Simple wait queue support Daniel Wagner
2016-01-28 14:44 ` [PATCH tip v6 1/5] wait.[ch]: Introduce the simple waitqueue (swait) implementation Daniel Wagner
2016-01-28 14:44 ` [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error Daniel Wagner
2016-01-29 12:17 ` Daniel Wagner
2016-01-29 18:55 ` Paul Gortmaker
2016-02-01 6:49 ` Daniel Wagner
2016-02-05 8:16 ` Daniel Wagner
2016-02-07 4:39 ` Paul Gortmaker
2016-02-17 13:04 ` Daniel Wagner
2016-01-28 14:44 ` [PATCH tip v6 3/5] KVM: use simple waitqueue for vcpu->wq Daniel Wagner
2016-01-29 12:18 ` Daniel Wagner
2016-01-28 14:44 ` Daniel Wagner [this message]
2016-01-28 14:44 ` [PATCH tip v6 5/5] rcu: use simple wait queues where possible in rcutree Daniel Wagner
2016-01-29 13:23 ` [PATCH] video: Use bool instead int pointer for get_opt_bool() argument Daniel Wagner
2016-01-29 13:28 ` [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header Daniel Wagner
2016-02-01 0:52 ` Maciej W. Rozycki
2016-02-01 16:07 ` Daniel Wagner
2016-02-06 17:16 ` Maciej W. Rozycki
2016-02-08 15:44 ` [PATCH v3 0/3] " Daniel Wagner
2016-02-08 15:44 ` [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version Daniel Wagner
2016-02-08 17:19 ` Maciej W. Rozycki
2016-02-09 7:01 ` Daniel Wagner
2016-02-09 11:46 ` Maciej W. Rozycki
2016-02-09 12:37 ` Daniel Wagner
2016-02-09 14:51 ` Maciej W. Rozycki
2016-02-10 8:51 ` Daniel Wagner
2016-02-08 15:44 ` [PATCH v3 2/3] crash_dump: Add vmcore_elf32_check_arch Daniel Wagner
2016-02-08 17:05 ` Maciej W. Rozycki
2016-02-08 15:44 ` [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header Daniel Wagner
2016-02-08 16:22 ` kbuild test robot
2016-02-09 8:03 ` Daniel Wagner
2016-02-09 12:32 ` Maciej W. Rozycki
2016-02-09 12:38 ` Daniel Wagner
2016-02-09 19:44 ` Maciej W. Rozycki
2016-02-10 6:28 ` Daniel Wagner
2016-02-10 9:21 ` [PATCH v4 0/2] " Daniel Wagner
2016-02-10 9:21 ` [PATCH v4 1/2] crash_dump: Add vmcore_elf32_check_arch Daniel Wagner
2016-02-10 9:21 ` [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header Daniel Wagner
2016-02-11 10:49 ` Ralf Baechle
2016-02-11 12:04 ` Maciej W. Rozycki
2016-02-11 12:14 ` Daniel Wagner
2016-02-11 14:58 ` Maciej W. Rozycki
2016-02-11 15:30 ` Ralf Baechle
2016-02-08 16:58 ` [PATCH v3 3/3] " Maciej W. Rozycki
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=1453992270-4688-5-git-send-email-daniel.wagner@bmw-carit.de \
--to=daniel.wagner@bmw-carit.de \
--cc=boqun.feng@gmail.com \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rt-users@vger.kernel.org \
--cc=mtosatti@redhat.com \
--cc=paul.gortmaker@windriver.com \
--cc=paulmck@linux.vnet.ibm.com \
--cc=pbonzini@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).