Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "Chuck Lever III" <chuck.lever@oracle.com>
Cc:     "Bruce Fields" <bfields@fieldses.org>,
        "Linux NFS Mailing List" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 00/14] SUNRPC: clean up server thread management.
In-reply-to: <9E7E7AC8-FE88-4C07-9DE7-DA5F0CB2B9E2@oracle.com>
References: <163710954700.5485.5622638225352156964.stgit@noble.brown>,
 <9E7E7AC8-FE88-4C07-9DE7-DA5F0CB2B9E2@oracle.com>
Date:   Fri, 19 Nov 2021 14:19:47 +1100
Message-id: <163729198702.13692.3585664864399341321@noble.neil.brown.name>
Precedence: bulk

On Thu, 18 Nov 2021, Chuck Lever III wrote:
> 
> > On Nov 16, 2021, at 7:46 PM, NeilBrown <neilb@suse.de> 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.

I've split out the renaming of svc_destroy to svc_put and related
changes.

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

Will do.

> 
> 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?

As Bruce discovered, it is the starting and stopping of services that
are most affected.  Races between the two might be interesting.
Failures during start could be interesting.  Signalling nfsd threads vs
"rpc.nfsd 0" might have races.

> 
> 
> > 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.

Agreed.

Thanks,
NeilBrown