Re: [PATCH] NFS: Retry a zero-length short read

From: Benjamin Coddington <bcodding@redhat.com>
To: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Anna Schumaker <anna.schumaker@netapp.com>,
	Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] NFS: Retry a zero-length short read
Date: Wed, 16 Mar 2016 10:56:02 -0400 (EDT)	[thread overview]
Message-ID: <alpine.OSX.2.19.9992.1603161050370.11199@planck> (raw)
In-Reply-To: <CAHQdGtThhkdmiYUgbWq1Jz00K8CKtOsgfxJUADhHXe5d3ygrSQ@mail.gmail.com>

On Wed, 16 Mar 2016, Trond Myklebust wrote:

> On Wed, Mar 16, 2016 at 10:22 AM, Benjamin Coddington
> <bcodding@redhat.com> wrote:
> > On Wed, 16 Mar 2016, Trond Myklebust wrote:
> >
> >> On Wed, Mar 16, 2016 at 5:17 AM, Benjamin Coddington
> >> <bcodding@redhat.com> wrote:
> >> >
> >> > A zero-length short read without eof should be retried rather than sending
> >> > an error to the application.
> >>
> >>
> >> In what situation would returning a 0 length read not be a bug? If the
> >> server intended that we back off and retry, it has the alternative of
> >> sending a JUKEBOX/DELAY error.
> >
> > If the server completes a local read but then another writer comes in and
> > appends to the file before the server checks if it needs to set EOF, then
> > the response might be 0 length without EOF set.
>
> Why isn't that EOF check done atomically with the read itself? This
> still sounds like a server bug to me.

I don't know -- I would guess because doing that atomically is harder than
not, and I don't know where the RFCs say that a zero length response without
eof is to be treated as an error or condition to be avoided.

I'll look into that, and respond here.

> > I'm also using https://tools.ietf.org/html/rfc7530#section-16.23.5 to guide
> > how I think the client should behave; it says that the client should retry
> > a short read without eof set.  I think that should include a response with
> > 0 length.

Here's the verbatim from section 12.23.5:

   If the server returns a "short read" (i.e., less data than requested
   and eof is set to FALSE), the client should send another READ to get
   the remaining data.  A server may return less data than requested
   under several circumstances.  The file may have been truncated by
   another client or perhaps on the server itself, changing the file
   size from what the requesting client believes to be the case.  This
   would reduce the actual amount of data available to the client.  It
   is possible that the server reduces the transfer size and so returns
   a short read result.  Server resource exhaustion may also result in a
   short read.

Ben

> >> > Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> >> > ---
> >> >  fs/nfs/read.c |    5 -----
> >> >  1 files changed, 0 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/fs/nfs/read.c b/fs/nfs/read.c
> >> > index eb31e23..7269d42 100644
> >> > --- a/fs/nfs/read.c
> >> > +++ b/fs/nfs/read.c
> >> > @@ -244,11 +244,6 @@ static void nfs_readpage_retry(struct rpc_task *task,
> >> >
> >> >         /* This is a short read! */
> >> >         nfs_inc_stats(hdr->inode, NFSIOS_SHORTREAD);
> >> > -       /* Has the server at least made some progress? */
> >> > -       if (resp->count == 0) {
> >> > -               nfs_set_pgio_error(hdr, -EIO, argp->offset);
> >> > -               return;
> >> > -       }
> >> >
> >> >         /* For non rpc-based layout drivers, retry-through-MDS */
> >> >         if (!task->tk_ops) {
> >> > --
> >> > 1.7.1
> >> >
> >>
>