[pnfs] [PATCH 12/17] [SQUASHME] nfs41: return 0 from nfs4_read_done

Benny Halevy bhalevy at panasas.com
Wed Jun 4 04:11:20 EDT 2008


On Jun. 04, 2008, 2:40 +0300, Ricardo Labiaga <ricardo.labiaga at netapp.com> wrote:
> On Tue, 2008-06-03 at 20:40 +0300, Benny Halevy wrote:
>> nfs4 code always returned 0 from nfs4_read_done
>> there is no reason for nfs41 to change that afaict.
>>
>> Signed-off-by: Benny Halevy <bhalevy at panasas.com>
>> ---
>>  fs/nfs/nfs4proc.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 04cba79..09b3410 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -2936,8 +2936,8 @@ static int nfs4_read_done(struct rpc_task *task, struct nfs_read_data *data)
>>  	nfs_invalidate_atime(data->inode);
>>  	if (task->tk_status > 0)
>>  		renew_lease(server, data->timestamp);
>> -	dprintk("<-- %s status= 0\n", __func__);
>> -	return status;
>> +	dprintk("<-- %s status= %d. returning 0\n", __func__, status);
>> +	return 0;
> 
> We used to return status (which could have been 0 or task->tk_status).
> This now changes to always return 0.  Are you sure this is what we want?

This is how the original code looks:

git show master:fs/nfs/nfs4proc.c:
...
        nfs_invalidate_atime(data->inode);
        if (task->tk_status > 0)
                renew_lease(server, data->timestamp);
        return 0;

The question we need to ask is why we need to change that.
If there's a good reason to return status rather than 0
we need to spell it out and if it's not nfs4.1 specific we
need to send a respective patch upstream.

> 
> - ricardo
> 
>>  }
>>  
>>  static void nfs4_proc_read_setup(struct nfs_read_data *data, struct rpc_message *msg)



More information about the pNFS mailing list