All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: David Howells <dhowells@redhat.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>,
	Trond.Myklebust@netapp.com, linux-nfs@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] NFS: Fix RCU warnings in nfs_inode_return_delegation_noreclaim() [ver #2]
Date: Mon, 29 Mar 2010 16:26:36 -0700	[thread overview]
Message-ID: <20100329232636.GT2569@linux.vnet.ibm.com> (raw)
In-Reply-To: <26760.1269903543@redhat.com>

On Mon, Mar 29, 2010 at 11:59:03PM +0100, David Howells wrote:
> Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> 
> > Only on Alpha.  Otherwise only a volatile access.
> 
> Whilst that is true, it's the principle of the thing.  The extra barrier
> shouldn't be emitted on Alpha.  If Alpha's no longer important, then can we
> scrap smp_read_barrier_depends()?
> 
> My point is that some of these rcu_dereference*()'s are unnecessary.  If
> there're required for correctness tracking purposes, fine; but can we have a
> macro that is just a dummy for the purpose of stripping the pointer Sparse
> annotation?  One that doesn't invoke rcu_dereference_raw() and interpolate a
> barrier, pretend or otherwise, when there's no second reference to order
> against.

Interesting point.  Perhaps an rcu_dereference_update(p, c) for the
cases where the data structure cannot change.  Also, such a name makes
it more clear that this is an update-side access, and it further documents
the update-side lock.

Something like the following, then?  Untested, probably does not even
compile.

							Thanx, Paul

------------------------------------------------------------------------

rcu: Add update-side variant of rcu_dereference()

Upcoming consistency-checking features are requiring that even update-side
accesses to RCU-protected pointers use some variant of rcu_dereference().
Even though rcu_dereference() is quite lightweight, it does constrain the
compiler, thus producing code that is worse than required.  This patch
therefore adds rcu_dereference_update(), which allows lockdep-style
checks for holding the correct update-side lock, but which does not
constrain the compiler.

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

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

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 872a98e..0a6047f 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -209,9 +209,26 @@ static inline int rcu_read_lock_sched_held(void)
 		rcu_dereference_raw(p); \
 	})
 
+/**
+ * rcu_dereference_update - rcu_dereference on structure that cannot change
+ *
+ * Do rcu_dereference() checking, but just pick up the pointer without
+ * the normal RCU read-side precautions.  These precautions are only
+ * needed if the data structure can change.  The caller is responsible
+ * for doing whatever is necessary (such as holding locks) to prevent
+ * such changes from occurring.
+ */
+#define rcu_dereference_update(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_dereference_update(p, c)	(p)
 
 #endif /* #else #ifdef CONFIG_PROVE_RCU */
 

  reply	other threads:[~2010-03-29 23:26 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-18 13:33 [PATCH] NFS: Fix RCU warnings in nfs_inode_return_delegation_noreclaim() [ver #2] David Howells
     [not found] ` <20100318133302.29754.1584.stgit-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>
2010-03-19  2:25   ` Paul E. McKenney
2010-03-19  2:25     ` Paul E. McKenney
2010-03-29 19:02     ` David Howells
2010-03-29 19:21       ` Paul E. McKenney
2010-03-29 20:15         ` David Howells
2010-03-29 20:26           ` Eric Dumazet
2010-03-29 20:26             ` Eric Dumazet
2010-03-29 21:05           ` Paul E. McKenney
2010-03-29 22:22             ` David Howells
2010-03-29 22:36               ` Paul E. McKenney
2010-03-29 22:59                 ` David Howells
2010-03-29 23:26                   ` Paul E. McKenney [this message]
2010-03-30 15:40                     ` Paul E. McKenney
2010-03-30 16:39                       ` David Howells
2010-03-30 16:49                         ` Paul E. McKenney
2010-03-30 17:04                           ` Eric Dumazet
2010-03-30 17:04                             ` Eric Dumazet
2010-03-30 17:25                             ` Paul E. McKenney
2010-03-30 17:25                               ` Paul E. McKenney
2010-03-30 23:51                           ` David Howells
2010-03-31  0:08                             ` Paul E. McKenney
2010-03-31 14:04                               ` David Howells
2010-03-31 15:16                                 ` Paul E. McKenney
2010-03-31 17:37                                   ` David Howells
2010-03-31 18:30                                     ` Paul E. McKenney
2010-03-31 18:32                                     ` Eric Dumazet
2010-03-31 18:32                                       ` Eric Dumazet
2010-03-31 22:53                                       ` David Howells
2010-04-01  1:29                                         ` Paul E. McKenney
2010-04-01 11:45                                           ` David Howells
2010-04-01 14:39                                             ` Paul E. McKenney
2010-04-01 14:46                                               ` David Howells
2010-04-05 17:57                                                 ` Paul E. McKenney
2010-04-06  9:30                                                   ` David Howells
2010-04-06 16:14                                                   ` David Howells
2010-04-06 17:29                                                     ` Paul E. McKenney
2010-04-06 19:34                                                       ` David Howells
2010-04-07  0:02                                                         ` Paul E. McKenney
2010-04-07 13:22                                                           ` David Howells
2010-04-07 15:57                                                             ` Paul E. McKenney
2010-04-07 16:35                                                               ` RCU condition checks David Howells
2010-04-07 17:10                                                                 ` Paul E. McKenney
2010-04-11 22:57                                                                   ` Trond Myklebust
     [not found]                                                                     ` <1271026643.6620.37.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2010-04-12 16:47                                                                       ` Paul E. McKenney
2010-04-12 16:47                                                                         ` Paul E. McKenney
2010-03-30 16:37                     ` [PATCH] NFS: Fix RCU warnings in nfs_inode_return_delegation_noreclaim() [ver #2] David Howells
2010-03-30 17:01                       ` Paul E. McKenney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100329232636.GT2569@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=Trond.Myklebust@netapp.com \
    --cc=dhowells@redhat.com \
    --cc=eric.dumazet@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.