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;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Anna Schumaker <anna@kernel.org>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 0/1] NFSD: Simplify READ_PLUS
Thread-Topic: [PATCH 0/1] NFSD: Simplify READ_PLUS
Thread-Index: AQHYvjFgBg77Wrnv30+1UKhkeKONIa3N+vyAgATClICAAAFzAA==
Date:   Tue, 6 Sep 2022 18:23:02 +0000
Message-ID: <B09AA7C8-A15A-444D-8B29-766B1BB67876@oracle.com>
References: <20220901183341.1543827-1-anna@kernel.org>
 <0125F316-1D9C-4A0E-B1B9-1919C9F26718@oracle.com>
 <CAFX2JfnQ=KygdjYMk3uTy2T9N+We3BH_xiWT8eCG9Y7rrVOD3Q@mail.gmail.com>
In-Reply-To: <CAFX2JfnQ=KygdjYMk3uTy2T9N+We3BH_xiWT8eCG9Y7rrVOD3Q@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?xHNrzMblMb94RLlULolOdrdhavivxrz770Yqp3jmTGbZ/apUY7YNNkMsVk3v?=
 =?us-ascii?Q?vKofh7YTmSbskNKvDEU3FoP/UgPiNmIDpVmz627Pfoa+1MROe02USsKGu+p9?=
 =?us-ascii?Q?LBE1vvSF9tv9lyT9tMltTd+TKuSup5vldf5dJ4qP5av4iOo5SC+0I66CO5VZ?=
 =?us-ascii?Q?TcopEXKGiFFpjkMrZE2eXHtKgdH7f1urNrFh7Y6NDIMzkB36HHR+tVxix8Th?=
 =?us-ascii?Q?rldXymg8mgp+chuNVeRxg6d+G/6L9hFCq0zrKZX+rn16GLslJQxuvuRcvvHe?=
 =?us-ascii?Q?jG1oP6R5Bzlvwh0e4LP/2YOAr7FbLtqezNnPk1+ZD+Ei+vbysk/aInzze+MP?=
 =?us-ascii?Q?SWTycq1MM/+27arsYaM9Y64PxTei8pisfy9Myt8Yujxoe6H8p5zdkn5veCyU?=
 =?us-ascii?Q?Ew/GrwbKoUdMOHNc2uZ7tsGKBScagPlsEdsOjb6Dn1FPfr0nCnRsKd/Osaku?=
 =?us-ascii?Q?wbTWPbiTDSM49rASLnzmZfSn/5jiQF2OcTtJ5p4mSUqqfoDchqNip9ZvuxWR?=
 =?us-ascii?Q?3Fxia0iQujdwEqpOlnbN0pdGaj97rlK+aCEeAD63RE3Zbx9vS92h4vTjWhS/?=
 =?us-ascii?Q?hSAHrbNdUf0mTh3p8ewt7B9IHVmho+YR2zd/Nwfx0iagIukujfKMaPh84iGx?=
 =?us-ascii?Q?inqWYus/iWgXlIPuYL7G5cpKTF6y6vOAt/Ky/tu8QSwza9QWMv1GifFbI4TS?=
 =?us-ascii?Q?zA5XFEdmTPJ/Li0l7/2ucDVK03psxEu2lIzG1B7xl4AfeoezZZH+/7BZJ+FX?=
 =?us-ascii?Q?MtT82ItVnfkQ+x24QTNHSK6qwMNKBOFS3ExcrUoc5qMxmKLcGSuXLVf1jJ06?=
 =?us-ascii?Q?LFnRhiDBel0ZhN6xpEJBL0BY9gYL/yCqj+NO3EPZqBOVvNdZiZHzoFljGKI4?=
 =?us-ascii?Q?OgCHbnHV17LRaWFgd688Yt5qyQeaBfiAoEamOsl6HJfOr/pC/Qvwsp25x2KG?=
 =?us-ascii?Q?x0RKR1hljZvOVX6R0uTVKuPPfQI3ODjBYZ8lWfWFcty3q79dEfinvL8pI8DO?=
 =?us-ascii?Q?zBeSLow3bRUgMAgwPuTHEMXnU21qDKcjX9sSOgi7ZxVdNXVKzMGBADE1krwr?=
 =?us-ascii?Q?VuXY7URVeAN7yoifYy7GV4JJuyw/asoAIBPuxYh4rCX1rVv1tsNzzobvP20e?=
 =?us-ascii?Q?XTZdFaKxKciEuSBa3DJMKw3t6gbAyU3/ee2oc977VwHb9nN7xkOAbvoZOe/3?=
 =?us-ascii?Q?fOqoei2JQpMZe+p5LVjU+HWjR+8PRqjS/FIA58iHqUe7U7Bj/EcCdj+pHQ+y?=
 =?us-ascii?Q?wcvA4729fPPL+wAdagjc0N1lI3CMTFREzzSlxX1COQS6T7+GTnXR66GXM7Gx?=
 =?us-ascii?Q?8ou2GYLUSLt1mu4mbOsQVWevT2f6GE/LKpR95oGimhQ538qcrwuTHjJaS1si?=
 =?us-ascii?Q?R2OU1B2ua6yiUPuhHNZ8DtgGVuVg+3YHAwA8UJ+mJ++G+MoSghAzFl3wUTpO?=
 =?us-ascii?Q?eucdwGQ+r7+GqdyWVGUYJYCQAG4+c/kfw6QRXBK7yWEQhfaRHqcERMxiFOcy?=
 =?us-ascii?Q?FkubA0MEiOioCXSFikePY4tBXcKyNNP8bP0n4P1N0fF9VjcKdcFmO1qLnfRG?=
 =?us-ascii?Q?sePVey5l8AYaxepABTS1MfojJfHr+fStutMlSG27PUJpzhV74M9wRPpEfAqg?=
 =?us-ascii?Q?wg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1E14C663E5DF8047A280C804A20E4144@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 541ff9ea-c8b7-4c6e-9f53-08da9034d601
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Sep 2022 18:23:02.8991
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: eTGZETAe7QXvyRLx6brJQJwNpOHPJ24yjihpMCMs8jgDgwODkkMCpdngXkPqiajx/3dJIhsh5OnJH2I69RT4pg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR10MB4357
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.528,FMLib:17.11.122.1
 definitions=2022-09-06_09,2022-09-06_02,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 mlxscore=0 adultscore=0
 malwarescore=0 mlxlogscore=999 bulkscore=0 suspectscore=0 spamscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000
 definitions=main-2209060085
