> 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