svc_set_num_threads() will set the number of running threads for a given
pool. If the pool argument is set to NULL however, it will distribute
the threads among all of the pools evenly.

These divergent codepaths complicate the move to dynamic threading.
Simplify the API by splitting these two cases into different helpers:

Add a new svc_set_pool_threads() function that sets the number of
threads in a single, given pool. Modify svc_set_num_threads() to
distribute the threads evenly between all of the pools and then call
svc_set_pool_threads() for each.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/lockd/svc.c             |  4 ++--
 fs/nfs/callback.c          |  8 +++----
 fs/nfsd/nfssvc.c           | 21 ++++++++----------
 include/linux/sunrpc/svc.h |  3 ++-
 net/sunrpc/svc.c           | 54 +++++++++++++++++++++++++++++++++++++---------
 5 files changed, 61 insertions(+), 29 deletions(-)

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index d68afa196535a8785bab2931c2b14f03a1174ef9..fbf132b4e08d11a91784c21ee0209fd7c149fd9d 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -340,7 +340,7 @@ static int lockd_get(void)
 		return -ENOMEM;
 	}
 
-	error = svc_set_num_threads(serv, NULL, 1);
+	error = svc_set_num_threads(serv, 1);
 	if (error < 0) {
 		svc_destroy(&serv);
 		return error;
@@ -368,7 +368,7 @@ static void lockd_put(void)
 	unregister_inet6addr_notifier(&lockd_inet6addr_notifier);
 #endif
 
-	svc_set_num_threads(nlmsvc_serv, NULL, 0);
+	svc_set_num_threads(nlmsvc_serv, 0);
 	timer_delete_sync(&nlmsvc_retry);
 	svc_destroy(&nlmsvc_serv);
 	dprintk("lockd_down: service destroyed\n");
diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index c8b837006bb27277ab34fe516f1b63992fee9b7f..44b35b7f8dc022f1d8c069eaf2f7d334c93f77fc 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -119,9 +119,9 @@ static int nfs_callback_start_svc(int minorversion, struct rpc_xprt *xprt,
 	if (serv->sv_nrthreads == nrservs)
 		return 0;
 
-	ret = svc_set_num_threads(serv, NULL, nrservs);
+	ret = svc_set_num_threads(serv, nrservs);
 	if (ret) {
-		svc_set_num_threads(serv, NULL, 0);
+		svc_set_num_threads(serv, 0);
 		return ret;
 	}
 	dprintk("nfs_callback_up: service started\n");
@@ -242,7 +242,7 @@ int nfs_callback_up(u32 minorversion, struct rpc_xprt *xprt)
 	cb_info->users++;
 err_net:
 	if (!cb_info->users) {
-		svc_set_num_threads(cb_info->serv, NULL, 0);
+		svc_set_num_threads(cb_info->serv, 0);
 		svc_destroy(&cb_info->serv);
 	}
 err_create:
@@ -268,7 +268,7 @@ void nfs_callback_down(int minorversion, struct net *net)
 	nfs_callback_down_net(minorversion, serv, net);
 	cb_info->users--;
 	if (cb_info->users == 0) {
-		svc_set_num_threads(serv, NULL, 0);
+		svc_set_num_threads(serv, 0);
 		dprintk("nfs_callback_down: service destroyed\n");
 		svc_destroy(&cb_info->serv);
 	}
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 93f7435cafd2362d9ddb28815277c824067cb370..aafec8ff588b85b0e26d40b76ef00953dc6472b4 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -594,7 +594,7 @@ void nfsd_shutdown_threads(struct net *net)
 	}
 
 	/* Kill outstanding nfsd threads */
-	svc_set_num_threads(serv, NULL, 0);
+	svc_set_num_threads(serv, 0);
 	nfsd_destroy_serv(net);
 	mutex_unlock(&nfsd_mutex);
 }
@@ -702,12 +702,9 @@ int nfsd_set_nrthreads(int n, int *nthreads, struct net *net)
 	if (nn->nfsd_serv == NULL || n <= 0)
 		return 0;
 
-	/*
-	 * Special case: When n == 1, pass in NULL for the pool, so that the
-	 * change is distributed equally among them.
-	 */
+	/* Special case: When n == 1, distribute threads equally among pools. */
 	if (n == 1)
