Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
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:     "Jeff Layton" <jlayton@kernel.org>,
        "Linux NFS Mailing List" <linux-nfs@vger.kernel.org>,
        "Olga Kornievskaia" <kolga@netapp.com>,
        "Dai Ngo" <dai.ngo@oracle.com>, "Tom Talpey" <tom@talpey.com>
Subject: Re: [PATCH] lockd: hold a reference to nlmsvc_serv while stopping thread.
In-reply-to: <834A724C-4A44-4277-AFB5-D639FCF2900B@oracle.com>
References: <169689454310.26263.15848180396022999880@noble.neil.brown.name>,
 <834A724C-4A44-4277-AFB5-D639FCF2900B@oracle.com>
Date:   Wed, 11 Oct 2023 09:30:06 +1100
Message-id: <169697700659.26263.12600384009390271074@noble.neil.brown.name>
Precedence: bulk

On Tue, 10 Oct 2023, Chuck Lever III wrote:
> 
> > On Oct 9, 2023, at 7:35 PM, NeilBrown <neilb@suse.de> wrote:
> > We are required to hold a reference to the svc_serv struct while
> > stopping the last thread, as doing that could otherwise drop the last
> > reference itself and the svc_serv would be freed while still in use.
> > 
> > lockd doesn't do this.  After startup, the only reference is held by the
> > running thread.
> > 
> > So change locked to hold a reference on nlmsvc_serv while-ever the
> > service is active, and only drop it after the last thread has been
> > stopped.
> > 
> > Note: it doesn't really make sense for threads to hold references to the
> > svc_serv any more.  The fact threads are included in serv->sv_nrthreads
> > is sufficient.  Maybe a future patch could address this.
> > 
> > Reported-by: Jeff Layton <jlayton@kernel.org>
> > Fixes: 68cc388c3238 ("SUNRPC: change how svc threads are asked to exit.")
> > Signed-off-by: NeilBrown <neilb@suse.de>
> 
> Thanks for the fast response!
> 
> Should I squash this into 68cc, or apply it before? I would
> like to ensure that bisect works nicely over this series of
> commits.

Probably makes sense to put it before.  In that case the patch
description needs re-wording.

And on reflection I think the code should be changed a little so that it
matches similar code in nfsd and nfs4-callback.
So I'll repost.
I'll take the liberty of preserving Jeff's review/test even though I've
changed the code .... I hope that's OK.

NeilBrown