[PATCH 0/8] kdevops: random fixes

[PATCH 0/8] kdevops: random fixes

[Jeff Layton]

Just a number of random fixes and cleanups I've had sitting in various
trees. I think most should be uncontroversial.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Jeff Layton (8):
      fstests: remove the nfs_default guest
      fstests: add an option for testing nfs over rdma
      fstests: enable the "codeready" repo on RHEL and CentOS
      kotd: display the running kernel version after updating
      devconfig: run it on nfsd in addition to the all group
      devconfig: don't install dump on redhat family hosts
      redhat: fix up some install-deps cases for CentOS
      nfsd: default to a more sane number of threads

 kconfigs/Kconfig.nfsd                                     |  2 +-
 .../roles/devconfig/tasks/install-deps/redhat/main.yml    |  1 -
 playbooks/roles/devconfig/tasks/kotd-rev-kernel/main.yml  |  7 +++++++
 playbooks/roles/fstests/defaults/main.yml                 |  1 +
 .../roles/fstests/tasks/install-deps/redhat/main.yml      |  5 +++++
 playbooks/roles/fstests/templates/nfs/nfs.config          |  8 +++++---
 playbooks/roles/ktls/tasks/install-deps/redhat/main.yml   |  2 +-
 playbooks/roles/nfsd/templates/nfs.conf.j2                |  1 +
 playbooks/roles/pynfs/tasks/install-deps/redhat/main.yml  |  3 +--
 scripts/devconfig.Makefile                                |  5 +++--
 scripts/nfsd-thread-est.sh                                | 15 +++++++++++++++
 workflows/fstests/nfs/Kconfig                             | 14 +++++++-------
 workflows/fstests/nfs/Makefile                            |  3 +++
 13 files changed, 50 insertions(+), 17 deletions(-)
---
base-commit: 2b5a4f5c3255641a23617fcf3cb9aa05e1b0513e
change-id: 20240320-fixes-078837225fb3

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>

[PATCH 1/8] fstests: remove the nfs_default guest

[Jeff Layton]

Chuck recently added an explicit v4.2 guest, which is really the same
thing. Remove the redundant nfs_default guest.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 playbooks/roles/fstests/templates/nfs/nfs.config |  5 -----
 workflows/fstests/nfs/Kconfig                    | 10 +---------
 2 files changed, 1 insertion(+), 14 deletions(-)

diff --git a/playbooks/roles/fstests/templates/nfs/nfs.config b/playbooks/roles/fstests/templates/nfs/nfs.config
index 3b30114cce62..f301075a8928 100644
--- a/playbooks/roles/fstests/templates/nfs/nfs.config
+++ b/playbooks/roles/fstests/templates/nfs/nfs.config
@@ -10,11 +10,6 @@ SCRATCH_DEV="{{ fstests_nfs_scratch_devpool }}"
 RESULT_BASE=$PWD/results/$HOST/$(uname -r)
 TEST_DEV={{ fstests_nfs_test_dev }}
 CANON_DEVS=yes
-{% if fstests_nfs_section_default -%}
-
-# Test with default mount options
-[nfs_default]
-{% endif %}
 {% if fstests_nfs_section_tls -%}
 
 # Test NFS with RPC over TLS
diff --git a/workflows/fstests/nfs/Kconfig b/workflows/fstests/nfs/Kconfig
index 86e930a69447..b6daeeb490c5 100644
--- a/workflows/fstests/nfs/Kconfig
+++ b/workflows/fstests/nfs/Kconfig
@@ -46,14 +46,6 @@ config FSTESTS_NFS_MANUAL_COVERAGE
 
 if FSTESTS_NFS_MANUAL_COVERAGE
 
-config FSTESTS_NFS_SECTION_DEFAULT
-	bool "Enable testing section: nfs_default"
-	default y
-	help
-	  Enabling this will test nfs with the default mount options. At the
-	  time of this writing, this makes the client autonegotiate an NFS
-	  version, starting with v4.2 if it's available.
-
 config FSTESTS_NFS_SECTION_TLS
 	bool "Enable testing section: nfs_tls"
 	default n
@@ -63,7 +55,7 @@ config FSTESTS_NFS_SECTION_TLS
 
 config FSTESTS_NFS_SECTION_V42
 	bool "Enable testing section: nfs_v42"
-	default n
+	default y
 	help
 	  Enabling this will test NFSv4.2.
 

-- 
2.44.0

[PATCH 2/8] fstests: add an option for testing nfs over rdma

[Jeff Layton]

This adds a new Kconfig option for testing NFS over RDMA.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 playbooks/roles/fstests/defaults/main.yml        | 1 +
 playbooks/roles/fstests/templates/nfs/nfs.config | 7 +++++++
 playbooks/roles/nfsd/templates/nfs.conf.j2       | 1 +
 workflows/fstests/nfs/Kconfig                    | 8 ++++++++
 workflows/fstests/nfs/Makefile                   | 3 +++
 5 files changed, 20 insertions(+)

