Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 31 Oct 2018 11:17:48 -0700
From:   "Paul E. McKenney" <paulmck@linux.ibm.com>
To:     Joel Fernandes <joel@joelfernandes.org>
Cc:     linux-kernel@vger.kernel.org
Subject: Re: [RFC] doc: rcu: remove note on smp_mb during synchronize_rcu
Reply-To: paulmck@linux.ibm.com
References: <20181028043046.198403-1-joel@joelfernandes.org>
 <20181030222649.GA105735@joelaf.mtv.corp.google.com>
 <20181030234336.GW4170@linux.ibm.com>
 <20181031011119.GF224709@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20181031011119.GF224709@google.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Message-Id: <20181031181748.GG4170@linux.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Oct 30, 2018 at 06:11:19PM -0700, Joel Fernandes wrote:
> Hi Paul,
> 
> On Tue, Oct 30, 2018 at 04:43:36PM -0700, Paul E. McKenney wrote:
> > On Tue, Oct 30, 2018 at 03:26:49PM -0700, Joel Fernandes wrote:
> > > Hi Paul,
> > > 
> > > On Sat, Oct 27, 2018 at 09:30:46PM -0700, Joel Fernandes (Google) wrote:
> > > > As per this thread [1], it seems this smp_mb isn't needed anymore:
> > > > "So the smp_mb() that I was trying to add doesn't need to be there."
> > > > 
> > > > So let us remove this part from the memory ordering documentation.
> > > > 
> > > > [1] https://lkml.org/lkml/2017/10/6/707
> > > > 
> > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > > 
> > > I was just checking about this patch. Do you feel it is correct to remove
> > > this part from the docs? Are you satisified that a barrier isn't needed there
> > > now? Or did I miss something?
> > 
> > Apologies, it got lost in the shuffle.  I have now applied it with a
> > bit of rework to the commit log, thank you!
> 
> No worries, thanks for taking it!
> 
> Just wanted to update you on my progress reading/correcting the docs. The
> 'Memory Ordering' is taking a bit of time so I paused that and I'm focusing
> on finishing all the other low hanging fruit. This activity is mostly during
> night hours after the baby is asleep but some times I also manage to sneak it
> into the day job ;-)

If there is anything I can do to make this a more sustainable task for
you, please do not keep it a secret!!!

> BTW I do want to discuss about this smp_mb patch above with you at LPC if you
> had time, even though we are removing it from the documentation. I thought
> about it a few times, and I was not able to fully appreciate the need for the
> barrier (that is even assuming that complete() etc did not do the right
> thing).  Specifically I was wondering same thing Peter said in the above
> thread I think that - if that rcu_read_unlock() triggered all the spin
> locking up the tree of nodes, then why is that locking not sufficient to
> prevent reads from the read-side section from bleeding out? That would
> prevent the reader that just unlocked from seeing anything that happens
> _after_ the synchronize_rcu.

Actually, I recall an smp_mb() being added, but am not seeing it anywhere
relevant to wait_for_completion().  So I might need to add the smp_mb()
to synchronize_rcu() and remove the patch (retaining the typo fix).  :-/

The short form answer is that anything before a grace period on any CPU
must be seen by any CPU as being before anything on any CPU after that
same grace period.  This guarantee requires a rather big hammer.

But yes, let's talk at LPC!

> Also about GP memory ordering and RCU-tree-locking, I think you mentioned to
> me that the RCU reader-sections are virtually extended both forward and
> backward and whereever it ends, those paths do heavy-weight synchronization
> that should be sufficient to prevent memory ordering issues (such as those
> you mentioned in the Requierments document). That is exactly why we don't
> need explicit barriers during rcu_read_unlock. If I recall I asked you why
> those are not needed. So that answer made sense, but then now on going
> through the 'Memory Ordering' document, I see that you mentioned there is
> reliance on the locking. Is that reliance on locking necessary to maintain
> ordering then?

There is a "network" of locking augmented by smp_mb__after_unlock_lock()
that implements the all-to-all memory ordering mentioned above.  But it
also needs to handle all the possible complete()/wait_for_completion()
races, even those assisted by hypervisor vCPU preemption.

> Or did I miss the points completely? :(

A question for the ages for both of us!  ;-)

> ----------------------
> TODO list of the index file marking which ones I have finished perusing:
> 
> arrayRCU.txt			DONE
> checklist.txt			DONE
> listRCU.txt			DONE
> lockdep.txt			DONE
> lockdep-splat.txt		DONE
> NMI-RCU.txt
> rcu_dereference.txt
> rcubarrier.txt
> rculist_nulls.txt
> rcuref.txt
> rcu.txt
> RTFP.txt			DONE
> stallwarn.txt			DONE
> torture.txt
> UP.txt
> whatisRCU.txt			DONE
> 
> Design
>  - Data-Structures		DONE
>  - Requirements			DONE
>  - Expedited-Grace-Periods
>  - Memory Ordering		next

Great progress, and again, thank you!!!

							Thanx, Paul