[pnfs] [PATCH 08/11] pnfs: enable short read handling for all drivers

[Fredric Isaman]

On Sun, 13 Apr 2008, Benny Halevy wrote:

> On Apr. 10, 2008, 17:05 +0300, Fred Isaman <iisaman at citi.umich.edu> wrote:
>> The nfs short read infrastructure is useful even for layout drivers
>> that set PNFS_NO_RPC, so enable it.  In particular, it is necessary
>> for the correct functioning of nfs_readpage_result_partial for the
>> (NO_RPC) layout drivers to either enable short read handling, or
>> ensure they return an error in task->tk_status upon a short read.
>>
>> Signed-off-by: Fred Isaman <iisaman at citi.umich.edu>
>> ---
>>  fs/nfs/read.c |   11 +++++++----
>>  1 files changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/nfs/read.c b/fs/nfs/read.c
>> index 4055126..774a21a 100644
>> --- a/fs/nfs/read.c
>> +++ b/fs/nfs/read.c
>> @@ -375,10 +375,6 @@ static int nfs_readpage_retry(struct rpc_task *task, struct nfs_read_data *data)
>>  	struct nfs_readargs *argp = &data->args;
>>  	struct nfs_readres *resp = &data->res;
>>
>> -#ifdef CONFIG_PNFS
>> -	if (data->pnfsflags & PNFS_NO_RPC)
>> -		return 0;
>> -#endif /* CONFIG_PNFS */
>>  	if (resp->eof || resp->count == argp->count)
>>  		return 0;
>>
>> @@ -392,6 +388,13 @@ static int nfs_readpage_retry(struct rpc_task *task, struct nfs_read_data *data)
>>  	argp->offset += resp->count;
>>  	argp->pgbase += resp->count;
>>  	argp->count -= resp->count;
>> +#ifdef CONFIG_PNFS
>> +	if (data->pnfsflags & PNFS_NO_RPC) {
>> +		/* Note can't use return value, shouldn't really have one */
>
> Hmm, I don't really like that.
> What happens if we fail to get the layout?
>

Good point.  I'm going to have to think on this.

>> +		pnfs_try_to_read_data(data, data->call_ops);
>> +		return -EAGAIN;
>> +	}
>> +#endif /* CONFIG_PNFS */
>>  	rpc_restart_call(task);
>
> Remind me why not go through rpc_restart_call for the residue?
> (i.e. also for the PNFS_NO_RPC path)
>

Because the cleanup code is called directly, as opposed to from the RPC 
state machine.  So rpc_restart_call ends up being basically a nop.

 	Fred

>>  	return -EAGAIN;
>>  }
>
>