[pnfs] [PATCH 1/8] pnfs: async nfs4_proc_pnfs_layoutget

[Benny Halevy]

On Mar. 10, 2008, 20:39 +0200, andros at umich.edu wrote:
> From: Andy Adamson <andros at umich.edu>
> 
> Parallel LAYOUTGET operations require an async rpc. This patch provides
> the async framework.
> 
> Signed-off-by: Andy Adamson<andros at umich.edu>
> ---
>  fs/nfs/nfs4proc.c |   75 +++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 70 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 2db50bf..bcf9ea6 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5219,24 +5219,89 @@ int nfs41_proc_async_sequence(struct nfs_client *clp, struct rpc_cred *cred)
>  #endif /* CONFIG_NFS_V4_1 */
>  
>  #ifdef CONFIG_PNFS
> -static int nfs4_proc_pnfs_layoutget(struct nfs4_pnfs_layoutget *layout)
> +static void nfs4_pnfs_layoutget_prepare(struct rpc_task *task, void *calldata)
>  {
> -	struct inode *ino = layout->args->inode;
> -	int status;
> +	struct nfs4_pnfs_layoutget *layout = calldata;
>  	struct rpc_message msg = {
>  		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_PNFS_LAYOUTGET],
>  		.rpc_argp = layout->args,
>  		.rpc_resp = layout->res,
>  	};
> +
> +	dprintk("--> %s \n", __func__);
                   ^^^
Is this white space intentional?
I presume not :)

> +	rpc_call_setup(task, &msg, 0);
> +	dprintk("<-- %s\n", __func__);
> +}
> +
> +static int
> +nfs4_pnfs_layoutget_validate(struct rpc_task *task, void *calldata)
> +{
> +	struct nfs4_pnfs_layoutget *lgp = calldata;
> +	struct inode *ino = lgp->args->inode;
>  	struct nfs_server *server = NFS_SERVER(ino);
> +	struct nfs4_session *session = server->session;
> +
> +	dprintk("--> %s \n", __func__);

ditto

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

Hmm, that won't work.
nfs4_sequence_done must be called in order to clear the
slot's busy bit...

Please reverse the order (like in all other nfs41 *_done functions)

	nfs4_sequence_done(server, &data->o_res.seq_res, data->rpc_status);
	if (RPC_ASSASSINATED(task))
		return;

> +
> +	nfs4_sequence_done(server, &lgp->res->seq_res, task->tk_status);
> +	dprintk("<-- %s\n", __func__);
> +}
> +
> +static void nfs4_pnfs_layoutget_release(void *calldata)
> +{
> +	dprintk("--> %s\n", __func__);
> +	/* pnfs_layout_release here */
> +	dprintk("<-- %s\n", __func__);
> +}
> +
> +static const struct rpc_call_ops nfs4_pnfs_layoutget_call_ops = {
> +	.rpc_call_validate_args = nfs4_pnfs_layoutget_validate,
> +	.rpc_call_prepare = nfs4_pnfs_layoutget_prepare,
> +	.rpc_call_done = nfs4_pnfs_layoutget_done,
> +	.rpc_release = nfs4_pnfs_layoutget_release,
> +};
> +
> +static int nfs4_proc_pnfs_layoutget(struct nfs4_pnfs_layoutget *lgp)
> +{
> +	struct inode *ino = lgp->args->inode;
> +	struct nfs_server *server = NFS_SERVER(ino);
> +	struct rpc_task *task;
> +	int status;
> +
> +	dprintk("--> %s \n", __func__);

ditto space

>  
>  	NFS4_VALIDATE_STATE(server);
> -	status = NFS4_RPC_CALL_SYNC(server, NFS_CLIENT(ino), &msg,
> -		layout->args, layout->res, 0);
> +	task = rpc_run_task(server->client, RPC_TASK_ASYNC,
> +			    &nfs4_pnfs_layoutget_call_ops, lgp);
> +	if (IS_ERR(task))
> +		return PTR_ERR(task);
> +	status = nfs4_wait_for_completion_rpc_task(task);
> +	if (status != 0) {
> +		smp_wmb();

Is this really needed?  You aren't writing anything in memory
(that must to be synchronously seen by some other process running
on a different, cache incoherent cpu)

Benny

> +	} else
> +		status = task->tk_status;
> +	rpc_put_task(task);
>  
> +	dprintk("<-- %s\n", __func__);
>  	return status;
>  }
>  
> +
>  static void pnfs4_proc_layoutcommit_setup(struct pnfs_layoutcommit_data *data)
>  {
>  	struct rpc_task *task = &data->task;