> On Sep 6, 2022, at 2:17 PM, Anna Schumaker <anna@xxxxxxxxxx> wrote:
>
> Hi Chuck,
>
> On Sat, Sep 3, 2022 at 1:36 PM Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote:
>>
>>
>>
>>> On Sep 1, 2022, at 2:33 PM, Anna Schumaker <anna@xxxxxxxxxx> 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?
Yes.
Given the comment in nfsd4_read() I think READ_PLUS can't use splice
read for anything but the last data segment and only if this READ_PLUS
is the final operation in the COMPOUND. That will need some attention
at some later point.
Meanwhile I'm going to try to bisect the XFS READ failures right now.
--
Chuck Lever