diff --git a/playbooks/roles/fstests/defaults/main.yml b/playbooks/roles/fstests/defaults/main.yml
index 1d14568a03d0..a46a4672dfa5 100644
--- a/playbooks/roles/fstests/defaults/main.yml
+++ b/playbooks/roles/fstests/defaults/main.yml
@@ -161,6 +161,7 @@ fstests_btrfs_section_simple_zns: False
 
 fstests_nfs_enable: False
 fstests_nfs_use_kdevops_nfsd: False
+fstests_nfs_section_rdma: False
 fstests_nfs_section_tls: False
 fstests_nfs_section_default: False
 fstests_nfs_section_v42: False
diff --git a/playbooks/roles/fstests/templates/nfs/nfs.config b/playbooks/roles/fstests/templates/nfs/nfs.config
index f301075a8928..b5c90534f4df 100644
--- a/playbooks/roles/fstests/templates/nfs/nfs.config
+++ b/playbooks/roles/fstests/templates/nfs/nfs.config
@@ -10,6 +10,13 @@ SCRATCH_DEV="{{ fstests_nfs_scratch_devpool }}"
 RESULT_BASE=$PWD/results/$HOST/$(uname -r)
 TEST_DEV={{ fstests_nfs_test_dev }}
 CANON_DEVS=yes
+{% if fstests_nfs_section_rdma -%}
+
+# Test over RDMA
+[nfs_rdma]
+TEST_FS_MOUNT_OPTS="-o rdma"
+MOUNT_OPTIONS="-o rdma"
+{% endif %}
 {% if fstests_nfs_section_tls -%}
 
 # Test NFS with RPC over TLS
diff --git a/playbooks/roles/nfsd/templates/nfs.conf.j2 b/playbooks/roles/nfsd/templates/nfs.conf.j2
index 8e89eba4a095..a5f4a714ec34 100644
--- a/playbooks/roles/nfsd/templates/nfs.conf.j2
+++ b/playbooks/roles/nfsd/templates/nfs.conf.j2
@@ -3,6 +3,7 @@ pipefs-directory={{ pipefs_directory }}
 
 [nfsd]
 udp=y
+rdma=y
 threads={{ nfsd_threads }}
 grace-time={{ nfsd_lease_time }}
 lease-time={{ nfsd_lease_time }}
diff --git a/workflows/fstests/nfs/Kconfig b/workflows/fstests/nfs/Kconfig
index b6daeeb490c5..9f47761712b2 100644
--- a/workflows/fstests/nfs/Kconfig
+++ b/workflows/fstests/nfs/Kconfig
@@ -46,6 +46,14 @@ config FSTESTS_NFS_MANUAL_COVERAGE
 
 if FSTESTS_NFS_MANUAL_COVERAGE
 
+config FSTESTS_NFS_SECTION_RDMA
+	bool "Enable testing section: nfs_rdma"
+	default n
+	help
+	  Enabling this will test with the "rdma" mount option. Unless the
+	  hosts have RDMA hardware already, you probably also want to enable
+	  CONFIG_KDEVOPS_SETUP_SIW as well.
+
 config FSTESTS_NFS_SECTION_TLS
 	bool "Enable testing section: nfs_tls"
 	default n
diff --git a/workflows/fstests/nfs/Makefile b/workflows/fstests/nfs/Makefile
index ba4387e128f5..f06fdb9f2a51 100644
--- a/workflows/fstests/nfs/Makefile
+++ b/workflows/fstests/nfs/Makefile
@@ -9,6 +9,9 @@ FSTESTS_ARGS += fstests_nfs_server_host='$(FSTESTS_NFS_SERVER_HOST)'
 ifeq (y,$(CONFIG_FSTESTS_NFS_SECTION_DEFAULT))
 FSTESTS_ARGS += fstests_nfs_section_default=True
 endif
+ifeq (y,$(CONFIG_FSTESTS_NFS_SECTION_RDMA))
+FSTESTS_ARGS += fstests_nfs_section_rdma=True
+endif
 ifeq (y,$(CONFIG_FSTESTS_NFS_SECTION_TLS))
 FSTESTS_ARGS += fstests_nfs_section_tls=True
 endif

-- 
2.44.0

[PATCH 3/8] fstests: enable the "codeready" repo on RHEL and CentOS

[Jeff Layton]

