INFO: suspicious rcu_dereference_check()

INFO: suspicious rcu_dereference_check()

[Zdenek Kabelac]

Hi

I've enabled rcu correctness for my  todays 2.6.34-rc2 kernel.

I'm getting this INFO: from my kvm guest (which uses host's nfs
exported directory.)
This

===================================================
[ INFO: suspicious rcu_dereference_check() usage. ]
---------------------------------------------------
fs/nfs/delegation.c:348 invoked rcu_dereference_check() without protection!

other info that might help us debug this:


rcu_scheduler_active = 1, debug_locks = 0
2 locks held by rm/1820:
 #0:  (&sb->s_type->i_mutex_key#13/1){+.+.+.}, at:
[<ffffffff81138e1b>] do_unlinkat+0x9b/0x1c0
 #1:  (&sb->s_type->i_mutex_key#13){+.+.+.}, at: [<ffffffff81136a76>]
vfs_unlink+0x56/0xf0

stack backtrace:
Pid: 1820, comm: rm Not tainted 2.6.34-rc2-00186-ge79a302 #60
Call Trace:
 [<ffffffff810819fb>] lockdep_rcu_dereference+0xbb/0xc0
 [<ffffffffa0228351>] nfs_inode_return_delegation+0x101/0x110 [nfs]
 [<ffffffffa01fd58d>] nfs_unlink+0xad/0x2a0 [nfs]
 [<ffffffff81136aba>] vfs_unlink+0x9a/0xf0
 [<ffffffff81148365>] ? mnt_want_write+0x65/0xb0
 [<ffffffff81138f03>] do_unlinkat+0x183/0x1c0
 [<ffffffff8142b66d>] ? retint_swapgs+0xe/0x13
 [<ffffffff81083985>] ? trace_hardirqs_on_caller+0x155/0x1a0
 [<ffffffff8142a232>] ? trace_hardirqs_on_thunk+0x3a/0x3f
 [<ffffffff811390a2>] sys_unlinkat+0x22/0x40
 [<ffffffff81031dc8>] sysenter_dispatch+0x7/0x2c


Zdenek

Re: INFO: suspicious rcu_dereference_check()

[Trond Myklebust]

On Thu, 2010-03-25 at 11:48 +0100, Zdenek Kabelac wrote: 
> Hi
> 
> I've enabled rcu correctness for my  todays 2.6.34-rc2 kernel.
> 
> I'm getting this INFO: from my kvm guest (which uses host's nfs
> exported directory.)
> This
> 
> ===================================================
> [ INFO: suspicious rcu_dereference_check() usage. ]
> ---------------------------------------------------
> fs/nfs/delegation.c:348 invoked rcu_dereference_check() without protection!
> 
> other info that might help us debug this:
> 
> 
> rcu_scheduler_active = 1, debug_locks = 0
> 2 locks held by rm/1820:
>  #0:  (&sb->s_type->i_mutex_key#13/1){+.+.+.}, at:
> [<ffffffff81138e1b>] do_unlinkat+0x9b/0x1c0
>  #1:  (&sb->s_type->i_mutex_key#13){+.+.+.}, at: [<ffffffff81136a76>]
> vfs_unlink+0x56/0xf0
> 
> stack backtrace:
> Pid: 1820, comm: rm Not tainted 2.6.34-rc2-00186-ge79a302 #60
> Call Trace:
>  [<ffffffff810819fb>] lockdep_rcu_dereference+0xbb/0xc0
>  [<ffffffffa0228351>] nfs_inode_return_delegation+0x101/0x110 [nfs]
>  [<ffffffffa01fd58d>] nfs_unlink+0xad/0x2a0 [nfs]
>  [<ffffffff81136aba>] vfs_unlink+0x9a/0xf0
>  [<ffffffff81148365>] ? mnt_want_write+0x65/0xb0
>  [<ffffffff81138f03>] do_unlinkat+0x183/0x1c0
>  [<ffffffff8142b66d>] ? retint_swapgs+0xe/0x13
>  [<ffffffff81083985>] ? trace_hardirqs_on_caller+0x155/0x1a0
>  [<ffffffff8142a232>] ? trace_hardirqs_on_thunk+0x3a/0x3f
>  [<ffffffff811390a2>] sys_unlinkat+0x22/0x40
>  [<ffffffff81031dc8>] sysenter_dispatch+0x7/0x2c
> 

It is a 100% bogus warning. There are tentative patches floating around
to fix the above warning, but they haven't been merged yet.

In the meantime, please ignore...

Cheers
  Trond

Re: INFO: suspicious rcu_dereference_check()

[Trond Myklebust]

On Thu, 2010-03-25 at 11:48 +0100, Zdenek Kabelac wrote: 
> Hi
> 
> I've enabled rcu correctness for my  todays 2.6.34-rc2 kernel.
> 
> I'm getting this INFO: from my kvm guest (which uses host's nfs
> exported directory.)
> This
> 
> ===================================================
> [ INFO: suspicious rcu_dereference_check() usage. ]
> ---------------------------------------------------
> fs/nfs/delegation.c:348 invoked rcu_dereference_check() without protection!
> 
> other info that might help us debug this:
> 
> 
> rcu_scheduler_active = 1, debug_locks = 0
> 2 locks held by rm/1820:
>  #0:  (&sb->s_type->i_mutex_key#13/1){+.+.+.}, at:
> [<ffffffff81138e1b>] do_unlinkat+0x9b/0x1c0
>  #1:  (&sb->s_type->i_mutex_key#13){+.+.+.}, at: [<ffffffff81136a76>]
> vfs_unlink+0x56/0xf0
> 
> stack backtrace:
> Pid: 1820, comm: rm Not tainted 2.6.34-rc2-00186-ge79a302 #60
> Call Trace:
>  [<ffffffff810819fb>] lockdep_rcu_dereference+0xbb/0xc0
>  [<ffffffffa0228351>] nfs_inode_return_delegation+0x101/0x110 [nfs]
>  [<ffffffffa01fd58d>] nfs_unlink+0xad/0x2a0 [nfs]
>  [<ffffffff81136aba>] vfs_unlink+0x9a/0xf0
>  [<ffffffff81148365>] ? mnt_want_write+0x65/0xb0
>  [<ffffffff81138f03>] do_unlinkat+0x183/0x1c0
>  [<ffffffff8142b66d>] ? retint_swapgs+0xe/0x13
>  [<ffffffff81083985>] ? trace_hardirqs_on_caller+0x155/0x1a0
>  [<ffffffff8142a232>] ? trace_hardirqs_on_thunk+0x3a/0x3f
>  [<ffffffff811390a2>] sys_unlinkat+0x22/0x40
>  [<ffffffff81031dc8>] sysenter_dispatch+0x7/0x2c
> 

