Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     "trondmy@kernel.org" <trondmy@kernel.org>
CC:     Bruce Fields <bfields@redhat.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] nfsd: Ensure knfsd shuts down when the "nfsd" pseudofs is
 unmounted
Thread-Topic: [PATCH] nfsd: Ensure knfsd shuts down when the "nfsd" pseudofs
 is unmounted
Thread-Index: AQHXGE0TeKcnmbpiEkis/ppV016GpKqFHtkA
Date:   Mon, 15 Mar 2021 14:30:21 +0000
Message-ID: <3E0EA1A3-8257-4C20-97FD-C8712029F41D@oracle.com>
References: <20210313210847.569041-1-trondmy@kernel.org>
In-Reply-To: <20210313210847.569041-1-trondmy@kernel.org>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <699332976C30EA428A804AD0E23FBBEA@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 0e69e1d7-0f21-43b7-93aa-08d8e7bedd19
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Mar 2021 14:30:21.0944
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: hWE+/Lt35uW4RcJamqwEGFnADuVamusir+ffvsG0gk5iPd2x5qlNIHwTbPV3shmYPAbC8g9TsI7tc9IVSmOPHA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB2869
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9923 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 spamscore=0 bulkscore=0
 malwarescore=0 adultscore=0 mlxscore=0 mlxlogscore=999 phishscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000
 definitions=main-2103150105
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9923 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxlogscore=999
 spamscore=0 mlxscore=0 bulkscore=0 suspectscore=0 priorityscore=1501
 lowpriorityscore=0 clxscore=1015 adultscore=0 phishscore=0 impostorscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000
 definitions=main-2103150105
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Mar 13, 2021, at 4:08 PM, trondmy@kernel.org wrote:
>=20
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>=20
> In order to ensure that knfsd threads don't linger once the nfsd
> pseudofs is unmounted (e.g. when the container is killed) we let
> nfsd_umount() shut down those threads and wait for them to exit.
>=20
> This also should ensure that we don't need to do a kernel mount of
> the pseudofs, since the thread lifetime is now limited by the
> lifetime of the filesystem.
>=20
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>

Hey Trond, I've included your patch in the for-next topic branch at

git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git


