2015-04-17 10:34:03

by Milos Vyletel

[permalink] [raw]
Subject: [PATCH] rcu: small rcu_dereference doc update

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 <[email protected]>
Signed-off-by: Milos Vyletel <[email protected]>
---
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


2015-04-17 14:06:48

by Paul E. McKenney

[permalink] [raw]
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 <[email protected]>
> Signed-off-by: Milos Vyletel <[email protected]>

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

> ---
> 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
>

2015-04-17 14:14:01

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] rcu: small rcu_dereference doc update

On Fri, 17 Apr 2015 07:06:38 -0700
"Paul E. McKenney" <[email protected]> wrote:

> 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 <[email protected]>
> > Signed-off-by: Milos Vyletel <[email protected]>
>
> 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.

Before you queue it, there's a few articles that are screaming to be
present...

>
> Thanx, Paul
>
> > ---
> > 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.

ugly, do not guarantee that the same pointer will be returned
if an update happened while in the critical section, and incur
unnecessary overhead on Alpha CPUs.

-- Steve

> >
> > Note that the value returned by rcu_dereference() is valid
> > only within the enclosing RCU read-side critical section.
> > --
> > 2.1.0
> >

2015-04-17 14:24:46

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH] rcu: small rcu_dereference doc update

On Fri, Apr 17, 2015 at 10:13:50AM -0400, Steven Rostedt wrote:
> On Fri, 17 Apr 2015 07:06:38 -0700
> "Paul E. McKenney" <[email protected]> wrote:
>
> > 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 <[email protected]>
> > > Signed-off-by: Milos Vyletel <[email protected]>
> >
> > 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.
>
> Before you queue it, there's a few articles that are screaming to be
> present...

OK, I removed it. Milos, when you send me a version that Steve is happy
with, I will requeue it.

Thanx, Paul

> > > ---
> > > 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.
>
> ugly, do not guarantee that the same pointer will be returned
> if an update happened while in the critical section, and incur
> unnecessary overhead on Alpha CPUs.
>
> -- Steve
>
> > >
> > > Note that the value returned by rcu_dereference() is valid
> > > only within the enclosing RCU read-side critical section.
> > > --
> > > 2.1.0
> > >
>

2015-04-17 14:37:15

by Milos Vyletel

[permalink] [raw]
Subject: Re: [PATCH] rcu: small rcu_dereference doc update

On Fri, Apr 17, 2015 at 10:13:50AM -0400, Steven Rostedt wrote:

> On Fri, 17 Apr 2015 07:06:38 -0700
> "Paul E. McKenney" <[email protected]> wrote:
>
> > 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 <[email protected]>
> > > Signed-off-by: Milos Vyletel <[email protected]>
> >
> > 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.
>
> Before you queue it, there's a few articles that are screaming to be
> present...
>
> >
> > Thanx, Paul
> >
> > > ---
> > > 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.
>
> ugly, do not guarantee that the same pointer will be returned
> if an update happened while in the critical section, and incur
> unnecessary overhead on Alpha CPUs.
>
Thanks. Corrected. Sending v2 in a bit.

Milos

> -- Steve
>
> > >
> > > Note that the value returned by rcu_dereference() is valid
> > > only within the enclosing RCU read-side critical section.
> > > --
> > > 2.1.0
> > >
>

2015-04-17 15:55:54

by Pranith Kumar

[permalink] [raw]
Subject: Re: [PATCH] rcu: small rcu_dereference doc update

On Fri, Apr 17, 2015 at 6:33 AM, Milos Vyletel <[email protected]> wrote:
> Make a note stating that repeated calls of rcu_dereference() may not
> return the same pointer if update happens while in critical section.

Might as well make it more explicit with an example then. See below:

>
> Reported-by: Jeff Haran <[email protected]>
> Signed-off-by: Milos Vyletel <[email protected]>
> ---
> 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.
>

An example like follows:

struct some_ds {
int data;
bool ready;
};

struct some_ds *p = ...;