This is necessary in order to install certain development packages
that the testcases need.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 playbooks/roles/fstests/tasks/install-deps/redhat/main.yml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/playbooks/roles/fstests/tasks/install-deps/redhat/main.yml b/playbooks/roles/fstests/tasks/install-deps/redhat/main.yml
index 44d8c062dd8d..94756d1f9ca4 100644
--- a/playbooks/roles/fstests/tasks/install-deps/redhat/main.yml
+++ b/playbooks/roles/fstests/tasks/install-deps/redhat/main.yml
@@ -1,4 +1,9 @@
 ---
+- name: Enable the CodeReady repo
+  become: yes
+  command: /usr/bin/dnf config-manager --enable codeready-builder-for-rhel-{{ ansible_distribution_major_version }}-{{ ansible_architecture }}-rpms
+  when: ansible_facts['distribution'] == 'RedHat' or ansible_facts['distribution'] == 'CentOS'
+
 - name: Install epel-release if we're not on Fedora
   become: yes
   become_method: sudo

-- 
2.44.0

[PATCH 4/8] kotd: display the running kernel version after updating

[Jeff Layton]

Just a sanity check that's nice to see when running make kotd.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 playbooks/roles/devconfig/tasks/kotd-rev-kernel/main.yml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/playbooks/roles/devconfig/tasks/kotd-rev-kernel/main.yml b/playbooks/roles/devconfig/tasks/kotd-rev-kernel/main.yml
index c49d3cf77cd5..f453afb2a6d9 100644
--- a/playbooks/roles/devconfig/tasks/kotd-rev-kernel/main.yml
+++ b/playbooks/roles/devconfig/tasks/kotd-rev-kernel/main.yml
@@ -49,6 +49,13 @@
     - ansible_facts['os_family']|lower == 'redhat'
     - devconfig_try_refresh_repos|bool
 
+- name: Check kernel uname
+  tags: [ 'kotd' ]
+  debug:
+    msg: "Running kernel {{ running_kernel }}"
+  vars:
+    running_kernel: "{{ uname_cmd.stdout_lines.0 }}"
+
 - name: Get used target kernel version after reving kernel
   tags: [ 'kotd' ]
   command: "uname -r"

-- 
2.44.0

[PATCH 5/8] devconfig: run it on nfsd in addition to the all group

[Jeff Layton]

Sometimes we need to log into the nfs server as well.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 scripts/devconfig.Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/devconfig.Makefile b/scripts/devconfig.Makefile
index 0c1653ff12a2..c5a8061b02cc 100644
--- a/scripts/devconfig.Makefile
+++ b/scripts/devconfig.Makefile
@@ -57,8 +57,9 @@ extend-extra-args-devconfig:
 ifeq (y,$(CONFIG_SYSCTL_TUNING))
 PHONY += sysctl-tunings
 sysctl-tunings: $(KDEVOPS_NODES)
-	$(Q)ansible-playbook $(ANSIBLE_VERBOSE) -i \
-		$(KDEVOPS_HOSTFILE) $(KDEVOPS_PLAYBOOKS_DIR)/devconfig.yml \
+	$(Q)ansible-playbook $(ANSIBLE_VERBOSE) -i $(KDEVOPS_HOSTFILE) \
+		-l all,nfsd \
+		$(KDEVOPS_PLAYBOOKS_DIR)/devconfig.yml \
 		--extra-vars="$(BOOTLINUX_ARGS)" $(LIMIT_HOSTS) --tags vars,sysctl
 
 devconfig-help-menu:

-- 
2.44.0

[PATCH 6/8] devconfig: don't install dump on redhat family hosts

[Jeff Layton]

It doesn't seem to be installable everywhere.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 playbooks/roles/devconfig/tasks/install-deps/redhat/main.yml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/playbooks/roles/devconfig/tasks/install-deps/redhat/main.yml b/playbooks/roles/devconfig/tasks/install-deps/redhat/main.yml
index d629b1becc1d..838e4d293135 100644
--- a/playbooks/roles/devconfig/tasks/install-deps/redhat/main.yml
+++ b/playbooks/roles/devconfig/tasks/install-deps/redhat/main.yml
@@ -98,7 +98,6 @@
       - make
       - gawk
       - bc
-      - dump
       - libtool
       - psmisc
       - sed

-- 
2.44.0

[PATCH 7/8] redhat: fix up some install-deps cases for CentOS

[Jeff Layton]

Fix up some "when" cases for install-deps in ktls and pynfs.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 playbooks/roles/ktls/tasks/install-deps/redhat/main.yml  | 2 +-
 playbooks/roles/pynfs/tasks/install-deps/redhat/main.yml | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/playbooks/roles/ktls/tasks/install-deps/redhat/main.yml b/playbooks/roles/ktls/tasks/install-deps/redhat/main.yml
index 5d6a18c9d6c3..802ef2a4c3ed 100644
--- a/playbooks/roles/ktls/tasks/install-deps/redhat/main.yml
+++ b/playbooks/roles/ktls/tasks/install-deps/redhat/main.yml
@@ -9,7 +9,7 @@
   set_fact:
     epel_package:
       - epel-release
