Re: [PATCH] rcu: small rcu_dereference doc update
On Fri, Apr 17, 2015 at 11:48:16PM +, Jeff Haran wrote: > > -Original Message- > > From: Paul E. McKenney [mailto:paul...@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 +, Jeff Haran wrote: > > > > -Original Message- > > > > From: Paul E. McKenney [mailto:paul...@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 > > > > > Signed-off-by: Milos Vyletel > > > > > > > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rcu: small rcu_dereference doc update
On Fri, Apr 17, 2015 at 11:48:16PM +, Jeff Haran wrote: -Original Message- From: Paul E. McKenney [mailto:paul...@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 +, Jeff Haran wrote: -Original Message- From: Paul E. McKenney [mailto:paul...@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.ha...@citrix.com Signed-off-by: Milos Vyletel mi...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] rcu: small rcu_dereference doc update
> -Original Message- > From: Paul E. McKenney [mailto:paul...@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 +, Jeff Haran wrote: > > > -Original Message- > > > From: Paul E. McKenney [mailto:paul...@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 > > > > Signed-off-by: Milos Vyletel > > > > > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rcu: small rcu_dereference doc update
On Fri, Apr 17, 2015 at 04:53:15PM +, Jeff Haran wrote: > > -Original Message- > > From: Paul E. McKenney [mailto:paul...@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 > > > Signed-off-by: Milos Vyletel > > > > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rcu: small rcu_dereference doc update
On Fri, Apr 17, 2015 at 12:15 PM, Paul E. McKenney 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] rcu: small rcu_dereference doc update
> -Original Message- > From: Paul E. McKenney [mailto:paul...@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 > > Signed-off-by: Milos Vyletel > > 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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 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
> > Signed-off-by: Milos Vyletel
> > ---
> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rcu: small rcu_dereference doc update
On Fri, Apr 17, 2015 at 6:33 AM, 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.
Might as well make it more explicit with an example then. See below:
>
> Reported-by: Jeff Haran
> Signed-off-by: Milos Vyletel
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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" 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 > > > Signed-off-by: Milos Vyletel > > > > 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 > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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" 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 > > > Signed-off-by: Milos Vyletel > > > > 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 > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rcu: small rcu_dereference doc update
On Fri, 17 Apr 2015 07:06:38 -0700 "Paul E. McKenney" 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 > > Signed-off-by: Milos Vyletel > > 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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 > Signed-off-by: Milos Vyletel 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 paul...@linux.vnet.ibm.com 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 jeff.ha...@citrix.com Signed-off-by: Milos Vyletel mi...@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. 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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.ha...@citrix.com Signed-off-by: Milos Vyletel mi...@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 --- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 paul...@linux.vnet.ibm.com 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 jeff.ha...@citrix.com Signed-off-by: Milos Vyletel mi...@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. 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rcu: small rcu_dereference doc update
On Fri, 17 Apr 2015 07:06:38 -0700 Paul E. McKenney paul...@linux.vnet.ibm.com 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 jeff.ha...@citrix.com Signed-off-by: Milos Vyletel mi...@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. 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rcu: small rcu_dereference doc update
On Fri, Apr 17, 2015 at 12:15 PM, Paul E. McKenney paul...@linux.vnet.ibm.com 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rcu: small rcu_dereference doc update
On Fri, Apr 17, 2015 at 04:53:15PM +, Jeff Haran wrote: -Original Message- From: Paul E. McKenney [mailto:paul...@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.ha...@citrix.com Signed-off-by: Milos Vyletel mi...@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 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] rcu: small rcu_dereference doc update
-Original Message- From: Paul E. McKenney [mailto:paul...@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.ha...@citrix.com Signed-off-by: Milos Vyletel mi...@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. 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 mi...@redhat.com 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 jeff.ha...@citrix.com
Signed-off-by: Milos Vyletel mi...@redhat.com
---
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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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 paul...@linux.vnet.ibm.com 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rcu: small rcu_dereference doc update
On Fri, Apr 17, 2015 at 6:33 AM, Milos Vyletel mi...@redhat.com 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 jeff.ha...@citrix.com
Signed-off-by: Milos Vyletel mi...@redhat.com
---
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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] rcu: small rcu_dereference doc update
-Original Message- From: Paul E. McKenney [mailto:paul...@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 +, Jeff Haran wrote: -Original Message- From: Paul E. McKenney [mailto:paul...@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.ha...@citrix.com Signed-off-by: Milos Vyletel mi...@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. Thanks, Jeff Haran -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
