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

[Fredric Isaman]

On Fri, 15 Feb 2008, Benny Halevy wrote:

> 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.
>

Actually, I was alooking at the call near the end of pnfs_writepages.
I hadn't noticed it was also (conditionally) called during writeback_done.


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

No, since patch 01 was guaranteed to break file layout.

> 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;
>
>