[pnfs] [PATCH 02/25] pnfs: fix pnfs4_*_done routines

[Benny Halevy]

On Feb. 14, 2008, 20:21 +0200, Fred Isaman <iisaman at citi.umich.edu> wrote:
> The pnfs4_*_done routines are ignoring the possibility that the code
> fell back to nfs.  This patch chops out layoutcommit setup that is done
> there (incorrectly in the fallback case) which is already done by
> pnfs_writepages anyway.

Well, in the async case it is done in pnfs_writeback_done, but
only for pnfs_get_type(data->inode) != LAYOUT_NFSV4_FILES.
IIRC, last time I looked into this, I came to the conclusion
that the call to pnfs_update_last_write in pnfs4_write_done
is for the file layout rpc (via MDS) path.

Have you tested this patchset with over the file layout and made
sure layoutcommit is generated correctly after write?

Benny

> 
> TO CHECK: pnfs4_read_done does not do a nfs_invalidate_atime that
> nfs4_read_done does.  Does it need to be done in the fallback case?
> In that case, do we need to add a flag to data->pnfsflags?
> 
> Signed-off-by: Fred Isaman <iisaman at citi.umich.edu>
> ---
>  fs/nfs/nfs4proc.c |   22 +++++++++-------------
>  1 files changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index eaaaeed..87c7451 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3039,6 +3039,10 @@ static int pnfs4_read_done(struct rpc_task *task, struct nfs_read_data *data)
>  		return -EAGAIN;
>  	}
>  
> +	/* FRED - if we fell back to nfs, do we need to call
> +	 * nfs_invalidate_atime(data->inode) ?
> +	 */
> +
>  	/* Only renew lease if this was a read call to MDS */
>  	if (task->tk_status > 0 && !data->ds_nfs_client)
>  		renew_lease(mds_svr, data->timestamp);
> @@ -3083,15 +3087,10 @@ static int pnfs4_write_done(struct rpc_task *task, struct nfs_write_data *data)
>  	 * MDS write: renew lease
>  	 * DS write: update lastbyte written
>  	 */
> -	if (task->tk_status > 0) {
> -		if (!data->ds_nfs_client) {
> -			nfs_post_op_update_inode_force_wcc(data->inode,
> -							data->res.fattr);
> -			renew_lease(mds_svr, data->timestamp);
> -		} else
> -			pnfs_update_last_write(NFS_I(data->inode),
> -					       data->args.offset,
> -					       data->res.count);
> +	if (task->tk_status > 0 && !data->ds_nfs_client) {
> +		nfs_post_op_update_inode_force_wcc(data->inode,
> +						   data->res.fattr);
> +		renew_lease(mds_svr, data->timestamp);
>  	}
>  	return 0;
>  }
> @@ -3125,13 +3124,10 @@ static int pnfs4_commit_done(struct rpc_task *task, struct nfs_write_data *data)
>  		return -EAGAIN;
>  	}
>  
> -	if (task->tk_status >= 0) {
> +	if (task->tk_status >= 0 && !data->ds_nfs_client) {
>  		/* Update inode if commit to MDS */
>  		if (!data->ds_nfs_client)
>  			nfs_refresh_inode(data->inode, data->res.fattr);
> -
> -		/* Mark for LAYOUTCOMMIT */
> -		pnfs_need_layoutcommit(NFS_I(data->inode), data->args.context);

This might actually need to move to pnfs4_write_done after the call to 
pnfs_update_last_write if it's indeed needed for the file layout type.

>  	}
>  	dprintk("<-- %s\n", __func__);
>  	return 0;