From: "Paul E. McKenney" <paulmck@kernel.org>
To: Boqun Feng <boqun.feng@gmail.com>
Cc: rcu@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-team@fb.com, mingo@kernel.org, jiangshanlai@gmail.com,
dipankar@in.ibm.com, akpm@linux-foundation.org,
mathieu.desnoyers@efficios.com, josh@joshtriplett.org,
tglx@linutronix.de, peterz@infradead.org, rostedt@goodmis.org,
dhowells@redhat.com, edumazet@google.com, fweisbec@gmail.com,
oleg@redhat.com, joel@joelfernandes.org,
"# 5 . 5 . x" <stable@vger.kernel.org>
Subject: Re: [PATCH tip/core/rcu 30/30] rcu: Make rcu_barrier() account for offline no-CBs CPUs
Date: Tue, 25 Feb 2020 19:14:55 -0800 [thread overview]
Message-ID: <20200226031455.GZ2935@paulmck-ThinkPad-P72> (raw)
In-Reply-To: <20200225102436.GF110915@debian-boqun.qqnc3lrjykvubdpftowmye0fmh.lx.internal.cloudapp.net>
On Tue, Feb 25, 2020 at 06:24:36PM +0800, Boqun Feng wrote:
> Hi Paul,
>
> On Fri, Feb 14, 2020 at 03:56:07PM -0800, paulmck@kernel.org wrote:
> > From: "Paul E. McKenney" <paulmck@kernel.org>
> >
> > Currently, rcu_barrier() ignores offline CPUs, However, it is possible
> > for an offline no-CBs CPU to have callbacks queued, and rcu_barrier()
> > must wait for those callbacks. This commit therefore makes rcu_barrier()
> > directly invoke the rcu_barrier_func() with interrupts disabled for such
> > CPUs. This requires passing the CPU number into this function so that
> > it can entrain the rcu_barrier() callback onto the correct CPU's callback
> > list, given that the code must instead execute on the current CPU.
> >
> > While in the area, this commit fixes a bug where the first CPU's callback
> > might have been invoked before rcu_segcblist_entrain() returned, which
> > would also result in an early wakeup.
> >
> > Fixes: 5d6742b37727 ("rcu/nocb: Use rcu_segcblist for no-CBs CPUs")
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > Cc: <stable@vger.kernel.org> # 5.5.x
> > ---
> > include/trace/events/rcu.h | 1 +
> > kernel/rcu/tree.c | 32 ++++++++++++++++++++------------
> > 2 files changed, 21 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h
> > index 5e49b06..d56d54c 100644
> > --- a/include/trace/events/rcu.h
> > +++ b/include/trace/events/rcu.h
> > @@ -712,6 +712,7 @@ TRACE_EVENT_RCU(rcu_torture_read,
> > * "Begin": rcu_barrier() started.
> > * "EarlyExit": rcu_barrier() piggybacked, thus early exit.
> > * "Inc1": rcu_barrier() piggyback check counter incremented.
> > + * "OfflineNoCBQ": rcu_barrier() found offline no-CBs CPU with callbacks.
> > * "OnlineQ": rcu_barrier() found online CPU with callbacks.
> > * "OnlineNQ": rcu_barrier() found online CPU, no callbacks.
> > * "IRQ": An rcu_barrier_callback() callback posted on remote CPU.
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index d15041f..160643e 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -3098,9 +3098,10 @@ static void rcu_barrier_callback(struct rcu_head *rhp)
> > /*
> > * Called with preemption disabled, and from cross-cpu IRQ context.
> > */
> > -static void rcu_barrier_func(void *unused)
> > +static void rcu_barrier_func(void *cpu_in)
> > {
> > - struct rcu_data *rdp = raw_cpu_ptr(&rcu_data);
> > + uintptr_t cpu = (uintptr_t)cpu_in;
> > + struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
> >
> > rcu_barrier_trace(TPS("IRQ"), -1, rcu_state.barrier_sequence);
> > rdp->barrier_head.func = rcu_barrier_callback;
> > @@ -3127,7 +3128,7 @@ static void rcu_barrier_func(void *unused)
> > */
> > void rcu_barrier(void)
> > {
> > - int cpu;
> > + uintptr_t cpu;
> > struct rcu_data *rdp;
> > unsigned long s = rcu_seq_snap(&rcu_state.barrier_sequence);
> >
> > @@ -3150,13 +3151,14 @@ void rcu_barrier(void)
> > rcu_barrier_trace(TPS("Inc1"), -1, rcu_state.barrier_sequence);
> >
> > /*
> > - * Initialize the count to one rather than to zero in order to
> > - * avoid a too-soon return to zero in case of a short grace period
> > - * (or preemption of this task). Exclude CPU-hotplug operations
> > - * to ensure that no offline CPU has callbacks queued.
> > + * Initialize the count to two rather than to zero in order
> > + * to avoid a too-soon return to zero in case of an immediate
> > + * invocation of the just-enqueued callback (or preemption of
> > + * this task). Exclude CPU-hotplug operations to ensure that no
> > + * offline non-offloaded CPU has callbacks queued.
> > */
> > init_completion(&rcu_state.barrier_completion);
> > - atomic_set(&rcu_state.barrier_cpu_count, 1);
> > + atomic_set(&rcu_state.barrier_cpu_count, 2);
> > get_online_cpus();
> >
> > /*
> > @@ -3166,13 +3168,19 @@ void rcu_barrier(void)
> > */
> > for_each_possible_cpu(cpu) {
> > rdp = per_cpu_ptr(&rcu_data, cpu);
> > - if (!cpu_online(cpu) &&
> > + if (cpu_is_offline(cpu) &&
> > !rcu_segcblist_is_offloaded(&rdp->cblist))
> > continue;
> > - if (rcu_segcblist_n_cbs(&rdp->cblist)) {
> > + if (rcu_segcblist_n_cbs(&rdp->cblist) && cpu_online(cpu)) {
> > rcu_barrier_trace(TPS("OnlineQ"), cpu,
> > rcu_state.barrier_sequence);
> > - smp_call_function_single(cpu, rcu_barrier_func, NULL, 1);
> > + smp_call_function_single(cpu, rcu_barrier_func, (void *)cpu, 1);
> > + } else if (cpu_is_offline(cpu)) {
>
> I wonder whether this should be:
>
> else if (rcu_segcblist_n_cbs(&rdp->cblist) && cpu_is_offline(cpu))
>
> ? Because I think we only want to queue the barrier call back if there
> are callbacks for a particular CPU. Am I missing something subtle?
I don't believe that you are missing anything at all!
Thank you very much -- this bug would not have shown up in any validation
setup that I am aware of. ;-)
Thanx, Paul
> Regards,
> Boqun
>
> > + rcu_barrier_trace(TPS("OfflineNoCBQ"), cpu,
> > + rcu_state.barrier_sequence);
> > + local_irq_disable();
> > + rcu_barrier_func((void *)cpu);
> > + local_irq_enable();
> > } else {
> > rcu_barrier_trace(TPS("OnlineNQ"), cpu,
> > rcu_state.barrier_sequence);
> > @@ -3184,7 +3192,7 @@ void rcu_barrier(void)
> > * Now that we have an rcu_barrier_callback() callback on each
> > * CPU, and thus each counted, remove the initial count.
> > */
> > - if (atomic_dec_and_test(&rcu_state.barrier_cpu_count))
> > + if (atomic_sub_and_test(2, &rcu_state.barrier_cpu_count))
> > complete(&rcu_state.barrier_completion);
> >
> > /* Wait for all rcu_barrier_callback() callbacks to be invoked. */
> > --
> > 2.9.5
> >
next prev parent reply other threads:[~2020-02-26 3:14 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-14 23:55 [PATCH tip/core/rcu 0/30] Miscellaneous fixes for v5.7 Paul E. McKenney
2020-02-14 23:55 ` [PATCH tip/core/rcu 01/30] nfs: Fix nfs_access_get_cached_rcu() sparse error paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 02/30] rcu: Warn on for_each_leaf_node_cpu_mask() from non-leaf paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 03/30] rcu: Fix exp_funnel_lock()/rcu_exp_wait_wake() datarace paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 04/30] rcu: Provide debug symbols and line numbers in KCSAN runs paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 05/30] rcu: Add WRITE_ONCE() to rcu_node ->qsmask update paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 06/30] rcu: Add WRITE_ONCE to rcu_node ->exp_seq_rq store paulmck
2020-02-15 3:47 ` Steven Rostedt
2020-02-15 10:58 ` Paul E. McKenney
2020-02-17 21:11 ` Joel Fernandes
2020-02-17 21:36 ` Paul E. McKenney
2020-02-14 23:55 ` [PATCH tip/core/rcu 07/30] rcu: Add READ_ONCE() to rcu_node ->gp_seq paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 08/30] rcu: Add WRITE_ONCE() to rcu_state ->gp_req_activity paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 09/30] rcu: Add WRITE_ONCE() to rcu_node ->qsmaskinitnext paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 10/30] locking/rtmutex: rcu: Add WRITE_ONCE() to rt_mutex ->owner paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 11/30] rcu: Add READ_ONCE() to rcu_segcblist ->tails[] paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 12/30] rcu: *_ONCE() for grace-period progress indicators paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 13/30] rcu: Fix typos in beginning comments paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 14/30] rcu: Add READ_ONCE() to rcu_data ->gpwrap paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 15/30] rcu: Add *_ONCE() to rcu_data ->rcu_forced_tick paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 16/30] rcu: Add *_ONCE() to rcu_node ->boost_kthread_status paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 17/30] timer: Use hlist_unhashed_lockless() in timer_pending() paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 18/30] rcu: Remove dead code from rcu_segcblist_insert_pend_cbs() paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 19/30] rcu: Add WRITE_ONCE() to rcu_state ->gp_start paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 20/30] rcu: Fix rcu_barrier_callback() race condition paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 21/30] rculist: Add brackets around cond argument in __list_check_rcu macro paulmck
2020-02-14 23:55 ` [PATCH tip/core/rcu 22/30] rcu: Don't flag non-starting GPs before GP kthread is running paulmck
2020-02-15 3:53 ` Steven Rostedt
2020-02-15 11:01 ` Paul E. McKenney
2020-02-15 13:42 ` Paul E. McKenney
2020-02-17 20:25 ` Steven Rostedt
2020-02-17 22:03 ` Paul E. McKenney
2020-02-17 22:21 ` Steven Rostedt
2020-02-17 23:03 ` Paul E. McKenney
2020-02-14 23:56 ` [PATCH tip/core/rcu 23/30] rcu: Add missing annotation for rcu_nocb_bypass_lock() paulmck
2020-02-14 23:56 ` [PATCH tip/core/rcu 24/30] rcu/nocb: Add missing annotation for rcu_nocb_bypass_unlock() paulmck
2020-02-14 23:56 ` [PATCH tip/core/rcu 25/30] rcu: Optimize and protect atomic_cmpxchg() loop paulmck
2020-02-14 23:56 ` [PATCH tip/core/rcu 26/30] rcu: Tighten rcu_lockdep_assert_cblist_protected() check paulmck
2020-02-14 23:56 ` [PATCH tip/core/rcu 27/30] rcu: Make nocb_gp_wait() double-check unexpected-callback warning paulmck
2020-02-14 23:56 ` [PATCH tip/core/rcu 28/30] rcu: Mark rcu_state.ncpus to detect concurrent writes paulmck
2020-02-14 23:56 ` [PATCH tip/core/rcu 29/30] rcu: Mark rcu_state.gp_seq " paulmck
2020-02-14 23:56 ` [PATCH tip/core/rcu 30/30] rcu: Make rcu_barrier() account for offline no-CBs CPUs paulmck
2020-02-25 10:24 ` Boqun Feng
2020-02-26 3:14 ` Paul E. McKenney [this message]
2020-02-26 4:18 ` Paul E. McKenney
2020-02-26 6:14 ` Boqun Feng
2020-02-26 15:02 ` 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=20200226031455.GZ2935@paulmck-ThinkPad-P72 \
--to=paulmck@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=boqun.feng@gmail.com \
--cc=dhowells@redhat.com \
--cc=dipankar@in.ibm.com \
--cc=edumazet@google.com \
--cc=fweisbec@gmail.com \
--cc=jiangshanlai@gmail.com \
--cc=joel@joelfernandes.org \
--cc=josh@joshtriplett.org \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mingo@kernel.org \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=rcu@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=stable@vger.kernel.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 NNTP newsgroup(s).