-  when: ansible_distribution == 'OracleLinux'
+  when: ansible_distribution == 'OracleLinux' or ansible_distribution == 'CentOS'
 
 - name: Install epel-release
   become: yes
diff --git a/playbooks/roles/pynfs/tasks/install-deps/redhat/main.yml b/playbooks/roles/pynfs/tasks/install-deps/redhat/main.yml
index 9a133c0da661..be5ded1da8df 100644
--- a/playbooks/roles/pynfs/tasks/install-deps/redhat/main.yml
+++ b/playbooks/roles/pynfs/tasks/install-deps/redhat/main.yml
@@ -3,8 +3,7 @@
   become: yes
   command: /usr/bin/dnf config-manager --enable codeready-builder-for-rhel-{{ ansible_distribution_major_version }}-{{ ansible_architecture }}-rpms
   when:
-    - rhel_org_id is defined
-    - rhel_activation_key is defined
+    - ansible_distribution == 'RedHat' or ansible_distribution == 'CentOS'
 
 - name: Install build dependencies for pynfs
   become: yes

-- 
2.44.0

[PATCH 8/8] nfsd: default to a more sane number of threads

[Jeff Layton]

Add a small script that can estimate the number of threads for
nfsd. We default to 32 threads for every GB of guest memory.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 kconfigs/Kconfig.nfsd      |  2 +-
 scripts/nfsd-thread-est.sh | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/kconfigs/Kconfig.nfsd b/kconfigs/Kconfig.nfsd
index dec98c1e964f..7c28ad98955a 100644
--- a/kconfigs/Kconfig.nfsd
+++ b/kconfigs/Kconfig.nfsd
@@ -69,7 +69,7 @@ config NFSD_EXPORT_OPTIONS
 
 config NFSD_THREADS
 	int "Number of nfsd threads to spawn"
-	default 8
+	default $(shell, scripts/nfsd-thread-est.sh)
 	help
 	  Number of nfsd threads to start up for testing.
 
diff --git a/scripts/nfsd-thread-est.sh b/scripts/nfsd-thread-est.sh
new file mode 100755
index 000000000000..dc5a1deb1215
--- /dev/null
+++ b/scripts/nfsd-thread-est.sh
@@ -0,0 +1,15 @@
+#!/bin/bash
+#
+# The default number of 8 nfsd threads is pitifully low!
+#
+#
+# Each nfsd thread consumes ~1MB of memory, long-term:
+#
+# stack, kthread overhead, svc_rqst, and the rq_pages array, plus
+# temporary working space.
+#
+# Allow 32 threads for each GB of guest memory:
+
+. "${TOPDIR}/.config"
+
+echo "$(( 32 * $CONFIG_LIBVIRT_MEM_MB / 1024 ))"

-- 
2.44.0

Re: [PATCH 8/8] nfsd: default to a more sane number of threads

[Chuck Lever]

On Wed, Mar 20, 2024 at 09:11:31AM -0400, Jeff Layton wrote:
> Add a small script that can estimate the number of threads for
> nfsd. We default to 32 threads for every GB of guest memory.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  kconfigs/Kconfig.nfsd      |  2 +-
>  scripts/nfsd-thread-est.sh | 15 +++++++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/kconfigs/Kconfig.nfsd b/kconfigs/Kconfig.nfsd
> index dec98c1e964f..7c28ad98955a 100644
> --- a/kconfigs/Kconfig.nfsd
> +++ b/kconfigs/Kconfig.nfsd
> @@ -69,7 +69,7 @@ config NFSD_EXPORT_OPTIONS
>  
>  config NFSD_THREADS
>  	int "Number of nfsd threads to spawn"
> -	default 8
> +	default $(shell, scripts/nfsd-thread-est.sh)
>  	help
>  	  Number of nfsd threads to start up for testing.
>  
> diff --git a/scripts/nfsd-thread-est.sh b/scripts/nfsd-thread-est.sh
> new file mode 100755
> index 000000000000..dc5a1deb1215
> --- /dev/null
> +++ b/scripts/nfsd-thread-est.sh
> @@ -0,0 +1,15 @@
> +#!/bin/bash
> +#
> +# The default number of 8 nfsd threads is pitifully low!
> +#
> +#
> +# Each nfsd thread consumes ~1MB of memory, long-term:
> +#
> +# stack, kthread overhead, svc_rqst, and the rq_pages array, plus
> +# temporary working space.
> +#
> +# Allow 32 threads for each GB of guest memory:

I agree that 8 is a small number, but that multiplier seems
excessive. On my 16GB test systems, that would create 512 threads.
I set the nfsd thread count to 32 on my test NFS server, and I've
never seen more than 10-15 threads for any test. (OK, maybe I should
run tests that drive the test server harder).

