Re: [PATCH] rcu: small rcu_dereference doc update

[Paul E. McKenney]

On Fri, Apr 17, 2015 at 04:53:15PM +0000, Jeff Haran wrote:
> > -----Original Message-----
> > From: Paul E. McKenney [mailto:paulmck@xxxxxxxxxxxxxxxxxx]
> > 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@xxxxxxxxxx>
> > > Signed-off-by: Milos Vyletel <milos@xxxxxxxxxx>
> > 
> > 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

> Thanks,
> 
> Jeff Haran
> 
> > > ---
> > >  Documentation/RCU/whatisRCU.txt | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/RCU/whatisRCU.txt
> > > b/Documentation/RCU/whatisRCU.txt index 88dfce1..82b1b2c 100644
> > > --- a/Documentation/RCU/whatisRCU.txt
> > > +++ b/Documentation/RCU/whatisRCU.txt
> > > @@ -256,7 +256,9 @@ rcu_dereference()
> > >  	If you are going to be fetching multiple fields from the
> > >  	RCU-protected structure, using the local variable is of
> > >  	course preferred.  Repeated rcu_dereference() calls look
> > > -	ugly and incur unnecessary overhead on Alpha CPUs.
> > > +	ugly, do not guarantee that same pointer will be returned
> > > +	if update happened while in critical section and incur
> > > +	unnecessary overhead on Alpha CPUs.
> > >
> > >  	Note that the value returned by rcu_dereference() is valid
> > >  	only within the enclosing RCU read-side critical section.
> > > --
> > > 2.1.0
> > >
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/