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:     Hugh Dickins <hughd@google.com>
CC:     Andrew Morton <akpm@linux-foundation.org>,
        Mark Hemment <markhemm@googlemail.com>,
        Patrice CHOTARD <patrice.chotard@foss.st.com>,
        Mikulas Patocka <mpatocka@redhat.com>,
        Lukas Czerner <lczerner@redhat.com>,
        Christoph Hellwig <hch@lst.de>,
        "Darrick J. Wong" <djwong@kernel.org>,
        Linux-MM <linux-mm@kvack.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: Regression in xfstests on tmpfs-backed NFS exports
Thread-Topic: Regression in xfstests on tmpfs-backed NFS exports
Thread-Index: AQHYSdpNL2d3Xru4B0OzHZJ1LCngEazjlm2AgAE//ICAADMPAIABKSsAgAAyAwCAAAv0gA==
Date:   Fri, 8 Apr 2022 19:52:20 +0000
Message-ID: <E99BC043-47BE-49FF-A882-0CF1820B500B@oracle.com>
References: <673D708E-2DFA-4812-BB63-6A437E0C72EE@oracle.com>
 <11f319-c9a-4648-bfbb-dc5a83c774@google.com>
 <2B7AF707-67B1-4ED8-A29F-957C26B7F87A@oracle.com>
 <c5ea49a-1a76-8cf9-5c76-4bb31aa3d458@google.com>
 <C7966059-D583-4B20-A838-067BAE86FB3E@oracle.com>
 <8058d02d-d8b2-4d7a-a535-a78719e996@google.com>