Plus, no distribution uses that configuration. Maybe we should stick
to what's a common deployment scenario?

Do you have data that shows creating more threads improves test run
time?


> +
> +. "${TOPDIR}/.config"
> +
> +echo "$(( 32 * $CONFIG_LIBVIRT_MEM_MB / 1024 ))"

How would this work on cloud (non-libvirt) systems?

-- 
Chuck Lever

Re: [PATCH 8/8] nfsd: default to a more sane number of threads

[Jeff Layton]

On Wed, 2024-03-20 at 10:10 -0400, Chuck Lever wrote:
> On Wed, Mar 20, 2024 at 09:11:31AM -0400, Jeff Layton wrote:
> > Add a small script that can estimate the number of threads for
> > nfsd. We default to 32 threads for every GB of guest memory.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  kconfigs/Kconfig.nfsd      |  2 +-
> >  scripts/nfsd-thread-est.sh | 15 +++++++++++++++
> >  2 files changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kconfigs/Kconfig.nfsd b/kconfigs/Kconfig.nfsd
> > index dec98c1e964f..7c28ad98955a 100644
> > --- a/kconfigs/Kconfig.nfsd
> > +++ b/kconfigs/Kconfig.nfsd
> > @@ -69,7 +69,7 @@ config NFSD_EXPORT_OPTIONS
> >  
> >  config NFSD_THREADS
> >  	int "Number of nfsd threads to spawn"
> > -	default 8
> > +	default $(shell, scripts/nfsd-thread-est.sh)
> >  	help
> >  	  Number of nfsd threads to start up for testing.
> >  
> > diff --git a/scripts/nfsd-thread-est.sh b/scripts/nfsd-thread-est.sh
> > new file mode 100755
> > index 000000000000..dc5a1deb1215
> > --- /dev/null
> > +++ b/scripts/nfsd-thread-est.sh
> > @@ -0,0 +1,15 @@
> > +#!/bin/bash
> > +#
> > +# The default number of 8 nfsd threads is pitifully low!
> > +#
> > +#
> > +# Each nfsd thread consumes ~1MB of memory, long-term:
> > +#
> > +# stack, kthread overhead, svc_rqst, and the rq_pages array, plus
> > +# temporary working space.
> > +#
> > +# Allow 32 threads for each GB of guest memory:
> 
> I agree that 8 is a small number, but that multiplier seems
> excessive. On my 16GB test systems, that would create 512 threads.
> I set the nfsd thread count to 32 on my test NFS server, and I've
> never seen more than 10-15 threads for any test. (OK, maybe I should
> run tests that drive the test server harder).
> 

How are you measuring that? I'm open to a different formula. This one is
based on a total SWAG.

> Plus, no distribution uses that configuration. Maybe we should stick
> to what's a common deployment scenario?
> 

They don't use that configuration because rpc.nfsd still defaults to 8.
Admins have to _know_ to increase the thread count, which sort of sucks.
I'd like to make that more dynamic, but that's a separate project.
Still, maybe worthwhile to look at soon...

> Do you have data that shows creating more threads improves test run
> time?
> 

No, but I think the overhead from that number of threads is pretty
minimal and the whole purpose of the kdevops nfsd host is to serve NFS
requests. Also, this _is_ just a default.

BTW, do we collect stats on how often we can't find a thread to run? I
guess that's all the svc_defer stuff?

> 
> > +
> > +. "${TOPDIR}/.config"
> > +
> > +echo "$(( 32 * $CONFIG_LIBVIRT_MEM_MB / 1024 ))"
> 
> How would this work on cloud (non-libvirt) systems?
> 

Yeah, that is a problem. The way you specify guest sizes on cloud hosts
is totally different (and different for each cloud provider). For that,
we should probably just assume a 4GB guest. With this formula that's 128
threads which I guess is still excessive by your estimation.

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 8/8] nfsd: default to a more sane number of threads

[Chuck Lever]

