Message-ID: <b9aebe4951bb7af4fa07dbdacd2f1751944f7cf2.camel@kernel.org>
Subject: Re: [PATCH 1/1] SUNRPC: Fix a server shutdown leak
From:   Jeff Layton <jlayton@kernel.org>
To:     Benjamin Coddington <bcodding@redhat.com>, chuck.lever@oracle.com,
        trond.myklebust@hammerspace.com, anna@kernel.org,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, bfields@redhat.com
Cc:     linux-nfs@vger.kernel.org
Date:   Wed, 08 Mar 2023 05:48:08 -0500
In-Reply-To: <65d0248533fbdea2f6190faa1ee150d2d615344b.1677877233.git.bcodding@redhat.com>
References: <cover.1677877233.git.bcodding@redhat.com>
         <65d0248533fbdea2f6190faa1ee150d2d615344b.1677877233.git.bcodding@redhat.com>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Fri, 2023-03-03 at 16:08 -0500, Benjamin Coddington wrote:
> Fix a race where kthread_stop() may prevent the threadfn from ever gettin=
g
> called.  If that happens the svc_rqst will not be cleaned up.
>=20
> Fixes: ed6473ddc704 ("NFSv4: Fix callback server shutdown")
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---
>  net/sunrpc/svc.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>=20
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 1fd3f5e57285..fea7ce8fba14 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -798,6 +798,7 @@ svc_start_kthreads(struct svc_serv *serv, struct svc_=
pool *pool, int nrservs)
>  static int
>  svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrse=
rvs)
>  {
> +	struct svc_rqst	*rqstp;
>  	struct task_struct *task;
>  	unsigned int state =3D serv->sv_nrthreads-1;
> =20
> @@ -806,7 +807,10 @@ svc_stop_kthreads(struct svc_serv *serv, struct svc_=
pool *pool, int nrservs)
>  		task =3D choose_victim(serv, pool, &state);
>  		if (task =3D=3D NULL)
>  			break;
> -		kthread_stop(task);
> +		rqstp =3D kthread_data(task);
> +		/* Did we lose a race to svo_function threadfn? */
> +		if (kthread_stop(task) =3D=3D -EINTR)
> +			svc_exit_thread(rqstp);
>  		nrservs++;
>  	} while (nrservs < 0);
>  	return 0;

Reviewed-by: Jeff Layton <jlayton@kernel.org>