vfs_mount.c: Don't call VFS_MOUNT() if only exports are being updated

PR#293198 reports a hang within ZFS when exports
are being updated concurrently with a VOP_SETEXTATTR().
The hang appears to be caused by mishandling of the
z_teardown_lock, but fixing handling of this lock appears
to be a major effort. Since the hang occurs when
VFS_MOUNT() acquires a write/exclusive z_teardown_lock,
which rarely occurs, except when exports are being updated,
this patch avoids the VFS_MOUNT() call for this case.

Avoiding a VFS_MOUNT() call fixes the hang for the case
reported by PR#293198 and is also an optimization.
As such, this patch avoids the VFS_MOUNT() call when only exports
are being updated similar to what was already being done
within vnet prisons.

PR:	293198
Reviewed by:	kib, markj
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D55318

1 files changed, 31 insertions, 21 deletions

diff --git a/sys/kern/vfs_mount.c b/sys/kern/vfs_mount.c
index 2237fcc6b423..383ccf98c10e 100644
--- a/sys/kern/vfs_mount.c
+++ b/sys/kern/vfs_mount.c
@@ -74,7 +74,7 @@
 #define	VFS_MOUNTARG_SIZE_MAX	(1024 * 64)
 
 static int	vfs_domount(struct thread *td, const char *fstype, char *fspath,
-		    uint64_t fsflags, bool jail_export,
+		    uint64_t fsflags, bool only_export, bool jail_export,
 		    struct vfsoptlist **optlist);
 static void	free_mntarg(struct mntarg *ma);
 
@@ -806,7 +806,7 @@ vfs_donmount(struct thread *td, uint64_t fsflags, struct uio *fsoptions)
 	struct vfsopt *opt, *tmp_opt;
 	char *fstype, *fspath, *errmsg;
 	int error, fstypelen, fspathlen, errmsg_len, errmsg_pos;
-	bool autoro, has_nonexport, jail_export;
+	bool autoro, has_nonexport, only_export, jail_export;
 
 	errmsg = fspath = NULL;
 	errmsg_len = fspathlen = 0;
@@ -861,12 +861,11 @@ vfs_donmount(struct thread *td, uint64_t fsflags, struct uio *fsoptions)
 	 * when we want to update the root filesystem.
 	 */
 	has_nonexport = false;
-	jail_export = false;
+	only_export = false;
 	TAILQ_FOREACH_SAFE(opt, optlist, link, tmp_opt) {
 		int do_freeopt = 0;
 
-		if (jailed(td->td_ucred) &&
-		    strcmp(opt->name, "export") != 0 &&
+		if (strcmp(opt->name, "export") != 0 &&
 		    strcmp(opt->name, "update") != 0 &&
 		    strcmp(opt->name, "fstype") != 0 &&
 		    strcmp(opt->name, "fspath") != 0 &&
@@ -957,7 +956,7 @@ vfs_donmount(struct thread *td, uint64_t fsflags, struct uio *fsoptions)
 			fsflags |= MNT_UNION;
 		else if (strcmp(opt->name, "export") == 0) {
 			fsflags |= MNT_EXPORTED;
-			jail_export = true;
+			only_export = true;
 		} else if (strcmp(opt->name, "automounted") == 0) {
 			fsflags |= MNT_AUTOMOUNTED;
 			do_freeopt = 1;
@@ -989,14 +988,22 @@ vfs_donmount(struct thread *td, uint64_t fsflags, struct uio *fsoptions)
 	}
 
 	/*
-	 * If has_nonexport is true or the caller is not running within a
-	 * vnet prison that can run mountd(8), set jail_export false.
+	 * only_export is set to true only if exports are being
+	 * updated and nothing else is being updated.
+	 */
+	if (has_nonexport)
+		only_export = false;
+	/*
+	 * If only_export is true and the caller is running within a
+	 * vnet prison that can run mountd(8), set jail_export true.
 	 */
-	if (has_nonexport || !jailed(td->td_ucred) ||
-	    !prison_check_nfsd(td->td_ucred))
-		jail_export = false;
+	jail_export = false;
+	if (only_export && jailed(td->td_ucred) &&
+	    prison_check_nfsd(td->td_ucred))
+		jail_export = true;
 
-	error = vfs_domount(td, fstype, fspath, fsflags, jail_export, &optlist);
+	error = vfs_domount(td, fstype, fspath, fsflags, only_export,
+	    jail_export, &optlist);
 	if (error == ENODEV) {
 		error = EINVAL;
 		if (errmsg != NULL)
@@ -1014,8 +1021,8 @@ vfs_donmount(struct thread *td, uint64_t fsflags, struct uio *fsoptions)
 		printf("%s: R/W mount failed, possibly R/O media,"
 		    " trying R/O mount\n", __func__);
 		fsflags |= MNT_RDONLY;
-		error = vfs_domount(td, fstype, fspath, fsflags, jail_export,
-		    &optlist);
+		error = vfs_domount(td, fstype, fspath, fsflags, only_export,
+		    jail_export, &optlist);
 	}
 bail:
 	/* copyout the errmsg */
@@ -1307,6 +1314,7 @@ vfs_domount_update(
 	struct thread *td,		/* Calling thread. */
 	struct vnode *vp,		/* Mount point vnode. */
 	uint64_t fsflags,		/* Flags common to all filesystems. */
+	bool only_export,		/* Got export option. */
 	bool jail_export,		/* Got export option in vnet prison. */
 	struct vfsoptlist **optlist	/* Options local to the filesystem. */
 	)
@@ -1442,15 +1450,16 @@ vfs_domount_update(
 	 * get.  No freeing of cn_pnbuf.
 	 */
 	/*
+	 * When only updating mount exports, VFS_MOUNT() does not need to
+	 * be called, as indicated by only_export being set true.
 	 * For the case of mountd(8) doing exports from within a vnet jail,
 	 * "from" is typically not set correctly such that VFS_MOUNT() will
-	 * return ENOENT. It is not obvious that VFS_MOUNT() ever needs to be
-	 * called when mountd is doing exports, but this check only applies to
-	 * the specific case where it is running inside a vnet jail, to
-	 * avoid any POLA violation.
+	 * return ENOENT. For ZFS, there is a locking bug which can result in
+	 * deadlock if VFS_MOUNT() is called when extended attributes are
+	 * being updated.
 	 */
 	error = 0;
-	if (!jail_export)
+	if (!only_export)
 		error = VFS_MOUNT(mp);
 
 	export_error = 0;
@@ -1590,6 +1599,7 @@ vfs_domount(
 	const char *fstype,		/* Filesystem type. */
 	char *fspath,			/* Mount path. */
 	uint64_t fsflags,		/* Flags common to all filesystems. */
+	bool only_export,		/* Got export option. */
 	bool jail_export,		/* Got export option in vnet prison. */
 	struct vfsoptlist **optlist	/* Options local to the filesystem. */
 	)
@@ -1693,8 +1703,8 @@ vfs_domount(
 		}
 		free(pathbuf, M_TEMP);
 	} else
-		error = vfs_domount_update(td, vp, fsflags, jail_export,
-		    optlist);
+		error = vfs_domount_update(td, vp, fsflags, only_export,
+		    jail_export, optlist);
 
 out:
 	NDFREE_PNBUF(&nd);