It is a 100% bogus warning. There are tentative patches floating around
to fix the above warning, but they haven't been merged yet.

In the meantime, please ignore...

Cheers
  Trond

Re: INFO: suspicious rcu_dereference_check()

[Paul E. McKenney]

On Thu, Mar 25, 2010 at 11:36:40AM -0400, Trond Myklebust wrote:
> On Thu, 2010-03-25 at 11:48 +0100, Zdenek Kabelac wrote: 
> > Hi
> > 
> > I've enabled rcu correctness for my  todays 2.6.34-rc2 kernel.
> > 
> > I'm getting this INFO: from my kvm guest (which uses host's nfs
> > exported directory.)
> > This
> > 
> > ===================================================
> > [ INFO: suspicious rcu_dereference_check() usage. ]
> > ---------------------------------------------------
> > fs/nfs/delegation.c:348 invoked rcu_dereference_check() without protection!
> > 
> > other info that might help us debug this:
> > 
> > 
> > rcu_scheduler_active = 1, debug_locks = 0
> > 2 locks held by rm/1820:
> >  #0:  (&sb->s_type->i_mutex_key#13/1){+.+.+.}, at:
> > [<ffffffff81138e1b>] do_unlinkat+0x9b/0x1c0
> >  #1:  (&sb->s_type->i_mutex_key#13){+.+.+.}, at: [<ffffffff81136a76>]
> > vfs_unlink+0x56/0xf0
> > 
> > stack backtrace:
> > Pid: 1820, comm: rm Not tainted 2.6.34-rc2-00186-ge79a302 #60
> > Call Trace:
> >  [<ffffffff810819fb>] lockdep_rcu_dereference+0xbb/0xc0
> >  [<ffffffffa0228351>] nfs_inode_return_delegation+0x101/0x110 [nfs]
> >  [<ffffffffa01fd58d>] nfs_unlink+0xad/0x2a0 [nfs]
> >  [<ffffffff81136aba>] vfs_unlink+0x9a/0xf0
> >  [<ffffffff81148365>] ? mnt_want_write+0x65/0xb0
> >  [<ffffffff81138f03>] do_unlinkat+0x183/0x1c0
> >  [<ffffffff8142b66d>] ? retint_swapgs+0xe/0x13
> >  [<ffffffff81083985>] ? trace_hardirqs_on_caller+0x155/0x1a0
> >  [<ffffffff8142a232>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> >  [<ffffffff811390a2>] sys_unlinkat+0x22/0x40
> >  [<ffffffff81031dc8>] sysenter_dispatch+0x7/0x2c
> > 
> 
> It is a 100% bogus warning. There are tentative patches floating around
> to fix the above warning, but they haven't been merged yet.
> 
> In the meantime, please ignore...

Did you want to carry these patches, or would you rather that I do so?

(And sorry for the slow response, was on holiday last week.)

							Thanx, Paul

Re: INFO: suspicious rcu_dereference_check()

[Paul E. McKenney]

On Thu, Mar 25, 2010 at 11:36:40AM -0400, Trond Myklebust wrote:
> On Thu, 2010-03-25 at 11:48 +0100, Zdenek Kabelac wrote: 
> > Hi
> > 
> > I've enabled rcu correctness for my  todays 2.6.34-rc2 kernel.
> > 
> > I'm getting this INFO: from my kvm guest (which uses host's nfs
> > exported directory.)
> > This
> > 
> > ===================================================
> > [ INFO: suspicious rcu_dereference_check() usage. ]
> > ---------------------------------------------------
> > fs/nfs/delegation.c:348 invoked rcu_dereference_check() without protection!
> > 
> > other info that might help us debug this:
> > 
> > 
> > rcu_scheduler_active = 1, debug_locks = 0
> > 2 locks held by rm/1820:
> >  #0:  (&sb->s_type->i_mutex_key#13/1){+.+.+.}, at:
> > [<ffffffff81138e1b>] do_unlinkat+0x9b/0x1c0
> >  #1:  (&sb->s_type->i_mutex_key#13){+.+.+.}, at: [<ffffffff81136a76>]
> > vfs_unlink+0x56/0xf0
> > 
> > stack backtrace:
> > Pid: 1820, comm: rm Not tainted 2.6.34-rc2-00186-ge79a302 #60
> > Call Trace:
> >  [<ffffffff810819fb>] lockdep_rcu_dereference+0xbb/0xc0
> >  [<ffffffffa0228351>] nfs_inode_return_delegation+0x101/0x110 [nfs]
> >  [<ffffffffa01fd58d>] nfs_unlink+0xad/0x2a0 [nfs]
> >  [<ffffffff81136aba>] vfs_unlink+0x9a/0xf0
> >  [<ffffffff81148365>] ? mnt_want_write+0x65/0xb0
> >  [<ffffffff81138f03>] do_unlinkat+0x183/0x1c0
> >  [<ffffffff8142b66d>] ? retint_swapgs+0xe/0x13
> >  [<ffffffff81083985>] ? trace_hardirqs_on_caller+0x155/0x1a0
> >  [<ffffffff8142a232>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> >  [<ffffffff811390a2>] sys_unlinkat+0x22/0x40
> >  [<ffffffff81031dc8>] sysenter_dispatch+0x7/0x2c
> > 
> 
> It is a 100% bogus warning. There are tentative patches floating around
> to fix the above warning, but they haven't been merged yet.
> 
> In the meantime, please ignore...

Did you want to carry these patches, or would you rather that I do so?

(And sorry for the slow response, was on holiday last week.)

							Thanx, Paul

Re: INFO: suspicious rcu_dereference_check()

[Trond Myklebust]

On Mon, 2010-03-29 at 08:25 -0700, Paul E. McKenney wrote: 
> On Thu, Mar 25, 2010 at 11:36:40AM -0400, Trond Myklebust wrote:
> > On Thu, 2010-03-25 at 11:48 +0100, Zdenek Kabelac wrote: 
> > > Hi
> > > 
> > > I've enabled rcu correctness for my  todays 2.6.34-rc2 kernel.
> > > 
> > > I'm getting this INFO: from my kvm guest (which uses host's nfs
> > > exported directory.)
> > > This
> > > 
> > > ===================================================
> > > [ INFO: suspicious rcu_dereference_check() usage. ]
> > > ---------------------------------------------------
> > > fs/nfs/delegation.c:348 invoked rcu_dereference_check() without protection!
> > > 
> > > other info that might help us debug this:
> > > 
> > > 
> > > rcu_scheduler_active = 1, debug_locks = 0
> > > 2 locks held by rm/1820:
> > >  #0:  (&sb->s_type->i_mutex_key#13/1){+.+.+.}, at:
> > > [<ffffffff81138e1b>] do_unlinkat+0x9b/0x1c0
> > >  #1:  (&sb->s_type->i_mutex_key#13){+.+.+.}, at: [<ffffffff81136a76>]
> > > vfs_unlink+0x56/0xf0
> > > 
> > > stack backtrace:
> > > Pid: 1820, comm: rm Not tainted 2.6.34-rc2-00186-ge79a302 #60
> > > Call Trace:
> > >  [<ffffffff810819fb>] lockdep_rcu_dereference+0xbb/0xc0
> > >  [<ffffffffa0228351>] nfs_inode_return_delegation+0x101/0x110 [nfs]
> > >  [<ffffffffa01fd58d>] nfs_unlink+0xad/0x2a0 [nfs]
> > >  [<ffffffff81136aba>] vfs_unlink+0x9a/0xf0
> > >  [<ffffffff81148365>] ? mnt_want_write+0x65/0xb0
> > >  [<ffffffff81138f03>] do_unlinkat+0x183/0x1c0
> > >  [<ffffffff8142b66d>] ? retint_swapgs+0xe/0x13
> > >  [<ffffffff81083985>] ? trace_hardirqs_on_caller+0x155/0x1a0
> > >  [<ffffffff8142a232>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> > >  [<ffffffff811390a2>] sys_unlinkat+0x22/0x40
> > >  [<ffffffff81031dc8>] sysenter_dispatch+0x7/0x2c
> > > 
> > 
> > It is a 100% bogus warning. There are tentative patches floating around
> > to fix the above warning, but they haven't been merged yet.
> > 
> > In the meantime, please ignore...
> 
> Did you want to carry these patches, or would you rather that I do so?
> 
> (And sorry for the slow response, was on holiday last week.)
> 
> 							Thanx, Paul

Hi Paul,

I don't mind whether you or I push them to Linus, but IIRC you had a
couple of comments about the last patchset I saw from David, so I was
expecting to see either a reply from him or patch update. Did I miss
that reply? (Ccing: David)

Cheers
  Trond

Re: INFO: suspicious rcu_dereference_check()

[David Howells]

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]

