Message-ID: <79233008-4be2-4442-9600-f9ac1a654312@paulmck-laptop>
Date:   Fri, 20 Oct 2023 11:13:29 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Jonas Oberhauser <jonas.oberhauser@...weicloud.com>
Cc:     linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
        linux-doc@...r.kernel.org, Alan Stern <stern@...land.harvard.edu>,
        Andrea Parri <parri.andrea@...il.com>,
        Will Deacon <will@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Boqun Feng <boqun.feng@...il.com>,
        Nicholas Piggin <npiggin@...il.com>,
        David Howells <dhowells@...hat.com>,
        Jade Alglave <j.alglave@....ac.uk>,
        Luc Maranget <luc.maranget@...ia.fr>,
        Akira Yokosawa <akiyks@...il.com>,
        Daniel Lustig <dlustig@...dia.com>,
        Joel Fernandes <joel@...lfernandes.org>,
        Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH memory-model] docs: memory-barriers: Add note on compiler
 transformation and address deps

On Fri, Oct 20, 2023 at 06:00:19PM +0200, Jonas Oberhauser wrote:
> 
> Am 10/20/2023 um 3:57 PM schrieb Paul E. McKenney:
> > On Fri, Oct 20, 2023 at 11:29:24AM +0200, Jonas Oberhauser wrote:
> > > Am 10/19/2023 um 6:39 PM schrieb Paul E. McKenney:
> > > > On Wed, Oct 18, 2023 at 12:11:58PM +0200, Jonas Oberhauser wrote:
> > > > > Hi Paul,
> > > > > [...]
> > > > The compiler is forbidden from inventing pointer comparisons.
> > > TIL :) Btw, do you remember a discussion where this is clarified? A quick
> > > search didn't turn up anything.
> > This was a verbal discussion with Richard Smith at the 2020 C++ Standards
> > Committee meeting in Prague.  I honestly do not know what standardese
> > supports this.
> 
> Then this e-mail thread shall be my evidence for future discussion.

I am sure that Richard will be delighted, especially given that he
did not seem at all happy with this don't-invent-pointer-comparisons
rule.  ;-)

> > > > > Best wishes,
> > > > > 
> > > > > jonas
> > > > > 
> > > > > Am 10/6/2023 um 6:39 PM schrieb Jonas Oberhauser:
> > > > > > Hi Paul,
> > > > > > 
> > > > > > The "more up-to-date information" makes it sound like (some of) the
> > > > > > information in this section is out-of-date/no longer valid.
> > > > The old smp_read_barrier_depends() that these section cover really
> > > > does no longer exist.
> > > 
> > > (and the parts that are still there are all still relevant, while the parts
> > > that only the authors know was intended to be there and is out-of-date is
> > > already gone).
> > The question is instead what parts that are still relevant are missing
> > from rcu_dereference.rst.
> > 
> > > So I would add a disclaimer specifying that (since 4.15) *all* marked
> > > accesses imply read dependency barriers which resolve most of the issues
> > > mentioned in the remainder of the article.
> > > However, some issues remain because the dependencies that are preserved by
> > > such barriers are just *semantic* dependencies, and readers should check
> > > rcu_dereference.rst for examples of what that implies.
> > Or maybe it is now time to remove those sections from memory-barriers.txt,
> > leaving only the first section's pointer to rcu_dereference.rst.
> 
> That would also make sense to me.
> 
> > It still feels a bit early to me, and I am still trying to figure out
> > why you care so much about these sections.  ;-)
> 
> I honestly don't care about the sections themselves, but I do care about 1)
> address dependency ordering and 2) not confusing people more than necessary.
> IMHO the sections right now are more confusing than necessary.
> As I said before, I think they should clarify what exactly is historical in
> a short sentence. E.g.
> 
>  (2) Address-dependency barriers (historical).
>      [!] This section is marked as HISTORICAL: it covers the obsolete barrier
>      smp_read_barrier_depends(), the semantics of which is now implicit in all
>      marked accesses. For more up-to-date information, including how compiler
>      transformations related to pointer comparisons can sometimes cause problems,
>      see Documentation/RCU/rcu_dereference.rst.
> 
> I think this tiny rewrite makes it much more clear. Specifically it tells *why* the text is historical (and why we maybe don't need to read it anymore).

Good point!  I reworked this a bit and added it to both HISTORICAL
sections, with your Suggested-by.

> Btw, when I raised my concerns about what should be there I didn't mean to imply those points are missing, just trying to sketch what the paragraph should look like in my opinion.
> The paragraphs you are adding already had several of those points.

Very good, but I did have to ask.  It wouldn't be the first time that
I left something out.  ;-)

