[PATCH tip/urgent] rcu: add rcu_access_pointer and rcu_dereference_protected

[PATCH tip/urgent] rcu: add rcu_access_pointer and rcu_dereference_protected

[Paul E. McKenney]

This patch adds variants of rcu_dereference() that handle situations
where the RCU-protected data structure cannot change, perhaps due to
our holding the update-side lock, or where the RCU-protected pointer is
only to be fetched, not dereferenced.  These are needed due to some
performance concerns with using rcu_dereference() where it is not
required, aside from the need for lockdep/sparse checking.

The new rcu_access_pointer() primitive is for the case where the pointer
is be fetch and not dereferenced.  This primitive may be used without
protection, RCU or otherwise, due to the fact that it uses ACCESS_ONCE().

The new rcu_dereference_protected() primitive is for the case where updates
are prevented, for example, due to holding the update-side lock.  This
primitive does neither ACCESS_ONCE() nor smp_read_barrier_depends(), so
can only be used when updates are somehow prevented.

Suggested-by: David Howells <dhowells@redhat.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

 rcupdate.h |   38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 872a98e..3f06b3d 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -209,9 +209,47 @@ static inline int rcu_read_lock_sched_held(void)
 		rcu_dereference_raw(p); \
 	})
 
+/**
+ * rcu_access_pointer - fetch RCU pointer with no dereferencing
+ *
+ * Return the value of the specified RCU-protected pointer, but omit the
+ * smp_read_barrier_depends() and keep the ACCESS_ONCE().  This is useful
+ * when the value of this pointer is accessed, but the pointer is not
+ * dereferenced, for example, when testing an RCU-protected pointer against
+ * NULL.  This may also be used in cases where update-side locks prevent
+ * the value of the pointer from changing, but rcu_dereference_protected()
+ * is a lighter-weight primitive for this use case.
+ */
+#define rcu_access_pointer(p, c) \
+	({ \
+		if (debug_lockdep_rcu_enabled() && !(c)) \
+			lockdep_rcu_dereference(__FILE__, __LINE__); \
+		ACCESS_ONCE(p); \
+	})
+
+/**
+ * rcu_dereference_protected - fetch RCU pointer when updates prevented
+ *
+ * Return the value of the specified RCU-protected pointer, but omit
+ * both the smp_read_barrier_depends() and the ACCESS_ONCE().  This
+ * is useful in cases where update-side locks prevent the value of the
+ * pointer from changing.  Please note that this primitive does -not-
+ * prevent the compiler from repeating this reference or combining it
+ * with other references, so it should not be used without protection
+ * of appropriate locks.
+ */
+#define rcu_dereference_protected(p, c) \
+	({ \
+		if (debug_lockdep_rcu_enabled() && !(c)) \
+			lockdep_rcu_dereference(__FILE__, __LINE__); \
+		(p); \
+	})
+
 #else /* #ifdef CONFIG_PROVE_RCU */
 
 #define rcu_dereference_check(p, c)	rcu_dereference_raw(p)
+#define rcu_access_pointer(p, c)	ACCESS_ONCE(p)
+#define rcu_dereference_protected(p, c) (p)
 
 #endif /* #else #ifdef CONFIG_PROVE_RCU */
 

Re: [PATCH tip/urgent] rcu: add rcu_access_pointer and rcu_dereference_protected

[Eric Dumazet]

Le mercredi 07 avril 2010 à 09:26 -0700, Paul E. McKenney a écrit :
> This patch adds variants of rcu_dereference() that handle situations
> where the RCU-protected data structure cannot change, perhaps due to
> our holding the update-side lock, or where the RCU-protected pointer is
> only to be fetched, not dereferenced.  These are needed due to some
> performance concerns with using rcu_dereference() where it is not
> required, aside from the need for lockdep/sparse checking.
> 
> The new rcu_access_pointer() primitive is for the case where the pointer
> is be fetch and not dereferenced.  This primitive may be used without
> protection, RCU or otherwise, due to the fact that it uses ACCESS_ONCE().
> 
> The new rcu_dereference_protected() primitive is for the case where updates
> are prevented, for example, due to holding the update-side lock.  This
> primitive does neither ACCESS_ONCE() nor smp_read_barrier_depends(), so
> can only be used when updates are somehow prevented.
> 
> Suggested-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Re: [PATCH tip/urgent] rcu: add rcu_access_pointer and rcu_dereference_protected

[Mathieu Desnoyers]

* Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> This patch adds variants of rcu_dereference() that handle situations
> where the RCU-protected data structure cannot change, perhaps due to
> our holding the update-side lock, or where the RCU-protected pointer is
> only to be fetched, not dereferenced.  These are needed due to some
> performance concerns with using rcu_dereference() where it is not
> required, aside from the need for lockdep/sparse checking.
> 
> The new rcu_access_pointer() primitive is for the case where the pointer
> is be fetch and not dereferenced.  This primitive may be used without
> protection, RCU or otherwise, due to the fact that it uses ACCESS_ONCE().
> 
> The new rcu_dereference_protected() primitive is for the case where updates
> are prevented, for example, due to holding the update-side lock.  This
> primitive does neither ACCESS_ONCE() nor smp_read_barrier_depends(), so
> can only be used when updates are somehow prevented.
> 
> Suggested-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

We should probably pull this in liburcu eventually too (renaming
ACCESS_ONCE() into LOAD_SHARED()).

Thanks,

Mathieu

> 
>  rcupdate.h |   38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 872a98e..3f06b3d 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -209,9 +209,47 @@ static inline int rcu_read_lock_sched_held(void)
>  		rcu_dereference_raw(p); \
>  	})
>  
> +/**
> + * rcu_access_pointer - fetch RCU pointer with no dereferencing
> + *
> + * Return the value of the specified RCU-protected pointer, but omit the
> + * smp_read_barrier_depends() and keep the ACCESS_ONCE().  This is useful
> + * when the value of this pointer is accessed, but the pointer is not
> + * dereferenced, for example, when testing an RCU-protected pointer against
> + * NULL.  This may also be used in cases where update-side locks prevent
> + * the value of the pointer from changing, but rcu_dereference_protected()
> + * is a lighter-weight primitive for this use case.
> + */
> +#define rcu_access_pointer(p, c) \
> +	({ \
> +		if (debug_lockdep_rcu_enabled() && !(c)) \
> +			lockdep_rcu_dereference(__FILE__, __LINE__); \
> +		ACCESS_ONCE(p); \
> +	})
> +
> +/**
> + * rcu_dereference_protected - fetch RCU pointer when updates prevented
> + *
> + * Return the value of the specified RCU-protected pointer, but omit
> + * both the smp_read_barrier_depends() and the ACCESS_ONCE().  This
> + * is useful in cases where update-side locks prevent the value of the
> + * pointer from changing.  Please note that this primitive does -not-
> + * prevent the compiler from repeating this reference or combining it
> + * with other references, so it should not be used without protection
> + * of appropriate locks.
> + */
> +#define rcu_dereference_protected(p, c) \
> +	({ \
> +		if (debug_lockdep_rcu_enabled() && !(c)) \
> +			lockdep_rcu_dereference(__FILE__, __LINE__); \
> +		(p); \
> +	})
> +
>  #else /* #ifdef CONFIG_PROVE_RCU */
>  
>  #define rcu_dereference_check(p, c)	rcu_dereference_raw(p)
> +#define rcu_access_pointer(p, c)	ACCESS_ONCE(p)
> +#define rcu_dereference_protected(p, c) (p)
>  
>  #endif /* #else #ifdef CONFIG_PROVE_RCU */
>  

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH tip/urgent] rcu: add rcu_access_pointer and rcu_dereference_protected

[David Howells]

Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> The new rcu_access_pointer() primitive is for the case where the pointer
> is be fetch and not dereferenced.  This primitive may be used without
> protection, RCU or otherwise, due to the fact that it uses ACCESS_ONCE().
> ...
> +#define rcu_access_pointer(p, c) \

NAK.  This shouldn't have the conditional parameter 'c'.  Given that 'c' (by
analogy to rcu_dereference_check()) is there to describe the conditions under
which it's permitted to dereference the pointer, why is that relevant here?
What is it you're proving?

David

Re: [PATCH tip/urgent] rcu: add rcu_access_pointer and rcu_dereference_protected

[Paul E. McKenney]

On Wed, Apr 07, 2010 at 06:00:27PM +0100, David Howells wrote:
> Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> 
> > The new rcu_access_pointer() primitive is for the case where the pointer
> > is be fetch and not dereferenced.  This primitive may be used without
> > protection, RCU or otherwise, due to the fact that it uses ACCESS_ONCE().
> > ...
> > +#define rcu_access_pointer(p, c) \
> 
> NAK.  This shouldn't have the conditional parameter 'c'.  Given that 'c' (by
> analogy to rcu_dereference_check()) is there to describe the conditions under
> which it's permitted to dereference the pointer, why is that relevant here?
> What is it you're proving?

