On 6/5/25 10:01 AM, Benjamin Coddington wrote:
> While investigating some reports of memory-constrained NUMA machines
> failing to mount v3 and v4.0 nfs mounts, we found that svc_init_buffer()
> was not attempting to retry allocations from the bulk page allocator.
> Typically, this results in a single page allocation being returned and
> the mount attempt fails with -ENOMEM. A retry would have allowed the mount
> to succeed.
>
> Additionally, it seems that svc_init_buffer() is redundant because
> svc_alloc_arg() will perform the required allocation and does the correct
> thing to retry the allocations.
>
> The call to allocate memory in svc_alloc_arg() drops the preferred node
> argument, but I expect we'll still allocate on the preferred node because
> the allocation call happens within the svc thread context, which chooses
> the node with memory closest to the current thread's execution.
>
> This patch cleans out svc_init_buffer() to allow svc_alloc_arg() to handle
> the allocation of rq_pages.
>
> Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
> ---
> net/sunrpc/svc.c | 24 ------------------------
> 1 file changed, 24 deletions(-)
>
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index e7f9c295d13c..e14f2d5c15bf 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -635,27 +635,6 @@ 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)
> -{
> - unsigned long pages, ret;
> -
> - /* bc_xprt uses fore channel allocated buffers */
> - if (svc_is_backchannel(rqstp))
> - return true;
> -
> - 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;
> -
> - ret = alloc_pages_bulk_node(GFP_KERNEL, node, pages,
> - rqstp->rq_pages);
> - return ret == pages;
> -}
> -
> /*
> * Release an RPC server buffer
> */
> @@ -708,9 +687,6 @@ svc_prepare_thread(struct svc_serv *serv, struct svc_pool *pool, int node)
> if (!rqstp->rq_resp)
> goto out_enomem;
>
> - if (!svc_init_buffer(rqstp, serv->sv_max_mesg, node))
> - goto out_enomem;
> -
> rqstp->rq_err = -EAGAIN; /* No error yet */
>
> serv->sv_nrthreads += 1;
This doesn't apply to v6.16-rc1 due to recent changes to use a
dynamically-allocated rq_pages array. This array is allocated in
svc_init_buffer(); the array allocation has to remain.
--
Chuck Lever
