Re: [PATCH 1/2] NFSv4: Fix the locking innfs_inode_reclaim_delegation()

[Paul E. McKenney]

On Tue, Apr 20, 2010 at 11:26:08AM +0100, David Howells wrote:
> From: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
> 
> Ensure that we correctly rcu-dereference the delegation itself, and that we
> protect against removal while we're changing the contents.

I queued both of these, thank you Trond and David!

Trond, if you would rather carry these in your tree, please let me know.

							Thanx, Paul

> Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
> Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> ---
> 
>  fs/nfs/delegation.c |   42 ++++++++++++++++++++++++++++--------------
>  1 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
> index 1567124..8d9ec49 100644
> --- a/fs/nfs/delegation.c
> +++ b/fs/nfs/delegation.c
> @@ -129,21 +129,35 @@ again:
>   */
>  void nfs_inode_reclaim_delegation(struct inode *inode, struct rpc_cred *cred, struct nfs_openres *res)
>  {
> -	struct nfs_delegation *delegation = NFS_I(inode)->delegation;
> -	struct rpc_cred *oldcred;
> +	struct nfs_delegation *delegation;
> +	struct rpc_cred *oldcred = NULL;
> 
> -	if (delegation == NULL)
> -		return;
> -	memcpy(delegation->stateid.data, res->delegation.data,
> -			sizeof(delegation->stateid.data));
> -	delegation->type = res->delegation_type;
> -	delegation->maxsize = res->maxsize;
> -	oldcred = delegation->cred;
> -	delegation->cred = get_rpccred(cred);
> -	clear_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags);
> -	NFS_I(inode)->delegation_state = delegation->type;
> -	smp_wmb();
> -	put_rpccred(oldcred);
> +	rcu_read_lock();
> +	delegation = rcu_dereference(NFS_I(inode)->delegation);
> +	if (delegation != NULL) {
> +		spin_lock(&delegation->lock);
> +		if (delegation->inode != NULL) {
> +			memcpy(delegation->stateid.data, res->delegation.data,
> +			       sizeof(delegation->stateid.data));
> +			delegation->type = res->delegation_type;
> +			delegation->maxsize = res->maxsize;
> +			oldcred = delegation->cred;
> +			delegation->cred = get_rpccred(cred);
> +			clear_bit(NFS_DELEGATION_NEED_RECLAIM,
> +				  &delegation->flags);
> +			NFS_I(inode)->delegation_state = delegation->type;
> +			spin_unlock(&delegation->lock);
> +			put_rpccred(oldcred);
> +			rcu_read_unlock();
> +		} else {
> +			/* We appear to have raced with a delegation return. */
> +			spin_unlock(&delegation->lock);
> +			rcu_read_unlock();
> +			nfs_inode_set_delegation(inode, cred, res);
> +		}
> +	} else {
> +		rcu_read_unlock();
> +	}
>  }
> 
>  static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation *delegation, int issync)
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/