On Wed, Mar 20, 2024 at 02:48:37PM +0000, Jeff Layton wrote:
> On Wed, 2024-03-20 at 10:10 -0400, Chuck Lever wrote:
> > On Wed, Mar 20, 2024 at 09:11:31AM -0400, Jeff Layton wrote:
> > > Add a small script that can estimate the number of threads for
> > > nfsd. We default to 32 threads for every GB of guest memory.
> > > 
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > >  kconfigs/Kconfig.nfsd      |  2 +-
> > >  scripts/nfsd-thread-est.sh | 15 +++++++++++++++
> > >  2 files changed, 16 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/kconfigs/Kconfig.nfsd b/kconfigs/Kconfig.nfsd
> > > index dec98c1e964f..7c28ad98955a 100644
> > > --- a/kconfigs/Kconfig.nfsd
> > > +++ b/kconfigs/Kconfig.nfsd
> > > @@ -69,7 +69,7 @@ config NFSD_EXPORT_OPTIONS
> > >  
> > >  config NFSD_THREADS
> > >  	int "Number of nfsd threads to spawn"
> > > -	default 8
> > > +	default $(shell, scripts/nfsd-thread-est.sh)
> > >  	help
> > >  	  Number of nfsd threads to start up for testing.
> > >  
> > > diff --git a/scripts/nfsd-thread-est.sh b/scripts/nfsd-thread-est.sh
> > > new file mode 100755
> > > index 000000000000..dc5a1deb1215
> > > --- /dev/null
> > > +++ b/scripts/nfsd-thread-est.sh
> > > @@ -0,0 +1,15 @@
> > > +#!/bin/bash
> > > +#
> > > +# The default number of 8 nfsd threads is pitifully low!
> > > +#
> > > +#
> > > +# Each nfsd thread consumes ~1MB of memory, long-term:
> > > +#
> > > +# stack, kthread overhead, svc_rqst, and the rq_pages array, plus
> > > +# temporary working space.
> > > +#
> > > +# Allow 32 threads for each GB of guest memory:
> > 
> > I agree that 8 is a small number, but that multiplier seems
> > excessive. On my 16GB test systems, that would create 512 threads.
> > I set the nfsd thread count to 32 on my test NFS server, and I've
> > never seen more than 10-15 threads for any test. (OK, maybe I should
> > run tests that drive the test server harder).
> 
> How are you measuring that?

I watch "top" while the tests are running. You can see CPU time
accumulating for nfsd threads as they rise to the top of the display
window. I see between ten and fifteen nfsd threads in that window
after a test run is done. Other nfsd threads remain unused.

It's the unused threads that worry me -- that will be memory rather
permanently tied up in rq_pages that isn't serving any useful
purpose. We don't yet have a shrinker for reducing the thread
count, for instance.


> I'm open to a different formula. This one is
> based on a total SWAG.

So I think your rationale and formula works as a /maximum/ thread
count. But distributions can't assume that NFSD is going to be
the only memory consumer on a system, thus taking that much memory
for nominally idle nfsd threads seems undesirable.

I've toyed with the idea of 3 * CPU-core-count as a rough default
number of threads, though it really depends on NFS workload and the
speed of permanent storage. Sigh.


> > Plus, no distribution uses that configuration. Maybe we should stick
> > to what's a common deployment scenario?
> > 
> 
> They don't use that configuration because rpc.nfsd still defaults to 8.
> Admins have to _know_ to increase the thread count, which sort of sucks.
> I'd like to make that more dynamic, but that's a separate project.
> Still, maybe worthwhile to look at soon...

I agree that increasing the default thread count is a super idea,
but I don't think we know enough yet to pick a new number. No doubt
a fixed value of 8 is too low as a distribution default. Yes, they
will look to us to provide a strong rationale for a new choice.


> > Do you have data that shows creating more threads improves test run
> > time?
> > 
> 
> No, but I think the overhead from that number of threads is pretty
> minimal and the whole purpose of the kdevops nfsd host is to serve NFS
> requests. Also, this _is_ just a default.
> 
> BTW, do we collect stats on how often we can't find a thread to run? I
> guess that's all the svc_defer stuff?

Good questions...

I had added a trace point at one time, but Neil removed all that
when he rewrote the svc thread scheduler last year.

We also used to have a thread histogram, but that's also been
removed for reasons.

It might be nice if we can find a generic tool for looking at thread
utilization so we don't have to build and maintain our own.


-- 
Chuck Lever

Re: [PATCH 8/8] nfsd: default to a more sane number of threads

[Jeff Layton]

