| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <828c4fe7-930b-41b3-be73-62b2c76f43e4@app.fastmail.com>
Date: Sat, 13 Dec 2025 15:19:59 -0500
From: "Chuck Lever" <cel@...nel.org>
To: "Jeff Layton" <jlayton@...nel.org>,
"Chuck Lever" <chuck.lever@...cle.com>, NeilBrown <neil@...wn.name>,
"Olga Kornievskaia" <okorniev@...hat.com>, "Dai Ngo" <Dai.Ngo@...cle.com>,
"Tom Talpey" <tom@...pey.com>, "Trond Myklebust" <trondmy@...nel.org>,
"Anna Schumaker" <anna@...nel.org>
Cc: linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC 4/6] sunrpc: introduce the concept of a minimum number of
threads per pool
On Fri, Dec 12, 2025, at 5:39 PM, Jeff Layton wrote:
> Add a new pool->sp_nrthrmin field to track the minimum number of threads
> in a pool. Add min_threads parameters to both svc_set_num_threads() and
> svc_set_pool_threads(). If min_threads is non-zero, then have
> svc_set_num_threads() ensure that the number of running threads is
> between the min and the max.
>
> For now, the min_threads is always 0, but a later patch will pass the
> proper value through from nfsd.
>
> Signed-off-by: Jeff Layton <jlayton@...nel.org>
> ---
> fs/lockd/svc.c | 4 ++--
> fs/nfs/callback.c | 8 ++++----
> fs/nfsd/nfssvc.c | 8 ++++----
> include/linux/sunrpc/svc.h | 7 ++++---
> net/sunrpc/svc.c | 21 ++++++++++++++++++---
> 5 files changed, 32 insertions(+), 16 deletions(-)
>
> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> index
> fbf132b4e08d11a91784c21ee0209fd7c149fd9d..7899205314391415dfb698ab58fe97efc426d928
> 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, 1);
> + error = svc_set_num_threads(serv, 1, 0);
> 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, 0);
> + svc_set_num_threads(nlmsvc_serv, 0, 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
> 44b35b7f8dc022f1d8c069eaf2f7d334c93f77fc..32bbc0e688ff3988e4ba50eeb36b4808cec07c87
> 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, nrservs);
> + ret = svc_set_num_threads(serv, nrservs, 0);
> if (ret) {
> - svc_set_num_threads(serv, 0);
> + svc_set_num_threads(serv, 0, 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, 0);
> + svc_set_num_threads(cb_info->serv, 0, 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, 0);
> + svc_set_num_threads(serv, 0, 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
> aafec8ff588b85b0e26d40b76ef00953dc6472b4..993ed338764b0ccd7bdfb76bd6fbb5dc6ab4022d
> 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, 0);
> + svc_set_num_threads(serv, 0, 0);
> nfsd_destroy_serv(net);
> mutex_unlock(&nfsd_mutex);
> }
> @@ -704,7 +704,7 @@ int nfsd_set_nrthreads(int n, int *nthreads, struct
> net *net)
>
> /* Special case: When n == 1, distribute threads equally among pools. */
> if (n == 1)
> - return svc_set_num_threads(nn->nfsd_serv, nthreads[0]);
> + return svc_set_num_threads(nn->nfsd_serv, nthreads[0], 0);
>
> if (n > nn->nfsd_serv->sv_nrpools)
> n = nn->nfsd_serv->sv_nrpools;
> @@ -732,7 +732,7 @@ int nfsd_set_nrthreads(int n, int *nthreads, struct
> net *net)
> for (i = 0; i < n; i++) {
> err = svc_set_pool_threads(nn->nfsd_serv,
> &nn->nfsd_serv->sv_pools[i],
> - nthreads[i]);
> + nthreads[i], 0);
> if (err)
> goto out;
> }
> @@ -741,7 +741,7 @@ int nfsd_set_nrthreads(int n, int *nthreads, struct
> net *net)
> for (i = n; i < nn->nfsd_serv->sv_nrpools; ++i) {
> err = svc_set_pool_threads(nn->nfsd_serv,
> &nn->nfsd_serv->sv_pools[i],
> - 0);
> + 0, 0);
> if (err)
> goto out;
> }
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index
> ee9260ca908c907f4373f4cfa471b272bc7bcc8c..35bd3247764ae8dc5dcdfffeea36f7cfefd13372
> 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -36,6 +36,7 @@
> struct svc_pool {
> unsigned int sp_id; /* pool id; also node id on NUMA */
> unsigned int sp_nrthreads; /* # of threads currently running in pool
> */
> + unsigned int sp_nrthrmin; /* Min number of threads to run per pool */
> unsigned int sp_nrthrmax; /* Max requested number of threads in pool
> */
> struct lwq sp_xprts; /* pending transports */
> struct list_head sp_all_threads; /* all server threads */
> @@ -72,7 +73,7 @@ struct svc_serv {
> struct svc_stat * sv_stats; /* RPC statistics */
> spinlock_t sv_lock;
> unsigned int sv_nprogs; /* Number of sv_programs */
> - unsigned int sv_nrthreads; /* # of server threads */
> + unsigned int sv_nrthreads; /* # of running server threads */
> unsigned int sv_max_payload; /* datagram payload size */
> unsigned int sv_max_mesg; /* max_payload + 1 page for overheads */
> unsigned int sv_xdrsize; /* XDR buffer size */
> @@ -447,8 +448,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_pool_threads(struct svc_serv *, struct svc_pool *,
> int);
> -int svc_set_num_threads(struct svc_serv *, int);
> +int svc_set_pool_threads(struct svc_serv *, struct svc_pool *,
> int, unsigned int);
> +int svc_set_num_threads(struct svc_serv *, int, unsigned 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
> 8cd45f62ef74af6e0826b8f13cc903b0962af5e0..dc818158f8529b62dcf96c91bd9a9d4ab21df91f
> 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -821,6 +821,7 @@ svc_stop_kthreads(struct svc_serv *serv, struct
> svc_pool *pool, int nrservs)
> * @serv: RPC service to adjust
> * @pool: Specific pool from which to choose threads
> * @nrservs: New number of threads for @serv (0 or less means kill all
> threads)
> + * @min_threads: minimum number of threads per pool (0 means set to
> same as nrservs)
> *
> * Create or destroy threads in @pool to bring it to @nrservs.
> *
> @@ -831,12 +832,22 @@ svc_stop_kthreads(struct svc_serv *serv, struct
> svc_pool *pool, int nrservs)
> * starting a thread.
> */
> int
> -svc_set_pool_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,
> + unsigned int min_threads)
> {
> if (!pool)
> return -EINVAL;
>
> pool->sp_nrthrmax = nrservs;
> + if (min_threads) {
> + if (pool->sp_nrthreads > nrservs) {
> + // fallthrough to update nrservs
> + } else if (pool->sp_nrthreads < min_threads) {
> + nrservs = min_threads;
Nit: The mixed sign types here gives me hives. Can you think of
a reason nrservs is a signed int rather than unsigned?
> + } else {
> + return 0;
> + }
> + }
> nrservs -= pool->sp_nrthreads;
>
> if (nrservs > 0)
> @@ -851,6 +862,7 @@ 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)
> + * @min_threads: minimum number of threads per pool (0 means set to
> same as nrservs)
> *
> * 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
> @@ -863,20 +875,23 @@ EXPORT_SYMBOL_GPL(svc_set_pool_threads);
> * starting a thread.
> */
> int
> -svc_set_num_threads(struct svc_serv *serv, int nrservs)
> +svc_set_num_threads(struct svc_serv *serv, int nrservs, unsigned int
> min_threads)
> {
> int base = nrservs / serv->sv_nrpools;
> int remain = nrservs % serv->sv_nrpools;
> int i, err;
>
> for (i = 0; i < serv->sv_nrpools; ++i) {
> + struct svc_pool *pool = &serv->sv_pools[i];
> int threads = base;
>
> if (remain) {
> ++threads;
> --remain;
> }
> - err = svc_set_pool_threads(serv, &serv->sv_pools[i], threads);
> +
> + pool->sp_nrthrmin = min_threads;
> + err = svc_set_pool_threads(serv, pool, threads, min_threads);
> if (err)
> break;
> }
>
> --
> 2.52.0
--
Chuck Lever
Powered by blists - more mailing lists