On Mon, 2025-06-09 at 13:21 -0400, 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 the bulk allocation in 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 the bulk allocation in svc_init_buffer() to allow
> svc_alloc_arg() to handle the allocation/retry logic for rq_pages.
>
> Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
>
> --
> On v2:
> - rebased on nfsd-next
> - keep the rq_pages array allocation in svc_init_buffer(), defer
> the page allocation to svc_alloc_arg()
> ---
> net/sunrpc/svc.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 939b6239df8a..ef8a05aac87f 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -638,8 +638,6 @@ EXPORT_SYMBOL_GPL(svc_destroy);
> static bool
> svc_init_buffer(struct svc_rqst *rqstp, const struct svc_serv *serv, int node)
> {
> - unsigned long ret;
> -
> rqstp->rq_maxpages = svc_serv_maxpages(serv);
>
> /* rq_pages' last entry is NULL for historical reasons. */
> @@ -649,9 +647,7 @@ svc_init_buffer(struct svc_rqst *rqstp, const struct svc_serv *serv, int node)
> if (!rqstp->rq_pages)
> return false;
>
> - ret = alloc_pages_bulk_node(GFP_KERNEL, node, rqstp->rq_maxpages,
> - rqstp->rq_pages);
> - return ret == rqstp->rq_maxpages;
> + return true;
> }
>
> /*
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