-		return svc_set_num_threads(nn->nfsd_serv, NULL, nthreads[0]);
+		return svc_set_num_threads(nn->nfsd_serv, nthreads[0]);
 
 	if (n > nn->nfsd_serv->sv_nrpools)
 		n = nn->nfsd_serv->sv_nrpools;
@@ -733,18 +730,18 @@ int nfsd_set_nrthreads(int n, int *nthreads, struct net *net)
 
 	/* apply the new numbers */
 	for (i = 0; i < n; i++) {
-		err = svc_set_num_threads(nn->nfsd_serv,
-					  &nn->nfsd_serv->sv_pools[i],
-					  nthreads[i]);
+		err = svc_set_pool_threads(nn->nfsd_serv,
+					   &nn->nfsd_serv->sv_pools[i],
+					   nthreads[i]);
 		if (err)
 			goto out;
 	}
 
 	/* Anything undefined in array is considered to be 0 */
 	for (i = n; i < nn->nfsd_serv->sv_nrpools; ++i) {
-		err = svc_set_num_threads(nn->nfsd_serv,
-					  &nn->nfsd_serv->sv_pools[i],
-					  0);
+		err = svc_set_pool_threads(nn->nfsd_serv,
+					   &nn->nfsd_serv->sv_pools[i],
+					   0);
 		if (err)
 			goto out;
 	}
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 5506d20857c318774cd223272d4b0022cc19ffb8..dd5fbbf8b3d39df6c17a7624edf344557fffd32c 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -446,7 +446,8 @@ struct svc_serv *  svc_create_pooled(struct svc_program *prog,
 				     struct svc_stat *stats,
 				     unsigned int bufsize,
 				     int (*threadfn)(void *data));
-int		   svc_set_num_threads(struct svc_serv *, struct svc_pool *, int);
+int		   svc_set_pool_threads(struct svc_serv *, struct svc_pool *, int);
+int		   svc_set_num_threads(struct svc_serv *, int);
 int		   svc_pool_stats_open(struct svc_info *si, struct file *file);
 void		   svc_process(struct svc_rqst *rqstp);
 void		   svc_process_bc(struct rpc_rqst *req, struct svc_rqst *rqstp);
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 4704dce7284eccc9e2bc64cf22947666facfa86a..3fe5a7f8e57e3fa3837265ec06884b357d5373ff 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -856,15 +856,12 @@ svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs)
 }
 
 /**
- * svc_set_num_threads - adjust number of threads per RPC service
+ * svc_set_pool_threads - adjust number of threads per pool
  * @serv: RPC service to adjust
- * @pool: Specific pool from which to choose threads, or NULL
+ * @pool: Specific pool from which to choose threads
  * @nrservs: New number of threads for @serv (0 or less means kill all threads)
  *
- * Create or destroy threads to make the number of threads for @serv the
- * given number. If @pool is non-NULL, change only threads in that pool;
- * otherwise, round-robin between all pools for @serv. @serv's
- * sv_nrthreads is adjusted for each thread created or destroyed.
+ * Create or destroy threads in @pool to bring it to @nrservs.
  *
  * Caller must ensure mutual exclusion between this and server startup or
  * shutdown.
@@ -873,12 +870,12 @@ svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs)
  * starting a thread.
  */
 int