> > > > The longer-term direction, perhaps a few years from now, is for the
> > > > first section to simply reference rcu_dereference.rst and for the second
> > > > section to be removed completely.
> > > Sounds good to me, but that doesn't mean we need to compromise the
> > > readability in the interim :)
> > Some compromise is needed for people that read the document some time
> > back and are looking for something specific.
> 
> Yes. But the compromise should be "there's a blob of text other people don't
> need to read", not "there's a blob of text that will leave other people
> confused".

Fair enough in general, but I cannot promise to never confuse people.
This is after all memory ordering.  And different people will be confused
by different things.

But I do very much like your suggested clarification.  Please let me
know if I messed anything up in the translation.

							Thanx, Paul

------------------------------------------------------------------------

commit 566c71eee55b26ece5855ebbee6f8762495d78f7
Author: Paul E. McKenney <paulmck@...nel.org>
Date:   Fri Oct 20 11:04:27 2023 -0700

    doc: Clarify historical disclaimers in memory-barriers.txt
    
    This commit makes it clear that the reason that these sections are
    historical is that smp_read_barrier_depends() is no more.  It also
    removes the point about comparison operations, given that there are
    other optimizations that can break address dependencies.
    
    Suggested-by: Jonas Oberhauser <jonas.oberhauser@...weicloud.com>
    Signed-off-by: Paul E. McKenney <paulmck@...nel.org>
    Cc: Alan Stern <stern@...land.harvard.edu>
    Cc: Andrea Parri <parri.andrea@...il.com>
    Cc: Will Deacon <will@...nel.org>
    Cc: Peter Zijlstra <peterz@...radead.org>
    Cc: Boqun Feng <boqun.feng@...il.com>
    Cc: Nicholas Piggin <npiggin@...il.com>
    Cc: David Howells <dhowells@...hat.com>
    Cc: Jade Alglave <j.alglave@....ac.uk>
    Cc: Luc Maranget <luc.maranget@...ia.fr>
    Cc: Akira Yokosawa <akiyks@...il.com>
    Cc: Daniel Lustig <dlustig@...dia.com>
    Cc: Joel Fernandes <joel@...lfernandes.org>
    Cc: Jonathan Corbet <corbet@....net>
    Cc: <linux-arch@...r.kernel.org>
    Cc: <linux-doc@...r.kernel.org>

diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index d414e145f912..4202174a6262 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -396,10 +396,11 @@ Memory barriers come in four basic varieties:
 
 
  (2) Address-dependency barriers (historical).
-     [!] This section is marked as HISTORICAL: For more up-to-date
-     information, including how compiler transformations related to pointer
-     comparisons can sometimes cause problems, see
-     Documentation/RCU/rcu_dereference.rst.
+     [!] This section is marked as HISTORICAL: it covers the long-obsolete
+     smp_read_barrier_depends() macro, the semantics of which are now
+     implicit in all marked accesses.  For more up-to-date information,
+     including how compiler transformations can sometimes break address
+     dependencies, see Documentation/RCU/rcu_dereference.rst.
 
      An address-dependency barrier is a weaker form of read barrier.  In the
      case where two loads are performed such that the second depends on the
@@ -560,9 +561,11 @@ There are certain things that the Linux kernel memory barriers do not guarantee:
 
 ADDRESS-DEPENDENCY BARRIERS (HISTORICAL)
 ----------------------------------------
-[!] This section is marked as HISTORICAL: For more up-to-date information,
-including how compiler transformations related to pointer comparisons can
-sometimes cause problems, see Documentation/RCU/rcu_dereference.rst.
+[!] This section is marked as HISTORICAL: it covers the long-obsolete
+smp_read_barrier_depends() macro, the semantics of which are now implicit
+in all marked accesses.  For more up-to-date information, including
+how compiler transformations can sometimes break address dependencies,
+see Documentation/RCU/rcu_dereference.rst.
 
 As of v4.15 of the Linux kernel, an smp_mb() was added to READ_ONCE() for
 DEC Alpha, which means that about the only people who need to pay attention

Powered by blists - more mailing lists