In-Reply-To: <8058d02d-d8b2-4d7a-a535-a78719e996@google.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?VlHToBi5Q25YCs6OiUMZ6c7tzoY27BAT6EtVLdO4XLM00WFJUNjva8rInpyU?=
 =?us-ascii?Q?+gkEJbqy2D0my5GWf81TrTmOmGVcvCItZD7eOVLpoJRI4hDSnx6uHj016LIE?=
 =?us-ascii?Q?FXWTFfkCtE2XUX/17nOVoIciNvoa5028E66E9nn+jFBnblkbTRL3JTGTeDKb?=
 =?us-ascii?Q?Ti9KnwGobE03pWkJOFnV7qIcEFcDlIPxax6w+QrFcFAntw2idi4J+G7qjYGl?=
 =?us-ascii?Q?8yPDDUmeUS2RpqEGw06+K4EG1R/tcV+IaplrrLh2nqdTgJA/2zDjHuUguyfW?=
 =?us-ascii?Q?eyAOjfvSo/uAoEpAU1Mh+9ZX54v2gM0GehNjblMjldnVqP2nYViXyb4lrh39?=
 =?us-ascii?Q?Yhn1Wu9CepPcJne3kxGPCBey2Myd+ldONWzY9qi8p+pfYv1zaSuiXeJ2n8cP?=
 =?us-ascii?Q?3tHu+oilurHA8Hsmy3ZA9qwOxEOzyccm81LBxX5XtVoHbe+y+CpQ3G0YYpZM?=
 =?us-ascii?Q?vk6I3dwC+2JZc03ZFGk905CC22XjGMtyeUr17wuS9CvZRrndLUwniYYwxSAP?=
 =?us-ascii?Q?NjZVEIHLjmXZ6wXHpSMyNctWh8US11VCHRZHuGKAHZUamGxMhyAxS5AAMbHJ?=
 =?us-ascii?Q?eAK3WdY4NwnouQr6kYhSgzBNo7mlahhNxvnIryFxeZsPWnwbuUb7Oh5uekO8?=
 =?us-ascii?Q?qB7SIF4sJxdjIWIIr00vf2EO9Q70WICdbq1lM8F5oJe4T1B8okLFn2EL5eX5?=
 =?us-ascii?Q?UpQ8BXW65ThK31X1cWYnJYVjYEpIw2L/Ugwn5s8sYCouJRi9HqMK5PUCJMqe?=
 =?us-ascii?Q?Xtc9/Pnv+pCtMADb2Ubn7ptOxydfzkECR/nTF2rpgmU2yjtuf6o9jw3AsmsU?=
 =?us-ascii?Q?0bpIHSMYjBXw3hC1Sh1rZrkK6B1d1G3k5DWE4aC4cZCWHkbp8rIvc6OE26mA?=
 =?us-ascii?Q?C12RU8rY4lkmYYtqbh1N6FP2OcgJAxYMQoM8PHrFsaG+a6Xm3rPTT/FjJIfo?=
 =?us-ascii?Q?AICnfZ5yygcSyGLGPZiVyxjVrOKTiw7tBaAcrVLx7idhGDOU9N6fmlRG3IbD?=
 =?us-ascii?Q?FatT3mutzrVlrX3de4wzDWRxdGpbzHKsoxxZG9OoA5/aBpolzFez2L9oQ9FN?=
 =?us-ascii?Q?5IBSSc5NQuZWNiuig1utSQpETimwkJ/FY73ISaspCG4jgKdLeOxQYTkMs3tk?=
 =?us-ascii?Q?QtjwNXveN3sjBFoUTtGd6D5zH3FlqlZcgEgTGkvP5UoV1aLzcTWCfW/FY8+H?=
 =?us-ascii?Q?yH7Q9ddslwi/OKQ5EWV2efFJEjmpQmx9t5Zi4SAy2uZm4w9+aELzlsbLy/be?=
 =?us-ascii?Q?PxaxnjwLoI/cmQvUYyZcceJqiL+NWitXiqEfDnPTjjgeBYgEc8svoN3ecwD5?=
 =?us-ascii?Q?YTPUc9JvzQX/YgCCY8N5puNkKOMoE92IIHFT0RrYlWXrClEiLYISXMNzqCav?=
 =?us-ascii?Q?3Yy8okVPkooCmVFReMpymdUd47aYQBELnkl/QAZVQsnNInBxKOUZcF3vIkaG?=
 =?us-ascii?Q?2jUNigCn3hv8GtF6ErVldDHDlagS0Gab8GNyB7SCaLrmR5JGp9DgUoHHqJbz?=
 =?us-ascii?Q?c+R+F1wqV0VevMurFMokalqsXtT0VCX9Jaj/OG1xli74Wdhtm2RJ+B5NcKGe?=
 =?us-ascii?Q?bMDRcQWjjLMPsxnOl2SowZskwIV5vaU5klLCprNusl9gLOsR68juTgcgTz0k?=
 =?us-ascii?Q?ibzCJW5OK2LRnEo8ygfInaDv4NdbmYoNvd7yB5XalbNUc1oGJIEvYPg9XSsF?=
 =?us-ascii?Q?VppYupZccNw48/vDy5zCk2/MTlmTgkCMNLV2p/GxaxddSN8vzmcHmD5+jSY4?=
 =?us-ascii?Q?UJ3fDu4eDmHJKOd3LtILlrQMdnzntvQ=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <53DCE6478193E54B86897606D003416F@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: 76a7d22e-bbd9-412d-e9ff-08da19994b27
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Apr 2022 19:52:20.7163
 (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: Fl8x3awYsRUbECsIjxKqw0A2e8ncYTTV0WBszWTz8+6mIi0NkAPgAyYSfaJ3Bl0l1uvxPl5CqiMV/Ldi2O8RQQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR10MB2877
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.425,18.0.858
 definitions=2022-04-08_07:2022-04-08,2022-04-08 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 mlxlogscore=999 spamscore=0
 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2204080100
X-Proofpoint-GUID: ZC2GdCcdd8krDcEzEm8YKouVRMHB4R3H
X-Proofpoint-ORIG-GUID: ZC2GdCcdd8krDcEzEm8YKouVRMHB4R3H
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_H5,RCVD_IN_MSPIKE_WL,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 Apr 8, 2022, at 3:09 PM, Hugh Dickins <hughd@google.com> wrote:
>=20
> On Fri, 8 Apr 2022, Chuck Lever III wrote:
>>> On Apr 7, 2022, at 6:26 PM, Hugh Dickins <hughd@google.com> wrote:
>>> On Thu, 7 Apr 2022, Chuck Lever III wrote:
>>>>=20
>>>> 847 static int
>>>> 848 nfsd_splice_actor(struct pipe_inode_info *pipe, struct pipe_buffer=
 *buf,
>>>> 849                   struct splice_desc *sd)
>>>> 850 {
>>>> 851         struct svc_rqst *rqstp =3D sd->u.data;
>>>> 852         struct page **pp =3D rqstp->rq_next_page;
>>>> 853         struct page *page =3D buf->page;
>>>> 854=20
>>>> 855         if (rqstp->rq_res.page_len =3D=3D 0) {
>>>> 856                 svc_rqst_replace_page(rqstp, page);
>>>> 857                 rqstp->rq_res.page_base =3D buf->offset;
>>>> 858         } else if (page !=3D pp[-1]) {
>>>> 859                 svc_rqst_replace_page(rqstp, page);
>>>> 860         }
>>>> 861         rqstp->rq_res.page_len +=3D sd->len;
>>>> 862=20
>>>> 863         return sd->len;
>>>> 864 }
>>>>=20
>>>> rq_next_page should point to the first unused element of
>>>> rqstp->rq_pages, so IIUC that check is looking for the
>>>> final page that is part of the READ payload.
>>>>=20
>>>> But that does suggest that if page -> ZERO_PAGE and so does
>>>> pp[-1], then svc_rqst_replace_page() would not be invoked.
>>=20
>> To put a little more color on this, I think the idea here
>> is to prevent releasing the same page twice. It might be
>> possible that NFSD can add the same page to the rq_pages
>> array more than once, and we don't want to do a double
>> put_page().
>>=20
>> The only time I can think this might happen is if the
>> READ payload is partially contained in the page that
>> contains the NFS header. I'm not sure that can ever
>> happen these days.
>=20
> I'd have thought that if a page were repeated, then its refcount would
> have been raised twice, and so require a double put_page().  But it's
> no concern of mine.  The only thing I'd say is, if you do find a good
> way to robustify that code for duplicates, please don't make it
> conditional on ZERO_PAGE - that's just a special case which I
> mis-introduced and is now about to go away.

100% agree on both counts: not sure (yet) why get_page() was not
used here, and a special case for ZERO_PAGE would be a band-aid.

Anyway, I haven't found a case yet where a duplicate struct page
appears in rq_pages with current kernels.


--
Chuck Lever