Trond Myklebust <trond.myklebust@fys.uio.no> wrote:

> I don't mind whether you or I push them to Linus, but IIRC you had a
> couple of comments about the last patchset I saw from David, so I was
> expecting to see either a reply from him or patch update. Did I miss
> that reply? (Ccing: David)

I was waiting to see what Arnd did, since Paul said:

	This looks good at the moment, however, the sparse changes that Arnd
	Bergmann is working on will invalidate a couple of the changes below.
	Of course, better a future problem than a here-and-now problem, but
	is there an easy way to fix both?

My message and Paul's reply are attached for Arnd's convenience.  Maybe Arnd
could include my changes, or supply me with his patch?

David


[-- Attachment #2: 10878 --]
[-- Type: message/rfc822, Size: 8118 bytes --]

From: David Howells <dhowells@redhat.com>
To: Trond.Myklebust@netapp.com, paulmck@linux.vnet.ibm.com
Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, David Howells <dhowells@redhat.com>
Subject: [PATCH] NFS: Fix RCU warnings in nfs_inode_return_delegation_noreclaim() [ver #2]
Date: Thu, 18 Mar 2010 13:33:02 +0000
Message-ID: <20100318133302.29754.1584.stgit@warthog.procyon.org.uk>

Fix a number of RCU warnings in nfs_inode_return_delegation_noreclaim().
nfs_inode_return_delegation_noreclaim() and nfs_inode_return_delegation() don't
need to use rcu_dereference() outside the spinlocked region as they merely
examin the pointer and don't follow it, thus rendering unnecessary the need to
impose a partial ordering over the one item of interest.

nfs_detach_delegation_locked() doesn't need rcu_derefence() because it can only
be called if nfs_client::cl_lock is held, and that guards against anyone
changing nfsi->delegation under it.  Furthermore, the barrier in
rcu_derefence() is superfluous, given that the spin_lock() is also a barrier.

nfs_free_delegation() should be using rcu_dereference_check() to validate the
state that the data is in (the delegation inode must have been cleared).  By
this point, the delegation is being released, so no one else should be
attempting to use the saved credentials, and they can be cleared.  However,
rcu_assign_pointer() should be used to clear them, and the delegation itself
must still use call_rcu() as the list of delegations could be being traversed.


[ INFO: suspicious rcu_dereference_check() usage. ]
---------------------------------------------------
fs/nfs/delegation.c:332 invoked rcu_dereference_check() without protection!

other info that might help us debug this:


rcu_scheduler_active = 1, debug_locks = 0
2 locks held by mount.nfs4/2281:
 #0:  (&type->s_umount_key#34){+.+...}, at: [<ffffffff810b25b4>] deactivate_super+0x60/0x80
 #1:  (iprune_sem){+.+...}, at: [<ffffffff810c332a>] invalidate_inodes+0x39/0x13a

stack backtrace:
Pid: 2281, comm: mount.nfs4 Not tainted 2.6.34-rc1-cachefs #110
Call Trace:
 [<ffffffff8105149f>] lockdep_rcu_dereference+0xaa/0xb2
 [<ffffffffa00b4591>] nfs_inode_return_delegation_noreclaim+0x5b/0xa0 [nfs]
 [<ffffffffa0095d63>] nfs4_clear_inode+0x11/0x1e [nfs]
 [<ffffffff810c2d92>] clear_inode+0x9e/0xf8
 [<ffffffff810c3028>] dispose_list+0x67/0x10e
 [<ffffffff810c340d>] invalidate_inodes+0x11c/0x13a
 [<ffffffff810b1dc1>] generic_shutdown_super+0x42/0xf4
 [<ffffffff810b1ebe>] kill_anon_super+0x11/0x4f
 [<ffffffffa009893c>] nfs4_kill_super+0x3f/0x72 [nfs]
 [<ffffffff810b25bc>] deactivate_super+0x68/0x80
 [<ffffffff810c6744>] mntput_no_expire+0xbb/0xf8
 [<ffffffff810c681b>] release_mounts+0x9a/0xb0
 [<ffffffff810c689b>] put_mnt_ns+0x6a/0x79
 [<ffffffffa00983a1>] nfs_follow_remote_path+0x5a/0x146 [nfs]
 [<ffffffffa0098334>] ? nfs_do_root_mount+0x82/0x95 [nfs]
 [<ffffffffa00985a9>] nfs4_try_mount+0x75/0xaf [nfs]
 [<ffffffffa0098874>] nfs4_get_sb+0x291/0x31a [nfs]
 [<ffffffff810b2059>] vfs_kern_mount+0xb8/0x177
 [<ffffffff810b2176>] do_kern_mount+0x48/0xe8
 [<ffffffff810c810b>] do_mount+0x782/0x7f9
 [<ffffffff810c8205>] sys_mount+0x83/0xbe
 [<ffffffff81001eeb>] system_call_fastpath+0x16/0x1b

Also on:

fs/nfs/delegation.c:215 invoked rcu_dereference_check() without protection!
 [<ffffffff8105149f>] lockdep_rcu_dereference+0xaa/0xb2
 [<ffffffffa00b4223>] nfs_inode_set_delegation+0xfe/0x219 [nfs]
 [<ffffffffa00a9c6f>] nfs4_opendata_to_nfs4_state+0x2c2/0x30d [nfs]
 [<ffffffffa00aa15d>] nfs4_do_open+0x2a6/0x3a6 [nfs]
 ...

And:

fs/nfs/delegation.c:40 invoked rcu_dereference_check() without protection!
 [<ffffffff8105149f>] lockdep_rcu_dereference+0xaa/0xb2
 [<ffffffffa00b3bef>] nfs_free_delegation+0x3d/0x6e [nfs]
 [<ffffffffa00b3e71>] nfs_do_return_delegation+0x26/0x30 [nfs]
 [<ffffffffa00b406a>] __nfs_inode_return_delegation+0x1ef/0x1fe [nfs]
 [<ffffffffa00b448a>] nfs_client_return_marked_delegations+0xc9/0x124 [nfs]
 ...


Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/nfs/delegation.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 2563beb..fa9b7c5 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -37,7 +37,8 @@ static void nfs_free_delegation(struct nfs_delegation *delegation)
 {
 	struct rpc_cred *cred;
 
-	cred = rcu_dereference(delegation->cred);
+	cred = rcu_dereference_check(delegation->cred,
+				     delegation->inode == NULL);
 	rcu_assign_pointer(delegation->cred, NULL);
 	call_rcu(&delegation->rcu, nfs_free_delegation_callback);
 	if (cred)
@@ -167,7 +168,7 @@ static struct inode *nfs_delegation_grab_inode(struct nfs_delegation *delegation
 
 static struct nfs_delegation *nfs_detach_delegation_locked(struct nfs_inode *nfsi, const nfs4_stateid *stateid)
 {
-	struct nfs_delegation *delegation = rcu_dereference(nfsi->delegation);
+	struct nfs_delegation *delegation = nfsi->delegation;
 
 	if (delegation == NULL)
 		goto nomatch;
@@ -212,7 +213,7 @@ int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, struct
 	spin_lock_init(&delegation->lock);
 
 	spin_lock(&clp->cl_lock);
-	if (rcu_dereference(nfsi->delegation) != NULL) {
+	if (nfsi->delegation != NULL) {
 		if (memcmp(&delegation->stateid, &nfsi->delegation->stateid,
 					sizeof(delegation->stateid)) == 0 &&
 				delegation->type == nfsi->delegation->type) {
@@ -329,7 +330,7 @@ void nfs_inode_return_delegation_noreclaim(struct inode *inode)
 	struct nfs_inode *nfsi = NFS_I(inode);
 	struct nfs_delegation *delegation;
 
-	if (rcu_dereference(nfsi->delegation) != NULL) {
+	if (nfsi->delegation != NULL) {
 		spin_lock(&clp->cl_lock);
 		delegation = nfs_detach_delegation_locked(nfsi, NULL);
 		spin_unlock(&clp->cl_lock);
@@ -345,7 +346,7 @@ int nfs_inode_return_delegation(struct inode *inode)
 	struct nfs_delegation *delegation;
 	int err = 0;
 
-	if (rcu_dereference(nfsi->delegation) != NULL) {
+	if (nfsi->delegation != NULL) {
 		spin_lock(&clp->cl_lock);
 		delegation = nfs_detach_delegation_locked(nfsi, NULL);
 		spin_unlock(&clp->cl_lock);


[-- Attachment #3: 10889 --]
[-- Type: message/rfc822, Size: 10028 bytes --]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: David Howells <dhowells@redhat.com>
Cc: 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: Thu, 18 Mar 2010 19:25:27 -0700
Message-ID: <20100319022527.GC2894@linux.vnet.ibm.com>

On Thu, Mar 18, 2010 at 01:33:02PM +0000, David Howells wrote:
> Fix a number of RCU warnings in nfs_inode_return_delegation_noreclaim().
> nfs_inode_return_delegation_noreclaim() and nfs_inode_return_delegation() don't
> need to use rcu_dereference() outside the spinlocked region as they merely
> examin the pointer and don't follow it, thus rendering unnecessary the need to
> impose a partial ordering over the one item of interest.
> 
> nfs_detach_delegation_locked() doesn't need rcu_derefence() because it can only
> be called if nfs_client::cl_lock is held, and that guards against anyone
> changing nfsi->delegation under it.  Furthermore, the barrier in
> rcu_derefence() is superfluous, given that the spin_lock() is also a barrier.
> 
> nfs_free_delegation() should be using rcu_dereference_check() to validate the
> state that the data is in (the delegation inode must have been cleared).  By
> this point, the delegation is being released, so no one else should be
> attempting to use the saved credentials, and they can be cleared.  However,
> rcu_assign_pointer() should be used to clear them, and the delegation itself
> must still use call_rcu() as the list of delegations could be being traversed.

Thank you for fixing these up!

This looks good at the moment, however, the sparse changes that Arnd
Bergmann is working on will invalidate a couple of the changes below.
Of course, better a future problem than a here-and-now problem, but
is there an easy way to fix both?

							Thanx, Paul

> [ INFO: suspicious rcu_dereference_check() usage. ]
> ---------------------------------------------------
> fs/nfs/delegation.c:332 invoked rcu_dereference_check() without protection!
> 
> other info that might help us debug this:
> 
> 
> rcu_scheduler_active = 1, debug_locks = 0
> 2 locks held by mount.nfs4/2281:
>  #0:  (&type->s_umount_key#34){+.+...}, at: [<ffffffff810b25b4>] deactivate_super+0x60/0x80
>  #1:  (iprune_sem){+.+...}, at: [<ffffffff810c332a>] invalidate_inodes+0x39/0x13a
> 
> stack backtrace:
> Pid: 2281, comm: mount.nfs4 Not tainted 2.6.34-rc1-cachefs #110
> Call Trace:
>  [<ffffffff8105149f>] lockdep_rcu_dereference+0xaa/0xb2
>  [<ffffffffa00b4591>] nfs_inode_return_delegation_noreclaim+0x5b/0xa0 [nfs]
>  [<ffffffffa0095d63>] nfs4_clear_inode+0x11/0x1e [nfs]
>  [<ffffffff810c2d92>] clear_inode+0x9e/0xf8
>  [<ffffffff810c3028>] dispose_list+0x67/0x10e
>  [<ffffffff810c340d>] invalidate_inodes+0x11c/0x13a
>  [<ffffffff810b1dc1>] generic_shutdown_super+0x42/0xf4
>  [<ffffffff810b1ebe>] kill_anon_super+0x11/0x4f
>  [<ffffffffa009893c>] nfs4_kill_super+0x3f/0x72 [nfs]
>  [<ffffffff810b25bc>] deactivate_super+0x68/0x80
>  [<ffffffff810c6744>] mntput_no_expire+0xbb/0xf8
>  [<ffffffff810c681b>] release_mounts+0x9a/0xb0
>  [<ffffffff810c689b>] put_mnt_ns+0x6a/0x79
>  [<ffffffffa00983a1>] nfs_follow_remote_path+0x5a/0x146 [nfs]
>  [<ffffffffa0098334>] ? nfs_do_root_mount+0x82/0x95 [nfs]
>  [<ffffffffa00985a9>] nfs4_try_mount+0x75/0xaf [nfs]
>  [<ffffffffa0098874>] nfs4_get_sb+0x291/0x31a [nfs]
>  [<ffffffff810b2059>] vfs_kern_mount+0xb8/0x177
>  [<ffffffff810b2176>] do_kern_mount+0x48/0xe8
>  [<ffffffff810c810b>] do_mount+0x782/0x7f9
>  [<ffffffff810c8205>] sys_mount+0x83/0xbe
>  [<ffffffff81001eeb>] system_call_fastpath+0x16/0x1b
> 
> Also on:
> 
> fs/nfs/delegation.c:215 invoked rcu_dereference_check() without protection!
>  [<ffffffff8105149f>] lockdep_rcu_dereference+0xaa/0xb2
>  [<ffffffffa00b4223>] nfs_inode_set_delegation+0xfe/0x219 [nfs]
>  [<ffffffffa00a9c6f>] nfs4_opendata_to_nfs4_state+0x2c2/0x30d [nfs]
>  [<ffffffffa00aa15d>] nfs4_do_open+0x2a6/0x3a6 [nfs]
>  ...
> 
> And:
> 
> fs/nfs/delegation.c:40 invoked rcu_dereference_check() without protection!
>  [<ffffffff8105149f>] lockdep_rcu_dereference+0xaa/0xb2
>  [<ffffffffa00b3bef>] nfs_free_delegation+0x3d/0x6e [nfs]
>  [<ffffffffa00b3e71>] nfs_do_return_delegation+0x26/0x30 [nfs]
>  [<ffffffffa00b406a>] __nfs_inode_return_delegation+0x1ef/0x1fe [nfs]
>  [<ffffffffa00b448a>] nfs_client_return_marked_delegations+0xc9/0x124 [nfs]
>  ...
> 
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 
>  fs/nfs/delegation.c |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
> index 2563beb..fa9b7c5 100644
> --- a/fs/nfs/delegation.c
> +++ b/fs/nfs/delegation.c
> @@ -37,7 +37,8 @@ static void nfs_free_delegation(struct nfs_delegation *delegation)
>  {
>  	struct rpc_cred *cred;
> 
> -	cred = rcu_dereference(delegation->cred);
> +	cred = rcu_dereference_check(delegation->cred,
> +				     delegation->inode == NULL);
>  	rcu_assign_pointer(delegation->cred, NULL);
>  	call_rcu(&delegation->rcu, nfs_free_delegation_callback);
>  	if (cred)
> @@ -167,7 +168,7 @@ static struct inode *nfs_delegation_grab_inode(struct nfs_delegation *delegation
> 
>  static struct nfs_delegation *nfs_detach_delegation_locked(struct nfs_inode *nfsi, const nfs4_stateid *stateid)
>  {
> -	struct nfs_delegation *delegation = rcu_dereference(nfsi->delegation);
> +	struct nfs_delegation *delegation = nfsi->delegation;

Arnd's work will flag this one.

> 
>  	if (delegation == NULL)
>  		goto nomatch;
> @@ -212,7 +213,7 @@ int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, struct
>  	spin_lock_init(&delegation->lock);
> 
>  	spin_lock(&clp->cl_lock);
> -	if (rcu_dereference(nfsi->delegation) != NULL) {
> +	if (nfsi->delegation != NULL) {

And this one.  I thought that Trond said that clp->cl_lock protects
this one, in which case this should work:

	if (rcu_dereference_check(nfsi->delegation,
				  lockdep_is_held(&clp->cl_lock)) != NULL) {

>  		if (memcmp(&delegation->stateid, &nfsi->delegation->stateid,
>  					sizeof(delegation->stateid)) == 0 &&
>  				delegation->type == nfsi->delegation->type) {
> @@ -329,7 +330,7 @@ void nfs_inode_return_delegation_noreclaim(struct inode *inode)
>  	struct nfs_inode *nfsi = NFS_I(inode);
>  	struct nfs_delegation *delegation;
> 
> -	if (rcu_dereference(nfsi->delegation) != NULL) {
> +	if (nfsi->delegation != NULL) {

And this one, although the check for cp->cl_lock obviously won't work here.

>  		spin_lock(&clp->cl_lock);
>  		delegation = nfs_detach_delegation_locked(nfsi, NULL);
>  		spin_unlock(&clp->cl_lock);
> @@ -345,7 +346,7 @@ int nfs_inode_return_delegation(struct inode *inode)
>  	struct nfs_delegation *delegation;
>  	int err = 0;
> 
> -	if (rcu_dereference(nfsi->delegation) != NULL) {
> +	if (nfsi->delegation != NULL) {

Ditto...

>  		spin_lock(&clp->cl_lock);
>  		delegation = nfs_detach_delegation_locked(nfsi, NULL);
>  		spin_unlock(&clp->cl_lock);
> 

Re: INFO: suspicious rcu_dereference_check()

[David Howells]

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]

Trond Myklebust <trond.myklebust@fys.uio.no> wrote:

> I don't mind whether you or I push them to Linus, but IIRC you had a
> couple of comments about the last patchset I saw from David, so I was
> expecting to see either a reply from him or patch update. Did I miss
> that reply? (Ccing: David)

I was waiting to see what Arnd did, since Paul said:

	This looks good at the moment, however, the sparse changes that Arnd
	Bergmann is working on will invalidate a couple of the changes below.
	Of course, better a future problem than a here-and-now problem, but
	is there an easy way to fix both?

My message and Paul's reply are attached for Arnd's convenience.  Maybe Arnd
could include my changes, or supply me with his patch?

David


[-- Attachment #2: 10878 --]
[-- Type: message/rfc822, Size: 8174 bytes --]

From: David Howells <dhowells@redhat.com>
To: Trond.Myklebust@netapp.com, paulmck@linux.vnet.ibm.com
Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, David Howells <dhowells@redhat.com>
Subject: [PATCH] NFS: Fix RCU warnings in nfs_inode_return_delegation_noreclaim() [ver #2]
Date: Thu, 18 Mar 2010 13:33:02 +0000
Message-ID: <20100318133302.29754.1584.stgit-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>

Fix a number of RCU warnings in nfs_inode_return_delegation_noreclaim().
nfs_inode_return_delegation_noreclaim() and nfs_inode_return_delegation() don't
need to use rcu_dereference() outside the spinlocked region as they merely
examin the pointer and don't follow it, thus rendering unnecessary the need to
impose a partial ordering over the one item of interest.

nfs_detach_delegation_locked() doesn't need rcu_derefence() because it can only
be called if nfs_client::cl_lock is held, and that guards against anyone
changing nfsi->delegation under it.  Furthermore, the barrier in
rcu_derefence() is superfluous, given that the spin_lock() is also a barrier.

nfs_free_delegation() should be using rcu_dereference_check() to validate the
state that the data is in (the delegation inode must have been cleared).  By
this point, the delegation is being released, so no one else should be
attempting to use the saved credentials, and they can be cleared.  However,
rcu_assign_pointer() should be used to clear them, and the delegation itself
must still use call_rcu() as the list of delegations could be being traversed.


[ INFO: suspicious rcu_dereference_check() usage. ]
---------------------------------------------------
fs/nfs/delegation.c:332 invoked rcu_dereference_check() without protection!

other info that might help us debug this:


rcu_scheduler_active = 1, debug_locks = 0
2 locks held by mount.nfs4/2281:
 #0:  (&type->s_umount_key#34){+.+...}, at: [<ffffffff810b25b4>] deactivate_super+0x60/0x80
 #1:  (iprune_sem){+.+...}, at: [<ffffffff810c332a>] invalidate_inodes+0x39/0x13a

stack backtrace:
Pid: 2281, comm: mount.nfs4 Not tainted 2.6.34-rc1-cachefs #110
Call Trace:
 [<ffffffff8105149f>] lockdep_rcu_dereference+0xaa/0xb2
 [<ffffffffa00b4591>] nfs_inode_return_delegation_noreclaim+0x5b/0xa0 [nfs]
 [<ffffffffa0095d63>] nfs4_clear_inode+0x11/0x1e [nfs]
 [<ffffffff810c2d92>] clear_inode+0x9e/0xf8
 [<ffffffff810c3028>] dispose_list+0x67/0x10e
 [<ffffffff810c340d>] invalidate_inodes+0x11c/0x13a
 [<ffffffff810b1dc1>] generic_shutdown_super+0x42/0xf4
 [<ffffffff810b1ebe>] kill_anon_super+0x11/0x4f
 [<ffffffffa009893c>] nfs4_kill_super+0x3f/0x72 [nfs]
 [<ffffffff810b25bc>] deactivate_super+0x68/0x80
 [<ffffffff810c6744>] mntput_no_expire+0xbb/0xf8
 [<ffffffff810c681b>] release_mounts+0x9a/0xb0
 [<ffffffff810c689b>] put_mnt_ns+0x6a/0x79
 [<ffffffffa00983a1>] nfs_follow_remote_path+0x5a/0x146 [nfs]
 [<ffffffffa0098334>] ? nfs_do_root_mount+0x82/0x95 [nfs]
 [<ffffffffa00985a9>] nfs4_try_mount+0x75/0xaf [nfs]
 [<ffffffffa0098874>] nfs4_get_sb+0x291/0x31a [nfs]
 [<ffffffff810b2059>] vfs_kern_mount+0xb8/0x177
 [<ffffffff810b2176>] do_kern_mount+0x48/0xe8
 [<ffffffff810c810b>] do_mount+0x782/0x7f9
 [<ffffffff810c8205>] sys_mount+0x83/0xbe
 [<ffffffff81001eeb>] system_call_fastpath+0x16/0x1b

Also on:

fs/nfs/delegation.c:215 invoked rcu_dereference_check() without protection!
 [<ffffffff8105149f>] lockdep_rcu_dereference+0xaa/0xb2
 [<ffffffffa00b4223>] nfs_inode_set_delegation+0xfe/0x219 [nfs]
 [<ffffffffa00a9c6f>] nfs4_opendata_to_nfs4_state+0x2c2/0x30d [nfs]
 [<ffffffffa00aa15d>] nfs4_do_open+0x2a6/0x3a6 [nfs]
 ...

And:

fs/nfs/delegation.c:40 invoked rcu_dereference_check() without protection!
 [<ffffffff8105149f>] lockdep_rcu_dereference+0xaa/0xb2
 [<ffffffffa00b3bef>] nfs_free_delegation+0x3d/0x6e [nfs]
 [<ffffffffa00b3e71>] nfs_do_return_delegation+0x26/0x30 [nfs]
 [<ffffffffa00b406a>] __nfs_inode_return_delegation+0x1ef/0x1fe [nfs]
 [<ffffffffa00b448a>] nfs_client_return_marked_delegations+0xc9/0x124 [nfs]
 ...


Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/nfs/delegation.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 2563beb..fa9b7c5 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -37,7 +37,8 @@ static void nfs_free_delegation(struct nfs_delegation *delegation)
 {
 	struct rpc_cred *cred;
 
-	cred = rcu_dereference(delegation->cred);
+	cred = rcu_dereference_check(delegation->cred,
+				     delegation->inode == NULL);
 	rcu_assign_pointer(delegation->cred, NULL);
 	call_rcu(&delegation->rcu, nfs_free_delegation_callback);
 	if (cred)
@@ -167,7 +168,7 @@ static struct inode *nfs_delegation_grab_inode(struct nfs_delegation *delegation
 
 static struct nfs_delegation *nfs_detach_delegation_locked(struct nfs_inode *nfsi, const nfs4_stateid *stateid)
 {
-	struct nfs_delegation *delegation = rcu_dereference(nfsi->delegation);
+	struct nfs_delegation *delegation = nfsi->delegation;
 
 	if (delegation == NULL)
 		goto nomatch;
@@ -212,7 +213,7 @@ int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, struct
 	spin_lock_init(&delegation->lock);
 
 	spin_lock(&clp->cl_lock);
-	if (rcu_dereference(nfsi->delegation) != NULL) {
+	if (nfsi->delegation != NULL) {
 		if (memcmp(&delegation->stateid, &nfsi->delegation->stateid,
 					sizeof(delegation->stateid)) == 0 &&
 				delegation->type == nfsi->delegation->type) {
@@ -329,7 +330,7 @@ void nfs_inode_return_delegation_noreclaim(struct inode *inode)
 	struct nfs_inode *nfsi = NFS_I(inode);
 	struct nfs_delegation *delegation;
 
-	if (rcu_dereference(nfsi->delegation) != NULL) {
+	if (nfsi->delegation != NULL) {
 		spin_lock(&clp->cl_lock);
 		delegation = nfs_detach_delegation_locked(nfsi, NULL);
 		spin_unlock(&clp->cl_lock);
@@ -345,7 +346,7 @@ int nfs_inode_return_delegation(struct inode *inode)
 	struct nfs_delegation *delegation;
 	int err = 0;
 
-	if (rcu_dereference(nfsi->delegation) != NULL) {
+	if (nfsi->delegation != NULL) {
 		spin_lock(&clp->cl_lock);
 		delegation = nfs_detach_delegation_locked(nfsi, NULL);
 		spin_unlock(&clp->cl_lock);


[-- Attachment #3: 10889 --]
[-- Type: message/rfc822, Size: 10149 bytes --]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: David Howells <dhowells@redhat.com>
Cc: 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: Thu, 18 Mar 2010 19:25:27 -0700
Message-ID: <20100319022527.GC2894@linux.vnet.ibm.com>

On Thu, Mar 18, 2010 at 01:33:02PM +0000, David Howells wrote:
> Fix a number of RCU warnings in nfs_inode_return_delegation_noreclaim().
> nfs_inode_return_delegation_noreclaim() and nfs_inode_return_delegation() don't
> need to use rcu_dereference() outside the spinlocked region as they merely
> examin the pointer and don't follow it, thus rendering unnecessary the need to
> impose a partial ordering over the one item of interest.
> 
> nfs_detach_delegation_locked() doesn't need rcu_derefence() because it can only
> be called if nfs_client::cl_lock is held, and that guards against anyone
> changing nfsi->delegation under it.  Furthermore, the barrier in
> rcu_derefence() is superfluous, given that the spin_lock() is also a barrier.
> 
> nfs_free_delegation() should be using rcu_dereference_check() to validate the
> state that the data is in (the delegation inode must have been cleared).  By
> this point, the delegation is being released, so no one else should be
> attempting to use the saved credentials, and they can be cleared.  However,
> rcu_assign_pointer() should be used to clear them, and the delegation itself
> must still use call_rcu() as the list of delegations could be being traversed.

Thank you for fixing these up!

This looks good at the moment, however, the sparse changes that Arnd
Bergmann is working on will invalidate a couple of the changes below.
Of course, better a future problem than a here-and-now problem, but
is there an easy way to fix both?

							Thanx, Paul

> [ INFO: suspicious rcu_dereference_check() usage. ]
> ---------------------------------------------------
> fs/nfs/delegation.c:332 invoked rcu_dereference_check() without protection!
> 
> other info that might help us debug this:
> 
> 
> rcu_scheduler_active = 1, debug_locks = 0
> 2 locks held by mount.nfs4/2281:
>  #0:  (&type->s_umount_key#34){+.+...}, at: [<ffffffff810b25b4>] deactivate_super+0x60/0x80
>  #1:  (iprune_sem){+.+...}, at: [<ffffffff810c332a>] invalidate_inodes+0x39/0x13a
> 
> stack backtrace:
> Pid: 2281, comm: mount.nfs4 Not tainted 2.6.34-rc1-cachefs #110
> Call Trace:
>  [<ffffffff8105149f>] lockdep_rcu_dereference+0xaa/0xb2
>  [<ffffffffa00b4591>] nfs_inode_return_delegation_noreclaim+0x5b/0xa0 [nfs]
>  [<ffffffffa0095d63>] nfs4_clear_inode+0x11/0x1e [nfs]
>  [<ffffffff810c2d92>] clear_inode+0x9e/0xf8
>  [<ffffffff810c3028>] dispose_list+0x67/0x10e
>  [<ffffffff810c340d>] invalidate_inodes+0x11c/0x13a
>  [<ffffffff810b1dc1>] generic_shutdown_super+0x42/0xf4
>  [<ffffffff810b1ebe>] kill_anon_super+0x11/0x4f
>  [<ffffffffa009893c>] nfs4_kill_super+0x3f/0x72 [nfs]
>  [<ffffffff810b25bc>] deactivate_super+0x68/0x80
>  [<ffffffff810c6744>] mntput_no_expire+0xbb/0xf8
>  [<ffffffff810c681b>] release_mounts+0x9a/0xb0
>  [<ffffffff810c689b>] put_mnt_ns+0x6a/0x79
>  [<ffffffffa00983a1>] nfs_follow_remote_path+0x5a/0x146 [nfs]
>  [<ffffffffa0098334>] ? nfs_do_root_mount+0x82/0x95 [nfs]
>  [<ffffffffa00985a9>] nfs4_try_mount+0x75/0xaf [nfs]
>  [<ffffffffa0098874>] nfs4_get_sb+0x291/0x31a [nfs]
>  [<ffffffff810b2059>] vfs_kern_mount+0xb8/0x177
>  [<ffffffff810b2176>] do_kern_mount+0x48/0xe8
>  [<ffffffff810c810b>] do_mount+0x782/0x7f9
>  [<ffffffff810c8205>] sys_mount+0x83/0xbe
>  [<ffffffff81001eeb>] system_call_fastpath+0x16/0x1b
> 
> Also on:
> 
> fs/nfs/delegation.c:215 invoked rcu_dereference_check() without protection!
>  [<ffffffff8105149f>] lockdep_rcu_dereference+0xaa/0xb2
>  [<ffffffffa00b4223>] nfs_inode_set_delegation+0xfe/0x219 [nfs]
>  [<ffffffffa00a9c6f>] nfs4_opendata_to_nfs4_state+0x2c2/0x30d [nfs]
>  [<ffffffffa00aa15d>] nfs4_do_open+0x2a6/0x3a6 [nfs]
>  ...
> 
> And:
> 
> fs/nfs/delegation.c:40 invoked rcu_dereference_check() without protection!
>  [<ffffffff8105149f>] lockdep_rcu_dereference+0xaa/0xb2
>  [<ffffffffa00b3bef>] nfs_free_delegation+0x3d/0x6e [nfs]
>  [<ffffffffa00b3e71>] nfs_do_return_delegation+0x26/0x30 [nfs]
>  [<ffffffffa00b406a>] __nfs_inode_return_delegation+0x1ef/0x1fe [nfs]
>  [<ffffffffa00b448a>] nfs_client_return_marked_delegations+0xc9/0x124 [nfs]
>  ...
> 
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 
>  fs/nfs/delegation.c |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
> index 2563beb..fa9b7c5 100644
> --- a/fs/nfs/delegation.c
> +++ b/fs/nfs/delegation.c
> @@ -37,7 +37,8 @@ static void nfs_free_delegation(struct nfs_delegation *delegation)
>  {
>  	struct rpc_cred *cred;
> 
> -	cred = rcu_dereference(delegation->cred);
> +	cred = rcu_dereference_check(delegation->cred,
> +				     delegation->inode == NULL);
>  	rcu_assign_pointer(delegation->cred, NULL);
>  	call_rcu(&delegation->rcu, nfs_free_delegation_callback);
>  	if (cred)
> @@ -167,7 +168,7 @@ static struct inode *nfs_delegation_grab_inode(struct nfs_delegation *delegation
> 
>  static struct nfs_delegation *nfs_detach_delegation_locked(struct nfs_inode *nfsi, const nfs4_stateid *stateid)
>  {
> -	struct nfs_delegation *delegation = rcu_dereference(nfsi->delegation);
> +	struct nfs_delegation *delegation = nfsi->delegation;

Arnd's work will flag this one.

> 
>  	if (delegation == NULL)
>  		goto nomatch;
> @@ -212,7 +213,7 @@ int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, struct
>  	spin_lock_init(&delegation->lock);
> 
>  	spin_lock(&clp->cl_lock);
> -	if (rcu_dereference(nfsi->delegation) != NULL) {
> +	if (nfsi->delegation != NULL) {

And this one.  I thought that Trond said that clp->cl_lock protects
this one, in which case this should work:

	if (rcu_dereference_check(nfsi->delegation,
				  lockdep_is_held(&clp->cl_lock)) != NULL) {

>  		if (memcmp(&delegation->stateid, &nfsi->delegation->stateid,
>  					sizeof(delegation->stateid)) == 0 &&
>  				delegation->type == nfsi->delegation->type) {
> @@ -329,7 +330,7 @@ void nfs_inode_return_delegation_noreclaim(struct inode *inode)
>  	struct nfs_inode *nfsi = NFS_I(inode);
>  	struct nfs_delegation *delegation;
> 
> -	if (rcu_dereference(nfsi->delegation) != NULL) {
> +	if (nfsi->delegation != NULL) {

And this one, although the check for cp->cl_lock obviously won't work here.

>  		spin_lock(&clp->cl_lock);
>  		delegation = nfs_detach_delegation_locked(nfsi, NULL);
>  		spin_unlock(&clp->cl_lock);
> @@ -345,7 +346,7 @@ int nfs_inode_return_delegation(struct inode *inode)
>  	struct nfs_delegation *delegation;
>  	int err = 0;
> 
> -	if (rcu_dereference(nfsi->delegation) != NULL) {
> +	if (nfsi->delegation != NULL) {

Ditto...

>  		spin_lock(&clp->cl_lock);
>  		delegation = nfs_detach_delegation_locked(nfsi, NULL);
>  		spin_unlock(&clp->cl_lock);
>