From: Anna Schumaker <[email protected]>
We might need this to better handle shifting around data in the reply
buffer.
Suggested-by: Trond Myklebust <[email protected]>
Signed-off-by: Anna Schumaker <[email protected]>
---
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);
+
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
> On Dec 3, 2020, at 3:18 PM, [email protected] wrote:
>
> From: Anna Schumaker <[email protected]>
>
> We might need this to better handle shifting around data in the reply
> buffer.
>
> Suggested-by: Trond Myklebust <[email protected]>
> Signed-off-by: Anna Schumaker <[email protected]>
> ---
> 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
But seems like enough callers need a scratch buffer that the XDR
layer should set up it transparently for all requests.
> +
> 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
On Thu, Dec 3, 2020 at 3:27 PM Chuck Lever <[email protected]> wrote:
>
>
>
> > On Dec 3, 2020, at 3:18 PM, [email protected] wrote:
> >
> > From: Anna Schumaker <[email protected]>
> >
> > We might need this to better handle shifting around data in the reply
> > buffer.
> >
> > Suggested-by: Trond Myklebust <[email protected]>
> > Signed-off-by: Anna Schumaker <[email protected]>
> > ---
> > 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
>
>
>
On Thu, 2020-12-03 at 15:31 -0500, Anna Schumaker wrote:
> On Thu, Dec 3, 2020 at 3:27 PM Chuck Lever <[email protected]>
> wrote:
> >
> >
> >
> > > On Dec 3, 2020, at 3:18 PM, [email protected] wrote:
> > >
> > > From: Anna Schumaker <[email protected]>
> > >
> > > We might need this to better handle shifting around data in the
> > > reply
> > > buffer.
> > >
> > > Suggested-by: Trond Myklebust <[email protected]>
> > > Signed-off-by: Anna Schumaker <[email protected]>
> > > ---
> > > 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
[email protected]
> On Dec 3, 2020, at 3:39 PM, Trond Myklebust <[email protected]> wrote:
>
> On Thu, 2020-12-03 at 15:31 -0500, Anna Schumaker wrote:
>> On Thu, Dec 3, 2020 at 3:27 PM Chuck Lever <[email protected]>
>> wrote:
>>>
>>>
>>>
>>>> On Dec 3, 2020, at 3:18 PM, [email protected] wrote:
>>>>
>>>> From: Anna Schumaker <[email protected]>
>>>>
>>>> We might need this to better handle shifting around data in the
>>>> reply
>>>> buffer.
>>>>
>>>> Suggested-by: Trond Myklebust <[email protected]>
>>>> Signed-off-by: Anna Schumaker <[email protected]>
>>>> ---
>>>> 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?
Sounds OK.
For comparison, to support xdr_stream decoding on the server, I've
changed svc_rqst_alloc() to grab a page that stays around until the
nfsd thread dies. There is a new svcxdr_init_decode() API that
attaches that page for use as the decoding scratch buffer.
Since it's a new API, the call sites that set up new streams with
xdr_init_decode() are not affected.
See:
https://git.linux-nfs.org/?p=cel/cel-2.6.git;a=commit;h=5191955d6fc65e6d4efe8f4f10a6028298f57281
--
Chuck Lever