Re: [PATCH 3/5] NFSv4: clean up state ref counting in open recover

From: "Myklebust, Trond" <Trond.Myklebust@netapp.com>
To: Weston Andros Adamson <dros@netapp.com>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 3/5] NFSv4: clean up state ref counting in open recover
Date: Mon, 28 Oct 2013 21:06:07 +0000	[thread overview]
Message-ID: <1382994367.3314.7.camel@leira.trondhjem.org> (raw)
In-Reply-To: <1382375414-5854-4-git-send-email-dros@netapp.com>

On Mon, 2013-10-21 at 13:10 -0400, Weston Andros Adamson wrote:
> There's already a valid state (the one being recovered), so just
> reference it. Also clean up error paths to avoid ref leaks.
> 
> Signed-off-by: Weston Andros Adamson <dros@netapp.com>
> ---
>  fs/nfs/nfs4proc.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 8140366..8ae1589 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -1323,14 +1323,14 @@ _nfs4_opendata_reclaim_to_nfs4_state(struct nfs4_opendata *data)
>  		goto err;
>  	}
>  
> -	ret = -ENOMEM;
> -	state = nfs4_get_open_state(inode, data->owner);
> -	if (state == NULL)
> +	/* referenced the passed state */
> +	ret = -EINVAL;
> +	if (state == NULL || !atomic_inc_not_zero(&state->count))
>  		goto err;

We already know that state != NULL, and that state->count != 0 here, so
I applied a simplified version of this patch that just does an
atomic_inc(&state->count) just before the function return.

>  
>  	ret = nfs_refresh_inode(inode, &data->f_attr);
>  	if (ret)
> -		goto err;
> +		goto err_put;
>  
>  	nfs_setsecurity(inode, &data->f_attr, data->f_label);
>  
> @@ -1340,9 +1340,12 @@ _nfs4_opendata_reclaim_to_nfs4_state(struct nfs4_opendata *data)
>  			    data->o_arg.fmode);
>  
>  	return state;
> +
> +err_put:
> +	nfs4_put_open_state(state);
> +
>  err:
>  	return ERR_PTR(ret);
> -
>  }
>  
>  static struct nfs4_state *

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com