[PATCH RFC 0/6] nfsd: allow for a dynamically-sized threadpool

Jeff Layton posted 6 patches 1 day, 3 hours ago
Documentation/netlink/specs/nfsd.yaml |   5 +
fs/lockd/svc.c                        |   4 +-
fs/nfs/callback.c                     |   8 +-
fs/nfsd/netlink.c                     |   5 +-
fs/nfsd/netns.h                       |   6 ++
fs/nfsd/nfsctl.c                      |  50 ++++++++++
fs/nfsd/nfssvc.c                      |  56 ++++++++---
fs/nfsd/trace.h                       |  54 ++++++++++
include/linux/sunrpc/svc.h            |  11 +-
include/linux/sunrpc/svcsock.h        |   2 +-
include/uapi/linux/nfsd_netlink.h     |   1 +
net/sunrpc/svc.c                      | 182 +++++++++++++++++++---------------
net/sunrpc/svc_xprt.c                 |  45 +++++++--
13 files changed, 315 insertions(+), 114 deletions(-)
[PATCH RFC 0/6] nfsd: allow for a dynamically-sized threadpool
Posted by Jeff Layton 1 day, 3 hours ago
This patchset changes nfsd to dynamically size its threadpool as
needed. The main user-visible change is the addition of new controls
that allow the admin to set a minimum number of threads.

When the minimum is set to a non-zero value, the traditional "threads"
setting is interpreted as a maximum number of threads instead of a
static count. The server will start the minimum number of threads, and
then ramp up the thread count as needed. When the server is idle, it
will gradually ramp down the thread count.

This control scheme should allow us to sanely switch between kernels
that do and do not support dynamic threading. In the case where dynamic
threading is not supported, the user will just get the static maximum
number of threads.

The series is based on a set of draft patches by Neil. There are a
number of changes from his work:

1/ his original series was based around a new setting that defined a
maximum number of threads. This one instead adds a control to define a
minimum number of threads.

2/ if the thread's wait doesn't hit the timeout, then svc_recv() will
not return -ETIMEDOUT and the thread will stay running. Timeouts only
happens if a thread is wakes up without finding work to do.

3/ the printks in his original patches have been changed to tracepoints

So far this is only lightly tested, but it seems to work well. I
still need to do some benchmarking to see whether this affects
performance, so I'm posting this as an RFC for now.

Does this approach look sane to everyone?

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Jeff Layton (6):
      sunrpc: split svc_set_num_threads() into two functions
      sunrpc: remove special handling of NULL pool from svc_start/stop_kthreads()
      sunrpc: track the max number of requested threads in a pool
      sunrpc: introduce the concept of a minimum number of threads per pool
      nfsd: adjust number of running nfsd threads based on activity
      nfsd: add controls to set the minimum number of threads per pool

 Documentation/netlink/specs/nfsd.yaml |   5 +
 fs/lockd/svc.c                        |   4 +-
 fs/nfs/callback.c                     |   8 +-
 fs/nfsd/netlink.c                     |   5 +-
 fs/nfsd/netns.h                       |   6 ++
 fs/nfsd/nfsctl.c                      |  50 ++++++++++
 fs/nfsd/nfssvc.c                      |  56 ++++++++---
 fs/nfsd/trace.h                       |  54 ++++++++++
 include/linux/sunrpc/svc.h            |  11 +-
 include/linux/sunrpc/svcsock.h        |   2 +-
 include/uapi/linux/nfsd_netlink.h     |   1 +
 net/sunrpc/svc.c                      | 182 +++++++++++++++++++---------------
 net/sunrpc/svc_xprt.c                 |  45 +++++++--
 13 files changed, 315 insertions(+), 114 deletions(-)
---
base-commit: b6cf9ca7e7555f7f079bb062e3cd99a501e0d611
change-id: 20251212-nfsd-dynathread-9f7a31172005

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>
Re: [PATCH RFC 0/6] nfsd: allow for a dynamically-sized threadpool
Posted by Chuck Lever 6 hours ago

