[PATCH] SUNRPC: Restore NUMA_NO_NODE for svc thread allocations in global mode

Ameer Hamza posted 1 patch 2 days, 4 hours ago
net/sunrpc/svc.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
[PATCH] SUNRPC: Restore NUMA_NO_NODE for svc thread allocations in global mode
Posted by Ameer Hamza 2 days, 4 hours ago
Commit d57e43b72bf2 ("SUNRPC: Update svcxdr_init_decode() to call
xdr_set_scratch_folio()") changed svc_pool_map_get_node() to return
numa_mem_id() instead of NUMA_NO_NODE, because __folio_alloc_node()
cannot accept NUMA_NO_NODE. That return value is not equivalent: it
is evaluated in the context of the task creating the nfsd threads,
once per thread created, and it is passed to kthread_create_on_node()
and to the per-thread allocations in svc_prepare_thread().

Since commit d1a89197589c ("kthread: Default affine kthread to its
preferred NUMA node"), the node argument of kthread_create_on_node()
no longer only places the task structure and stack: a kthread created
with a real node id normally affines itself to that node's CPUs when
it is first woken to run its thread function. All nfsd threads are
typically started together, by one task writing to
/proc/fs/nfsd/threads, so under the default pool_mode=global each
nfsd thread is now affined to the local-memory node of the CPU its
creating iteration happened to run on - typically the same node for
every thread. The CPUs of the other nodes are then unable to run
nfsd at all, and the threads' allocations - svc_rqst structures,
page pointer arrays, newly allocated task stacks, and the per-RPC
pages allocated at run time - all prefer that one node.

Restore the NUMA_NO_NODE behaviour that global mode has had since
commit 11fd165c68b7 ("sunrpc: use better NUMA affinities"), and
handle NUMA_NO_NODE at the one call site that cannot take it by
resolving it to numa_mem_id() there, exactly as alloc_pages_node()
did for the scratch page before the conversion. The mapped percpu
and pernode branches are unchanged. Unpooled services such as lockd
and the NFS client callback service also take this fallback when no
percpu or pernode map is active, restoring their thread placement in
that case.

A bisect of a 2x NFS READ throughput regression between v6.17 and
v6.18 converged on d57e43b72bf2. On the affected 4-node server every
nfsd thread comes up with its CPU affinity restricted to the CPUs of
a single node; with this change the threads are runnable on all CPUs
again and the observed regression is resolved.

Fixes: d57e43b72bf2 ("SUNRPC: Update svcxdr_init_decode() to call xdr_set_scratch_folio()")
Signed-off-by: Ameer Hamza <ameer.hamza@truenas.com>
---
The pool-mode rework in nfsd-testing already removes global mode,
so this fix is based on nfsd-fixes and aimed there and at stable
(v6.18+), where the regression ships. Tested on a v6.18-based
4-node server: without this patch all nfsd threads come up affined
to one node; with it they are runnable on all CPUs.

 net/sunrpc/svc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index ae9ec4bf34f7..16b1ca1a7ae3 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -352,7 +352,7 @@ static int svc_pool_map_get_node(unsigned int pidx)
 		if (m->mode == SVC_POOL_PERNODE)
 			return m->pool_to[pidx];
 	}
-	return numa_mem_id();
+	return NUMA_NO_NODE;
 }
 /*
  * Set the given thread's cpus_allowed mask so that it
@@ -711,7 +711,9 @@ svc_prepare_thread(struct svc_serv *serv, struct svc_pool *pool, int node)
 	rqstp->rq_server = serv;
 	rqstp->rq_pool = pool;
 
-	rqstp->rq_scratch_folio = __folio_alloc_node(GFP_KERNEL, 0, node);
+	rqstp->rq_scratch_folio = __folio_alloc_node(GFP_KERNEL, 0,
+						     node == NUMA_NO_NODE ?
+						     numa_mem_id() : node);
 	if (!rqstp->rq_scratch_folio)
 		goto out_enomem;
 

base-commit: 5c4851e4cdfc3204128f88991be64a6e3929d3d0
-- 
2.53.0
Re: [PATCH] SUNRPC: Restore NUMA_NO_NODE for svc thread allocations in global mode
Posted by Chuck Lever 2 days ago
On Wed, 22 Jul 2026 23:20:11 +0500, Ameer Hamza wrote:
> Commit d57e43b72bf2 ("SUNRPC: Update svcxdr_init_decode() to call
> xdr_set_scratch_folio()") changed svc_pool_map_get_node() to return
> numa_mem_id() instead of NUMA_NO_NODE, because __folio_alloc_node()
> cannot accept NUMA_NO_NODE. That return value is not equivalent: it
> is evaluated in the context of the task creating the nfsd threads,
> once per thread created, and it is passed to kthread_create_on_node()
> and to the per-thread allocations in svc_prepare_thread().
> 
> [...]

Applied to nfsd-testing, thanks!

nfsd-testing has been re-ordered so this patch is positioned before
Jeff's restructuring of svc pools.

[1/1] SUNRPC: Restore NUMA_NO_NODE for svc thread allocations in global mode
      commit: ea94cbfb34c7151d79da28a8c7e4bec0290b3e27

--
Chuck Lever
Re: [PATCH] SUNRPC: Restore NUMA_NO_NODE for svc thread allocations in global mode
Posted by Jeff Layton 2 days, 3 hours ago
On Wed, 2026-07-22 at 23:20 +0500, Ameer Hamza wrote:
> Commit d57e43b72bf2 ("SUNRPC: Update svcxdr_init_decode() to call
> xdr_set_scratch_folio()") changed svc_pool_map_get_node() to return
> numa_mem_id() instead of NUMA_NO_NODE, because __folio_alloc_node()
> cannot accept NUMA_NO_NODE. That return value is not equivalent: it
> is evaluated in the context of the task creating the nfsd threads,
> once per thread created, and it is passed to kthread_create_on_node()
> and to the per-thread allocations in svc_prepare_thread().
> 
> Since commit d1a89197589c ("kthread: Default affine kthread to its
> preferred NUMA node"), the node argument of kthread_create_on_node()
> no longer only places the task structure and stack: a kthread created
> with a real node id normally affines itself to that node's CPUs when
> it is first woken to run its thread function. All nfsd threads are
> typically started together, by one task writing to
> /proc/fs/nfsd/threads, so under the default pool_mode=global each
> nfsd thread is now affined to the local-memory node of the CPU its
> creating iteration happened to run on - typically the same node for
> every thread. The CPUs of the other nodes are then unable to run
> nfsd at all, and the threads' allocations - svc_rqst structures,
> page pointer arrays, newly allocated task stacks, and the per-RPC
> pages allocated at run time - all prefer that one node.
> 
> Restore the NUMA_NO_NODE behaviour that global mode has had since
> commit 11fd165c68b7 ("sunrpc: use better NUMA affinities"), and
> handle NUMA_NO_NODE at the one call site that cannot take it by
> resolving it to numa_mem_id() there, exactly as alloc_pages_node()
> did for the scratch page before the conversion. The mapped percpu
> and pernode branches are unchanged. Unpooled services such as lockd
> and the NFS client callback service also take this fallback when no
> percpu or pernode map is active, restoring their thread placement in
> that case.
> 
> A bisect of a 2x NFS READ throughput regression between v6.17 and
> v6.18 converged on d57e43b72bf2. On the affected 4-node server every
> nfsd thread comes up with its CPU affinity restricted to the CPUs of
> a single node; with this change the threads are runnable on all CPUs
> again and the observed regression is resolved.
> 
> Fixes: d57e43b72bf2 ("SUNRPC: Update svcxdr_init_decode() to call xdr_set_scratch_folio()")
> Signed-off-by: Ameer Hamza <ameer.hamza@truenas.com>
> ---
> The pool-mode rework in nfsd-testing already removes global mode,
> so this fix is based on nfsd-fixes and aimed there and at stable
> (v6.18+), where the regression ships. Tested on a v6.18-based
> 4-node server: without this patch all nfsd threads come up affined
> to one node; with it they are runnable on all CPUs.
> 
>  net/sunrpc/svc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index ae9ec4bf34f7..16b1ca1a7ae3 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -352,7 +352,7 @@ static int svc_pool_map_get_node(unsigned int pidx)
>  		if (m->mode == SVC_POOL_PERNODE)
>  			return m->pool_to[pidx];
>  	}
> -	return numa_mem_id();
> +	return NUMA_NO_NODE;
>  }
>  /*
>   * Set the given thread's cpus_allowed mask so that it
> @@ -711,7 +711,9 @@ svc_prepare_thread(struct svc_serv *serv, struct svc_pool *pool, int node)
>  	rqstp->rq_server = serv;
>  	rqstp->rq_pool = pool;
>  
> -	rqstp->rq_scratch_folio = __folio_alloc_node(GFP_KERNEL, 0, node);
> +	rqstp->rq_scratch_folio = __folio_alloc_node(GFP_KERNEL, 0,
> +						     node == NUMA_NO_NODE ?
> +						     numa_mem_id() : node);
>  	if (!rqstp->rq_scratch_folio)
>  		goto out_enomem;
>  
> 
> base-commit: 5c4851e4cdfc3204128f88991be64a6e3929d3d0

The patch seems sane. I wonder how much we should care about people
using global mode on real NUMA hw though? The better fix for those
folks would be to move to pernode, most likely.

Oh well...

Reviewed-by: Jeff Layton <jlayton@kernel.org>