From: Frederic Weisbecker <frederic@kernel.org>
To: Valentin Schneider <vschneid@redhat.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
"Paul E . McKenney" <paulmck@kernel.org>,
Boqun Feng <boqun.feng@gmail.com>,
Joel Fernandes <joel@joelfernandes.org>,
Neeraj Upadhyay <neeraj.upadhyay@amd.com>,
Uladzislau Rezki <urezki@gmail.com>,
Zqiang <qiang.zhang1211@gmail.com>, rcu <rcu@vger.kernel.org>
Subject: Re: [PATCH 2/6] rcu: Remove superfluous full memory barrier upon first EQS snapshot
Date: Thu, 16 May 2024 18:08:26 +0200 [thread overview]
Message-ID: <ZkYvemdrEOVFNtVu@lothringen> (raw)
In-Reply-To: <xhsmhfruhhixv.mognet@vschneid-thinkpadt14sgen2i.remote.csb>
On Thu, May 16, 2024 at 05:31:40PM +0200, Valentin Schneider wrote:
> On 15/05/24 14:53, Frederic Weisbecker wrote:
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 58415cdc54f8..f5354de5644b 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -773,7 +773,12 @@ static void rcu_gpnum_ovf(struct rcu_node *rnp, struct rcu_data *rdp)
> > */
> > static int dyntick_save_progress_counter(struct rcu_data *rdp)
> > {
> > - rdp->dynticks_snap = rcu_dynticks_snap(rdp->cpu);
>
> So for PPC, which gets the smp_mb() at the lock acquisition, this is an
> "obvious" redundant smp_mb().
>
> For the other archs, per the definition of smp_mb__after_unlock_lock() it
> seems implied that UNLOCK+LOCK is a full memory barrier, but I wanted to
> see it explicitly stated somewhere. From a bit of spelunking below I still
> think it's the case, but is there a "better" source of truth?
>
> 01352fb81658 ("locking: Add an smp_mb__after_unlock_lock() for UNLOCK+BLOCK barrier")
> """
> The Linux kernel has traditionally required that an UNLOCK+LOCK pair act as a
> full memory barrier when either (1) that UNLOCK+LOCK pair was executed by the
> same CPU or task, or (2) the same lock variable was used for the UNLOCK and
> LOCK.
> """
>
> and
>
> https://lore.kernel.org/all/1436789704-10086-1-git-send-email-will.deacon@arm.com/
> """
> This ordering guarantee is already provided without the barrier on
> all architectures apart from PowerPC
> """
You seem to have found the accurate informations! But I must admit
they are hard to find and it would be welcome to document that properly, for example
in Documentation/memory-barriers.txt
I think the reason is that it's not supposed to be used outside RCU, perhaps
because its semantics are too fragile to use for general purpose? Even that
could be stated along in Documentation/memory-barriers.txt
Another thing is that its semantics are similar to smp_mb__after_spinlock()
(itself badly documented), although slightly different. I'm not even completely
sure how. I assume that smp_mb__after_spinlock() can be just used once to
produce the required ordering and subsequent lock on that spinlock don't need
to repeat the barrier to propagate the ordering against what is before the
smp_mb__after_spinlock. However IUUC smp_mb__after_unlock_lock() has to be
chained/repeated on all subsequent locking of the spinlock...
Thanks.
>
> > + /*
> > + * Full ordering against accesses prior current GP and also against
> ^^^^^
> prior to
>
> > + * current GP sequence number is enforced by current rnp locking
> > + * with chained smp_mb__after_unlock_lock().
> > + */
> > + rdp->dynticks_snap = ct_dynticks_cpu_acquire(rdp->cpu);
> > if (rcu_dynticks_in_eqs(rdp->dynticks_snap)) {
> > trace_rcu_fqs(rcu_state.name, rdp->gp_seq, rdp->cpu, TPS("dti"));
> > rcu_gpnum_ovf(rdp->mynode, rdp);
> > --
> > 2.44.0
>
next prev parent reply other threads:[~2024-05-16 16:08 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-15 12:53 [PATCH 0/6] rcu: Remove several redundant memory barriers Frederic Weisbecker
2024-05-15 12:53 ` [PATCH 1/6] rcu: Remove full ordering on second EQS snapshot Frederic Weisbecker
2024-05-15 17:32 ` Valentin Schneider
2024-05-15 12:53 ` [PATCH 2/6] rcu: Remove superfluous full memory barrier upon first " Frederic Weisbecker
2024-05-16 15:31 ` Valentin Schneider
2024-05-16 16:08 ` Frederic Weisbecker [this message]
2024-05-16 17:08 ` Valentin Schneider
2024-05-17 7:29 ` Andrea Parri
2024-05-17 11:40 ` Frederic Weisbecker
2024-05-17 16:27 ` Andrea Parri
2024-05-15 12:53 ` [PATCH 3/6] rcu/exp: " Frederic Weisbecker
2024-05-15 12:53 ` [PATCH 4/6] rcu: Remove full memory barrier on boot time eqs sanity check Frederic Weisbecker
2024-05-16 17:09 ` Valentin Schneider
2024-05-15 12:53 ` [PATCH 5/6] rcu: Remove full memory barrier on RCU stall printout Frederic Weisbecker
2024-05-16 17:09 ` Valentin Schneider
2024-05-15 12:53 ` [PATCH 6/6] rcu/exp: Remove redundant full memory barrier at the end of GP Frederic Weisbecker
2024-05-15 17:32 ` [PATCH 0/6] rcu: Remove several redundant memory barriers Valentin Schneider
2024-05-15 23:13 ` Frederic Weisbecker
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=ZkYvemdrEOVFNtVu@lothringen \
--to=frederic@kernel.org \
--cc=boqun.feng@gmail.com \
--cc=joel@joelfernandes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=neeraj.upadhyay@amd.com \
--cc=paulmck@kernel.org \
--cc=qiang.zhang1211@gmail.com \
--cc=rcu@vger.kernel.org \
--cc=urezki@gmail.com \
--cc=vschneid@redhat.com \
/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).