> ---
> fs/nfsd/netns.h     |  6 +++---
> fs/nfsd/nfs4state.c |  8 +-------
> fs/nfsd/nfsctl.c    | 14 ++------------
> fs/nfsd/nfsd.h      |  3 +--
> fs/nfsd/nfssvc.c    | 35 ++++++++++++++++++++++++++++++++++-
> 5 files changed, 41 insertions(+), 25 deletions(-)
>=20
> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> index c330f5bd0cf3..a75abeb1e698 100644
> --- a/fs/nfsd/netns.h
> +++ b/fs/nfsd/netns.h
> @@ -51,9 +51,6 @@ struct nfsd_net {
> 	bool grace_ended;
> 	time64_t boot_time;
>=20
> -	/* internal mount of the "nfsd" pseudofilesystem: */
> -	struct vfsmount *nfsd_mnt;
> -
> 	struct dentry *nfsd_client_dir;
>=20
> 	/*
> @@ -130,6 +127,9 @@ struct nfsd_net {
> 	wait_queue_head_t ntf_wq;
> 	atomic_t ntf_refcnt;
>=20
> +	/* Allow umount to wait for nfsd state cleanup */
> +	struct completion nfsd_shutdown_complete;
> +
> 	/*
> 	 * clientid and stateid data for construction of net unique COPY
> 	 * stateids.
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 423fd6683f3a..8bf840661d67 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -7346,14 +7346,9 @@ nfs4_state_start_net(struct net *net)
> 	struct nfsd_net *nn =3D net_generic(net, nfsd_net_id);
> 	int ret;
>=20
> -	ret =3D get_nfsdfs(net);
> -	if (ret)
> -		return ret;
> 	ret =3D nfs4_state_create_net(net);
> -	if (ret) {
> -		mntput(nn->nfsd_mnt);
> +	if (ret)
> 		return ret;
> -	}
> 	locks_start_grace(net, &nn->nfsd4_manager);
> 	nfsd4_client_tracking_init(net);
> 	if (nn->track_reclaim_completes && nn->reclaim_str_hashtbl_size =3D=3D 0=
)
> @@ -7423,7 +7418,6 @@ nfs4_state_shutdown_net(struct net *net)
>=20
> 	nfsd4_client_tracking_exit(net);
> 	nfs4_state_destroy_net(net);
> -	mntput(nn->nfsd_mnt);
> }
>=20
> void
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index ef86ed23af82..02ff7f762e2d 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1416,6 +1416,8 @@ static void nfsd_umount(struct super_block *sb)
> {
> 	struct net *net =3D sb->s_fs_info;
>=20
> +	nfsd_shutdown_threads(net);
> +
> 	kill_litter_super(sb);
> 	put_net(net);
> }
> @@ -1428,18 +1430,6 @@ static struct file_system_type nfsd_fs_type =3D {
> };
> MODULE_ALIAS_FS("nfsd");
>=20
> -int get_nfsdfs(struct net *net)
> -{
> -	struct nfsd_net *nn =3D net_generic(net, nfsd_net_id);
> -	struct vfsmount *mnt;
> -
> -	mnt =3D  vfs_kern_mount(&nfsd_fs_type, SB_KERNMOUNT, "nfsd", NULL);
> -	if (IS_ERR(mnt))
> -		return PTR_ERR(mnt);
> -	nn->nfsd_mnt =3D mnt;
> -	return 0;
> -}
> -
> #ifdef CONFIG_PROC_FS
> static int create_proc_exports_entry(void)
> {
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 8bdc37aa2c2e..27c1308ffc2b 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -93,13 +93,12 @@ int		nfsd_get_nrthreads(int n, int *, struct net *);
> int		nfsd_set_nrthreads(int n, int *, struct net *);
> int		nfsd_pool_stats_open(struct inode *, struct file *);
> int		nfsd_pool_stats_release(struct inode *, struct file *);
> +void		nfsd_shutdown_threads(struct net *net);
>=20
> void		nfsd_destroy(struct net *net);
>=20
> bool		i_am_nfsd(void);
>=20
> -int get_nfsdfs(struct net *);
> -
> struct nfsdfs_client {
> 	struct kref cl_ref;
> 	void (*cl_release)(struct kref *kref);
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index 6de406322106..f014b7aa0726 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -596,6 +596,37 @@ static const struct svc_serv_ops nfsd_thread_sv_ops =
=3D {
> 	.svo_module		=3D THIS_MODULE,
> };
>=20
> +static void nfsd_complete_shutdown(struct net *net)
> +{
> +	struct nfsd_net *nn =3D net_generic(net, nfsd_net_id);
> +
> +	WARN_ON(!mutex_is_locked(&nfsd_mutex));
> +
> +	nn->nfsd_serv =3D NULL;
> +	complete(&nn->nfsd_shutdown_complete);
> +}
> +
> +void nfsd_shutdown_threads(struct net *net)
> +{
> +	struct nfsd_net *nn =3D net_generic(net, nfsd_net_id);
> +	struct svc_serv *serv;
> +
> +	mutex_lock(&nfsd_mutex);
> +	serv =3D nn->nfsd_serv;
> +	if (serv =3D=3D NULL) {
> +		mutex_unlock(&nfsd_mutex);
> +		return;
> +	}
> +
> +	svc_get(serv);
> +	/* Kill outstanding nfsd threads */
> +	serv->sv_ops->svo_setup(serv, NULL, 0);
> +	nfsd_destroy(net);
> +	mutex_unlock(&nfsd_mutex);
> +	/* Wait for shutdown of nfsd_serv to complete */
> +	wait_for_completion(&nn->nfsd_shutdown_complete);
> +}
> +
> bool i_am_nfsd(void)
> {
> 	return kthread_func(current) =3D=3D nfsd;
> @@ -618,11 +649,13 @@ int nfsd_create_serv(struct net *net)
> 						&nfsd_thread_sv_ops);
> 	if (nn->nfsd_serv =3D=3D NULL)
> 		return -ENOMEM;
> +	init_completion(&nn->nfsd_shutdown_complete);
>=20
> 	nn->nfsd_serv->sv_maxconn =3D nn->max_connections;
> 	error =3D svc_bind(nn->nfsd_serv, net);
> 	if (error < 0) {
> 		svc_destroy(nn->nfsd_serv);
> +		nfsd_complete_shutdown(net);
> 		return error;
> 	}
>=20
> @@ -671,7 +704,7 @@ void nfsd_destroy(struct net *net)
> 		svc_shutdown_net(nn->nfsd_serv, net);
> 	svc_destroy(nn->nfsd_serv);
> 	if (destroy)
> -		nn->nfsd_serv =3D NULL;
> +		nfsd_complete_shutdown(net);
> }
>=20
> int nfsd_set_nrthreads(int n, int *nthreads, struct net *net)
> --=20
> 2.30.2
>=20

--
Chuck Lever