Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210331193025.25724-1-olga.kornievskaia@gmail.com>
 <0ca40f087491acec8f26816b43b6d64bb624c35e.camel@hammerspace.com>
 <CAN-5tyHQyHO8K7UjwhKhN9Xoz1nSXMB2cv8De=w9Rx-qphaHgg@mail.gmail.com> <93c81355cfe7df2d0f42fae52a84c869a4298cef.camel@hammerspace.com>
In-Reply-To: <93c81355cfe7df2d0f42fae52a84c869a4298cef.camel@hammerspace.com>
From:   Olga Kornievskaia <olga.kornievskaia@gmail.com>
Date:   Wed, 31 Mar 2021 17:25:30 -0400
Message-ID: <CAN-5tyHhL5xNaWO1kw0Z5FBbngT8YHa_GJxaXu1wFLWB6-r33Q@mail.gmail.com>
Subject: Re: [PATCH 1/1] NFSv4.2 fix handling of sr_eof in SEEK's reply
To:     Trond Myklebust <trondmy@hammerspace.com>
Cc:     "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
        "anna.schumaker@netapp.com" <anna.schumaker@netapp.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Mar 31, 2021 at 5:07 PM Trond Myklebust <trondmy@hammerspace.com> wrote:
>
> On Wed, 2021-03-31 at 16:34 -0400, Olga Kornievskaia wrote:
> > On Wed, Mar 31, 2021 at 3:42 PM Trond Myklebust
> > <trondmy@hammerspace.com> wrote:
> > >
> > > On Wed, 2021-03-31 at 15:30 -0400, Olga Kornievskaia wrote:
> > > > From: Olga Kornievskaia <kolga@netapp.com>
> > > >
> > > > Currently the client ignores the value of the sr_eof of the SEEK
> > > > operation. According to the spec, if the server didn't find the
> > > > requested extent and reached the end of the file, the server
> > > > would return sr_eof=true. In case the request for DATA and no
> > > > data was found (ie in the middle of the hole), then the lseek
> > > > expects that ENXIO would be returned.
> > > >
> > > > Fixes: 1c6dcbe5ceff8 ("NFS: Implement SEEK")
> > > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> > > > ---
> > > >  fs/nfs/nfs42proc.c | 5 ++++-
> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> > > > index 094024b0aca1..d359e712c11d 100644
> > > > --- a/fs/nfs/nfs42proc.c
> > > > +++ b/fs/nfs/nfs42proc.c
> > > > @@ -659,7 +659,10 @@ static loff_t _nfs42_proc_llseek(struct file
> > > > *filep,
> > > >         if (status)
> > > >                 return status;
> > > >
> > > > -       return vfs_setpos(filep, res.sr_offset, inode->i_sb-
> > > > > s_maxbytes);
> > > > +       if (whence == SEEK_DATA && res.sr_eof)
> > > > +               return -NFS4ERR_NXIO;
> > > > +       else
> > > > +               return vfs_setpos(filep, res.sr_offset, inode-
> > > > >i_sb-
> > > > > s_maxbytes);
> > > >  }
> > > >
> > > >  loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int
> > > > whence)
> > >
> > > Don't we also need to deal with SEEK_HOLE with the offset being
> > > greater
> > > than the end-of-file in the same way?
> >
> > We do not because if there is no hole extent after the requested
> > offset, then there is an implied hole which is at the end of the file.
> > So if sr_eof is true we still need to pay attention to the returned
> > offset (ie it should be end of the file) and it's not an error
> > condition.
>
> I'm asking because according to the manpage for lseek():
>
>        ENXIO  whence is SEEK_DATA or SEEK_HOLE, and offset is beyond  the  end
>               of  the file, or whence is SEEK_DATA and offset is within a hole
>               at the end of the file.
>
> i.e. the manpage implies that we should also be returning an error for
> SEEK_HOLE if the supplied offset is beyond eof. Are you saying that we
> currently do so?

Yes the server should do that. Client doesn't need to do anything
extra. This patch is to handle the discrepancy between what the
NFSv4.2 spec says and what the linux lseek expects (specifically the
last condition is what linux expects but spec says it has to be a
non-error condition).

>
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com
>
>