[PATCH] [fs/9p] Let the read retry on short reads.

[PATCH] [fs/9p] Let the read retry on short reads.

[Venkateswararao Jujjuri (JV)]

A simple fix to retry on short reads.

Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
---
 fs/9p/vfs_file.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
index 2695491..cae984d 100644
--- a/fs/9p/vfs_file.c
+++ b/fs/9p/vfs_file.c
@@ -166,7 +166,7 @@ v9fs_file_readn(struct file *filp, char *data, char __user *udata, u32 count,
 		offset += n;
 		count -= n;
 		total += n;
-	} while (count > 0 && n == size);
+	} while (count > 0);
 
 	if (n < 0)
 		total = n;
-- 
1.6.5.2

Re: [V9fs-developer] [PATCH] [fs/9p] Let the read retry on short reads.

[Latchesar Ionkov]

What problem does that change solve? It adds an additional call when
EOF is reached. The way most user apps are written, that'll mean two
calls that return Rread count 0.

I was thinking of doing a similar change, but decided against it.

Thanks,
    Lucho

On Tue, Aug 17, 2010 at 12:46 PM, Venkateswararao Jujjuri (JV)
<jvrao@linux.vnet.ibm.com> wrote:
> A simple fix to retry on short reads.
>
> Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
> ---
>  fs/9p/vfs_file.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
> index 2695491..cae984d 100644
> --- a/fs/9p/vfs_file.c
> +++ b/fs/9p/vfs_file.c
> @@ -166,7 +166,7 @@ v9fs_file_readn(struct file *filp, char *data, char __user *udata, u32 count,
>                offset += n;
>                count -= n;
>                total += n;
> -       } while (count > 0 && n == size);
> +       } while (count > 0);
>
>        if (n < 0)
>                total = n;
> --
> 1.6.5.2
>
>
> ------------------------------------------------------------------------------
> This SF.net email is sponsored by
>
> Make an app they can't live without
> Enter the BlackBerry Developer Challenge
> http://p.sf.net/sfu/RIM-dev2dev
> _______________________________________________
> V9fs-developer mailing list
> V9fs-developer@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/v9fs-developer
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [V9fs-developer] [PATCH] [fs/9p] Let the read retry on short reads.

[Venkateswararao Jujjuri (JV)]

Latchesar Ionkov wrote:
> What problem does that change solve? It adds an additional call when
> EOF is reached. The way most user apps are written, that'll mean two
> calls that return Rread count 0.
> 
> I was thinking of doing a similar change, but decided against it.

Yeah I ran into the issue when tried with o_direct and dd.

Can we depend on the application retrying?
Yes I also observed the behavior you are mentioning (2 Rreads returning 0)
Wondering if we have a better way to address this.

Thanks,
JV

> 
> Thanks,
>     Lucho
> 
> On Tue, Aug 17, 2010 at 12:46 PM, Venkateswararao Jujjuri (JV)
> <jvrao@linux.vnet.ibm.com> wrote:
>> A simple fix to retry on short reads.
>>
>> Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
>> ---
>>  fs/9p/vfs_file.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
>> index 2695491..cae984d 100644
>> --- a/fs/9p/vfs_file.c
>> +++ b/fs/9p/vfs_file.c
>> @@ -166,7 +166,7 @@ v9fs_file_readn(struct file *filp, char *data, char __user *udata, u32 count,
>>                offset += n;
>>                count -= n;
>>                total += n;
>> -       } while (count > 0 && n == size);
>> +       } while (count > 0);
>>
>>        if (n < 0)
>>                total = n;
>> --
>> 1.6.5.2
>>
>>
>> ------------------------------------------------------------------------------
>> This SF.net email is sponsored by
>>
>> Make an app they can't live without
>> Enter the BlackBerry Developer Challenge
>> http://p.sf.net/sfu/RIM-dev2dev
>> _______________________________________________
>> V9fs-developer mailing list
>> V9fs-developer@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/v9fs-developer
>>

Re: [V9fs-developer] [PATCH] [fs/9p] Let the read retry on short reads.

[Latchesar Ionkov]

The only solution I can think of is to use the iounit value the file
server returns (and not the msize as we do now). If iounit is 0, allow
short reads, if iounit is more than zero and the read returns less
that the iounit, don't try anymore.

This kind of passes the problem to the file server, but I guess that's
the right place to solve it anyway.

Thanks,
    Lucho

