Re: [PATCH 00/14 RFC] support automatic changes to nfsd thread count

[Jeff Layton <jlayton@kernel.org>]

On Mon, 2024-07-15 at 17:14 +1000, NeilBrown wrote:
> This patch set (against nfsd-next) enables automatic adjustment of the
> number of nfsd threads.  The number can increase under high load, and
> reduce after idle periods.
> 
> The first few patches (1-6) are cleanups that may not be entirely
> relevant to the current series.  They could safely land any time and
> only need minimal review.
> 
> Patch 9,10,11 remove some places were sv_nrthreads are used for things
> other than counting threads.  It is use to adjust other limits.  At the
> time this seemed like an easy and sensible solution.  I now have to
> repent of that short-cut and find a better way to impose reasonable
> limits.
> 
> These and the other sundry patches (7,8,12) can, I think safely land
> whenever that get sufficient review.  I think they are sensible even if
> we won't end up adjusting threads dynamically.
> 
> Patches 13 and 14 build on all this to provide the desired
> functionality.  Patch 13 allows the maximum to be configured, and patch
> 14 starts or stops threads based on some simple triggers.
> 
> For 13 I decided that if the user/admin makes no explicit configuration,
> then the currently request number of threads becomes a minimum, and a
> maximum is determined based on the amount of memory.  This will make
> the patch set immediately useful but shouldn't unduly impact existing
> configurations.
> 
> For patch 14 I only implemented starting a thread when there is work to
> do but no threads to do it, and stopping a thread when it has been idle
> for 5 seconds.  The start-up is deliberately serialised so at least one
> NFS request is serviced between the decision to start a thread and the
> action of starting it.  This hopefully encourages a ramping up of thread
> count rather than a sudden jump.
> 
> There is certain room for discussion around the wisdom of these
> heuristics, and what other heuristics are needed - we probably want a
> shrinker to impose memory pressure of the number of threads.  We
> probably want a thread to exit rather than retry when a memory
> allocation in svc_alloc_arg() fails.
> 
> I certainly wouldn't recommend patch 14 landing in any hurry at all.
> 
> I'd love to hear what y'all think, and what experiences you have when
> testing it.
> 
> 

This looks mostly reasonable, modulo a few nits on the later patches.
You can add my Reviewed-by to 1-9. 10-12 others look tentatively OK
too, but I'm less familiar with the slot handling code, and it sounds
like you're going to rework that part anyway.

For 13 I have some ideas about how we should present this from a user
interface standpoint that I wrote in my reply. The heuristics you came
up with in 14 look like a fine place to start.

Cheers!
-- 
Jeff Layton <jlayton@kernel.org>