Messages in this thread | | | From | David Howells <> | Subject | Re: [PATCH] NFS: Fix RCU warnings in nfs_inode_return_delegation_noreclaim() [ver #2] | Date | Mon, 29 Mar 2010 20:02:28 +0100 |
| |
Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> > 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 clp->cl_lock protects this pointer, why the need for rcu_dereference_check() at all? The check is redundant since the line above gets the very lock we're checking for.
> > - 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);
On this one, why does nfsi->delegation need a memory barrier interpolating afterwards? It has an implicit one in the form of the spin_lock() immediately after, if the value of the pointer wasn't NULL. What two memory accesses is the memory barrier ordering?
Ditto on the next one.
David
| |