Re: [PATCH 00/14] SUNRPC: clean up server thread management.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On Nov 16, 2021, at 7:46 PM, NeilBrown <neilb@xxxxxxx> wrote:
> 
> I have a dream of making nfsd threads start and stop dynamically.
> This would free the admin of having to choose a number of threads to
> start.
> I'm not there yet, and I may never get there, but the current state of
> the thread management code makes it harder to experiment than it needs
> to be.  There is a lot of technical debt that needs to be repaid first.
> 
> This series addresses much of this debt.  There are three users of
> service threads: nfsd, lockd, and nfs-callback.
> nfs-callback, the newest, is quite clean.  This patch brings nfsd and
> lockd up to a similar standard, and takes advantage of this increased
> uniformity to simplify some shared interfaces.

NFSD is venerable and ancient code. I'm a fan of careful and
tasteful clean up efforts!

I've started to familiarize myself with these changes. A couple
of thoughts for v2:

1. 1/14 is doing some heavy lifting and might be split into a
   lockd/nfs_cb patch and an nfsd-only patch. Or maybe there's
   another cleavage that makes more sense.

2. When introducing "static inline" functions I like to see full
   kerneldoc comments for those.

I'd also like to have some idea how we should be exercising this
series to ensure it is as bug-free as is reasonable. Do you have
any thoughts about that?


> It doesn't introduce any functionality improvements,

I might quibble with that assessment: replacing heavyweight
synchronization with spinlocks and atomics will have some
functional impact. That's probably where we need to be most
careful.


> and (as far as I
> know) only fixes one minor bug (can you spot it?  If not, look at
> c20106944eb6 and if you can see a second place that it could have
> fixed).
> 
> Thanks for your review,
> NeilBrown
> 
> 
> ---
> 
> NeilBrown (14):
>      SUNRPC: stop using ->sv_nrthreads as a refcount
>      nfsd: make nfsd_stats.th_cnt atomic_t
>      NFSD: narrow nfsd_mutex protection in nfsd thread
>      SUNRPC: use sv_lock to protect updates to sv_nrthreads.
>      NFSD: Make it possible to use svc_set_num_threads_sync
>      SUNRPC: discard svo_setup and rename svc_set_num_threads_sync()
>      NFSD: simplify locking for network notifier.
>      lockd: introduce nlmsvc_serv
>      lockd: simplify management of network status notifiers
>      lockd: move lockd_start_svc() call into lockd_create_svc()
>      lockd: move svc_exit_thread() into the thread
>      lockd: introduce lockd_put()
>      lockd: rename lockd_create_svc() to lockd_get()
>      lockd: use svc_set_num_threads() for thread start and stop
> 
> 
> fs/lockd/svc.c             | 190 ++++++++++++-------------------------
> fs/nfs/callback.c          |   8 +-
> fs/nfsd/netns.h            |   6 --
> fs/nfsd/nfsctl.c           |   2 -
> fs/nfsd/nfssvc.c           |  99 +++++++++----------
> fs/nfsd/stats.c            |   2 +-
> fs/nfsd/stats.h            |   4 +-
> include/linux/sunrpc/svc.h |  11 +--
> net/sunrpc/svc.c           |  61 ++----------
> 9 files changed, 128 insertions(+), 255 deletions(-)
> 
> --
> Signature
> 

--
Chuck Lever







[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux