Hello everyone, When launching nfsd without the -n parameter a function called get_tuned_nfsdcount set the number of threads to launch at start to (number of cpu * 8) but do not check if this result is greater than MAXNFSDCNT (which is 256). On a processor with 72 cores it makes nfsd launch 576 threads per nfsd, if you have multiple nfsd (with jails for example) it grows very quickly. This problem do not appear if you manually set the number of threads through the -n command line parameter to nfsd. It would seem this regression appeared in commit ae3e3df45c75ba60ec2cd2d425e4921bf2a6321d (or SVN revision 282272) nearly 10 years ago. A patch like this one should be enough to restore this check : diff --git a/usr.sbin/nfsd/nfsd.c b/usr.sbin/nfsd/nfsd.c index c1995580e142..cb506a4b1740 100644 --- a/usr.sbin/nfsd/nfsd.c +++ b/usr.sbin/nfsd/nfsd.c @@ -1033,6 +1033,8 @@ get_tuned_nfsdcount(void) tuned_nfsdcnt = DEFNFSDCNT; } else { tuned_nfsdcnt = ncpu * 8; + if (tuned_nfsdcnt > MAXNFSDCNT) + tuned_nfsdcnt = MAXNFSDCNT; } return tuned_nfsdcnt; }
^Triage: note that there is an inline patch. (fwiw, we prefer patches to be attachments.)
Created attachment 257305 [details] patch for get_tuned_nfsdcount sorry for the inline patch, here is the attachment version.
(In reply to florian.millet from comment #0) A few comments. I would consider this behaviour (ignoring MAXNFSDCNT) a feature and not a bug. MAXNFSDCNT is cruft left over from decades ago when nfsd kernel threads were processes. You'll notice some ancient cruft where children[MAXNFSCNT] is defined, but only children[0] is actually used. (That cruft should be cleared out, but it is harmless.) You'll also notice that MAXNFSDCNT is defined as 256 and I am pretty sure that there are servers that want to use ore than that limit now (and have been able to do so for more than a decade, due to the commit you mention. To change that now would be a POLA violation, imho. Why don't you just use the #maxthreads option to set the limit to what you need? (If this doesn't work, that is a bug that needs to be fixed.) I'll admit that "man nfsd" is misleading, in that it states that "-n" is the equivalent of setting both #minthreads and #maxthreads to the same value. This is inaccurate, as you note. It should say that "-n" is deprecated in favor of #minthreads, #maxthreads. Note that having too many kernel threads does not impact the system much. An inactive kernel thread mostly just uses a few pages for its kernel stack. (Minimal comapred to the "processes" used decades ago.) I will ask on freebsd-current@freebsd.org to see what others think w.r.t. this.
--minthreads and --maxthreads works correctly and I will use that to better control how many threads is spawned. If my original bug report is indeed considered not a bug, then I can close it. But you are right, a modification of the man page or the removal of the -n parameter should be considered to avoid such confusion in the future.
(In reply to florian.millet from comment #4) I didn't see any comments on freebsd-current@ so I am going to go with a patch to the man page that states "-n" is deprecated in favour of --minthreads, --maxthreads. Unless others disagree, I'll close this PR once the man page update is committed and MFC'd.
I have put a patch for the nfsd.8 man page up on phabricator as D49102. I will leave this PR open until the man page is patched. If anyone is interested in reviewing D49102, please do so.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=f27afbd850021b9cb7cf4e66f0621c793ad37de0 commit f27afbd850021b9cb7cf4e66f0621c793ad37de0 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2025-03-10 13:35:23 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2025-03-10 13:35:23 +0000 nfsd.8: Note that the -n option is deprecated PR#284616 reported that --maxthreads did not obey the 256 thread limit defined as MAXNFSDCNT in nfsd.c. This is actually a feature and not a bug, since many NFS servers will now want to run more than 256 threads and --maxthreads can be used to set the upper bound on the number of threads. (MAXNFSDCNT was used long ago to define how many daemons would be forked, before daemons were replaced by kernel threads.) However, the nfsd.8 man page was misleading, since it indicated that "-n" was the equivalent to setting both --minthreads and --maxthreads to the same value. This patch fixes the man page. This is a content change. PR: 284616 Reviewed by: 0mp (manpages) MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D49102 usr.sbin/nfsd/nfsd.8 | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-)
A patch to the nfsd.8 man page that notes that "-n" is deprecated in favor of --minthreads/--maxthreads has been committed. I think this PR can be closed once this patch is MFC'd.
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=f03c8869127c84cccec468ab4bf2ea5caf3cfade commit f03c8869127c84cccec468ab4bf2ea5caf3cfade Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2025-03-10 13:35:23 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2025-03-25 21:35:53 +0000 nfsd.8: Note that the -n option is deprecated PR#284616 reported that --maxthreads did not obey the 256 thread limit defined as MAXNFSDCNT in nfsd.c. This is actually a feature and not a bug, since many NFS servers will now want to run more than 256 threads and --maxthreads can be used to set the upper bound on the number of threads. (MAXNFSDCNT was used long ago to define how many daemons would be forked, before daemons were replaced by kernel threads.) However, the nfsd.8 man page was misleading, since it indicated that "-n" was the equivalent to setting both --minthreads and --maxthreads to the same value. This patch fixes the man page. This is a content change. PR: 284616 (cherry picked from commit f27afbd850021b9cb7cf4e66f0621c793ad37de0) usr.sbin/nfsd/nfsd.8 | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-)
The nfsd.8 patch has been committed and MFC'd.