On Tue, Aug 17, 2010 at 2:10 PM, Venkateswararao Jujjuri (JV)
<jvrao@linux.vnet.ibm.com> wrote:
> Latchesar Ionkov wrote:
>> What problem does that change solve? It adds an additional call when
>> EOF is reached. The way most user apps are written, that'll mean two
>> calls that return Rread count 0.
>>
>> I was thinking of doing a similar change, but decided against it.
>
> Yeah I ran into the issue when tried with o_direct and dd.
>
> Can we depend on the application retrying?
> Yes I also observed the behavior you are mentioning (2 Rreads returning 0)
> Wondering if we have a better way to address this.
>
> Thanks,
> JV
>
>>
>> Thanks,
>>     Lucho
>>
>> On Tue, Aug 17, 2010 at 12:46 PM, Venkateswararao Jujjuri (JV)
>> <jvrao@linux.vnet.ibm.com> wrote:
>>> A simple fix to retry on short reads.
>>>
>>> Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
>>> ---
>>>  fs/9p/vfs_file.c |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
>>> index 2695491..cae984d 100644
>>> --- a/fs/9p/vfs_file.c
>>> +++ b/fs/9p/vfs_file.c
>>> @@ -166,7 +166,7 @@ v9fs_file_readn(struct file *filp, char *data, char __user *udata, u32 count,
>>>                offset += n;
>>>                count -= n;
>>>                total += n;
>>> -       } while (count > 0 && n == size);
>>> +       } while (count > 0);
>>>
>>>        if (n < 0)
>>>                total = n;
>>> --
>>> 1.6.5.2
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> This SF.net email is sponsored by
>>>
>>> Make an app they can't live without
>>> Enter the BlackBerry Developer Challenge
>>> http://p.sf.net/sfu/RIM-dev2dev
>>> _______________________________________________
>>> V9fs-developer mailing list
>>> V9fs-developer@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/v9fs-developer
>>>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [V9fs-developer] [PATCH] [fs/9p] Let the read retry on short reads.

[Eric Van Hensbergen]

On Tue, Aug 17, 2010 at 3:39 PM, Latchesar Ionkov <lucho@ionkov.net> wrote:
> The only solution I can think of is to use the iounit value the file
> server returns (and not the msize as we do now). If iounit is 0, allow
> short reads, if iounit is more than zero and the read returns less
> that the iounit, don't try anymore.
>
> This kind of passes the problem to the file server, but I guess that's
> the right place to solve it anyway.
>

What's the potential impact on existing serves?  What does Plan 9 expect?

read(5) says:
The count field in the reply indicates the number of bytes returned.
This may be less than the requested amount. If the offset field is
greater than or equal to the number of bytes in the file, a count of
zero will be returned.

I'd say if we wanted to do something different, I would need to be
conditional by 9p2000.u/9p2000.L -- I don't want to break existing
servers/clients that might rely on this behavior to frame messages
(like pipes, the IP stack, etc.)

         -eric

Re: [V9fs-developer] [PATCH] [fs/9p] Let the read retry on short reads.

[Latchesar Ionkov]

The potential impact is that if the existing server returns iounit==0,
there will be two Rread count 0. Which is perfectly fine with 9P, just
a bit slower. I am not sure how Plan9 handles the case.

Thanks,
    Lucho

On Mon, Aug 23, 2010 at 10:11 AM, Eric Van Hensbergen <ericvh@gmail.com> wrote:
> On Tue, Aug 17, 2010 at 3:39 PM, Latchesar Ionkov <lucho@ionkov.net> wrote:
>> The only solution I can think of is to use the iounit value the file
>> server returns (and not the msize as we do now). If iounit is 0, allow
>> short reads, if iounit is more than zero and the read returns less
>> that the iounit, don't try anymore.
>>
>> This kind of passes the problem to the file server, but I guess that's
>> the right place to solve it anyway.
>>
>
> What's the potential impact on existing serves?  What does Plan 9 expect?
>
> read(5) says:
> The count field in the reply indicates the number of bytes returned.
> This may be less than the requested amount. If the offset field is
> greater than or equal to the number of bytes in the file, a count of
> zero will be returned.
>
> I'd say if we wanted to do something different, I would need to be
> conditional by 9p2000.u/9p2000.L -- I don't want to break existing
> servers/clients that might rely on this behavior to frame messages
> (like pipes, the IP stack, etc.)
>
>         -eric
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html