Linux NFS development
 help / color / mirror / Atom feed
From: Steve Dickson <steved@redhat.com>
To: Jeff Layton <jlayton@kernel.org>
Cc: Chuck Lever <chuck.lever@oracle.com>, NeilBrown <neil@brown.name>,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
	linux-nfs@vger.kernel.org
Subject: Re: [PATCH nfs-utils v2] nfsdctl: add support for min-threads parameter
Date: Sun, 1 Feb 2026 12:31:37 -0500	[thread overview]
Message-ID: <671224fc-7c42-420e-9dbe-ae125bfdad4d@redhat.com> (raw)
In-Reply-To: <20260123-minthreads-v2-1-9bfbae745845@kernel.org>



On 1/23/26 1:48 PM, Jeff Layton wrote:
> This patch adds proper support to nfsdctl to manage nfsd's new dynamic
> threading when NFSD_A_SERVER_MIN_THREADS is defined in the header:
> 
> If the user sets min-threads, and has a new enough kernel, the
> traditional threads parameters will represent the max number of threads.
> The min-threads value represents the minimum number of threads to run in
> each pool. The kernel will start the minimum number of threads at
> startup time, and the thread count will dynamically fluctuate between
> the min and max based on load.
> 
> Allow the "autostart" subcommand to find the "min-threads" parameter in
> nfs.conf and set it in the netlink call appropriately. If it's not set,
> then assume that it's 0, which will disable dynamic threading.
> 
> For the "threads" subcommand, add a new command-line option. If it's not
> set, then don't send it in the netlink command at all. That allows the
> kernel's existing setting to remain as-is, unless explicitly changed.
> 
> Also, update the documentation with info about min-threads.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
Committed... (tag: nfs-utils-2-8-5-rc3)

steved.