-svc_set_num_threads(struct svc_serv *serv, struct svc_pool *pool, int nrservs)
+svc_set_pool_threads(struct svc_serv *serv, struct svc_pool *pool, int nrservs)
 {
 	if (!pool)
-		nrservs -= serv->sv_nrthreads;
-	else
-		nrservs -= pool->sp_nrthreads;
+		return -EINVAL;
+
+	nrservs -= pool->sp_nrthreads;
 
 	if (nrservs > 0)
 		return svc_start_kthreads(serv, pool, nrservs);
@@ -886,6 +883,43 @@ svc_set_num_threads(struct svc_serv *serv, struct svc_pool *pool, int nrservs)
 		return svc_stop_kthreads(serv, pool, nrservs);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(svc_set_pool_threads);
+
+/**
+ * svc_set_num_threads - adjust number of threads in serv
+ * @serv: RPC service to adjust
+ * @nrservs: New number of threads for @serv (0 or less means kill all threads)
+ *
+ * Create or destroy threads in @serv to bring it to @nrservs. If there
+ * are multiple pools then the new threads or victims will be distributed
+ * evenly among them.
+ *
+ * Caller must ensure mutual exclusion between this and server startup or
+ * shutdown.
+ *
+ * Returns zero on success or a negative errno if an error occurred while
+ * starting a thread.
+ */
+int
+svc_set_num_threads(struct svc_serv *serv, int nrservs)
+{
+	int base = nrservs / serv->sv_nrpools;
+	int remain = nrservs % serv->sv_nrpools;
+	int i, err;
+
+	for (i = 0; i < serv->sv_nrpools; ++i) {
+		int threads = base;
+
+		if (remain) {
+			++threads;
+			--remain;
+		}
+		err = svc_set_pool_threads(serv, &serv->sv_pools[i], threads);
+		if (err)
+			break;
+	}
+	return err;
+}
 EXPORT_SYMBOL_GPL(svc_set_num_threads);
 
 /**

-- 
2.52.0

On Fri, Dec 12, 2025, at 5:39 PM, Jeff Layton wrote:
> svc_set_num_threads() will set the number of running threads for a given
> pool. If the pool argument is set to NULL however, it will distribute
> the threads among all of the pools evenly.
>
> These divergent codepaths complicate the move to dynamic threading.
> Simplify the API by splitting these two cases into different helpers:
>
> Add a new svc_set_pool_threads() function that sets the number of
> threads in a single, given pool. Modify svc_set_num_threads() to
> distribute the threads evenly between all of the pools and then call
> svc_set_pool_threads() for each.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/lockd/svc.c             |  4 ++--
>  fs/nfs/callback.c          |  8 +++----
>  fs/nfsd/nfssvc.c           | 21 ++++++++----------
>  include/linux/sunrpc/svc.h |  3 ++-
>  net/sunrpc/svc.c           | 54 +++++++++++++++++++++++++++++++++++++---------
>  5 files changed, 61 insertions(+), 29 deletions(-)
>
> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> index 
> d68afa196535a8785bab2931c2b14f03a1174ef9..fbf132b4e08d11a91784c21ee0209fd7c149fd9d 
> 100644
> --- a/fs/lockd/svc.c
> +++ b/fs/lockd/svc.c
> @@ -340,7 +340,7 @@ static int lockd_get(void)
>  		return -ENOMEM;
>  	}
> 
> -	error = svc_set_num_threads(serv, NULL, 1);
> +	error = svc_set_num_threads(serv, 1);
>  	if (error < 0) {
>  		svc_destroy(&serv);
>  		return error;
> @@ -368,7 +368,7 @@ static void lockd_put(void)
>  	unregister_inet6addr_notifier(&lockd_inet6addr_notifier);
>  #endif
> 
> -	svc_set_num_threads(nlmsvc_serv, NULL, 0);
> +	svc_set_num_threads(nlmsvc_serv, 0);
>  	timer_delete_sync(&nlmsvc_retry);
>  	svc_destroy(&nlmsvc_serv);
>  	dprintk("lockd_down: service destroyed\n");
> diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> index 
> c8b837006bb27277ab34fe516f1b63992fee9b7f..44b35b7f8dc022f1d8c069eaf2f7d334c93f77fc 
> 100644
> --- a/fs/nfs/callback.c
> +++ b/fs/nfs/callback.c
> @@ -119,9 +119,9 @@ static int nfs_callback_start_svc(int minorversion, 
> struct rpc_xprt *xprt,
>  	if (serv->sv_nrthreads == nrservs)
>  		return 0;
> 
> -	ret = svc_set_num_threads(serv, NULL, nrservs);
> +	ret = svc_set_num_threads(serv, nrservs);
>  	if (ret) {
> -		svc_set_num_threads(serv, NULL, 0);
> +		svc_set_num_threads(serv, 0);
>  		return ret;
>  	}
>  	dprintk("nfs_callback_up: service started\n");
> @@ -242,7 +242,7 @@ int nfs_callback_up(u32 minorversion, struct 
> rpc_xprt *xprt)
>  	cb_info->users++;
>  err_net:
>  	if (!cb_info->users) {
> -		svc_set_num_threads(cb_info->serv, NULL, 0);
> +		svc_set_num_threads(cb_info->serv, 0);
>  		svc_destroy(&cb_info->serv);
>  	}
>  err_create:
> @@ -268,7 +268,7 @@ void nfs_callback_down(int minorversion, struct net 
> *net)
>  	nfs_callback_down_net(minorversion, serv, net);
>  	cb_info->users--;
>  	if (cb_info->users == 0) {
> -		svc_set_num_threads(serv, NULL, 0);
> +		svc_set_num_threads(serv, 0);
>  		dprintk("nfs_callback_down: service destroyed\n");
>  		svc_destroy(&cb_info->serv);
>  	}
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index 
> 93f7435cafd2362d9ddb28815277c824067cb370..aafec8ff588b85b0e26d40b76ef00953dc6472b4 
> 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -594,7 +594,7 @@ void nfsd_shutdown_threads(struct net *net)
>  	}
> 
>  	/* Kill outstanding nfsd threads */
> -	svc_set_num_threads(serv, NULL, 0);
> +	svc_set_num_threads(serv, 0);
>  	nfsd_destroy_serv(net);
>  	mutex_unlock(&nfsd_mutex);
>  }
> @@ -702,12 +702,9 @@ int nfsd_set_nrthreads(int n, int *nthreads, 
> struct net *net)
>  	if (nn->nfsd_serv == NULL || n <= 0)
>  		return 0;
> 
> -	/*
> -	 * Special case: When n == 1, pass in NULL for the pool, so that the
> -	 * change is distributed equally among them.
> -	 */
> +	/* Special case: When n == 1, distribute threads equally among pools. */
>  	if (n == 1)
> -		return svc_set_num_threads(nn->nfsd_serv, NULL, nthreads[0]);
> +		return svc_set_num_threads(nn->nfsd_serv, nthreads[0]);
> 
>  	if (n > nn->nfsd_serv->sv_nrpools)
>  		n = nn->nfsd_serv->sv_nrpools;
> @@ -733,18 +730,18 @@ int nfsd_set_nrthreads(int n, int *nthreads, 
> struct net *net)
> 
>  	/* apply the new numbers */
>  	for (i = 0; i < n; i++) {
> -		err = svc_set_num_threads(nn->nfsd_serv,
> -					  &nn->nfsd_serv->sv_pools[i],
> -					  nthreads[i]);
> +		err = svc_set_pool_threads(nn->nfsd_serv,
> +					   &nn->nfsd_serv->sv_pools[i],
> +					   nthreads[i]);
>  		if (err)
>  			goto out;
>  	}
> 
>  	/* Anything undefined in array is considered to be 0 */
>  	for (i = n; i < nn->nfsd_serv->sv_nrpools; ++i) {
> -		err = svc_set_num_threads(nn->nfsd_serv,
> -					  &nn->nfsd_serv->sv_pools[i],
> -					  0);
> +		err = svc_set_pool_threads(nn->nfsd_serv,
> +					   &nn->nfsd_serv->sv_pools[i],
> +					   0);
>  		if (err)
>  			goto out;
>  	}
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index 
> 5506d20857c318774cd223272d4b0022cc19ffb8..dd5fbbf8b3d39df6c17a7624edf344557fffd32c 
> 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -446,7 +446,8 @@ struct svc_serv *  svc_create_pooled(struct 
> svc_program *prog,
>  				     struct svc_stat *stats,
>  				     unsigned int bufsize,
>  				     int (*threadfn)(void *data));
> -int		   svc_set_num_threads(struct svc_serv *, struct svc_pool *, int);
> +int		   svc_set_pool_threads(struct svc_serv *, struct svc_pool *, 
> int);
> +int		   svc_set_num_threads(struct svc_serv *, int);
>  int		   svc_pool_stats_open(struct svc_info *si, struct file *file);
>  void		   svc_process(struct svc_rqst *rqstp);
>  void		   svc_process_bc(struct rpc_rqst *req, struct svc_rqst *rqstp);
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 
> 4704dce7284eccc9e2bc64cf22947666facfa86a..3fe5a7f8e57e3fa3837265ec06884b357d5373ff 
> 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -856,15 +856,12 @@ svc_stop_kthreads(struct svc_serv *serv, struct 
> svc_pool *pool, int nrservs)
>  }
> 
>  /**
> - * svc_set_num_threads - adjust number of threads per RPC service
> + * svc_set_pool_threads - adjust number of threads per pool
>   * @serv: RPC service to adjust
> - * @pool: Specific pool from which to choose threads, or NULL
> + * @pool: Specific pool from which to choose threads
>   * @nrservs: New number of threads for @serv (0 or less means kill all 
> threads)
>   *
> - * Create or destroy threads to make the number of threads for @serv 
> the
> - * given number. If @pool is non-NULL, change only threads in that 
> pool;
> - * otherwise, round-robin between all pools for @serv. @serv's
> - * sv_nrthreads is adjusted for each thread created or destroyed.
> + * Create or destroy threads in @pool to bring it to @nrservs.
>   *
>   * Caller must ensure mutual exclusion between this and server startup 
> or
>   * shutdown.
> @@ -873,12 +870,12 @@ svc_stop_kthreads(struct svc_serv *serv, struct 
> svc_pool *pool, int nrservs)
>   * starting a thread.
>   */
>  int
> -svc_set_num_threads(struct svc_serv *serv, struct svc_pool *pool, int 
> nrservs)
> +svc_set_pool_threads(struct svc_serv *serv, struct svc_pool *pool, int 
> nrservs)
>  {
>  	if (!pool)
> -		nrservs -= serv->sv_nrthreads;
> -	else
> -		nrservs -= pool->sp_nrthreads;
> +		return -EINVAL;
> +
> +	nrservs -= pool->sp_nrthreads;
> 
>  	if (nrservs > 0)
>  		return svc_start_kthreads(serv, pool, nrservs);
> @@ -886,6 +883,43 @@ svc_set_num_threads(struct svc_serv *serv, struct 
> svc_pool *pool, int nrservs)
>  		return svc_stop_kthreads(serv, pool, nrservs);
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(svc_set_pool_threads);
> +
> +/**
> + * svc_set_num_threads - adjust number of threads in serv
> + * @serv: RPC service to adjust
> + * @nrservs: New number of threads for @serv (0 or less means kill all 
> threads)
> + *
> + * Create or destroy threads in @serv to bring it to @nrservs. If there
> + * are multiple pools then the new threads or victims will be 
> distributed
> + * evenly among them.
> + *
> + * Caller must ensure mutual exclusion between this and server startup 
> or
> + * shutdown.
> + *
> + * Returns zero on success or a negative errno if an error occurred 
> while
> + * starting a thread.
> + */
> +int
> +svc_set_num_threads(struct svc_serv *serv, int nrservs)
> +{
> +	int base = nrservs / serv->sv_nrpools;
> +	int remain = nrservs % serv->sv_nrpools;
> +	int i, err;
> +
> +	for (i = 0; i < serv->sv_nrpools; ++i) {

If sv_nrpools happens to be zero, then the loop doesn't
execute at all, and err is left containing stack garbage.
Is sv_nrpools guaranteed to be non-zero? If not then err
needs to be initialized before the loop runs. I see that
nfsd_set_nrthreads() in fs/nfsd/nfssvc.c has "int err = 0"
for a similar loop pattern.


> +		int threads = base;
> +
> +		if (remain) {
> +			++threads;
> +			--remain;
> +		}
> +		err = svc_set_pool_threads(serv, &serv->sv_pools[i], threads);
> +		if (err)
> +			break;
> +	}
> +	return err;
> +}
>  EXPORT_SYMBOL_GPL(svc_set_num_threads);
> 
>  /**
>
> -- 
> 2.52.0

-- 
Chuck Lever

On Sat, 2025-12-13 at 14:29 -0500, Chuck Lever wrote:
> 
> On Fri, Dec 12, 2025, at 5:39 PM, Jeff Layton wrote:
> > svc_set_num_threads() will set the number of running threads for a given
> > pool. If the pool argument is set to NULL however, it will distribute
> > the threads among all of the pools evenly.
> > 
> > These divergent codepaths complicate the move to dynamic threading.
> > Simplify the API by splitting these two cases into different helpers:
> > 
> > Add a new svc_set_pool_threads() function that sets the number of
> > threads in a single, given pool. Modify svc_set_num_threads() to
> > distribute the threads evenly between all of the pools and then call
> > svc_set_pool_threads() for each.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/lockd/svc.c             |  4 ++--
> >  fs/nfs/callback.c          |  8 +++----
> >  fs/nfsd/nfssvc.c           | 21 ++++++++----------
> >  include/linux/sunrpc/svc.h |  3 ++-
> >  net/sunrpc/svc.c           | 54 +++++++++++++++++++++++++++++++++++++---------
> >  5 files changed, 61 insertions(+), 29 deletions(-)
> > 
> > diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> > index 
> > d68afa196535a8785bab2931c2b14f03a1174ef9..fbf132b4e08d11a91784c21ee0209fd7c149fd9d 
> > 100644
> > --- a/fs/lockd/svc.c
> > +++ b/fs/lockd/svc.c
> > @@ -340,7 +340,7 @@ static int lockd_get(void)
> >  		return -ENOMEM;
> >  	}
> > 
> > -	error = svc_set_num_threads(serv, NULL, 1);
> > +	error = svc_set_num_threads(serv, 1);
> >  	if (error < 0) {
> >  		svc_destroy(&serv);
> >  		return error;
> > @@ -368,7 +368,7 @@ static void lockd_put(void)
> >  	unregister_inet6addr_notifier(&lockd_inet6addr_notifier);
> >  #endif
> > 
> > -	svc_set_num_threads(nlmsvc_serv, NULL, 0);
> > +	svc_set_num_threads(nlmsvc_serv, 0);
> >  	timer_delete_sync(&nlmsvc_retry);
> >  	svc_destroy(&nlmsvc_serv);
> >  	dprintk("lockd_down: service destroyed\n");
> > diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> > index 
> > c8b837006bb27277ab34fe516f1b63992fee9b7f..44b35b7f8dc022f1d8c069eaf2f7d334c93f77fc 
> > 100644
> > --- a/fs/nfs/callback.c
> > +++ b/fs/nfs/callback.c
> > @@ -119,9 +119,9 @@ static int nfs_callback_start_svc(int minorversion, 
> > struct rpc_xprt *xprt,
> >  	if (serv->sv_nrthreads == nrservs)
> >  		return 0;
> > 
> > -	ret = svc_set_num_threads(serv, NULL, nrservs);
> > +	ret = svc_set_num_threads(serv, nrservs);
> >  	if (ret) {
> > -		svc_set_num_threads(serv, NULL, 0);
> > +		svc_set_num_threads(serv, 0);
> >  		return ret;
> >  	}
> >  	dprintk("nfs_callback_up: service started\n");
> > @@ -242,7 +242,7 @@ int nfs_callback_up(u32 minorversion, struct 
> > rpc_xprt *xprt)
> >  	cb_info->users++;
> >  err_net:
> >  	if (!cb_info->users) {
> > -		svc_set_num_threads(cb_info->serv, NULL, 0);
> > +		svc_set_num_threads(cb_info->serv, 0);
> >  		svc_destroy(&cb_info->serv);
> >  	}
> >  err_create:
> > @@ -268,7 +268,7 @@ void nfs_callback_down(int minorversion, struct net 
> > *net)
> >  	nfs_callback_down_net(minorversion, serv, net);
> >  	cb_info->users--;
> >  	if (cb_info->users == 0) {
> > -		svc_set_num_threads(serv, NULL, 0);
> > +		svc_set_num_threads(serv, 0);
> >  		dprintk("nfs_callback_down: service destroyed\n");
> >  		svc_destroy(&cb_info->serv);
> >  	}
> > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> > index 
> > 93f7435cafd2362d9ddb28815277c824067cb370..aafec8ff588b85b0e26d40b76ef00953dc6472b4 
> > 100644
> > --- a/fs/nfsd/nfssvc.c
> > +++ b/fs/nfsd/nfssvc.c
> > @@ -594,7 +594,7 @@ void nfsd_shutdown_threads(struct net *net)
> >  	}
> > 
> >  	/* Kill outstanding nfsd threads */
> > -	svc_set_num_threads(serv, NULL, 0);
> > +	svc_set_num_threads(serv, 0);
> >  	nfsd_destroy_serv(net);
> >  	mutex_unlock(&nfsd_mutex);
> >  }
> > @@ -702,12 +702,9 @@ int nfsd_set_nrthreads(int n, int *nthreads, 
> > struct net *net)
> >  	if (nn->nfsd_serv == NULL || n <= 0)
> >  		return 0;
> > 
> > -	/*
> > -	 * Special case: When n == 1, pass in NULL for the pool, so that the
> > -	 * change is distributed equally among them.
> > -	 */
> > +	/* Special case: When n == 1, distribute threads equally among pools. */
> >  	if (n == 1)
> > -		return svc_set_num_threads(nn->nfsd_serv, NULL, nthreads[0]);
> > +		return svc_set_num_threads(nn->nfsd_serv, nthreads[0]);
> > 
> >  	if (n > nn->nfsd_serv->sv_nrpools)
> >  		n = nn->nfsd_serv->sv_nrpools;
> > @@ -733,18 +730,18 @@ int nfsd_set_nrthreads(int n, int *nthreads, 
> > struct net *net)
> > 
> >  	/* apply the new numbers */
> >  	for (i = 0; i < n; i++) {
> > -		err = svc_set_num_threads(nn->nfsd_serv,
> > -					  &nn->nfsd_serv->sv_pools[i],
> > -					  nthreads[i]);
> > +		err = svc_set_pool_threads(nn->nfsd_serv,
> > +					   &nn->nfsd_serv->sv_pools[i],
> > +					   nthreads[i]);
> >  		if (err)
> >  			goto out;
> >  	}
> > 
> >  	/* Anything undefined in array is considered to be 0 */
> >  	for (i = n; i < nn->nfsd_serv->sv_nrpools; ++i) {
> > -		err = svc_set_num_threads(nn->nfsd_serv,
> > -					  &nn->nfsd_serv->sv_pools[i],
> > -					  0);
> > +		err = svc_set_pool_threads(nn->nfsd_serv,
> > +					   &nn->nfsd_serv->sv_pools[i],
> > +					   0);
> >  		if (err)
> >  			goto out;
> >  	}
> > diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> > index 
> > 5506d20857c318774cd223272d4b0022cc19ffb8..dd5fbbf8b3d39df6c17a7624edf344557fffd32c 
> > 100644
> > --- a/include/linux/sunrpc/svc.h
> > +++ b/include/linux/sunrpc/svc.h
> > @@ -446,7 +446,8 @@ struct svc_serv *  svc_create_pooled(struct 
> > svc_program *prog,
> >  				     struct svc_stat *stats,
> >  				     unsigned int bufsize,
> >  				     int (*threadfn)(void *data));
> > -int		   svc_set_num_threads(struct svc_serv *, struct svc_pool *, int);
> > +int		   svc_set_pool_threads(struct svc_serv *, struct svc_pool *, 
> > int);
> > +int		   svc_set_num_threads(struct svc_serv *, int);
> >  int		   svc_pool_stats_open(struct svc_info *si, struct file *file);
> >  void		   svc_process(struct svc_rqst *rqstp);
> >  void		   svc_process_bc(struct rpc_rqst *req, struct svc_rqst *rqstp);
> > diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> > index 
> > 4704dce7284eccc9e2bc64cf22947666facfa86a..3fe5a7f8e57e3fa3837265ec06884b357d5373ff 
> > 100644
> > --- a/net/sunrpc/svc.c
> > +++ b/net/sunrpc/svc.c
> > @@ -856,15 +856,12 @@ svc_stop_kthreads(struct svc_serv *serv, struct 
> > svc_pool *pool, int nrservs)
> >  }
> > 
> >  /**
> > - * svc_set_num_threads - adjust number of threads per RPC service
> > + * svc_set_pool_threads - adjust number of threads per pool
> >   * @serv: RPC service to adjust
> > - * @pool: Specific pool from which to choose threads, or NULL
> > + * @pool: Specific pool from which to choose threads
> >   * @nrservs: New number of threads for @serv (0 or less means kill all 
> > threads)
> >   *
> > - * Create or destroy threads to make the number of threads for @serv 
> > the
> > - * given number. If @pool is non-NULL, change only threads in that 
> > pool;
> > - * otherwise, round-robin between all pools for @serv. @serv's
> > - * sv_nrthreads is adjusted for each thread created or destroyed.
> > + * Create or destroy threads in @pool to bring it to @nrservs.
> >   *
> >   * Caller must ensure mutual exclusion between this and server startup 
> > or
> >   * shutdown.
> > @@ -873,12 +870,12 @@ svc_stop_kthreads(struct svc_serv *serv, struct 
> > svc_pool *pool, int nrservs)
> >   * starting a thread.
> >   */
> >  int
> > -svc_set_num_threads(struct svc_serv *serv, struct svc_pool *pool, int 
> > nrservs)
> > +svc_set_pool_threads(struct svc_serv *serv, struct svc_pool *pool, int 
> > nrservs)
> >  {
> >  	if (!pool)
> > -		nrservs -= serv->sv_nrthreads;
> > -	else
> > -		nrservs -= pool->sp_nrthreads;
> > +		return -EINVAL;
> > +
> > +	nrservs -= pool->sp_nrthreads;
> > 
> >  	if (nrservs > 0)
> >  		return svc_start_kthreads(serv, pool, nrservs);
> > @@ -886,6 +883,43 @@ svc_set_num_threads(struct svc_serv *serv, struct 
> > svc_pool *pool, int nrservs)
> >  		return svc_stop_kthreads(serv, pool, nrservs);
> >  	return 0;
> >  }
> > +EXPORT_SYMBOL_GPL(svc_set_pool_threads);
> > +
> > +/**
> > + * svc_set_num_threads - adjust number of threads in serv
> > + * @serv: RPC service to adjust
> > + * @nrservs: New number of threads for @serv (0 or less means kill all 
> > threads)
> > + *
> > + * Create or destroy threads in @serv to bring it to @nrservs. If there
> > + * are multiple pools then the new threads or victims will be 
> > distributed
> > + * evenly among them.
> > + *
> > + * Caller must ensure mutual exclusion between this and server startup 
> > or
> > + * shutdown.
> > + *
> > + * Returns zero on success or a negative errno if an error occurred 
> > while
> > + * starting a thread.
> > + */
> > +int
> > +svc_set_num_threads(struct svc_serv *serv, int nrservs)
> > +{
> > +	int base = nrservs / serv->sv_nrpools;
> > +	int remain = nrservs % serv->sv_nrpools;
> > +	int i, err;
> > +
> > +	for (i = 0; i < serv->sv_nrpools; ++i) {
> 
> If sv_nrpools happens to be zero, then the loop doesn't
> execute at all, and err is left containing stack garbage.
> Is sv_nrpools guaranteed to be non-zero? If not then err
> needs to be initialized before the loop runs. I see that
> nfsd_set_nrthreads() in fs/nfsd/nfssvc.c has "int err = 0"
> for a similar loop pattern.
> 

sv_nrpools should always be non-zero. There are many places in the rpc
layer that depend on having at least one pool. From __svc_create:

        serv->sv_nrpools = npools;
        serv->sv_pools =
                kcalloc(serv->sv_nrpools, sizeof(struct svc_pool),
                        GFP_KERNEL);
        if (!serv->sv_pools) {
                kfree(serv);
                return NULL;
        }

I think kcalloc returns NULL if you pass it 0 for "n", so creation
should fail if that happens. None of the in-tree callers ever pass in 0
for the npools, but maybe that's worth an explicit check in
__svc_create(). I can add one.

> 
> > +		int threads = base;
> > +
> > +		if (remain) {
> > +			++threads;
> > +			--remain;
> > +		}
> > +		err = svc_set_pool_threads(serv, &serv->sv_pools[i], threads);
> > +		if (err)
> > +			break;
> > +	}
> > +	return err;
> > +}
> >  EXPORT_SYMBOL_GPL(svc_set_num_threads);
> > 
> >  /**
> > 
> > -- 
> > 2.52.0

-- 
Jeff Layton <jlayton@kernel.org>