Re: [PATCH] rcu: small rcu_dereference doc update

Paul E. McKenney
2015-04-21T16:32:25Z

On Fri, Apr 17, 2015 at 11:48:16PM +0000, Jeff Haran wrote:
> > -----Original Message-----
> > From: Paul E. McKenney [mailto:paulmck@linux.vnet.ibm.com]
> > Sent: Friday, April 17, 2015 11:41 AM
> > To: Jeff Haran
> > Cc: Milos Vyletel; Josh Triplett; Steven Rostedt; Mathieu Desnoyers; Lai
> > Jiangshan; Jonathan Corbet; open list:READ-COPY UPDATE...; open
> > list:DOCUMENTATION
> > Subject: Re: [PATCH] rcu: small rcu_dereference doc update
> >
> > On Fri, Apr 17, 2015 at 04:53:15PM +0000, Jeff Haran wrote:
> > > > -----Original Message-----
> > > > From: Paul E. McKenney [mailto:paulmck@linux.vnet.ibm.com]
> > > > Sent: Friday, April 17, 2015 7:07 AM
> > > > To: Milos Vyletel
> > > > Cc: Josh Triplett; Steven Rostedt; Mathieu Desnoyers; Lai Jiangshan;
> > > > Jonathan Corbet; open list:READ-COPY UPDATE...; open
> > > > list:DOCUMENTATION; Jeff Haran
> > > > Subject: Re: [PATCH] rcu: small rcu_dereference doc update
> > > >
> > > > On Fri, Apr 17, 2015 at 12:33:36PM +0200, Milos Vyletel wrote:
> > > > > Make a note stating that repeated calls of rcu_dereference() may
> > > > > not return the same pointer if update happens while in critical section.
> > > > >
> > > > > Reported-by: Jeff Haran <jeff.haran@citrix.com>
> > > > > Signed-off-by: Milos Vyletel <milos@redhat.com>
> > > >
> > > > Hmmm... Seems like that should be obvious, but on the other hand, I
> > > > have been using RCU for more than twenty years, so my obviousness
> > > > sensors might need recalibration.
> > > >
> > > > Queued for 4.2.
> > > >
> > > > Thanx, Paul
> > >
> > > It's just that the original text suggests repeated rcu_dereference() calls are
> > discouraged because they are ugly and not efficient on some architectures.
> > When I read that I concluded that those were the only reasons not to do it,
> > that despite the possible inefficiency it would always return the same
> > pointer. Depending on how one's code is structured, being able to do this
> > could be advantageous. Then I started looking at the code that implements it
> > and I couldn't see how it could possibly be the case. I even wrote a little
> > kernel module to prove to myself that doing this could return different
> > pointer values. If I misinterpreted the original text I figured others might also.
> > Milos even found some code in the kernel where it's author had done this,
> > so it might be a widely held misunderstanding. It's easy for people who have
> > worked with rwlock_ts to think an RCU read lock works the same.
> >
> > Fair point, and thank you the rationale! Are there any other parts of the RCU
> > documentation that are similarly blind to your initial point of view? If so, it
> > would be good for them to be fixed.
> >
> > Thanx, Paul
>
> I can't think of much off the top of my head, but I'm hoping I might get some time to review it again and perhaps provide some more concrete suggestions.
>
> One thing that does come to mind is the article you wrote in LWN, http://lwn.net/Articles/263130/, where you discussed RCU as a reader-write lock replacement. whatisRCU.txt seems to incorporated some of that. Something along the lines of the original section in the LWN article where there was some discussion of the differences between a rwlock_t read lock critical section and a RCU read lock critical section might be beneficial, a key thing being that with RCU there really is no locking, the value of the pointer can change in a RCU critical section because writers aren't blocked from updating it. Another thing might be some discussion that the cases where you'd call read_lock_bh() are way different than when you'd call rcu_read_lock_bh() and as a corollary why there is no rcu_read_lock_irq().
>
> To me it seems that the names of some of these functions are perhaps misleading. rcu_read_lock() sort of implies there is some locking going on when there isn't. It might have been easier to understand if rcu_read_lock() was called rcu_get() and rcu_read_unlock() was called rcu_put() to reflect that they are really as much about memory management as synchronization. Too late the change any of that obviously.

Indeed, proposing names for things that have not yet been used much is
always a bit hazardous. But we nevertheless did have to pick a name for
it back in the day. That said, RCU is used in different ways, so it is
not clear that any given change the the current set of names would be
an overall improvement.

Thanx, Paul