From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4EC76C433E0 for ; Fri, 26 Feb 2021 07:46:07 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id BF12F64EDB for ; Fri, 26 Feb 2021 07:46:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BF12F64EDB Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=brown.name Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 503906B0005; Fri, 26 Feb 2021 02:20:04 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4B3DB6B0006; Fri, 26 Feb 2021 02:20:04 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3F1D26B006C; Fri, 26 Feb 2021 02:20:04 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0020.hostedemail.com [216.40.44.20]) by kanga.kvack.org (Postfix) with ESMTP id 2A6D96B0005 for ; Fri, 26 Feb 2021 02:20:04 -0500 (EST) Received: from smtpin20.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id F1C3475BB for ; Fri, 26 Feb 2021 07:20:03 +0000 (UTC) X-FDA: 77859569886.20.34C47CD Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf01.hostedemail.com (Postfix) with ESMTP id 8B67F20001EA for ; Fri, 26 Feb 2021 07:20:02 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id C524BAAAE; Fri, 26 Feb 2021 07:20:01 +0000 (UTC) From: NeilBrown To: Chuck Lever , mgorman@techsingularity.net Date: Fri, 26 Feb 2021 18:19:56 +1100 Cc: linux-nfs@vger.kernel.org, linux-mm@kvack.org, kuba@kernel.org Subject: Re: [PATCH v2 4/4] SUNRPC: Cache pages that were replaced during a read splice In-Reply-To: <161400740732.195066.3792261943053910900.stgit@klimt.1015granger.net> References: <161400722731.195066.9584156841718557193.stgit@klimt.1015granger.net> <161400740732.195066.3792261943053910900.stgit@klimt.1015granger.net> Message-ID: <87k0qvi9cz.fsf@notabene.neil.brown.name> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 8B67F20001EA X-Stat-Signature: 684gpii6ikht8uzoauiw5ebj4hycp1jh Received-SPF: none (brown.name>: No applicable sender policy available) receiver=imf01; identity=mailfrom; envelope-from=""; helo=mx2.suse.de; client-ip=195.135.220.15 X-HE-DKIM-Result: none/none X-HE-Tag: 1614324002-342574 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable On Mon, Feb 22 2021, Chuck Lever wrote: > To avoid extra trips to the page allocator, don't free unused pages > in nfsd_splice_actor(), but instead place them in a local cache. > That cache is then used first when refilling rq_pages. > > On workloads that perform large NFS READs on splice-capable file > systems, this saves a considerable amount of work. > > Suggested-by: NeilBrown > Signed-off-by: Chuck Lever > --- > fs/nfsd/vfs.c | 4 ++-- > include/linux/sunrpc/svc.h | 1 + > include/linux/sunrpc/svc_xprt.h | 28 ++++++++++++++++++++++++++++ > net/sunrpc/svc.c | 7 +++++++ > net/sunrpc/svc_xprt.c | 12 ++++++++++++ > 5 files changed, 50 insertions(+), 2 deletions(-) > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c > index d316e11923c5..25cf41eaf3c4 100644 > --- a/fs/nfsd/vfs.c > +++ b/fs/nfsd/vfs.c > @@ -852,14 +852,14 @@ nfsd_splice_actor(struct pipe_inode_info *pipe, str= uct pipe_buffer *buf, >=20=20 > if (rqstp->rq_res.page_len =3D=3D 0) { > get_page(page); > - put_page(*rqstp->rq_next_page); > + svc_rqst_put_unused_page(rqstp, *rqstp->rq_next_page); > *(rqstp->rq_next_page++) =3D page; > rqstp->rq_res.page_base =3D buf->offset; > rqstp->rq_res.page_len =3D size; > } else if (page !=3D pp[-1]) { > get_page(page); > if (*rqstp->rq_next_page) > - put_page(*rqstp->rq_next_page); > + svc_rqst_put_unused_page(rqstp, *rqstp->rq_next_page); > *(rqstp->rq_next_page++) =3D page; > rqstp->rq_res.page_len +=3D size; > } else > diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h > index 31ee3b6047c3..340f4f3989c0 100644 > --- a/include/linux/sunrpc/svc.h > +++ b/include/linux/sunrpc/svc.h > @@ -250,6 +250,7 @@ struct svc_rqst { > struct xdr_stream rq_arg_stream; > struct page *rq_scratch_page; > struct xdr_buf rq_res; > + struct list_head rq_unused_pages; > struct page *rq_pages[RPCSVC_MAXPAGES + 1]; > struct page * *rq_respages; /* points into rq_pages */ > struct page * *rq_next_page; /* next reply page to use */ > diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_x= prt.h > index 571f605bc91e..49ef86499876 100644 > --- a/include/linux/sunrpc/svc_xprt.h > +++ b/include/linux/sunrpc/svc_xprt.h > @@ -150,6 +150,34 @@ static inline void svc_xprt_get(struct svc_xprt *xpr= t) > { > kref_get(&xprt->xpt_ref); > } > + > +/** > + * svc_rqst_get_unused_page - Tap a page from the local cache > + * @rqstp: svc_rqst with cached unused pages > + * > + * To save an allocator round trip, pages can be added to a > + * local cache and re-used later by svc_alloc_arg(). > + * > + * Returns an unused page, or NULL if the cache is empty. > + */ > +static inline struct page *svc_rqst_get_unused_page(struct svc_rqst *rqs= tp) > +{ > + return list_first_entry_or_null(&rqstp->rq_unused_pages, > + struct page, lru); > +} > + > +/** > + * svc_rqst_put_unused_page - Stash a page in the local cache > + * @rqstp: svc_rqst with cached unused pages > + * @page: page to cache > + * > + */ > +static inline void svc_rqst_put_unused_page(struct svc_rqst *rqstp, > + struct page *page) > +{ > + list_add(&page->lru, &rqstp->rq_unused_pages); > +} > + > static inline void svc_xprt_set_local(struct svc_xprt *xprt, > const struct sockaddr *sa, > const size_t salen) > diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c > index 61fb8a18552c..3920fa8f1146 100644 > --- a/net/sunrpc/svc.c > +++ b/net/sunrpc/svc.c > @@ -570,6 +570,8 @@ svc_init_buffer(struct svc_rqst *rqstp, unsigned int = size, int node) > if (svc_is_backchannel(rqstp)) > return 1; >=20=20 > + INIT_LIST_HEAD(&rqstp->rq_unused_pages); > + > pages =3D size / PAGE_SIZE + 1; /* extra page as we hold both request a= nd reply. > * We assume one is at most one page > */ > @@ -593,8 +595,13 @@ svc_init_buffer(struct svc_rqst *rqstp, unsigned int= size, int node) > static void > svc_release_buffer(struct svc_rqst *rqstp) > { > + struct page *page; > unsigned int i; >=20=20 > + while ((page =3D svc_rqst_get_unused_page(rqstp))) { > + list_del(&page->lru); > + put_page(page); > + } > for (i =3D 0; i < ARRAY_SIZE(rqstp->rq_pages); i++) > if (rqstp->rq_pages[i]) > put_page(rqstp->rq_pages[i]); > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c > index 15aacfa5ca21..84210e546a66 100644 > --- a/net/sunrpc/svc_xprt.c > +++ b/net/sunrpc/svc_xprt.c > @@ -678,6 +678,18 @@ static int svc_alloc_arg(struct svc_rqst *rqstp) > for (needed =3D 0, i =3D 0; i < pages ; i++) > if (!rqstp->rq_pages[i]) > needed++; > + if (needed) { > + for (i =3D 0; i < pages; i++) { > + if (!rqstp->rq_pages[i]) { > + page =3D svc_rqst_get_unused_page(rqstp); > + if (!page) > + break; > + list_del(&page->lru); > + rqstp->rq_pages[i] =3D page; > + needed--; > + } > + } > + } > if (needed) { > LIST_HEAD(list); >=20=20 This looks good! Probably simpler than the way I imagined it :-) I would do that last bit of code differently though... for (needed =3D 0, i =3D 0; i < pages ; i++) if (!rqstp->rq_pages[i]) { page =3D svc_rqst_get_unused_pages(rqstp); if (page) { list_del(&page->lru); rqstp->rq_pages[i] =3D page; } else needed++; } but it is really a minor style difference - I don't object to your version. Reviewed-by: NeilBrown Thanks, NeilBrown --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQJEBAEBCAAuFiEEG8Yp69OQ2HB7X0l6Oeye3VZigbkFAmA4oRwQHG5laWxAYnJv d24ubmFtZQAKCRA57J7dVmKBuYkCEACCwjdOIz4QAwZUH166WdVF8+mPmPYFilq1 OzTzVDDFN4oDZoUBDGWFd4xZAhwVNCJ/dJhLoAJo0znCOHeckM3mTWFF9fOdEhXM 67/+IXe6keX/8d9d/Vv3KN6vGN+IMjT2MfrW5Lvx7CnV3fYMQsQPAACeUkVtrDR5 s+DffgIlltKAKlFapA9x73h8nMzxvU6ngogsfKGWGd5dhhWVq6XlbzCbeVBAl4sL SG+PAcU+/1Fzubppob5xM8+p9EgpCiLdSbwSZcjZfDFgPFTWDU/xn3/40ikixQl7 mLWW/vcOjgOCniVibXSrlc0fAB4po/8/pZWvUCzoUOp1A2bJq3TASHGXSEIFzKWL lPuVurZvPKbhj96HAIr4Gio24HndDAqbI8zqH6r4idu6YGziSVMY8yYH4a/nFHFz Wuwf14NUSXR10v8Q9hi3CX31WLlpObcmTEd7E0ryB5ADPSRCJp7GSKal5XnMtOe4 +YVOiFbXVxZIduJsFmkcJE9YrcnZ+32spnQ2lXw7k5Nk7YAiIiGj+ayVfHsPieg0 vid2WcV4WVodzTkodql+F+dGWLJNOLBq2ISQLntB5FJCw0zCNTRfPOBME41cvT1a qYnyJ4TtqXyWDlpkP082hZXuzjAJwmoRcyd8ZnB6bG6T2IaYxzJHO9/J9UDaZ3by nVfm0AULsQ== =GFpw -----END PGP SIGNATURE----- --=-=-=--