Re: [PATCH 2/3] NFS: Allocate a scratch page for READ_PLUS

From: Anna Schumaker <schumaker.anna@gmail.com>
To: Chuck Lever <chuck.lever@oracle.com>
Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 2/3] NFS: Allocate a scratch page for READ_PLUS
Date: Thu, 3 Dec 2020 15:31:20 -0500	[thread overview]
Message-ID: <CAFX2JfnsHskDd+tsGcZ1-JNaWpZ9V4c-5=x2VE4mwC6p+MhfYQ@mail.gmail.com> (raw)
In-Reply-To: <8CE5CE8C-9301-4250-B077-ACE23969C19A@oracle.com>

On Thu, Dec 3, 2020 at 3:27 PM Chuck Lever <chuck.lever@oracle.com> wrote:
>
>
>
> > On Dec 3, 2020, at 3:18 PM, schumaker.anna@gmail.com wrote:
> >
> > From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> >
> > We might need this to better handle shifting around data in the reply
> > buffer.
> >
> > Suggested-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> > Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> > ---
> > fs/nfs/nfs42xdr.c       |  2 ++
> > fs/nfs/read.c           | 13 +++++++++++--
> > include/linux/nfs_xdr.h |  1 +
> > 3 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
> > index 8432bd6b95f0..ef095a5f86f7 100644
> > --- a/fs/nfs/nfs42xdr.c
> > +++ b/fs/nfs/nfs42xdr.c
> > @@ -1297,6 +1297,8 @@ static int nfs4_xdr_dec_read_plus(struct rpc_rqst *rqstp,
> >       struct compound_hdr hdr;
> >       int status;
> >
> > +     xdr_set_scratch_buffer(xdr, page_address(res->scratch), PAGE_SIZE);
>
> I intend to submit this for v5.11:
>
> https://git.linux-nfs.org/?p=cel/cel-2.6.git;a=commit;h=0ae4c3e8a64ace1b8d7de033b0751afe43024416

Thanks for the heads up! This patch can probably be deferred until
yours goes in.

>
> But seems like enough callers need a scratch buffer that the XDR
> layer should set up it transparently for all requests.

That could work too, and save some headache.

Anna

>
>
> > +
> >       status = decode_compound_hdr(xdr, &hdr);
> >       if (status)
> >               goto out;
> > diff --git a/fs/nfs/read.c b/fs/nfs/read.c
> > index eb854f1f86e2..012deb5a136f 100644
> > --- a/fs/nfs/read.c
> > +++ b/fs/nfs/read.c
> > @@ -37,15 +37,24 @@ static struct kmem_cache *nfs_rdata_cachep;
> >
> > static struct nfs_pgio_header *nfs_readhdr_alloc(void)
> > {
> > -     struct nfs_pgio_header *p = kmem_cache_zalloc(nfs_rdata_cachep, GFP_KERNEL);
> > +     struct nfs_pgio_header *p;
> > +     struct page *page;
> >
> > -     if (p)
> > +     page = alloc_page(GFP_KERNEL);
> > +     if (!page)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     p = kmem_cache_zalloc(nfs_rdata_cachep, GFP_KERNEL);
> > +     if (p) {
> >               p->rw_mode = FMODE_READ;
> > +             p->res.scratch = page;
> > +     }
> >       return p;
> > }
> >
> > static void nfs_readhdr_free(struct nfs_pgio_header *rhdr)
> > {
> > +     __free_page(rhdr->res.scratch);
> >       kmem_cache_free(nfs_rdata_cachep, rhdr);
> > }
> >
> > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> > index d63cb862d58e..e0a1c97f11a9 100644
> > --- a/include/linux/nfs_xdr.h
> > +++ b/include/linux/nfs_xdr.h
> > @@ -659,6 +659,7 @@ struct nfs_pgio_res {
> >       struct nfs_fattr *      fattr;
> >       __u64                   count;
> >       __u32                   op_status;
> > +     struct page *           scratch;
> >       union {
> >               struct {
> >                       unsigned int            replen;         /* used by read */
> > --
> > 2.29.2
> >
>
> --
> Chuck Lever
>
>
>