Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
Message-ID: <ccac5c0333b60cb37309a15f18f8592c19d73879.camel@kernel.org>
Subject: Re: [PATCH v1] SUNRPC: Remove BUG_ON call sites
From:   Jeff Layton <jlayton@kernel.org>
To:     Chuck Lever <cel@kernel.org>, neilb@suse.de, brauner@kernel.org
Cc:     Chuck Lever <chuck.lever@oracle.com>, linux-nfs@vger.kernel.org
Date:   Fri, 22 Sep 2023 15:19:27 -0400
In-Reply-To: <169504668787.134583.4338728458451666583.stgit@manet.1015granger.net>
References: <169504668787.134583.4338728458451666583.stgit@manet.1015granger.net>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

On Mon, 2023-09-18 at 10:18 -0400, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
>=20
> There is no need to take down the whole system for these assertions.
>=20
> I'd rather not attempt a heroic save here, as some bug has occurred
> that has left the transport data structures in an unknown state.
> Just warn and then leak the left-over resources.
>=20
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  net/sunrpc/svc.c |   11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>=20
> Let's start here. Comments?
>=20
>=20
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 587811a002c9..11a1d5e7f5c7 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -575,11 +575,14 @@ svc_destroy(struct kref *ref)
>  	timer_shutdown_sync(&serv->sv_temptimer);
> =20
>  	/*
> -	 * The last user is gone and thus all sockets have to be destroyed to
> -	 * the point. Check this.
> +	 * Remaining transports at this point are not expected.
>  	 */
> -	BUG_ON(!list_empty(&serv->sv_permsocks));
> -	BUG_ON(!list_empty(&serv->sv_tempsocks));
> +	if (unlikely(!list_empty(&serv->sv_permsocks)))
> +		pr_warn("SVC: permsocks remain for %s\n",
> +			serv->sv_program->pg_name);
> +	if (unlikely(!list_empty(&serv->sv_tempsocks)))
> +		pr_warn("SVC: tempsocks remain for %s\n",
> +			serv->sv_program->pg_name);
> =20
>  	cache_clean_deferred(serv);
> =20
>=20
>=20

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