Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     Chuck Lever <chuck.lever@oracle.com>,
        Jeff Layton <jlayton@kernel.org>
Cc:     linux-nfs@vger.kernel.org, Olga Kornievskaia <kolga@netapp.com>,
        Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>
Subject: [PATCH] lockd: hold a reference to nlmsvc_serv while stopping thread.
Date:   Tue, 10 Oct 2023 10:35:43 +1100
Message-id: <169689454310.26263.15848180396022999880@noble.neil.brown.name>
Precedence: bulk


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 report Jeff !!!

 fs/lockd/svc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index b441c706c2b8..7a5c90a00522 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -345,10 +345,10 @@ static int lockd_get(void)
 
 	serv->sv_maxconn = nlm_max_connections;
 	error = svc_set_num_threads(serv, NULL, 1);
-	/* The thread now holds the only reference */
-	svc_put(serv);
-	if (error < 0)
+	if (error < 0) {
+		svc_put(serv);
 		return error;
+	}
 
 	nlmsvc_serv = serv;
 	register_inetaddr_notifier(&lockd_inetaddr_notifier);
@@ -374,6 +374,7 @@ static void lockd_put(void)
 
 	svc_set_num_threads(nlmsvc_serv, NULL, 0);
 	timer_delete_sync(&nlmsvc_retry);
+	svc_put(nlmsvc_serv);
 	nlmsvc_serv = NULL;
 	dprintk("lockd_down: service destroyed\n");
 }
-- 
2.42.0