Re: [PATCH RFC 0/5] rcu doc updates for whatisRCU and checklist

From: "Paul E. McKenney" <paulmck@linux.ibm.com>
To: "Theodore Y. Ts'o" <tytso@mit.edu>,
	"Joel Fernandes (Google)" <joel@joelfernandes.org>,
	linux-kernel@vger.kernel.org, Jonathan Corbet <corbet@lwn.net>,
	Josh Triplett <josh@joshtriplett.org>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	linux-doc@vger.kernel.org,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	pantin@google.com
Subject: Re: [PATCH RFC 0/5] rcu doc updates for whatisRCU and checklist
Date: Sat, 6 Oct 2018 09:49:33 -0700	[thread overview]
Message-ID: <20181006164933.GP2674@linux.ibm.com> (raw)
In-Reply-To: <20181006053446.GA2529@thunk.org>

On Sat, Oct 06, 2018 at 01:34:46AM -0400, Theodore Y. Ts'o wrote:
> On Fri, Oct 05, 2018 at 08:45:40PM -0700, Paul E. McKenney wrote:
> > 
> > Shouldn't the synchronize_rcu() precede the loop doing the kfree()
> > calls?  Or am I missing something subtle?
> 
> No, that was a cut and paste error on my part.  I was removing the
> rcu_read_unlock() before the kfree loop, and accidentally removed the
> synchronize_rcu().  Then when I put it back, I put it back in the
> right place.

Been there, done that!  ;-)

> The longer version:
> 
> I originally used rcu_read_lock() and rcu_read_unlock() around setting
> up to_free[] --- since whatisRCU.txt didn't talk about
> rcu_derefence_proctected(), just rcu_dereference() in Section 2: "What
> is RCU's Core API?"   
> 
> Then when I looked at the example in Section 3, I was surprised when I
> didn't see the rcu_read_[un]lock() on the updater side, and spent some
> time trying to figure out how to use rcu_dereference_protected().
> 
> Then when I did the transumation from
> rcu_read_lock/rcu_dereference_protected/rcu_read_unlock to
> rcu_dereference_protected, I bobbled the location of
> synchronize_rcu().
> 
> 						- Ted
> 
> P.S.  Pedagogically, it might make sense to show an example that only
> uses the RCU core API --- I assume using rcu_read_[un]lock() and
> rcu_dereference() does work; it's just non-optimal, right?  --- and
> then introduce the use of rcu_dereference_protected() afterwards.

Yes, you can use rcu_dereference() on the update side and dispense
with rcu_dereference_protected(), but that will require you to add an
otherwise useless rcu_read_lock()/rcu_read_unlock() pair when accessing
the pointer on the update side.

Furthermore, if you are OK leaking memory rather than freeing it (which
is admittedly quite rare, but does sometimes happen), then yes, you
don't need call_rcu(), synchronize_rcu(), and friends.

So it is as you say, functional but non-optimal.

							Thanx, Paul