On Wed, 2024-03-20 at 11:15 -0400, Chuck Lever wrote:
> On Wed, Mar 20, 2024 at 02:48:37PM +0000, Jeff Layton wrote:
> > On Wed, 2024-03-20 at 10:10 -0400, Chuck Lever wrote:
> > > On Wed, Mar 20, 2024 at 09:11:31AM -0400, Jeff Layton wrote:
> > > > Add a small script that can estimate the number of threads for
> > > > nfsd. We default to 32 threads for every GB of guest memory.
> > > > 
> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > ---
> > > >  kconfigs/Kconfig.nfsd      |  2 +-
> > > >  scripts/nfsd-thread-est.sh | 15 +++++++++++++++
> > > >  2 files changed, 16 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/kconfigs/Kconfig.nfsd b/kconfigs/Kconfig.nfsd
> > > > index dec98c1e964f..7c28ad98955a 100644
> > > > --- a/kconfigs/Kconfig.nfsd
> > > > +++ b/kconfigs/Kconfig.nfsd
> > > > @@ -69,7 +69,7 @@ config NFSD_EXPORT_OPTIONS
> > > >  
> > > >  config NFSD_THREADS
> > > >  	int "Number of nfsd threads to spawn"
> > > > -	default 8
> > > > +	default $(shell, scripts/nfsd-thread-est.sh)
> > > >  	help
> > > >  	  Number of nfsd threads to start up for testing.
> > > >  
> > > > diff --git a/scripts/nfsd-thread-est.sh b/scripts/nfsd-thread-est.sh
> > > > new file mode 100755
> > > > index 000000000000..dc5a1deb1215
> > > > --- /dev/null
> > > > +++ b/scripts/nfsd-thread-est.sh
> > > > @@ -0,0 +1,15 @@
> > > > +#!/bin/bash
> > > > +#
> > > > +# The default number of 8 nfsd threads is pitifully low!
> > > > +#
> > > > +#
> > > > +# Each nfsd thread consumes ~1MB of memory, long-term:
> > > > +#
> > > > +# stack, kthread overhead, svc_rqst, and the rq_pages array, plus
> > > > +# temporary working space.
> > > > +#
> > > > +# Allow 32 threads for each GB of guest memory:
> > > 
> > > I agree that 8 is a small number, but that multiplier seems
> > > excessive. On my 16GB test systems, that would create 512 threads.
> > > I set the nfsd thread count to 32 on my test NFS server, and I've
> > > never seen more than 10-15 threads for any test. (OK, maybe I should
> > > run tests that drive the test server harder).
> > 
> > How are you measuring that?
> 
> I watch "top" while the tests are running. You can see CPU time
> accumulating for nfsd threads as they rise to the top of the display
> window. I see between ten and fifteen nfsd threads in that window
> after a test run is done. Other nfsd threads remain unused.
> 
> It's the unused threads that worry me -- that will be memory rather
> permanently tied up in rq_pages that isn't serving any useful
> purpose. We don't yet have a shrinker for reducing the thread
> count, for instance.
>
> 
> > I'm open to a different formula. This one is
> > based on a total SWAG.
> 
> So I think your rationale and formula works as a /maximum/ thread
> count. But distributions can't assume that NFSD is going to be
> the only memory consumer on a system, thus taking that much memory
> for nominally idle nfsd threads seems undesirable.
> 
> I've toyed with the idea of 3 * CPU-core-count as a rough default
> number of threads, though it really depends on NFS workload and the
> speed of permanent storage. Sigh.
> 

That seems really low, and I wouldn't base this off of CPU count.

nfsd threads spend most of their time sleeping while waiting on I/O, so
they are rarely cpu-bound. I think looking at memory makes a lot more
sense for sizing thread counts.

What's the worst-case memory usage per (idle) thread? I had figured
about 1-2MB per thread, given thread overhead, svc_rqst structure, and
completely filled-out rq_pages array. So with this formula, I figure
about 64MB per GB will be eaten by idle threads.


> 
> > > Plus, no distribution uses that configuration. Maybe we should stick
> > > to what's a common deployment scenario?
> > > 
> > 
> > They don't use that configuration because rpc.nfsd still defaults to 8.
> > Admins have to _know_ to increase the thread count, which sort of sucks.
> > I'd like to make that more dynamic, but that's a separate project.
> > Still, maybe worthwhile to look at soon...
> 
> I agree that increasing the default thread count is a super idea,
> but I don't think we know enough yet to pick a new number. No doubt
> a fixed value of 8 is too low as a distribution default. Yes, they
> will look to us to provide a strong rationale for a new choice.
> 

Again, this is a default for a particular use case. We're setting up a
dedicated server, so we don't need to replicate the (shitty) distro
default. But, I don't feel that strongly about this patch. I'll just
drop it for now.

> 
> > > Do you have data that shows creating more threads improves test run
> > > time?
> > > 
> > 
> > No, but I think the overhead from that number of threads is pretty
> > minimal and the whole purpose of the kdevops nfsd host is to serve NFS
> > requests. Also, this _is_ just a default.
> > 
> > BTW, do we collect stats on how often we can't find a thread to run? I
> > guess that's all the svc_defer stuff?
> 
> Good questions...
> 
> I had added a trace point at one time, but Neil removed all that
> when he rewrote the svc thread scheduler last year.
> 
> We also used to have a thread histogram, but that's also been
> removed for reasons.
> 
> It might be nice if we can find a generic tool for looking at thread
> utilization so we don't have to build and maintain our own.
> 
> 

Yeah, though I think we do have the machinery in sunrpc.ko to do this.
We know when we can't immediately find a thread to service an incoming
request. That would be a good stat to start counting, IMO.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 0/8] kdevops: random fixes

[Chuck Lever]

