[pnfs] [PATCH 1/1] nfsd41: Fix callback probe reference count bug

[J. Bruce Fields]

On Mon, Jan 14, 2008 at 04:36:05PM -0800, Ricardo Labiaga wrote:
> The server hits a BUG_ON() condition trying to free the nfs4_client
> structure during the callback probe.  It is releasing a reference
> to the structure with put_nfs4_client() without having first
> acquired a reference to it.
> 
> Fix by holding a reference in nfsd4*_probe_callback() before
> calling rpc_call_async().  In the vanilla NFSv4 callback probe,
> a reference is indeed acquired and a new kthread executes a
> synchronous RPC call.  This is equivalent to what we now do by
> holding a reference and scheduling an async RPC.  The reference
> is released after the async thread is done processing.

I believe these sorts of races should be gone in the nfs-server-stable
branch

	git://linux-nfs.org/~bfields/linux.git

If you have a good test case and got a chance to test with that kernel,
that could be helpful....

--b.

> 
> Signed-off-by: Ricardo Labiaga <ricardo.labiaga at netapp.com>
> ---
>  fs/nfsd/nfs4callback.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 7fb7786..22c4ca0 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -563,6 +563,12 @@ nfsd4_probe_callback(struct nfs4_client *clp)
>  		goto out_err;
>  	}
>  
> +	/*
> +	 * The task holds a reference to the nfs4_client struct.
> +	 * It is released in the callback function upon success.
> +	 */
> +	atomic_inc(&clp->cl_count);
> +
>  	status = rpc_call_async(cb->cb_client, &msg, RPC_TASK_ASYNC,
>  				&nfs4_cb_null_ops, NULL);
>  
> @@ -573,6 +579,7 @@ nfsd4_probe_callback(struct nfs4_client *clp)
>  	return;
>  
>  out_release_clp:
> +	atomic_dec(&clp->cl_count);
>  	rpc_shutdown_client(cb->cb_client);
>  out_err:
>  	cb->cb_client = NULL;
> @@ -645,6 +652,12 @@ nfsd41_probe_callback(struct nfs4_client *clp)
>  	dprintk("NFSD: %s: clp %p cb_client %p\n", __FUNCTION__,
>  		clp, clp->cl_callback.cb_client);
>  
> +	/*
> +	 * The task holds a reference to the nfs4_client struct.
> +	 * It is released in the callback function upon success.
> +	 */
> +	atomic_inc(&clp->cl_count);
> +
>  	status = rpc_call_async(cb->cb_client, &msg, RPC_TASK_ASYNC,
>  				&nfs4_cb_null_ops, NULL);
>  
> @@ -655,6 +668,7 @@ nfsd41_probe_callback(struct nfs4_client *clp)
>  	return;
>  
>  out_release_clp:
> +	atomic_dec(&clp->cl_count);
>  	rpc_shutdown_client(cb->cb_client);
>  out_err:
>  	cb->cb_client = NULL;
> -- 
> 1.5.3.3
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs