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

From: Trond Myklebust <trondmy@hammerspace.com>
To: "schumaker.anna@gmail.com" <schumaker.anna@gmail.com>,
	"chuck.lever@oracle.com" <chuck.lever@oracle.com>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 2/3] NFS: Allocate a scratch page for READ_PLUS
Date: Thu, 3 Dec 2020 20:39:57 +0000	[thread overview]
Message-ID: <dc191256b163d0c824c911fe7272d5443f2fd045.camel@hammerspace.com> (raw)
In-Reply-To: <CAFX2JfnsHskDd+tsGcZ1-JNaWpZ9V4c-5=x2VE4mwC6p+MhfYQ@mail.gmail.com>

On Thu, 2020-12-03 at 15:31 -0500, Anna Schumaker wrote:
> 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.
> 

Why not just integrate it into xdr_init_decode_pages(), and then add a
matching xdr_exit_decode_pages() to free up any allocated page?

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com