X-Proofpoint-GUID: ibzInqm8Mxtr1GqAaT8BSrTWv2qSxVkb
X-Proofpoint-ORIG-GUID: ibzInqm8Mxtr1GqAaT8BSrTWv2qSxVkb
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Sep 6, 2022, at 2:17 PM, Anna Schumaker <anna@kernel.org> wrote:
>=20
> Hi Chuck,
>=20
> On Sat, Sep 3, 2022 at 1:36 PM Chuck Lever III <chuck.lever@oracle.com> w=
rote:
>>=20
>>=20
>>=20
>>> On Sep 1, 2022, at 2:33 PM, Anna Schumaker <anna@kernel.org> wrote:
>>>=20
>>> 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?
>>>=20
>>> 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 =3D test_bit(RQ_SPLICE_OK, &resp->rqstp->rq_flags)=
;
>>>      unsigned long maxcount;
>>>      struct xdr_stream *xdr =3D resp->xdr;
>>>      struct file *file =3D read->rd_nf->nf_file;
>>> @@ -4747,7 +4748,10 @@ nfsd4_encode_read_plus_data(struct nfsd4_compoun=
dres *resp,
>>>      maxcount =3D min_t(unsigned long, read->rd_length,
>>>                       (xdr->buf->buflen - xdr->buf->len));
>>>=20
>>> -     nfserr =3D nfsd4_encode_readv(resp, read, file, maxcount);
>>> +     if (file->f_op->splice_read && splice_ok)
>>> +             nfserr =3D nfsd4_encode_splice_read(resp, read, file, max=
count);
>>> +     else
>>> +             nfserr =3D nfsd4_encode_readv(resp, read, file, maxcount)
>>>      if (nfserr)
>>>              return nfserr;
>>=20
>> 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.
>>=20
>> 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.
>>=20
>> 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.
>>=20
>> 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?
>=20
> 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.
>=20
> 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