[pnfs] [PATCH 6/8] pnfs: async pnfs4_proc_layoutreturn

[Benny Halevy]

On Mar. 10, 2008, 20:39 +0200, andros at umich.edu wrote:
> From: Andy Adamson <andros at umich.edu>
> 
> Parallel LAYOUTRETURN operations require an async rpc. This patch
> provides the async framework
> 
> Signed-off-by: Andy Adamson<andros at umich.edu>
> ---
>  fs/nfs/nfs4proc.c |   90 +++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 81 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 2ffebe5..3634814 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5370,26 +5370,98 @@ static int pnfs4_proc_layoutcommit(struct pnfs_layoutcommit_data *data)
>  	return err;
>  }
>  
> -static int pnfs4_proc_layoutreturn(struct nfs4_pnfs_layoutreturn *layout)
> +static void
> +nfs4_pnfs_layoutreturn_prepare(struct rpc_task *task, void *calldata)
>  {
> -	struct inode *ino = layout->args->inode;
> +	struct nfs4_pnfs_layoutreturn *lrp = calldata;
>  	struct rpc_message msg = {
>  		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_PNFS_LAYOUTRETURN],
> -		.rpc_argp = layout->args,
> -		.rpc_resp = layout->res,
> +		.rpc_argp = lrp->args,
> +		.rpc_resp = lrp->res,
>  	};
> +
> +	dprintk("--> %s\n", __func__);
> +	rpc_call_setup(task, &msg, 0);
> +	dprintk("<-- %s\n", __func__);
> +}
> +
> +static int
> +nfs4_pnfs_layoutreturn_validate(struct rpc_task *task, void *calldata)
> +{
> +	struct nfs4_pnfs_layoutreturn *lrp = calldata;
> +	struct inode *ino = lrp->args->inode;
> +	struct nfs_server *server = NFS_SERVER(ino);
> +	struct nfs4_session *session = server->session;
> +
> +	dprintk("--> %s \n", __func__);

nit: trailing white space in message.

> +	return nfs41_call_validate_seq_args(server, session,
> +					    &lrp->args->seq_args,
> +					    &lrp->res->seq_res,
> +					    0, task);
> +}
> +
> +static void nfs4_pnfs_layoutreturn_done(struct rpc_task *task, void *calldata)
> +{
> +	struct nfs4_pnfs_layoutreturn *lrp = calldata;
> +	struct inode *ino = lrp->args->inode;
> +	struct nfs_server *server = NFS_SERVER(ino);
> +
> +	dprintk("--> %s\n", __func__);
> +	if (RPC_ASSASSINATED(task))
> +		return;
> +
> +	nfs4_sequence_done(server, &lrp->res->seq_res, task->tk_status);

sequence_done must be called before the check for RPC_ASSASSINATED.

> +	dprintk("<-- %s\n", __func__);
> +}
> +
> +static void nfs4_pnfs_layoutreturn_release(void *calldata)
> +{
> +	struct nfs4_pnfs_layoutreturn *lrp = calldata;
> +	struct nfs_inode *nfsi = NFS_I(lrp->args->inode);
> +	struct pnfs_layout_type *lo;
> +
> +	lo = nfsi->current_layout;
> +	BUG_ON(!lo);
> +
> +	dprintk("--> %s\n", __func__);
> +	pnfs_layout_release(lo);
> +	dprintk("<-- %s\n", __func__);
> +}
> +
> +static const struct rpc_call_ops nfs4_pnfs_layoutreturn_call_ops = {
> +	.rpc_call_validate_args = nfs4_pnfs_layoutreturn_validate,
> +	.rpc_call_prepare = nfs4_pnfs_layoutreturn_prepare,
> +	.rpc_call_done = nfs4_pnfs_layoutreturn_done,
> +	.rpc_release = nfs4_pnfs_layoutreturn_release,
> +};
> +
> +static int pnfs4_proc_layoutreturn(struct nfs4_pnfs_layoutreturn *lrp)

I'm not sure about the allocation requirements for lrp
since it's being passed to the async rpc that may be executed in another
thread's context. Although we synchronously wait for completion
it might by safer to allocate and copy the nfs4_pnfs_layoutreturn arg and
release it when done.

> +{
> +	struct inode *ino = lrp->args->inode;
> +	struct nfs_server *server = NFS_SERVER(ino);
> +	struct rpc_task *task;
>  	int status;
> -	NFS4_VALIDATE_STATE(NFS_SERVER(ino));
>  
> -	status = NFS4_RPC_CALL_SYNC(NFS_SERVER(ino), NFS_CLIENT(ino), &msg,
> -						layout->args, layout->res, 0);
> -	dprintk("NFS reply layoutreturn: %d\n", status);
> +	dprintk("-->%s \n", __func__);

nit: trailing white space in message.

> +
> +	NFS4_VALIDATE_STATE(NFS_SERVER(ino));
> +	task = rpc_run_task(server->client, RPC_TASK_ASYNC,
> +				&nfs4_pnfs_layoutreturn_call_ops, lrp);
> +	if (IS_ERR(task))
> +		return PTR_ERR(task);
> +	status = nfs4_wait_for_completion_rpc_task(task);
> +	if (status != 0) {
> +		smp_wmb();

no need for that AFAICT.

> +	} else
> +		status = task->tk_status;
> +	rpc_put_task(task);
>  
> +	dprintk("<-- %s\n", __func__);
>  	return status;
>  }
>  
>  /*
> - * Retrieve the list of Data Server devices from the MDS.
> + * petrieve the list of Data Server devices from the MDS.

eh?
My spell checker complains about that ;-)

>   */
>  static int _nfs4_pnfs_getdevicelist(struct nfs_fh *fh,
>  				    struct nfs_server *server,