Re: [PATCH v4 3/4] sunrpc: guarantee a thread per CPU-bearing node when auto-distributing

[NeilBrown]

On Thu, 02 Jul 2026, Jeff Layton wrote:
> svc_set_num_threads() spreads the requested thread count evenly across
> the service's pools. In pernode mode each pool maps to a NUMA node, and
> svc_pool_for_cpu() steers an incoming transport to the pool for the node
> it arrived on. When fewer threads than pools are requested, even
> distribution leaves some nodes' pools empty, and a transport steered to
> an empty pool has no thread to service it.
> 
> Floor each CPU-bearing node's pool at one thread when auto-distributing a
> non-zero count, so no such pool is left empty. The resulting total may
> exceed the requested count. This only affects the auto-distribute path
> (a single-value array, i.e. svc_set_num_threads()); callers that set
> per-pool counts explicitly via svc_set_pool_threads() are unchanged and
> may still set a pool to zero. Nodes without CPUs (e.g. memory-only nodes)
> get no thread, as nothing is steered to them.
> 
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
>  net/sunrpc/svc.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index c9fba7edaace..ae93a6f51087 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -837,6 +837,12 @@ EXPORT_SYMBOL_GPL(svc_set_pool_threads);
>   * are multiple pools then the new threads or victims will be distributed
>   * evenly among them.
>   *
> + * When @nrservs is non-zero but smaller than the number of pools, even
> + * distribution would leave some pools empty. Since each pool maps to a
> + * NUMA node and only services transports steered to that node, every
> + * pool whose node has CPUs is instead guaranteed at least one thread.
> + * The resulting total may therefore exceed @nrservs.
> + *
>   * Caller must ensure mutual exclusion between this and server startup or
>   * shutdown.
>   *
> @@ -861,6 +867,15 @@ svc_set_num_threads(struct svc_serv *serv, unsigned int min_threads,
>  			--remain;
>  		}
>  
> +		/*
> +		 * Don't let a node's pool sit empty while threads are
> +		 * being auto-distributed: a transport steered there would
> +		 * have nothing to service it.
> +		 */
> +		if (threads == 0 && nrservs &&
> +		    nr_cpus_node(svc_pool_map_get_node(pool->sp_id)))

svc_pool_map_init_pernode() uses for_each_node_with_cpus() so we can be
certain that each node which has been allocated a pool will have at
least 1 cpu.  Thus that last condition isn't needed.

I would probably address the problem outside the loop with

if (base == 0 && nrservs != 0)
   /* We need at least one thread per pool for correct functionality */
   remain = serv->sv_nrpools;

or similar.  But your version works too and this isn't
performance-critical code.

Reviewed-by: NeilBrown <neil@xxxxxxxxxx>

Thanks,
NeilBrown