Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:58890 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966561AbcCPOWN (ORCPT ); Wed, 16 Mar 2016 10:22:13 -0400 Date: Wed, 16 Mar 2016 10:22:10 -0400 (EDT) From: Benjamin Coddington To: Trond Myklebust cc: Anna Schumaker , Linux NFS Mailing List Subject: Re: [PATCH] NFS: Retry a zero-length short read In-Reply-To: Message-ID: References: <79adfc63f33887475f6a0f0eeaacd8b52b2ed12c.1458119750.git.bcodding@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-nfs-owner@vger.kernel.org List-ID: On Wed, 16 Mar 2016, Trond Myklebust wrote: > On Wed, Mar 16, 2016 at 5:17 AM, Benjamin Coddington > 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. 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. > > Signed-off-by: Benjamin Coddington > > --- > > 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 > > >