From: Boqun Feng <boqun.feng@gmail.com>
To: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Colin Ian King <colin.king@canonical.com>,
Mark Rutland <mark.rutland@arm.com>,
linux-kernel@vger.kernel.org,
Josh Triplett <josh@joshtriplett.org>,
Steven Rostedt <rostedt@goodmis.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Lai Jiangshan <jiangshanlai@gmail.com>
Subject: Re: [RFC v2 4/5] rcu: Use for_each_leaf_node_cpu() in force_qs_rnp()
Date: Wed, 21 Dec 2016 12:18:08 +0800 [thread overview]
Message-ID: <20161221041808.GF1316@tardis.cn.ibm.com> (raw)
In-Reply-To: <20161221034024.GC3924@linux.vnet.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 3832 bytes --]
On Tue, Dec 20, 2016 at 07:40:24PM -0800, Paul E. McKenney wrote:
[...]
> >
> > Agreed, my intent is to keep this overcare check for couples of releases
> > and if no one shoots his/her foot, we can remove it, if not, it
> > definitely means this part is subtle, and we need to pay more attention
> > to it, maybe write some regression tests for this particular problem to
> > help developers avoid it.
> >
> > This check is supposed to be removed, so I'm not stick to keeping it.
>
> I suggest keeping through validation. If it triggers during that time,
> consider keeping it longer. If it does not trigger, remove it before
> it goes upstream.
>
Good point ;-)
[...]
> > > >
> > > > But this brings a side question, is the callsite of rcu_cpu_starting()
> > > > is correct? Given rcu_cpu_starting() ignores the @cpu parameter and only
> > > > set _this_ cpu's bit in a leaf node?
> > >
> > > The calls from notify_cpu_starting() are called from the various
> > > start_kernel_secondary(), secondary_start_kernel(), and similarly
> > > named functions. These are called on the incoming CPU early in that
> > > CPU's execution. The call from rcu_init() is correct until such time
> > > as more than one CPU can be running at rcu_init() time. And that
> > > day might be coming, so please see the untested patch below.
> >
> > Looks better than mine ;-)
> >
> > But do we need to worry that we start rcu on each CPU twice, which may
> > slow down the boot?
>
> We only start a given CPU once. The boot CPU at rcu_init() time, and
> the rest at CPU-hotplug time. Unless of course a CPU is later taken
Confused... we call rcu_cpu_starting() in a for_each_online_cpu() loop
in rcu_init(), so we basically start all online CPUs there after
applying your patch. And all the rest CPUs will get themselves start
again at CPU-hotplug time, right?
Besides, without your patch, we started the boot CPU many times in the
for_each_online_cpu() loop.
Am I missing something subtle?
Regards,
Boqun
> offline, in which case we start it again when it comes back online.
>
> Thanx, Paul
>
> > Regards,
> > Boqun
> >
> > > Thanx, Paul
> > >
> > > ------------------------------------------------------------------------
> > >
> > > commit 1e84402587173d6d4da8645689f0e24c877b3269
> > > Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > Date: Tue Dec 20 07:17:58 2016 -0800
> > >
> > > rcu: Make rcu_cpu_starting() use its "cpu" argument
> > >
> > > The rcu_cpu_starting() function uses this_cpu_ptr() to locate the
> > > incoming CPU's rcu_data structure. This works for the boot CPU and for
> > > all CPUs onlined after rcu_init() executes (during very early boot).
> > > Currently, this is the full set of CPUs, so all is well. But if
> > > anyone ever parallelizes boot before rcu_init() time, it will fail.
> > > This commit therefore substitutes the rcu_cpu_starting() function's
> > > this_cpu_pointer() for per_cpu_ptr(), future-proofing the code and
> > > (arguably) improving readability.
> > >
> > > Reported-by: Boqun Feng <boqun.feng@gmail.com>
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > >
> > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > index b9d3c0e30935..083cb8a6299c 100644
> > > --- a/kernel/rcu/tree.c
> > > +++ b/kernel/rcu/tree.c
> > > @@ -4017,7 +4017,7 @@ void rcu_cpu_starting(unsigned int cpu)
> > > struct rcu_state *rsp;
> > >
> > > for_each_rcu_flavor(rsp) {
> > > - rdp = this_cpu_ptr(rsp->rda);
> > > + rdp = per_cpu_ptr(rsp->rda, cpu);
> > > rnp = rdp->mynode;
> > > mask = rdp->grpmask;
> > > raw_spin_lock_irqsave_rcu_node(rnp, flags);
> > >
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2016-12-21 4:18 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-15 2:41 [RFC v2 0/5] rcu: Introduce for_each_leaf_node_cpu() Boqun Feng
2016-12-15 2:42 ` [RFC v2 1/5] " Boqun Feng
2016-12-15 11:43 ` Mark Rutland
2016-12-15 14:38 ` Boqun Feng
2016-12-15 15:10 ` Mark Rutland
2016-12-15 15:14 ` Boqun Feng
2016-12-15 15:21 ` [RFC v2.1 " Boqun Feng
2016-12-15 15:29 ` Mark Rutland
2016-12-15 2:42 ` [RFC v2 2/5] rcu: Use for_each_leaf_node_cpu() in RCU stall checking Boqun Feng
2016-12-15 2:42 ` [RFC v2 3/5] rcu: Use for_each_leaf_node_cpu() in ->expmask iteration Boqun Feng
2016-12-15 2:42 ` [RFC v2 4/5] rcu: Use for_each_leaf_node_cpu() in force_qs_rnp() Boqun Feng
2016-12-15 12:04 ` Mark Rutland
2016-12-15 14:42 ` Boqun Feng
2016-12-15 14:51 ` Colin Ian King
2016-12-19 15:15 ` Boqun Feng
2016-12-20 5:09 ` Paul E. McKenney
2016-12-20 5:59 ` Boqun Feng
2016-12-20 8:11 ` Boqun Feng
2016-12-20 15:32 ` Paul E. McKenney
2016-12-20 15:23 ` Paul E. McKenney
2016-12-21 2:34 ` Boqun Feng
2016-12-21 3:40 ` Paul E. McKenney
2016-12-21 4:18 ` Boqun Feng [this message]
2016-12-21 16:48 ` Paul E. McKenney
2016-12-22 1:08 ` Boqun Feng
2016-12-15 2:42 ` [RFC v2 5/5] rcu: Use for_each_leaf_node_cpu() in online CPU iteration Boqun Feng
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=20161221041808.GF1316@tardis.cn.ibm.com \
--to=boqun.feng@gmail.com \
--cc=colin.king@canonical.com \
--cc=jiangshanlai@gmail.com \
--cc=josh@joshtriplett.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mathieu.desnoyers@efficios.com \
--cc=paulmck@linux.vnet.ibm.com \
--cc=rostedt@goodmis.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;
as well as URLs for NNTP newsgroup(s).