From: Ed Tomlinson <edt@aei.ca>
To: paulmck@linux.vnet.ibm.com
Cc: Peter Zijlstra <peterz@infradead.org>,
linux-kernel@vger.kernel.org, mingo@elte.hu,
laijs@cn.fujitsu.com, dipankar@in.ibm.com,
akpm@linux-foundation.org, mathieu.desnoyers@polymtl.ca,
josh@joshtriplett.org, niv@us.ibm.com, tglx@linutronix.de,
rostedt@goodmis.org, Valdis.Kletnieks@vt.edu,
dhowells@redhat.com, eric.dumazet@gmail.com, darren@dvhart.com,
patches@linaro.org, greearb@candelatech.com
Subject: Re: [PATCH tip/core/urgent 1/7] rcu: decrease rcu_report_exp_rnp coupling with scheduler
Date: Wed, 20 Jul 2011 07:23:32 -0400 [thread overview]
Message-ID: <201107200723.33819.edt@aei.ca> (raw)
In-Reply-To: <20110720045414.GC2400@linux.vnet.ibm.com>
On Wednesday 20 July 2011 00:54:15 Paul E. McKenney wrote:
> On Wed, Jul 20, 2011 at 04:40:18AM +0200, Peter Zijlstra wrote:
> > On Tue, 2011-07-19 at 17:18 -0700, Paul E. McKenney wrote:
> > > +++ b/kernel/rcutree_plugin.h
> > > @@ -696,8 +696,10 @@ static void rcu_report_exp_rnp(struct rcu_state *rsp, struct rcu_node *rnp)
> > > raw_spin_lock_irqsave(&rnp->lock, flags);
> > > for (;;) {
> > > if (!sync_rcu_preempt_exp_done(rnp))
> > > + raw_spin_unlock_irqrestore(&rnp->lock, flags);
> > > break;
> >
> > I bet that'll all work much better if you wrap it in curly braces like:
> >
> > if (!sync_rcu_preempt_exp_done(rnp)) {
> > raw_spin_unlock_irqrestore(&rnp->lock, flags);
> > break;
> > }
> >
> > That might also explain those explosions Ed and Ben have been seeing.
>
> Indeed. Must be the call of the snake. :-(
>
> Thank you for catching this!
>
> > > if (rnp->parent == NULL) {
> > > + raw_spin_unlock_irqrestore(&rnp->lock, flags);
> > > wake_up(&sync_rcu_preempt_exp_wq);
> > > break;
> > > }
> > > @@ -707,7 +709,6 @@ static void rcu_report_exp_rnp(struct rcu_state *rsp, struct rcu_node *rnp)
> > > raw_spin_lock(&rnp->lock); /* irqs already disabled */
> > > rnp->expmask &= ~mask;
> > > }
> > > - raw_spin_unlock_irqrestore(&rnp->lock, flags);
> > > }
>
> So this time I am testing the exact patch series before resending.
> In the meantime, here is the updated version of this patch.
>
> Thanx, Paul
>
> ------------------------------------------------------------------------
>
> rcu: decrease rcu_report_exp_rnp coupling with scheduler
>
> PREEMPT_RCU read-side critical sections blocking an expedited grace
> period invoke rcu_report_exp_rnp(). When the last such critical section
> has completed, rcu_report_exp_rnp() invokes the scheduler to wake up the
> task that invoked synchronize_rcu_expedited() -- needlessly holding the
> root rcu_node structure's lock while doing so, thus needlessly providing
> a way for RCU and the scheduler to deadlock.
>
> This commit therefore releases the root rcu_node structure's lock before
> calling wake_up().
>
> Reported-by: Ed Tomlinson <edt@aei.ca>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>
> diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
> index 75113cb..6abef3c 100644
> --- a/kernel/rcutree_plugin.h
> +++ b/kernel/rcutree_plugin.h
> @@ -695,9 +695,12 @@ static void rcu_report_exp_rnp(struct rcu_state *rsp, struct rcu_node *rnp)
>
> raw_spin_lock_irqsave(&rnp->lock, flags);
> for (;;) {
> - if (!sync_rcu_preempt_exp_done(rnp))
> + if (!sync_rcu_preempt_exp_done(rnp)) {
> + raw_spin_unlock_irqrestore(&rnp->lock, flags);
> break;
> + }
> if (rnp->parent == NULL) {
> + raw_spin_unlock_irqrestore(&rnp->lock, flags);
> wake_up(&sync_rcu_preempt_exp_wq);
> break;
> }
> @@ -707,7 +710,6 @@ static void rcu_report_exp_rnp(struct rcu_state *rsp, struct rcu_node *rnp)
> raw_spin_lock(&rnp->lock); /* irqs already disabled */
> rnp->expmask &= ~mask;
> }
> - raw_spin_unlock_irqrestore(&rnp->lock, flags);
> }
>
> /*
Paul/Peter,
With all 7 fixes applied with the above version of patch 1, I get a warning during boot:
[ 3.443140] EXT4-fs (sdc3): mounted filesystem with ordered data mode. Opts: (null)
[ 3.456097] VFS: Mounted root (ext4 filesystem) readonly on device 8:35.
[ 3.469734] devtmpfs: mounted
[ 3.478488] Freeing unused kernel memory: 628k freed
[ 3.488590] Write protecting the kernel read-only data: 10240k
[ 3.500809] Freeing unused kernel memory: 468k freed
[ 3.514158] Freeing unused kernel memory: 1240k freed
[ 3.552337] ------------[ cut here ]------------
[ 3.553004] ------------[ cut here ]------------
[ 3.553004] WARNING: at kernel/rcutree_plugin.h:414 __rcu_read_unlock+0xc9/0x120()
[ 3.553004] Hardware name: System Product Name
[ 3.553004] Modules linked in:
[ 3.553004] Pid: 1, comm: init Not tainted 3.0.0-rcr-crc+ #342
[ 3.553004] Call Trace:
[ 3.553004] [<ffffffff8104b00f>] warn_slowpath_common+0x7f/0xc0
[ 3.553004] [<ffffffff8104b06a>] warn_slowpath_null+0x1a/0x20
[ 3.553004] [<ffffffff810bb479>] __rcu_read_unlock+0xc9/0x120
[ 3.553004] [<ffffffff8157d6b1>] __atomic_notifier_call_chain+0x91/0xb0
[ 3.553004] [<ffffffff8157d620>] ? notifier_call_chain+0x80/0x80
[ 3.553004] [<ffffffff812c5860>] ? bit_putcs+0x5b0/0x5b0
[ 3.553004] [<ffffffff8157d6e6>] atomic_notifier_call_chain+0x16/0x20
[ 3.553004] [<ffffffff813203c9>] notify_write+0x29/0x30
[ 3.553004] [<ffffffff81322024>] vt_console_print+0x164/0x3b0
[ 3.553004] [<ffffffff8104b29e>] __call_console_drivers+0x8e/0xb0
[ 3.553004] [<ffffffff8104b30a>] _call_console_drivers+0x4a/0x80
[ 3.553004] [<ffffffff8104b9ed>] console_unlock+0xfd/0x210
[ 3.553004] [<ffffffff8104c246>] vprintk+0x3f6/0x530
[ 3.553004] [<ffffffff810bb479>] ? __rcu_read_unlock+0xc9/0x120
[ 3.553004] [<ffffffff8157585c>] printk+0x41/0x45
[ 3.553004] [<ffffffff810bb479>] ? __rcu_read_unlock+0xc9/0x120
[ 3.553004] [<ffffffff8104afcd>] warn_slowpath_common+0x3d/0xc0
[ 3.553004] [<ffffffff8104b06a>] warn_slowpath_null+0x1a/0x20
[ 3.553004] [<ffffffff810bb479>] __rcu_read_unlock+0xc9/0x120
[ 3.553004] [<ffffffff8103fed8>] cpuacct_charge+0xc8/0xe0
[ 3.553004] [<ffffffff8103fe58>] ? cpuacct_charge+0x48/0xe0
[ 3.553004] [<ffffffff810326b7>] ? task_of+0x97/0xd0
[ 3.553004] [<ffffffff81040ef5>] update_curr+0x1a5/0x210
[ 3.553004] [<ffffffff81576d78>] ? preempt_schedule_irq+0x68/0xa0
[ 3.553004] [<ffffffff810419e0>] put_prev_task_fair+0x110/0x120
[ 3.553004] [<ffffffff81575f3a>] schedule+0x1da/0xc50
[ 3.553004] [<ffffffff81576d72>] ? preempt_schedule_irq+0x62/0xa0
[ 3.553004] [<ffffffff81576d78>] preempt_schedule_irq+0x68/0xa0
[ 3.553004] [<ffffffff8157a316>] retint_kernel+0x26/0x30
[ 3.553004] [<ffffffff810da494>] ? ftrace_likely_update+0x14/0x20
[ 3.553004] [<ffffffff810bb4ab>] __rcu_read_unlock+0xfb/0x120
[ 3.553004] [<ffffffff810f8190>] find_get_page+0x170/0x190
[ 3.553004] [<ffffffff810f8020>] ? find_get_pages+0x280/0x280
[ 3.553004] [<ffffffff81180a30>] __find_get_block_slow+0x40/0x130
[ 3.553004] [<ffffffff81180d5f>] __find_get_block+0xdf/0x210
[ 3.553004] [<ffffffff8157d541>] ? sub_preempt_count+0x51/0x60
[ 3.553004] [<ffffffff81184c5e>] __getblk+0x11e/0x300
[ 3.553004] [<ffffffff81184e55>] __breadahead+0x15/0x60
[ 3.553004] [<ffffffff811e5e9e>] __ext4_get_inode_loc+0x36e/0x3f0
[ 3.553004] [<ffffffff811e7dfe>] ext4_iget+0x7e/0x870
[ 3.553004] [<ffffffff8129247e>] ? do_raw_spin_lock+0xde/0x1c0
[ 3.553004] [<ffffffff811f25e5>] ext4_lookup+0xb5/0x150
[ 3.553004] [<ffffffff8115a5c3>] d_alloc_and_lookup+0xc3/0x100
[ 3.553004] [<ffffffff8115cd00>] do_lookup+0x260/0x480
[ 3.553004] [<ffffffff8157d541>] ? sub_preempt_count+0x51/0x60
[ 3.553004] [<ffffffff8115dc58>] link_path_walk+0x248/0x930
[ 3.553004] [<ffffffff812925f6>] ? __raw_spin_lock_init+0x36/0x60
[ 3.553004] [<ffffffff81160417>] path_openat+0x107/0x4f0
[ 3.553004] [<ffffffff8108b392>] ? lock_release_non_nested+0xb2/0x330
[ 3.553004] [<ffffffff81160979>] do_filp_open+0x49/0x100
[ 3.553004] [<ffffffff8129247e>] ? do_raw_spin_lock+0xde/0x1c0
[ 3.553004] [<ffffffff81579a3c>] ? _raw_spin_unlock+0x5c/0x70
[ 3.553004] [<ffffffff8116db4a>] ? alloc_fd+0xfa/0x140
[ 3.553004] [<ffffffff8114d892>] do_sys_open+0x172/0x220
[ 3.553004] [<ffffffff8114d980>] sys_open+0x20/0x30
[ 3.553004] [<ffffffff815814eb>] system_call_fastpath+0x16/0x1b
[ 3.553004] ---[ end trace 9137bd8a48443ea9 ]---
[ 3.553004] WARNING: at kernel/rcutree_plugin.h:414 __rcu_read_unlock+0xc9/0x120()
[ 3.553004] Hardware name: System Product Name
[ 3.553004] Modules linked in:
[ 3.553004] Pid: 1, comm: init Tainted: G W 3.0.0-rcr-crc+ #342
[ 3.553004] Call Trace:
[ 3.553004] [<ffffffff8104b00f>] warn_slowpath_common+0x7f/0xc0
[ 3.553004] [<ffffffff8104b06a>] warn_slowpath_null+0x1a/0x20
[ 3.553004] [<ffffffff810bb479>] __rcu_read_unlock+0xc9/0x120
[ 3.553004] [<ffffffff8103fed8>] cpuacct_charge+0xc8/0xe0
[ 3.553004] [<ffffffff8103fe58>] ? cpuacct_charge+0x48/0xe0
[ 3.553004] [<ffffffff810326b7>] ? task_of+0x97/0xd0
[ 3.553004] [<ffffffff81040ef5>] update_curr+0x1a5/0x210
[ 3.553004] [<ffffffff81576d78>] ? preempt_schedule_irq+0x68/0xa0
[ 3.553004] [<ffffffff810419e0>] put_prev_task_fair+0x110/0x120
[ 3.553004] [<ffffffff81575f3a>] schedule+0x1da/0xc50
[ 3.553004] [<ffffffff81576d72>] ? preempt_schedule_irq+0x62/0xa0
[ 3.553004] [<ffffffff81576d78>] preempt_schedule_irq+0x68/0xa0
[ 3.553004] [<ffffffff8157a316>] retint_kernel+0x26/0x30
[ 3.553004] [<ffffffff810da494>] ? ftrace_likely_update+0x14/0x20
[ 3.553004] [<ffffffff810bb4ab>] __rcu_read_unlock+0xfb/0x120
[ 3.553004] [<ffffffff810f8190>] find_get_page+0x170/0x190
[ 3.553004] [<ffffffff810f8020>] ? find_get_pages+0x280/0x280
[ 3.553004] [<ffffffff81180a30>] __find_get_block_slow+0x40/0x130
[ 3.553004] [<ffffffff81180d5f>] __find_get_block+0xdf/0x210
[ 3.553004] [<ffffffff8157d541>] ? sub_preempt_count+0x51/0x60
[ 3.553004] [<ffffffff81184c5e>] __getblk+0x11e/0x300
[ 3.553004] [<ffffffff81184e55>] __breadahead+0x15/0x60
[ 3.553004] [<ffffffff811e5e9e>] __ext4_get_inode_loc+0x36e/0x3f0
[ 3.553004] [<ffffffff811e7dfe>] ext4_iget+0x7e/0x870
[ 3.553004] [<ffffffff8129247e>] ? do_raw_spin_lock+0xde/0x1c0
[ 3.553004] [<ffffffff811f25e5>] ext4_lookup+0xb5/0x150
[ 3.553004] [<ffffffff8115a5c3>] d_alloc_and_lookup+0xc3/0x100
[ 3.553004] [<ffffffff8115cd00>] do_lookup+0x260/0x480
[ 3.553004] [<ffffffff8157d541>] ? sub_preempt_count+0x51/0x60
[ 3.553004] [<ffffffff8115dc58>] link_path_walk+0x248/0x930
[ 3.553004] [<ffffffff812925f6>] ? __raw_spin_lock_init+0x36/0x60
[ 3.553004] [<ffffffff81160417>] path_openat+0x107/0x4f0
[ 3.553004] [<ffffffff8108b392>] ? lock_release_non_nested+0xb2/0x330
[ 3.553004] [<ffffffff81160979>] do_filp_open+0x49/0x100
[ 3.553004] [<ffffffff8129247e>] ? do_raw_spin_lock+0xde/0x1c0
[ 3.553004] [<ffffffff81579a3c>] ? _raw_spin_unlock+0x5c/0x70
[ 3.553004] [<ffffffff8116db4a>] ? alloc_fd+0xfa/0x140
[ 3.553004] [<ffffffff8114d892>] do_sys_open+0x172/0x220
[ 3.553004] [<ffffffff8114d980>] sys_open+0x20/0x30
[ 3.553004] [<ffffffff815814eb>] system_call_fastpath+0x16/0x1b
[ 3.553004] ---[ end trace 9137bd8a48443eaa ]---
INIT: version 2.88 booting
G
OpenRC 0.8.3 is starting up Gentoo Linux (x86_64)
RCU settings are:grep RCU .config
# RCU Subsystem
CONFIG_TREE_PREEMPT_RCU=y
CONFIG_PREEMPT_RCU=y
# CONFIG_RCU_TRACE is not set
CONFIG_RCU_FANOUT=64
# CONFIG_RCU_FANOUT_EXACT is not set
# CONFIG_TREE_RCU_TRACE is not set
CONFIG_RCU_BOOST=y
CONFIG_RCU_BOOST_PRIO=1
CONFIG_RCU_BOOST_DELAY=500
# CONFIG_PROVE_RCU is not set
# CONFIG_SPARSE_RCU_POINTER is not set
CONFIG_RCU_TORTURE_TEST=m
CONFIG_RCU_CPU_STALL_TIMEOUT=60
# CONFIG_RCU_CPU_STALL_VERBOSE is not set
and threadirqs are enabled by boot.
Hope this helps,
Ed
next prev parent reply other threads:[~2011-07-20 11:23 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-20 0:17 [PATCH rcu/urgent 0/6] Fixes for RCU/scheduler/irq-threads trainwreck Paul E. McKenney
2011-07-20 0:18 ` [PATCH tip/core/urgent 1/7] rcu: decrease rcu_report_exp_rnp coupling with scheduler Paul E. McKenney
2011-07-20 2:40 ` Peter Zijlstra
2011-07-20 4:54 ` Paul E. McKenney
2011-07-20 11:23 ` Ed Tomlinson [this message]
2011-07-20 11:31 ` Ed Tomlinson
2011-07-20 12:35 ` Peter Zijlstra
2011-07-20 13:23 ` Paul E. McKenney
2011-07-20 0:18 ` [PATCH tip/core/urgent 2/7] rcu: Fix RCU_BOOST race handling current->rcu_read_unlock_special Paul E. McKenney
2011-07-20 0:18 ` [PATCH tip/core/urgent 3/7] rcu: Streamline code produced by __rcu_read_unlock() Paul E. McKenney
2011-07-20 0:18 ` [PATCH tip/core/urgent 4/7] rcu: protect __rcu_read_unlock() against scheduler-using irq handlers Paul E. McKenney
2011-07-20 12:54 ` Peter Zijlstra
2011-07-20 13:25 ` Paul E. McKenney
2011-07-20 0:18 ` [PATCH tip/core/urgent 5/7] sched: Add irq_{enter,exit}() to scheduler_ipi() Paul E. McKenney
2011-07-20 0:18 ` [PATCH tip/core/urgent 6/7] softirq,rcu: Inform RCU of irq_exit() activity Paul E. McKenney
2011-07-20 0:18 ` [PATCH tip/core/urgent 7/7] signal: align __lock_task_sighand() irq disabling and RCU Paul E. McKenney
2011-07-20 1:10 ` [PATCH rcu/urgent 0/6] Fixes for RCU/scheduler/irq-threads trainwreck Ben Greear
2011-07-20 1:30 ` Ed Tomlinson
2011-07-20 2:07 ` Ed Tomlinson
2011-07-20 4:44 ` Paul E. McKenney
2011-07-20 5:03 ` Linus Torvalds
2011-07-20 13:34 ` Paul E. McKenney
2011-07-20 17:02 ` Ben Greear
2011-07-20 17:15 ` Paul E. McKenney
2011-07-20 18:44 ` Ingo Molnar
2011-07-20 18:52 ` Peter Zijlstra
2011-07-20 19:01 ` Paul E. McKenney
2011-07-20 19:25 ` Peter Zijlstra
2011-07-20 20:06 ` Paul E. McKenney
2011-07-20 19:02 ` Linus Torvalds
2011-07-20 19:29 ` Paul E. McKenney
2011-07-20 19:39 ` Ingo Molnar
2011-07-20 19:57 ` Ingo Molnar
2011-07-20 20:33 ` Paul E. McKenney
2011-07-20 20:54 ` Ben Greear
2011-07-20 21:12 ` Paul E. McKenney
2011-07-21 3:25 ` Ben Greear
2011-07-21 16:04 ` Paul E. McKenney
2011-07-20 21:04 ` [GIT PULL] RCU fixes for v3.0 Ingo Molnar
2011-07-20 21:55 ` Ed Tomlinson
2011-07-20 22:06 ` Paul E. McKenney
2011-07-20 20:08 ` [PATCH rcu/urgent 0/6] Fixes for RCU/scheduler/irq-threads trainwreck Paul E. McKenney
2011-07-20 21:05 ` Peter Zijlstra
2011-07-20 21:39 ` Paul E. McKenney
2011-07-20 10:49 ` Ed Tomlinson
2011-07-20 18:25 ` [PATCH rcu/urgent 0/7 v2] " Paul E. McKenney
2011-07-20 18:26 ` [PATCH tip/core/urgent 1/7] rcu: decrease rcu_report_exp_rnp coupling with scheduler Paul E. McKenney
2011-07-20 18:26 ` [PATCH tip/core/urgent 2/7] rcu: Fix RCU_BOOST race handling current->rcu_read_unlock_special Paul E. McKenney
2011-07-20 18:26 ` [PATCH tip/core/urgent 3/7] rcu: Streamline code produced by __rcu_read_unlock() Paul E. McKenney
2011-07-20 22:44 ` Linus Torvalds
2011-07-21 5:09 ` Paul E. McKenney
2011-07-20 18:26 ` [PATCH tip/core/urgent 4/7] rcu: protect __rcu_read_unlock() against scheduler-using irq handlers Paul E. McKenney
2011-07-20 18:26 ` [PATCH tip/core/urgent 5/7] sched: Add irq_{enter,exit}() to scheduler_ipi() Paul E. McKenney
2011-07-20 18:26 ` [PATCH tip/core/urgent 6/7] softirq,rcu: Inform RCU of irq_exit() activity Paul E. McKenney
2011-07-20 18:26 ` [PATCH tip/core/urgent 7/7] signal: align __lock_task_sighand() irq disabling and RCU Paul E. McKenney
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=201107200723.33819.edt@aei.ca \
--to=edt@aei.ca \
--cc=Valdis.Kletnieks@vt.edu \
--cc=akpm@linux-foundation.org \
--cc=darren@dvhart.com \
--cc=dhowells@redhat.com \
--cc=dipankar@in.ibm.com \
--cc=eric.dumazet@gmail.com \
--cc=greearb@candelatech.com \
--cc=josh@joshtriplett.org \
--cc=laijs@cn.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@polymtl.ca \
--cc=mingo@elte.hu \
--cc=niv@us.ibm.com \
--cc=patches@linaro.org \
--cc=paulmck@linux.vnet.ibm.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 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.