Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220901183341.1543827-1-anna@kernel.org> <0125F316-1D9C-4A0E-B1B9-1919C9F26718@oracle.com>
In-Reply-To: <0125F316-1D9C-4A0E-B1B9-1919C9F26718@oracle.com>
From:   Anna Schumaker <anna@kernel.org>
Date:   Tue, 6 Sep 2022 14:17:51 -0400
Message-ID: <CAFX2JfnQ=KygdjYMk3uTy2T9N+We3BH_xiWT8eCG9Y7rrVOD3Q@mail.gmail.com>
Subject: Re: [PATCH 0/1] NFSD: Simplify READ_PLUS
To:     Chuck Lever III <chuck.lever@oracle.com>
Cc:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Chuck,

On Sat, Sep 3, 2022 at 1:36 PM Chuck Lever III <chuck.lever@oracle.com> wrote:
>
>
>
> > On Sep 1, 2022, at 2:33 PM, Anna Schumaker <anna@kernel.org> wrote:
> >
> > Chuck, I tried to add in sparse read support by adding this extra
> > change. Unfortunately it leads to a bunch of new failing xfstests. Do
> > you have any thoughts about what might be going on? Is the patch okay
> > without the splice support?
> >
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index adbff7737c14..e21e6cfd1c6d 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -4733,6 +4733,7 @@ static __be32
> > nfsd4_encode_read_plus_data(struct nfsd4_compoundres *resp,
> >                           struct nfsd4_read *read)
> > {
> > +     bool splice_ok = test_bit(RQ_SPLICE_OK, &resp->rqstp->rq_flags);
> >       unsigned long maxcount;
> >       struct xdr_stream *xdr = resp->xdr;
> >       struct file *file = read->rd_nf->nf_file;
> > @@ -4747,7 +4748,10 @@ nfsd4_encode_read_plus_data(struct nfsd4_compoundres *resp,
> >       maxcount = min_t(unsigned long, read->rd_length,
> >                        (xdr->buf->buflen - xdr->buf->len));
> >
> > -     nfserr = nfsd4_encode_readv(resp, read, file, maxcount);
> > +     if (file->f_op->splice_read && splice_ok)
> > +             nfserr = nfsd4_encode_splice_read(resp, read, file, maxcount);
> > +     else
> > +             nfserr = nfsd4_encode_readv(resp, read, file, maxcount)
> >       if (nfserr)
> >               return nfserr;
>
> I applied the above change to a test server, and was able to reproduce
> a bunch of new test failures when using NFSv4.2. I confirmed using nfsd
> tracepoints that splice read and READ_PLUS is being used.
>
> I then expanded the test. When using an XFS-based export, I reproduced
> the failures. But I was not able to reproduce these failures with
> exports based on tmpfs, btrfs, or ext4. Again, I confirmed using nfsd
> tracepoints that splice read was being used, and mountstats on my
> client showed READ_PLUS is being used.
>
> Then I tried testing the XFS-backed export with NFSv4.1, and found
> that most of the failures appeared again. Once again, I confirmed
> using nfsd tracepoints that splice read is being used during the tests.
>
> Can you confirm that you see test failures with NFSv4.1 and XFS but
> not with NFSv4.2 / READ_PLUS with btrfs, ext4, or tmpfs?

I can confirm that I'm seeing the same failures with NFS v4.1 and xfs,
but not with v4.2 and ext4. I didn't test btrfs or tmpfs, since the
ext4 test passed.

Should I re-add the splice change for v2 of this patch, in addition to
addressing the other comments you had?

Anna

>
>
> --
> Chuck Lever
>
>
>