In some cases, the value of 'c' will indeed be '1', and in those cases,
there needs to be a comment stating why it is OK, similar to those
required for smp_mb() and friends.

In other cases, there will be a reference counter or a "not yet fully
initialized" flag that can (and should) be tested.

							Thanx, Paul

Re: [PATCH tip/urgent] rcu: add rcu_access_pointer and rcu_dereference_protected

[David Howells]

Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> In other cases, there will be a reference counter or a "not yet fully
> initialized" flag that can (and should) be tested.

Why would you be using rcu_access_pointer() there?  Why wouldn't you be using
rcu_dereference_protected()?


Also, one other thing:  Should the default versions of these functions make
some reference to 'c' to prevent compiler warnings?  Should:

	#define rcu_dereference_check(p, c)	rcu_dereference_raw(p)

for example, be:

	#define rcu_dereference_check(p, c)	\
	({					\
		if (1 || !(c))			\
			rcu_dereference_raw(p);	\
	})

I'm not sure it's necessary, but it's possible to envisage a situation where
someone calculates something specifically for use in 'c', which will cause an
warning from the compiler if c isn't then checked.

David

Re: [PATCH tip/urgent] rcu: add rcu_access_pointer and rcu_dereference_protected

[Paul E. McKenney]

On Wed, Apr 07, 2010 at 06:20:48PM +0100, David Howells wrote:
> Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> 
> > In other cases, there will be a reference counter or a "not yet fully
> > initialized" flag that can (and should) be tested.
> 
> Why would you be using rcu_access_pointer() there?  Why wouldn't you be using
> rcu_dereference_protected()?

Excellent question.  I am writing up the documentation now, and will
either (1) have a good use case or (2) remove the condition.

> Also, one other thing:  Should the default versions of these functions make
> some reference to 'c' to prevent compiler warnings?  Should:
> 
> 	#define rcu_dereference_check(p, c)	rcu_dereference_raw(p)
> 
> for example, be:
> 
> 	#define rcu_dereference_check(p, c)	\
> 	({					\
> 		if (1 || !(c))			\
> 			rcu_dereference_raw(p);	\
> 	})
> 
> I'm not sure it's necessary, but it's possible to envisage a situation where
> someone calculates something specifically for use in 'c', which will cause an
> warning from the compiler if c isn't then checked.

I did try this.  The problem is that it breaks the build for non-lockdep
configurations due to the lockdep-check primitives not being defined.  :-(

							Thanx, Paul

Re: [PATCH tip/urgent] rcu: add rcu_access_pointer and rcu_dereference_protected

[Paul E. McKenney]

On Wed, Apr 07, 2010 at 04:00:09PM -0700, Paul E. McKenney wrote:
> On Wed, Apr 07, 2010 at 06:20:48PM +0100, David Howells wrote:
> > Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> > 
> > > In other cases, there will be a reference counter or a "not yet fully
> > > initialized" flag that can (and should) be tested.
> > 
> > Why would you be using rcu_access_pointer() there?  Why wouldn't you be using
> > rcu_dereference_protected()?
> 
> Excellent question.  I am writing up the documentation now, and will
> either (1) have a good use case or (2) remove the condition.

And all of the examples I could come up with that had c!=1 were contorted,
even by my standards.  So you were right, and I will drop the "c" on my
next set of patches.

							Thanx, Paul

Re: [PATCH tip/urgent] rcu: add rcu_access_pointer and rcu_dereference_protected

[David Howells]

Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> And all of the examples I could come up with that had c!=1 were contorted,
> even by my standards.  So you were right, and I will drop the "c" on my
> next set of patches.

:-)

When it's done, I'll rebuild my keys and NFS patches on top of it.

David

Re: [PATCH tip/urgent] rcu: add rcu_access_pointer and rcu_dereference_protected

[Paul E. McKenney]

On Thu, Apr 08, 2010 at 08:04:07PM +0100, David Howells wrote:
> Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> 
> > And all of the examples I could come up with that had c!=1 were contorted,
> > even by my standards.  So you were right, and I will drop the "c" on my
> > next set of patches.
> 
> :-)
> 
> When it's done, I'll rebuild my keys and NFS patches on top of it.

Testing in progress.  ;-)

							Thanx, Paul