On Fri, Dec 12, 2025, at 5:39 PM, Jeff Layton wrote:
> This patchset changes nfsd to dynamically size its threadpool as
> needed. The main user-visible change is the addition of new controls
> that allow the admin to set a minimum number of threads.
>
> When the minimum is set to a non-zero value, the traditional "threads"
> setting is interpreted as a maximum number of threads instead of a
> static count. The server will start the minimum number of threads, and
> then ramp up the thread count as needed. When the server is idle, it
> will gradually ramp down the thread count.
>
> This control scheme should allow us to sanely switch between kernels
> that do and do not support dynamic threading. In the case where dynamic
> threading is not supported, the user will just get the static maximum
> number of threads.

An important consideration!


> The series is based on a set of draft patches by Neil. There are a
> number of changes from his work:
>
> 1/ his original series was based around a new setting that defined a
> maximum number of threads. This one instead adds a control to define a
> minimum number of threads.

My concern is whether one or more clients can force this mechanism
to continue creating threads until resource exhaustion causes a
denial of service.

I'm not convinced that setting a minimum number of threads is all
that interesting. Can you elaborate on why you chose that design?


-- 
Chuck Lever
Re: [PATCH RFC 0/6] nfsd: allow for a dynamically-sized threadpool
Posted by Jeff Layton 4 hours ago
On Sat, 2025-12-13 at 14:34 -0500, Chuck Lever wrote:
> 
> On Fri, Dec 12, 2025, at 5:39 PM, Jeff Layton wrote:
> > This patchset changes nfsd to dynamically size its threadpool as
> > needed. The main user-visible change is the addition of new controls
> > that allow the admin to set a minimum number of threads.
> > 
> > When the minimum is set to a non-zero value, the traditional "threads"
> > setting is interpreted as a maximum number of threads instead of a
> > static count. The server will start the minimum number of threads, and
> > then ramp up the thread count as needed. When the server is idle, it
> > will gradually ramp down the thread count.
> > 
> > This control scheme should allow us to sanely switch between kernels
> > that do and do not support dynamic threading. In the case where dynamic
> > threading is not supported, the user will just get the static maximum
> > number of threads.
> 
> An important consideration!
> 
> 
> > The series is based on a set of draft patches by Neil. There are a
> > number of changes from his work:
> > 
> > 1/ his original series was based around a new setting that defined a
> > maximum number of threads. This one instead adds a control to define a
> > minimum number of threads.
> 
> My concern is whether one or more clients can force this mechanism
> to continue creating threads until resource exhaustion causes a
> denial of service.
> 

The old "threads" setting is repurposed as a maximum when "min-theads"
is set. If someone sets "threads" high enough that they can drive the
machine into resource exhaustion, then that's an administrative error
IMO.

> I'm not convinced that setting a minimum number of threads is all
> that interesting. Can you elaborate on why you chose that design?
> 

The main reason to do dynamic threading is that NFS activity can be
spotty. Servers often have periods where they are very busy and other
times where they are idle.

Today, admins usually just set "threads" to the maximum number that
they think they will ever need to deal with this. This is a waste of
resources when nfsd is idle, of course. For a dedicated NFS server that
isn't doing anything else, that's usually considered acceptable.

So, I see the dynamic threading as mostly useful for machines that are
running nfsd as a "side job". e.g. -- a compute-heavy server that runs
nfsd in order to make its results available to other hosts. In those
cases, it makes sense to allow the thread count to ramp down when no
one is accessing nfsd so that those resources can be used for other
things.

With that in mind, it makes sense to repurpose "threads" as a maximum,
since that reflects the reality for most people today. So, the new
control should have the effect of setting a minimum number of threads.
 
For my own testing, I've mostly set min-threads to 1. We could
certainly convert this into a "dynamic-threading" bool setting and just
hardcode the minimum to 1 or some other value in that case, but I think
it makes sense to allow the flexibility to set the value higher, at
least until we have a better feeling for how this affects performance.

Thanks for taking a look!
-- 
Jeff Layton <jlayton@kernel.org>