| 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: <fb148cf880aac410bf487a40a458acfe6a257a84.camel@kernel.org>
Date: Sun, 14 Dec 2025 06:47:21 +0900
From: Jeff Layton <jlayton@...nel.org>
To: Chuck Lever <cel@...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 6/6] nfsd: add controls to set the minimum number of
threads per pool
On Sat, 2025-12-13 at 16:10 -0500, Chuck Lever wrote:
>
> On Fri, Dec 12, 2025, at 5:39 PM, Jeff Layton wrote:
> > Add a new "min_threads" variable to the nfsd_net, along with the
> > corresponding nfsdfs and netlink interfaces to set that value from
> > userland. Pass that value to svc_set_pool_threads() and
> > svc_set_num_threads().
> >
> > Signed-off-by: Jeff Layton <jlayton@...nel.org>
> > ---
> > Documentation/netlink/specs/nfsd.yaml | 5 ++++
> > fs/nfsd/netlink.c | 5 ++--
> > fs/nfsd/netns.h | 6 +++++
> > fs/nfsd/nfsctl.c | 50 +++++++++++++++++++++++++++++++++++
> > fs/nfsd/nfssvc.c | 8 +++---
> > fs/nfsd/trace.h | 19 +++++++++++++
> > include/uapi/linux/nfsd_netlink.h | 1 +
> > 7 files changed, 88 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/netlink/specs/nfsd.yaml
> > b/Documentation/netlink/specs/nfsd.yaml
> > index
> > 100363029e82aed87295e34a008ab771a95d508c..badb2fe57c9859c6932c621a589da694782b0272
> > 100644
> > --- a/Documentation/netlink/specs/nfsd.yaml
> > +++ b/Documentation/netlink/specs/nfsd.yaml
> > @@ -78,6 +78,9 @@ attribute-sets:
> > -
> > name: scope
> > type: string
> > + -
> > + name: min-threads
> > + type: u32
> > -
> > name: version
> > attributes:
> > @@ -159,6 +162,7 @@ operations:
> > - gracetime
> > - leasetime
> > - scope
> > + - min-threads
> > -
> > name: threads-get
> > doc: get the number of running threads
> > @@ -170,6 +174,7 @@ operations:
> > - gracetime
> > - leasetime
> > - scope
> > + - min-threads
> > -
> > name: version-set
> > doc: set nfs enabled versions
> > diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
> > index
> > ac51a44e1065ec3f1d88165f70a831a828b58394..887525964451e640304371e33aa4f415b4ff2848
> > 100644
> > --- a/fs/nfsd/netlink.c
> > +++ b/fs/nfsd/netlink.c
> > @@ -24,11 +24,12 @@ const struct nla_policy
> > nfsd_version_nl_policy[NFSD_A_VERSION_ENABLED + 1] = {
> > };
> >
> > /* NFSD_CMD_THREADS_SET - do */
> > -static const struct nla_policy
> > nfsd_threads_set_nl_policy[NFSD_A_SERVER_SCOPE + 1] = {
> > +static const struct nla_policy
> > nfsd_threads_set_nl_policy[NFSD_A_SERVER_MIN_THREADS + 1] = {
> > [NFSD_A_SERVER_THREADS] = { .type = NLA_U32, },
> > [NFSD_A_SERVER_GRACETIME] = { .type = NLA_U32, },
> > [NFSD_A_SERVER_LEASETIME] = { .type = NLA_U32, },
> > [NFSD_A_SERVER_SCOPE] = { .type = NLA_NUL_STRING, },
> > + [NFSD_A_SERVER_MIN_THREADS] = { .type = NLA_U32, },
> > };
> >
> > /* NFSD_CMD_VERSION_SET - do */
> > @@ -57,7 +58,7 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
> > .cmd = NFSD_CMD_THREADS_SET,
> > .doit = nfsd_nl_threads_set_doit,
> > .policy = nfsd_threads_set_nl_policy,
> > - .maxattr = NFSD_A_SERVER_SCOPE,
> > + .maxattr = NFSD_A_SERVER_MIN_THREADS,
> > .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> > },
> > {
> > diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> > index
> > 3e2d0fde80a7ce434ef2cce9f1666c2bd16ab2eb..1c3449810eaefea8167ddd284af7bd66cac7e211
> > 100644
> > --- a/fs/nfsd/netns.h
> > +++ b/fs/nfsd/netns.h
> > @@ -128,6 +128,12 @@ struct nfsd_net {
> > seqlock_t writeverf_lock;
> > unsigned char writeverf[8];
> >
> > + /*
> > + * Minimum number of threads to run per pool. If 0 then the
> > + * min == max requested number of threads.
> > + */
> > + unsigned int min_threads;
> > +
> > u32 clientid_base;
> > u32 clientid_counter;
> > u32 clverifier_counter;
> > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > index
> > 206534fccf36a992026669fee6533adff1062c36..a5401015e62499d07150cde8822f1e7dd0515dfe
> > 100644
> > --- a/fs/nfsd/nfsctl.c
> > +++ b/fs/nfsd/nfsctl.c
> > @@ -48,6 +48,7 @@ enum {
> > NFSD_Versions,
> > NFSD_Ports,
> > NFSD_MaxBlkSize,
> > + NFSD_MinThreads,
> > NFSD_Filecache,
> > NFSD_Leasetime,
> > NFSD_Gracetime,
> > @@ -67,6 +68,7 @@ static ssize_t write_pool_threads(struct file *file,
> > char *buf, size_t size);
> > static ssize_t write_versions(struct file *file, char *buf, size_t
> > size);
> > static ssize_t write_ports(struct file *file, char *buf, size_t size);
> > static ssize_t write_maxblksize(struct file *file, char *buf, size_t
> > size);
> > +static ssize_t write_minthreads(struct file *file, char *buf, size_t
> > size);
> > #ifdef CONFIG_NFSD_V4
> > static ssize_t write_leasetime(struct file *file, char *buf, size_t
> > size);
> > static ssize_t write_gracetime(struct file *file, char *buf, size_t
> > size);
> > @@ -85,6 +87,7 @@ static ssize_t (*const write_op[])(struct file *,
> > char *, size_t) = {
> > [NFSD_Versions] = write_versions,
> > [NFSD_Ports] = write_ports,
> > [NFSD_MaxBlkSize] = write_maxblksize,
> > + [NFSD_MinThreads] = write_minthreads,
> > #ifdef CONFIG_NFSD_V4
> > [NFSD_Leasetime] = write_leasetime,
> > [NFSD_Gracetime] = write_gracetime,
> > @@ -899,6 +902,46 @@ static ssize_t write_maxblksize(struct file *file,
> > char *buf, size_t size)
> > nfsd_max_blksize);
> > }
> >
> > +/*
> > + * write_minthreads - Set or report the current min number of threads
> > + *
> > + * Input:
> > + * buf: ignored
> > + * size: zero
> > + * OR
> > + *
> > + * Input:
> > + * buf: C string containing an unsigned
> > + * integer value representing the new
> > + * max number of threads
>
> s/max number of threads/min number of threads
>
Will fix.
>
> > + * size: non-zero length of C string in @buf
> > + * Output:
> > + * On success: passed-in buffer filled with '\n'-terminated C string
> > + * containing numeric value of min_threads setting
> > + * for this net namespace;
> > + * return code is the size in bytes of the string
> > + * On error: return code is zero or a negative errno value
> > + */
> > +static ssize_t write_minthreads(struct file *file, char *buf, size_t
> > size)
> > +{
> > + char *mesg = buf;
> > + struct nfsd_net *nn = net_generic(netns(file), nfsd_net_id);
> > + unsigned int minthreads = nn->min_threads;
> > +
> > + if (size > 0) {
>
> What if @size is a very large number?
>
Then it'll basically be ignored and you'll get a static "threads" count
of threads.
>
> > + int rv = get_uint(&mesg, &minthreads);
> > +
> > + if (rv)
> > + return rv;
> > + trace_nfsd_ctl_minthreads(netns(file), minthreads);
> > + mutex_lock(&nfsd_mutex);
> > + nn->min_threads = minthreads;
> > + mutex_unlock(&nfsd_mutex);
> > + }
> > +
> > + return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%u\n", minthreads);
> > +}
> > +
> > #ifdef CONFIG_NFSD_V4
> > static ssize_t __nfsd4_write_time(struct file *file, char *buf, size_t
> > size,
> > time64_t *time, struct nfsd_net *nn)
> > @@ -1292,6 +1335,7 @@ static int nfsd_fill_super(struct super_block
> > *sb, struct fs_context *fc)
> > [NFSD_Versions] = {"versions", &transaction_ops, S_IWUSR|S_IRUSR},
> > [NFSD_Ports] = {"portlist", &transaction_ops, S_IWUSR|S_IRUGO},
> > [NFSD_MaxBlkSize] = {"max_block_size", &transaction_ops,
> > S_IWUSR|S_IRUGO},
> > + [NFSD_MinThreads] = {"min_threads", &transaction_ops,
> > S_IWUSR|S_IRUGO},
> > [NFSD_Filecache] = {"filecache", &nfsd_file_cache_stats_fops,
> > S_IRUGO},
> > #ifdef CONFIG_NFSD_V4
> > [NFSD_Leasetime] = {"nfsv4leasetime", &transaction_ops,
> > S_IWUSR|S_IRUSR},
> > @@ -1636,6 +1680,10 @@ int nfsd_nl_threads_set_doit(struct sk_buff
> > *skb, struct genl_info *info)
> > scope = nla_data(attr);
> > }
> >
> > + attr = info->attrs[NFSD_A_SERVER_MIN_THREADS];
> > + if (attr)
> > + nn->min_threads = nla_get_u32(attr);
> > +
> > ret = nfsd_svc(nrpools, nthreads, net, get_current_cred(), scope);
> > if (ret > 0)
> > ret = 0;
> > @@ -1675,6 +1723,8 @@ int nfsd_nl_threads_get_doit(struct sk_buff *skb,
> > struct genl_info *info)
> > nn->nfsd4_grace) ||
> > nla_put_u32(skb, NFSD_A_SERVER_LEASETIME,
> > nn->nfsd4_lease) ||
> > + nla_put_u32(skb, NFSD_A_SERVER_MIN_THREADS,
> > + nn->min_threads) ||
> > nla_put_string(skb, NFSD_A_SERVER_SCOPE,
> > nn->nfsd_name);
> > if (err)
> > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> > index
> > 26c3a6cb1f400f1b757d26f6ba77e27deb7e8ee2..d6120dd843ac1b6a42f0ef331700f4d6d70d8c38
> > 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, 0);
> > + svc_set_num_threads(serv, 0, nn->min_threads);
>
> Seems like this could actually /start/ threads during NFSD shutdown.
> At the very least it needs an explanatory comment.
>
I can add a comment, but if min-threads > threads, then min-threads
will have no effect.
>
> > 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], 0);
> > + return svc_set_num_threads(nn->nfsd_serv, nthreads[0], nn->min_threads);
> >
> > 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], 0);
> > + nthreads[i], nn->min_threads);
> > 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, nn->min_threads);
> > if (err)
> > goto out;
> > }
> > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > index
> > 8885fd9bead98ebf55379d68ab9c3701981a5150..d1d0b0dd054588a8c20e3386356dfa4e9632b8e0
> > 100644
> > --- a/fs/nfsd/trace.h
> > +++ b/fs/nfsd/trace.h
> > @@ -2164,6 +2164,25 @@ TRACE_EVENT(nfsd_ctl_maxblksize,
> > )
> > );
> >
> > +TRACE_EVENT(nfsd_ctl_minthreads,
> > + TP_PROTO(
> > + const struct net *net,
> > + int minthreads
> > + ),
> > + TP_ARGS(net, minthreads),
> > + TP_STRUCT__entry(
> > + __field(unsigned int, netns_ino)
> > + __field(int, minthreads)
> > + ),
> > + TP_fast_assign(
> > + __entry->netns_ino = net->ns.inum;
> > + __entry->minthreads = minthreads
> > + ),
> > + TP_printk("minthreads=%d",
> > + __entry->minthreads
> > + )
> > +);
> > +
> > TRACE_EVENT(nfsd_ctl_time,
> > TP_PROTO(
> > const struct net *net,
> > diff --git a/include/uapi/linux/nfsd_netlink.h
> > b/include/uapi/linux/nfsd_netlink.h
> > index
> > e157e2009ea8c1ef805301261d536c82677821ef..e9efbc9e63d83ed25fcd790b7a877c0023638f15
> > 100644
> > --- a/include/uapi/linux/nfsd_netlink.h
> > +++ b/include/uapi/linux/nfsd_netlink.h
> > @@ -35,6 +35,7 @@ enum {
> > NFSD_A_SERVER_GRACETIME,
> > NFSD_A_SERVER_LEASETIME,
> > NFSD_A_SERVER_SCOPE,
> > + NFSD_A_SERVER_MIN_THREADS,
> >
> > __NFSD_A_SERVER_MAX,
> > NFSD_A_SERVER_MAX = (__NFSD_A_SERVER_MAX - 1)
> >
> > --
> > 2.52.0
>
> Thanks, Neil and Jeff, for pulling all this together. It's good to
> have something we can noodle on now.
>
Definitely. Neil's patches were a great start to this.
--
Jeff Layton <jlayton@...nel.org>
Powered by blists - more mailing lists