On Wed, Mar 20, 2024 at 09:11:23AM -0400, Jeff Layton wrote:
> Just a number of random fixes and cleanups I've had sitting in various
> trees. I think most should be uncontroversial.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

For the series:

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>

Though I have reservations about 8/8.


> ---
> Jeff Layton (8):
>       fstests: remove the nfs_default guest
>       fstests: add an option for testing nfs over rdma
>       fstests: enable the "codeready" repo on RHEL and CentOS
>       kotd: display the running kernel version after updating
>       devconfig: run it on nfsd in addition to the all group
>       devconfig: don't install dump on redhat family hosts
>       redhat: fix up some install-deps cases for CentOS
>       nfsd: default to a more sane number of threads
> 
>  kconfigs/Kconfig.nfsd                                     |  2 +-
>  .../roles/devconfig/tasks/install-deps/redhat/main.yml    |  1 -
>  playbooks/roles/devconfig/tasks/kotd-rev-kernel/main.yml  |  7 +++++++
>  playbooks/roles/fstests/defaults/main.yml                 |  1 +
>  .../roles/fstests/tasks/install-deps/redhat/main.yml      |  5 +++++
>  playbooks/roles/fstests/templates/nfs/nfs.config          |  8 +++++---
>  playbooks/roles/ktls/tasks/install-deps/redhat/main.yml   |  2 +-
>  playbooks/roles/nfsd/templates/nfs.conf.j2                |  1 +
>  playbooks/roles/pynfs/tasks/install-deps/redhat/main.yml  |  3 +--
>  scripts/devconfig.Makefile                                |  5 +++--
>  scripts/nfsd-thread-est.sh                                | 15 +++++++++++++++
>  workflows/fstests/nfs/Kconfig                             | 14 +++++++-------
>  workflows/fstests/nfs/Makefile                            |  3 +++
>  13 files changed, 50 insertions(+), 17 deletions(-)
> ---
> base-commit: 2b5a4f5c3255641a23617fcf3cb9aa05e1b0513e
> change-id: 20240320-fixes-078837225fb3
> 
> Best regards,
> -- 
> Jeff Layton <jlayton@kernel.org>
> 
> 

-- 
Chuck Lever

Re: [PATCH 0/8] kdevops: random fixes

[Jeff Layton]

On Wed, 2024-03-20 at 10:11 -0400, Chuck Lever wrote:
> On Wed, Mar 20, 2024 at 09:11:23AM -0400, Jeff Layton wrote:
> > Just a number of random fixes and cleanups I've had sitting in various
> > trees. I think most should be uncontroversial.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> 
> For the series:
> 
> Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
> 
> Though I have reservations about 8/8.
> 

Thanks for the review. I'll plan to merge the first 7 soon, but wait on
#8 for a bit.

> 
> > ---
> > Jeff Layton (8):
> >       fstests: remove the nfs_default guest
> >       fstests: add an option for testing nfs over rdma
> >       fstests: enable the "codeready" repo on RHEL and CentOS
> >       kotd: display the running kernel version after updating
> >       devconfig: run it on nfsd in addition to the all group
> >       devconfig: don't install dump on redhat family hosts
> >       redhat: fix up some install-deps cases for CentOS
> >       nfsd: default to a more sane number of threads
> > 
> >  kconfigs/Kconfig.nfsd                                     |  2 +-
> >  .../roles/devconfig/tasks/install-deps/redhat/main.yml    |  1 -
> >  playbooks/roles/devconfig/tasks/kotd-rev-kernel/main.yml  |  7 +++++++
> >  playbooks/roles/fstests/defaults/main.yml                 |  1 +
> >  .../roles/fstests/tasks/install-deps/redhat/main.yml      |  5 +++++
> >  playbooks/roles/fstests/templates/nfs/nfs.config          |  8 +++++---
> >  playbooks/roles/ktls/tasks/install-deps/redhat/main.yml   |  2 +-
> >  playbooks/roles/nfsd/templates/nfs.conf.j2                |  1 +
> >  playbooks/roles/pynfs/tasks/install-deps/redhat/main.yml  |  3 +--
> >  scripts/devconfig.Makefile                                |  5 +++--
> >  scripts/nfsd-thread-est.sh                                | 15 +++++++++++++++
> >  workflows/fstests/nfs/Kconfig                             | 14 +++++++-------
> >  workflows/fstests/nfs/Makefile                            |  3 +++
> >  13 files changed, 50 insertions(+), 17 deletions(-)
> > ---
> > base-commit: 2b5a4f5c3255641a23617fcf3cb9aa05e1b0513e
> > change-id: 20240320-fixes-078837225fb3
> > 
> > Best regards,
> > -- 
> > Jeff Layton <jlayton@kernel.org>
> > 
> > 
> 

-- 
Jeff Layton <jlayton@kernel.org>