Skip to content

Commit 065bb78

Browse files
Daniel WagnerKAGA-KOKO
authored andcommitted
rcu: Do not call rcu_nocb_gp_cleanup() while holding rnp->lock
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 torvalds#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 torvalds#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 <[email protected]> Acked-by: Peter Zijlstra (Intel) <[email protected]> Cc: [email protected] Cc: Boqun Feng <[email protected]> Cc: Marcelo Tosatti <[email protected]> Cc: Steven Rostedt <[email protected]> Cc: Paul Gortmaker <[email protected]> Cc: Paolo Bonzini <[email protected]> Cc: "Paul E. McKenney" <[email protected]> Link: http://lkml.kernel.org/r/[email protected] Signed-off-by: Thomas Gleixner <[email protected]>
1 parent 8577370 commit 065bb78

File tree

3 files changed

+18
-5
lines changed

3 files changed

+18
-5
lines changed

kernel/rcu/tree.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1614,7 +1614,6 @@ static int rcu_future_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
16141614
int needmore;
16151615
struct rcu_data *rdp = this_cpu_ptr(rsp->rda);
16161616

1617-
rcu_nocb_gp_cleanup(rsp, rnp);
16181617
rnp->need_future_gp[c & 0x1] = 0;
16191618
needmore = rnp->need_future_gp[(c + 1) & 0x1];
16201619
trace_rcu_future_gp(rnp, rdp, c,
@@ -2010,6 +2009,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
20102009
int nocb = 0;
20112010
struct rcu_data *rdp;
20122011
struct rcu_node *rnp = rcu_get_root(rsp);
2012+
wait_queue_head_t *sq;
20132013

20142014
WRITE_ONCE(rsp->gp_activity, jiffies);
20152015
raw_spin_lock_irq_rcu_node(rnp);
@@ -2046,7 +2046,9 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
20462046
needgp = __note_gp_changes(rsp, rnp, rdp) || needgp;
20472047
/* smp_mb() provided by prior unlock-lock pair. */
20482048
nocb += rcu_future_gp_cleanup(rsp, rnp);
2049+
sq = rcu_nocb_gp_get(rnp);
20492050
raw_spin_unlock_irq(&rnp->lock);
2051+
rcu_nocb_gp_cleanup(sq);
20502052
cond_resched_rcu_qs();
20512053
WRITE_ONCE(rsp->gp_activity, jiffies);
20522054
rcu_gp_slow(rsp, gp_cleanup_delay);

kernel/rcu/tree.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,8 @@ static void zero_cpu_stall_ticks(struct rcu_data *rdp);
621621
static void increment_cpu_stall_ticks(void);
622622
static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu);
623623
static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq);
624-
static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp);
624+
static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp);
625+
static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq);
625626
static void rcu_init_one_nocb(struct rcu_node *rnp);
626627
static bool __call_rcu_nocb(struct rcu_data *rdp, struct rcu_head *rhp,
627628
bool lazy, unsigned long flags);

kernel/rcu/tree_plugin.h

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1811,9 +1811,9 @@ early_param("rcu_nocb_poll", parse_rcu_nocb_poll);
18111811
* Wake up any no-CBs CPUs' kthreads that were waiting on the just-ended
18121812
* grace period.
18131813
*/
1814-
static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
1814+
static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq)
18151815
{
1816-
wake_up_all(&rnp->nocb_gp_wq[rnp->completed & 0x1]);
1816+
wake_up_all(sq);
18171817
}
18181818

18191819
/*
@@ -1829,6 +1829,11 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
18291829
rnp->need_future_gp[(rnp->completed + 1) & 0x1] += nrq;
18301830
}
18311831

1832+
static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp)
1833+
{
1834+
return &rnp->nocb_gp_wq[rnp->completed & 0x1];
1835+
}
1836+
18321837
static void rcu_init_one_nocb(struct rcu_node *rnp)
18331838
{
18341839
init_waitqueue_head(&rnp->nocb_gp_wq[0]);
@@ -2502,14 +2507,19 @@ static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu)
25022507
return false;
25032508
}
25042509

2505-
static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
2510+
static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq)
25062511
{
25072512
}
25082513

25092514
static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
25102515
{
25112516
}
25122517

2518+
static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp)
2519+
{
2520+
return NULL;
2521+
}
2522+
25132523
static void rcu_init_one_nocb(struct rcu_node *rnp)
25142524
{
25152525
}

0 commit comments

Comments
 (0)