rcu_read_lock();
if (rcu_dereference(p->ready))
data = rcu_dereference(p->data); // bug
rcu_read_unlock();

or some such.

--
Pranith

2015-04-17 16:15:37

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH] rcu: small rcu_dereference doc update

On Fri, Apr 17, 2015 at 11:55:20AM -0400, Pranith Kumar wrote:
> On Fri, Apr 17, 2015 at 6:33 AM, Milos Vyletel <[email protected]> wrote:
> > Make a note stating that repeated calls of rcu_dereference() may not
> > return the same pointer if update happens while in critical section.
>
> Might as well make it more explicit with an example then. See below:
>
> >
> > Reported-by: Jeff Haran <[email protected]>
> > Signed-off-by: Milos Vyletel <[email protected]>
> > ---
> > 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.
> >
>
> An example like follows:
>
> struct some_ds {
> int data;
> bool ready;
> };
>
> struct some_ds *p = ...;
>
> rcu_read_lock();
> if (rcu_dereference(p->ready))
> data = rcu_dereference(p->data); // bug
> rcu_read_unlock();
>
> or some such.

Hello, Pranith!

Sounds like a good thought for a separate patch. Please take a look
through the rest of the documentation -- this might well be the right
place for such an example, but there might well be a better place.
Is this issue mentioned in the checklist? If not, another item might
be good.

Thanx, Paul

2015-04-17 16:53:19

by Jeff Haran

[permalink] [raw]
Subject: RE: [PATCH] rcu: small rcu_dereference doc update

> -----Original Message-----
> From: Paul E. McKenney [mailto:[email protected]]
> 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 <[email protected]>
> > Signed-off-by: Milos Vyletel <[email protected]>
>
> 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.

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
> >

2015-04-17 17:13:52

by Pranith Kumar

[permalink] [raw]
Subject: Re: [PATCH] rcu: small rcu_dereference doc update

On Fri, Apr 17, 2015 at 12:15 PM, Paul E. McKenney
<[email protected]> wrote:
> Sounds like a good thought for a separate patch. Please take a look
> through the rest of the documentation -- this might well be the right
> place for such an example, but there might well be a better place.
> Is this issue mentioned in the checklist? If not, another item might
> be good.
>

Yup, I will take a look and send a patch for this.

--
Pranith

2015-04-17 18:40:40

by Paul E. McKenney

[permalink] [raw]
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:[email protected]]
> > 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 <[email protected]>
> > > Signed-off-by: Milos Vyletel <[email protected]>
> >
> > 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
> > >
>

2015-04-17 21:27:18

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH] rcu: small rcu_dereference doc update

On Fri, Apr 17, 2015 at 01:13:18PM -0400, Pranith Kumar wrote:
> On Fri, Apr 17, 2015 at 12:15 PM, Paul E. McKenney
> <[email protected]> wrote:
> > Sounds like a good thought for a separate patch. Please take a look
> > through the rest of the documentation -- this might well be the right
> > place for such an example, but there might well be a better place.
> > Is this issue mentioned in the checklist? If not, another item might
> > be good.
>
> Yup, I will take a look and send a patch for this.

Sounds good!

Thanx, Paul

2015-04-17 23:48:26

by Jeff Haran

[permalink] [raw]
Subject: RE: [PATCH] rcu: small rcu_dereference doc update

> -----Original Message-----
> From: Paul E. McKenney [mailto:[email protected]]
> 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:[email protected]]
> > > 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 <[email protected]>
> > > > Signed-off-by: Milos Vyletel <[email protected]>
> > >
> > > 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.

Thanks,

Jeff Haran

2015-04-21 16:32:25

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH] rcu: small rcu_dereference doc update

On Fri, Apr 17, 2015 at 11:48:16PM +0000, Jeff Haran wrote:
> > -----Original Message-----
> > From: Paul E. McKenney [mailto:[email protected]]
> > 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:[email protected]]
> > > > 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 <[email protected]>
> > > > > Signed-off-by: Milos Vyletel <[email protected]>
> > > >
> > > > 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