Re: [PATCH v4 04/14] sunrpc: Replace the rq_pages array with dynamically-allocated memory

NeilBrown
2025-04-30T04:54:01Z

On Tue, 29 Apr 2025, cel@kernel.org wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
>
> As a step towards making NFSD's maximum rsize and wsize variable at
> run-time, replace the fixed-size rq_vec[] array in struct svc_rqst
> with a chunk of dynamically-allocated memory.
>
> On a system with 8-byte pointers and 4KB pages, pahole reports that
> the rq_pages[] array is 2080 bytes. This patch replaces that with
> a single 8-byte pointer field.
>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> include/linux/sunrpc/svc.h | 3 ++-
> net/sunrpc/svc.c | 34 ++++++++++++++++++-------------
> net/sunrpc/svc_xprt.c | 10 +--------
> net/sunrpc/xprtrdma/svc_rdma_rw.c | 2 +-
> 4 files changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index e83ac14267e8..ea3a33eec29b 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -205,7 +205,8 @@ struct svc_rqst {
> struct xdr_stream rq_res_stream;
> struct page *rq_scratch_page;
> struct xdr_buf rq_res;
> - struct page *rq_pages[RPCSVC_MAXPAGES + 1];
> + unsigned long rq_maxpages; /* num of entries in rq_pages */
> + struct page * *rq_pages;
> struct page * *rq_respages; /* points into rq_pages */
> struct page * *rq_next_page; /* next reply page to use */
> struct page * *rq_page_end; /* one past the last page */
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 8ce3e6b3df6a..682e11c9be36 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -636,20 +636,25 @@ svc_destroy(struct svc_serv **servp)
> EXPORT_SYMBOL_GPL(svc_destroy);
>
> static bool
> -svc_init_buffer(struct svc_rqst *rqstp, unsigned int size, int node)
> +svc_init_buffer(struct svc_rqst *rqstp, const struct svc_serv *serv, int node)
> {
> - unsigned long pages, ret;
> + unsigned long ret;
>
> - pages = size / PAGE_SIZE + 1; /* extra page as we hold both request and reply.
> - * We assume one is at most one page
> - */
> - WARN_ON_ONCE(pages > RPCSVC_MAXPAGES);
> - if (pages > RPCSVC_MAXPAGES)
> - pages = RPCSVC_MAXPAGES;
> + /* Add an extra page, as rq_pages holds both request and reply.
> + * We assume one of those is at most one page.
> + */
> + rqstp->rq_maxpages = svc_serv_maxpages(serv) + 1;

The calculation in svc_serv_maxpages() already allows for both request
and reply. I think the "+ 1" here is wrong.

NeilBrown