On Tue, 2023-10-10 at 10:35 +1100, NeilBrown 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@xxxxxxxxxx>
> Fixes: 68cc388c3238 ("SUNRPC: change how svc threads are asked to exit.")
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
> ---
>
> 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");
> }
Thanks Neil! You can add:
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
Tested-by: Jeff Layton <jlayton@xxxxxxxxxx>