> ---
> This just has a couple of minor revisions to the earlier set. Steve,
> does this look acceptable?
> ---
> Changes in v2:
> - properly handle getopt in threads_func()
> - #ifdef out the --minthreads help text when compiled out
> - Link to v1: https://lore.kernel.org/r/20260112-minthreads-v1-1-30c5f4113720@kernel.org
> ---
>   configure.ac               |  3 ++-
>   systemd/nfs.conf.man       |  3 ++-
>   utils/nfsdctl/nfsdctl.8    | 20 +++++++++++++++++--
>   utils/nfsdctl/nfsdctl.adoc | 13 ++++++++++++
>   utils/nfsdctl/nfsdctl.c    | 49 ++++++++++++++++++++++++++++++++++++++++------
>   5 files changed, 78 insertions(+), 10 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 6da23915836d34ff4d9bdef79af13499990688f9..33866e869666d8ebdebc8d7b5b08bf6ffbe92aa2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -262,6 +262,8 @@ AC_ARG_ENABLE(nfsdctl,
>   		PKG_CHECK_MODULES(LIBNLGENL3, libnl-genl-3.0 >= 3.1)
>   		PKG_CHECK_MODULES(LIBREADLINE, readline)
>   		AC_CHECK_HEADERS(linux/nfsd_netlink.h)
> +		AC_CHECK_DECLS([NFSD_A_SERVER_MIN_THREADS], , ,
> +			       [#include <linux/nfsd_netlink.h>])
>   
>   		# ensure we have the pool-mode commands
>   		AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <linux/nfsd_netlink.h>]],
> @@ -620,7 +622,6 @@ AC_CHECK_SIZEOF(socklen_t,, [AC_INCLUDES_DEFAULT
>                                # include <sys/socket.h>
>                                #endif])
>   
> -
>   dnl *************************************************************
>   dnl Export some path names to config.h
>   dnl *************************************************************
> diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
> index e6a84a9725da5a3cc40611f45d343a670fdb94ca..484de2c086db91ede38490e49411e1514a5da754 100644
> --- a/systemd/nfs.conf.man
> +++ b/systemd/nfs.conf.man
> @@ -162,6 +162,7 @@ option.
>   .B nfsd
>   Recognized values:
>   .BR threads ,
> +.BR min-threads ,
>   .BR host ,
>   .BR scope ,
>   .BR port ,
> @@ -179,7 +180,7 @@ Recognized values:
>   Version and protocol values are Boolean values as described above,
>   and are also used by
>   .BR rpc.mountd .
> -Threads and the two times are integers.
> +Threads, min-threads and the two times are integers.
>   .B port
>   and
>   .B rdma
> diff --git a/utils/nfsdctl/nfsdctl.8 b/utils/nfsdctl/nfsdctl.8
> index d69922448eb17fb155f05dc4ddc9aefffbf966e4..a86ffe8e1f4d39ef7041ac0f6867792466c40af0 100644
> --- a/utils/nfsdctl/nfsdctl.8
> +++ b/utils/nfsdctl/nfsdctl.8
> @@ -2,12 +2,12 @@
>   .\"     Title: nfsdctl
>   .\"    Author: Jeff Layton
>   .\" Generator: Asciidoctor 2.0.20
> -.\"      Date: 2025-01-09
> +.\"      Date: 2026-01-12
>   .\"    Manual: \ \&
>   .\"    Source: \ \&
>   .\"  Language: English
>   .\"
> -.TH "NFSDCTL" "8" "2025-01-09" "\ \&" "\ \&"
> +.TH "NFSDCTL" "8" "2026-01-12" "\ \&" "\ \&"
>   .ie \n(.g .ds Aq \(aq
>   .el       .ds Aq '
>   .ss \n[.ss] 0
> @@ -147,6 +147,22 @@ value of 0 will shut down the NFS server. Run this without arguments to
>   get the current number of running threads in each pool.
>   .RE
>   .sp
> +.if n .RS 4
> +.nf
> +.fam C
> +These options are specific to the "threads" subcommand:
> +
> +\-m, \-\-min\-threads=
> +    If set to a positive, non\-zero value, then dynamic threading is enabled for
> +    nfsd.  In this mode, the traditional "threads" values are treated as a maximum
> +    number of threads. This specifies the minimum number of threads per pool. The
> +    kernel will start the minimum number and dynamically start and stop threads as
> +    needed. If the minimum is larger than the maximum, then dynamic threadis is
> +    disabled, and the maximum number is started.
> +.fam
> +.fi
> +.if n .RE
> +.sp
>   \fBversion\fP
>   .RS 4
>   Get/set the enabled NFS versions for the server. Run without arguments to
> diff --git a/utils/nfsdctl/nfsdctl.adoc b/utils/nfsdctl/nfsdctl.adoc
> index 0207eff6118d2dcc5a794d2013c039d9beb11ddc..e85348e29ed815470d35b6365a212d8872cf645c 100644
> --- a/utils/nfsdctl/nfsdctl.adoc
> +++ b/utils/nfsdctl/nfsdctl.adoc
> @@ -84,6 +84,19 @@ Each subcommand can also accept its own set of options and arguments. The
>     value of 0 will shut down the NFS server. Run this without arguments to
>     get the current number of running threads in each pool.
>   
> +[source,bash]
> +----
> +These options are specific to the "threads" subcommand:
> +
> +-m, --min-threads=
> +    If set to a positive, non-zero value, then dynamic threading is enabled for
> +    nfsd.  In this mode, the traditional "threads" values are treated as a maximum
> +    number of threads. This specifies the minimum number of threads per pool. The
> +    kernel will start the minimum number and dynamically start and stop threads as
> +    needed. If the minimum is larger than the maximum, then dynamic threadis is
> +    disabled, and the maximum number is started.
> +----
> +
>   *version*::
>   
>     Get/set the enabled NFS versions for the server. Run without arguments to
> diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
> index e7a0e12495277d2e98a6c21c7cee29fe459f37cc..1e60b9ae6d9ec61ca243fc62623a1a4b9a3b45a7 100644
> --- a/utils/nfsdctl/nfsdctl.c
> +++ b/utils/nfsdctl/nfsdctl.c
> @@ -19,6 +19,7 @@
>   #include <string.h>
>   #include <sched.h>
>   #include <sys/queue.h>
> +#include <limits.h>
>   
>   #include <netlink/genl/family.h>
>   #include <netlink/genl/ctrl.h>
> @@ -323,6 +324,11 @@ static void parse_threads_get(struct genlmsghdr *gnlh)
>   		case NFSD_A_SERVER_THREADS:
>   			pool_threads[i++] = nla_get_u32(attr);
>   			break;
> +#if HAVE_DECL_NFSD_A_SERVER_MIN_THREADS
> +		case NFSD_A_SERVER_MIN_THREADS:
> +			printf("min-threads: %u\n", nla_get_u32(attr));
> +			break;
> +#endif
>   		default:
>   			break;
>   		}
> @@ -518,7 +524,7 @@ out:
>   }
>   
>   static int threads_doit(struct nl_sock *sock, int cmd, int grace, int lease,
> -			int pool_count, int *pool_threads, char *scope)
> +			int pool_count, int *pool_threads, char *scope, int minthreads)
>   {
>   	struct genlmsghdr *ghdr;
>   	struct nlmsghdr *nlh;
> @@ -540,6 +546,10 @@ static int threads_doit(struct nl_sock *sock, int cmd, int grace, int lease,
>   			nla_put_u32(msg, NFSD_A_SERVER_LEASETIME, lease);
>   		if (scope)
>   			nla_put_string(msg, NFSD_A_SERVER_SCOPE, scope);
> +#if HAVE_DECL_NFSD_A_SERVER_MIN_THREADS
> +		if (minthreads >= 0)
> +			nla_put_u32(msg, NFSD_A_SERVER_MIN_THREADS, minthreads);
> +#endif
>   		for (i = 0; i < pool_count; ++i)
>   			nla_put_u32(msg, NFSD_A_SERVER_THREADS, pool_threads[i]);
>   	}
> @@ -580,23 +590,49 @@ out:
>   
>   static void threads_usage(void)
>   {
> -	printf("Usage: %s threads [ pool0_count ] [ pool1_count ] ...\n", taskname);
> +	printf("Usage: %s threads { --min-threads=X } [ pool0_count ] [ pool1_count ] ...\n", taskname);
> +#if HAVE_DECL_NFSD_A_SERVER_MIN_THREADS
> +	printf("    --min-threads= set the minimum thread count per pool to value\n");
> +#endif
>   	printf("    pool0_count: thread count for pool0, etc...\n");
>   	printf("Omit any arguments to show current thread counts.\n");
>   }
>   
> +#if HAVE_DECL_NFSD_A_SERVER_MIN_THREADS
> +static const struct option threads_options[] = {
> +	{ "help", no_argument, NULL, 'h' },
> +	{ "min-threads", required_argument, NULL, 'm' },
> +	{ },
> +};
> +#define THREADS_OPTSTRING "hm:"
> +#else
> +#define threads_options help_only_options
> +#define THREADS_OPTSTRING "h"
> +#endif
> +
>   static int threads_func(struct nl_sock *sock, int argc, char **argv)
>   {
>   	uint8_t cmd = NFSD_CMD_THREADS_GET;
>   	int *pool_threads = NULL;
> +	int minthreads = -1;
>   	int opt, pools = 0;
>   
>   	optind = 1;
> -	while ((opt = getopt_long(argc, argv, "h", help_only_options, NULL)) != -1) {
> +	while ((opt = getopt_long(argc, argv, THREADS_OPTSTRING, threads_options, NULL)) != -1) {
>   		switch (opt) {
>   		case 'h':
>   			threads_usage();
>   			return 0;
> +#if HAVE_DECL_NFSD_A_SERVER_MIN_THREADS
> +		case 'm':
> +			errno = 0;
> +			minthreads = strtoul(optarg, NULL, 0);
> +			if (minthreads == ULONG_MAX && errno != 0) {
> +				fprintf(stderr, "Bad --min-threads value.");
> +				return 1;
> +			}
> +			break;
> +#endif
>   		}
>   	}
>   
> @@ -624,7 +660,7 @@ static int threads_func(struct nl_sock *sock, int argc, char **argv)
>   			}
>   		}
>   	}
> -	return threads_doit(sock, cmd, 0, 0, pools, pool_threads, NULL);
> +	return threads_doit(sock, cmd, 0, 0, pools, pool_threads, NULL, minthreads);
>   }
>   
>   /*
> @@ -1578,7 +1614,7 @@ static void autostart_usage(void)
>   
>   static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
>   {
> -	int *threads, grace, lease, idx, ret, opt, pools;
> +	int *threads, grace, lease, idx, ret, opt, pools, minthreads;
>   	struct conf_list *thread_str;
>   	struct conf_list_node *n;
>   	char *scope, *pool_mode;
> @@ -1659,9 +1695,10 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
>   
>   	lease = conf_get_num("nfsd", "lease-time", 0);
>   	scope = conf_get_str("nfsd", "scope");
> +	minthreads = conf_get_num("nfsd", "min-threads", 0);
>   
>   	ret = threads_doit(sock, NFSD_CMD_THREADS_SET, grace, lease, pools,
> -			   threads, scope);
> +			   threads, scope, minthreads);
>   out:
>   	free(threads);
>   	return ret;
> 
> ---
> base-commit: 0e71be58cdead21b7bc0285fa6afbf1d0eca3049
> change-id: 20260109-minthreads-7906eecedf99
> 
> Best regards,


      parent reply	other threads:[~2026-02-01 17:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-23 18:48 [PATCH nfs-utils v2] nfsdctl: add support for min-threads parameter Jeff Layton
2026-01-23 19:21 ` Mike Owen
2026-01-23 19:59   ` Jeff Layton
2026-02-01 17:31 ` Steve Dickson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=671224fc-7c42-420e-9dbe-ae125bfdad4d@redhat.com \
    --to=steved@redhat.com \
    --cc=Dai.Ngo@oracle.com \
    --cc=chuck.lever@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=okorniev@redhat.com \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox