zfs_ioctl: fix EBUSY race between quota queries and mount

zfsvfs_hold() fell back to zfsvfs_create() -> dmu_objset_own() (exclusive) for unmounted datasets. A concurrent zfs_domount() also calls dmu_objset_own(), causing EBUSY on the same dataset. Introduce zfsvfs_create_hold() using dmu_objset_hold() (shared hold) instead. Shared holds do not conflict with exclusive owns, eliminating the race. The release path (zfsvfs_rele, zfsvfs_create_impl error) uses dmu_objset_ds()->ds_owner to determine whether to disown or rele, avoiding the need for an extra flag in zfsvfs_t. Added tests userspace_005, groupspace_005, projectspace_006 (50 iter race test). Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Signed-off-by: HeonJe Lee <[email protected]> Closes #18611
2026-10-28 11:25:21 +01:00 · 2026-06-22 13:25:12 -07:00
1 parent 87593ea2b3
commit eb0c674c2a
@@ -73,6 +73,7 @@ struct zfsvfs {
 	boolean_t	z_atime;	/* enable atimes mount option */
 	boolean_t	z_relatime;	/* enable relatime mount option */
 	boolean_t	z_unmounted;	/* unmounted */
+	boolean_t	z_use_hold;	/* held via dmu_objset_hold */
 	zfs_teardown_lock_t z_teardown_lock;
 	zfs_teardown_inactive_lock_t z_teardown_inactive_lock;
 	list_t		z_all_znodes;	/* all vnodes in the fs */
@@ -226,6 +227,7 @@ extern int zfs_resume_fs(zfsvfs_t *zfsvfs, struct dsl_dataset *ds);
 extern int zfs_end_fs(zfsvfs_t *zfsvfs, struct dsl_dataset *ds);
 extern int zfs_set_version(zfsvfs_t *zfsvfs, uint64_t newvers);
 extern int zfsvfs_create(const char *name, boolean_t readonly, zfsvfs_t **zfvp);
+extern int zfsvfs_create_hold(const char *name, zfsvfs_t **zfvp);
 extern int zfsvfs_create_impl(zfsvfs_t **zfvp, zfsvfs_t *zfsvfs, objset_t *os);
 extern void zfsvfs_free(zfsvfs_t *zfsvfs);
 extern int zfs_check_global_label(const char *dsname, const char *hexsl);
@@ -100,6 +100,7 @@ struct zfsvfs {
 	int		z_norm;		/* normalization flags */
 	boolean_t	z_relatime;	/* enable relatime mount option */
 	boolean_t	z_unmounted;	/* unmounted */
+	boolean_t	z_use_hold;	/* held via dmu_objset_hold */
 	rrmlock_t	z_teardown_lock;
 	krwlock_t	z_teardown_inactive_lock;
 	list_t		z_all_znodes;	/* all znodes in the fs */
@@ -237,6 +238,7 @@ extern int zfs_end_fs(zfsvfs_t *zfsvfs, struct dsl_dataset *ds);
 extern void zfs_exit_fs(zfsvfs_t *zfsvfs);
 extern int zfs_set_version(zfsvfs_t *zfsvfs, uint64_t newvers);
 extern int zfsvfs_create(const char *name, boolean_t readony, zfsvfs_t **zfvp);
+extern int zfsvfs_create_hold(const char *name, zfsvfs_t **zfvp);
 extern int zfsvfs_create_impl(zfsvfs_t **zfvp, zfsvfs_t *zfsvfs, objset_t *os);
 extern void zfsvfs_free(zfsvfs_t *zfsvfs);
 extern int zfs_check_global_label(const char *dsname, const char *hexsl);
@@ -1046,6 +1046,32 @@ zfsvfs_create(const char *osname, boolean_t readonly, zfsvfs_t **zfvp)
 	return (error);
 }

+int
+zfsvfs_create_hold(const char *osname, zfsvfs_t **zfvp)
+{
+	objset_t *os;
+	zfsvfs_t *zfsvfs;
+	int error;
+
+	zfsvfs = kmem_zalloc(sizeof (zfsvfs_t), KM_SLEEP);
+
+	error = dmu_objset_hold(osname, zfsvfs, &os);
+	if (error != 0) {
+		kmem_free(zfsvfs, sizeof (zfsvfs_t));
+		return (error);
+	}
+
+	if (dmu_objset_type(os) != DMU_OST_ZFS) {
+		dmu_objset_rele(os, zfsvfs);
+		kmem_free(zfsvfs, sizeof (zfsvfs_t));
+		return (EINVAL);
+	}
+
+	zfsvfs->z_use_hold = B_TRUE;
+	error = zfsvfs_create_impl(zfvp, zfsvfs, os);
+
+	return (error);
+}

 int
 zfsvfs_create_impl(zfsvfs_t **zfvp, zfsvfs_t *zfsvfs, objset_t *os)
@@ -1069,7 +1095,10 @@ zfsvfs_create_impl(zfsvfs_t **zfvp, zfsvfs_t *zfsvfs, objset_t *os)

 	error = zfsvfs_init(zfsvfs, os);
 	if (error != 0) {
-		dmu_objset_disown(os, B_TRUE, zfsvfs);
+		if (zfsvfs->z_use_hold)
+			dmu_objset_rele(os, zfsvfs);
+		else
+			dmu_objset_disown(os, B_TRUE, zfsvfs);
 		*zfvp = NULL;
 		kmem_free(zfsvfs, sizeof (zfsvfs_t));
 		return (error);
@@ -621,6 +621,32 @@ zfsvfs_init(zfsvfs_t *zfsvfs, objset_t *os)
 	return (0);
 }

+int
+zfsvfs_create_hold(const char *osname, zfsvfs_t **zfvp)
+{
+	objset_t *os;
+	zfsvfs_t *zfsvfs;
+	int error;
+
+	zfsvfs = kmem_zalloc(sizeof (zfsvfs_t), KM_SLEEP);
+
+	error = dmu_objset_hold(osname, zfsvfs, &os);
+	if (error != 0) {
+		kmem_free(zfsvfs, sizeof (zfsvfs_t));
+		return (error);
+	}
+
+	if (dmu_objset_type(os) != DMU_OST_ZFS) {
+		dmu_objset_rele(os, zfsvfs);
+		kmem_free(zfsvfs, sizeof (zfsvfs_t));
+		return (EINVAL);
+	}
+
+	zfsvfs->z_use_hold = B_TRUE;
+	error = zfsvfs_create_impl(zfvp, zfsvfs, os);
+	return (error);
+}
+
 int
 zfsvfs_create(const char *osname, boolean_t readonly, zfsvfs_t **zfvp)
 {
@@ -678,7 +704,10 @@ zfsvfs_create_impl(zfsvfs_t **zfvp, zfsvfs_t *zfsvfs, objset_t *os)

 	error = zfsvfs_init(zfsvfs, os);
 	if (error != 0) {
-		dmu_objset_disown(os, B_TRUE, zfsvfs);
+		if (zfsvfs->z_use_hold)
+			dmu_objset_rele(os, zfsvfs);
+		else
+			dmu_objset_disown(os, B_TRUE, zfsvfs);
 		*zfvp = NULL;
 		zfsvfs_free(zfsvfs);
 		return (error);
@@ -1635,8 +1635,17 @@ zfsvfs_hold(const char *name, const void *tag, zfsvfs_t **zfvp,
 	int error = 0;

 	if (getzfsvfs(name, zfvp) != 0)
-		error = zfsvfs_create(name, B_FALSE, zfvp);
+		error = zfsvfs_create_hold(name, zfvp);
 	if (error == 0) {
+		/*
+		 * dmu_objset_hold() keeps the pool config read lock held.
+		 * Drop it before acquiring the teardown lock to avoid ABBA
+		 * deadlock with zfs_resume_fs(), which holds teardown write
+		 * then acquires the config lock.
+		 */
+		if ((*zfvp)->z_use_hold)
+			dsl_pool_config_exit(
+			    dmu_objset_pool((*zfvp)->z_os), *zfvp);
 		if (writer)
 			ZFS_TEARDOWN_ENTER_WRITE(*zfvp, tag);
 		else
@@ -1663,7 +1672,18 @@ zfsvfs_rele(zfsvfs_t *zfsvfs, const void *tag)
 	if (zfs_vfs_held(zfsvfs)) {
 		zfs_vfs_rele(zfsvfs);
 	} else {
-		dmu_objset_disown(zfsvfs->z_os, B_TRUE, zfsvfs);
+		objset_t *os = zfsvfs->z_os;
+		if (zfsvfs->z_use_hold) {
+			/*
+			 * Opened via dmu_objset_hold(): re-acquire the pool
+			 * config lock (released in zfsvfs_hold() before the
+			 * teardown lock) so that dmu_objset_rele() can exit it.
+			 */
+			dsl_pool_config_enter(dmu_objset_pool(os), zfsvfs);
+			dmu_objset_rele(os, zfsvfs);
+		} else {
+			dmu_objset_disown(os, B_TRUE, zfsvfs);
+		}
 		zfsvfs_free(zfsvfs);
 	}
 }
@@ -889,7 +889,7 @@ tests = ['defaultprojectquota_002_pos', 'defaultprojectquota_003_neg',
    'defaultprojectquota_007_pos', 'projectquota_002_pos',
    'projectquota_004_neg', 'projectquota_005_pos', 'projectquota_007_pos',
    'projectquota_008_pos', 'projectquota_009_pos', 'projecttree_002_pos',
-    'projecttree_003_neg']
+    'projecttree_003_neg', 'projectspace_006_pos']
 tags = ['functional', 'projectquota']

 [tests/functional/poolversion]
@@ -1098,8 +1098,9 @@ tests = [
    'userquota_004_pos', 'userquota_005_neg', 'userquota_006_pos',
    'userquota_007_pos', 'userquota_008_pos', 'userquota_009_pos',
    'userquota_010_pos', 'userquota_011_pos', 'userquota_012_neg',
+    'groupspace_005_pos',
    'userspace_001_pos', 'userspace_002_pos', 'userspace_004_pos',
-    'userspace_encrypted', 'userspace_send_encrypted',
+    'userspace_005_pos', 'userspace_encrypted', 'userspace_send_encrypted',
    'userspace_encrypted_13709']
 tags = ['functional', 'userquota']

@@ -1880,6 +1880,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
 	functional/projectquota/projectspace_003_pos.ksh \
 	functional/projectquota/projectspace_004_pos.ksh \
 	functional/projectquota/projectspace_005_pos.ksh \
+	functional/projectquota/projectspace_006_pos.ksh \
 	functional/projectquota/projecttree_001_pos.ksh \
 	functional/projectquota/projecttree_002_pos.ksh \
 	functional/projectquota/projecttree_003_neg.ksh \
@@ -2252,6 +2253,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
 	functional/userquota/groupspace_002_pos.ksh \
 	functional/userquota/groupspace_003_pos.ksh \
 	functional/userquota/groupspace_004_pos.ksh \
+	functional/userquota/groupspace_005_pos.ksh \
 	functional/userquota/setup.ksh \
 	functional/userquota/defaultuserquota_001_pos.ksh \
 	functional/userquota/defaultuserquota_002_pos.ksh \
@@ -2283,6 +2285,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
 	functional/userquota/userspace_002_pos.ksh \
 	functional/userquota/userspace_003_pos.ksh \
 	functional/userquota/userspace_004_pos.ksh \
+	functional/userquota/userspace_005_pos.ksh \
 	functional/userquota/userspace_encrypted.ksh \
 	functional/userquota/userspace_send_encrypted.ksh \
 	functional/userquota/userspace_encrypted_13709.ksh \
@@ -0,0 +1,97 @@
+#!/bin/ksh -p
+# SPDX-License-Identifier: CDDL-1.0
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
+# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or https://opensource.org/licenses/CDDL-1.0.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, include this CDDL HEADER in each
+# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright (c) 2026, Gluesys. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/projectquota/projectquota_common.kshlib
+
+#
+# DESCRIPTION:
+#	Verify 'zfs projectspace' reports correct quota values whether the
+#	dataset is mounted or not, and does not fail with EBUSY when
+#	raced against 'zfs mount'.
+#
+# STRATEGY:
+#	1. Create a dataset with a projectquota and write 50M of data.
+#	2. Check quota and used values while mounted.
+#	3. Unmount and verify the same values are still readable.
+#	4. Remount and re-verify values are consistent.
+#	5. Race 'zfs projectspace' against 'zfs mount' 50 times; expect no EBUSY.
+#
+
+function cleanup
+{
+	zfs unmount $RACE_FS 2>/dev/null
+	datasetexists $RACE_FS && destroy_dataset $RACE_FS
+	rm -f "$ERRFILE" "$DATAFILE"
+}
+
+log_onexit cleanup
+
+typeset RACE_FS=$QFS/projectspace_race
+typeset ERRFILE=/tmp/projectspace_006_err.$$
+typeset DATAFILE
+typeset -i ITERS=50
+
+log_assert "zfs projectspace shows correct quota values and does not EBUSY on mount race"
+
+log_must zfs create $RACE_FS
+log_must zfs set projectquota@$PRJID1=100m $RACE_FS
+mkmount_writable $RACE_FS
+DATAFILE=$(get_prop mountpoint $RACE_FS)/projectspace_006_data.$$
+log_must mkfile 50m $DATAFILE
+sync_all_pools
+
+log_note "check projectspace values while mounted"
+log_must eval "zfs projectspace $RACE_FS | grep $PRJID1 | grep 100M"
+
+log_must zfs unmount $RACE_FS
+log_note "check projectspace values while unmounted"
+log_must eval "zfs projectspace $RACE_FS | grep $PRJID1 | grep 100M"
+
+log_must zfs mount $RACE_FS
+log_note "check projectspace values after remount"
+log_must eval "zfs projectspace $RACE_FS | grep $PRJID1 | grep 100M"
+
+typeset -i i=0 ebusy=0
+while (( i < ITERS )); do
+	log_must zfs unmount $RACE_FS
+	zfs mount $RACE_FS &
+	typeset mpid=$!
+	zfs projectspace $RACE_FS >"$ERRFILE" 2>&1
+	typeset rc=$?
+	wait $mpid
+	if (( rc != 0 )) && grep -qi "busy" "$ERRFILE" 2>/dev/null; then
+		(( ebusy++ ))
+		log_note "Iteration $i: EBUSY: $(cat $ERRFILE)"
+	fi
+	rm -f "$ERRFILE"
+	(( i++ ))
+done
+
+(( ebusy > 0 )) && log_fail "EBUSY seen $ebusy/$ITERS times"
+
+log_pass "zfs projectspace shows correct quota values and does not EBUSY on mount race"
@@ -0,0 +1,100 @@
+#!/bin/ksh -p
+# SPDX-License-Identifier: CDDL-1.0
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
+# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or https://opensource.org/licenses/CDDL-1.0.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, include this CDDL HEADER in each
+# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright (c) 2026, Gluesys. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/userquota/userquota_common.kshlib
+
+#
+# DESCRIPTION:
+#	Verify 'zfs groupspace' reports correct quota values whether the
+#	dataset is mounted or not, and does not fail with EBUSY when
+#	raced against 'zfs mount'.
+#
+# STRATEGY:
+#	1. Create a dataset with a groupquota and write 50M of data.
+#	2. Check quota and used values while mounted.
+#	3. Unmount and verify the same values are still readable.
+#	4. Remount and re-verify values are consistent.
+#	5. Race 'zfs groupspace' against 'zfs mount' 50 times; expect no EBUSY.
+#
+
+function cleanup
+{
+	zfs unmount $RACE_FS 2>/dev/null
+	datasetexists $RACE_FS && destroy_dataset $RACE_FS
+	rm -f "$ERRFILE" "$DATAFILE"
+}
+
+log_onexit cleanup
+
+typeset RACE_FS=$QFS/groupspace_race
+typeset ERRFILE=/tmp/groupspace_005_err.$$
+typeset DATAFILE
+typeset -i ITERS=50
+
+log_assert "zfs groupspace shows correct quota values and does not EBUSY on mount race"
+
+log_must zfs create $RACE_FS
+log_must zfs set groupquota@$QGROUP=100m $RACE_FS
+mkmount_writable $RACE_FS
+DATAFILE=$(get_prop mountpoint $RACE_FS)/groupspace_005_data.$$
+log_must user_run $QUSER1 mkfile 50m $DATAFILE
+sync_all_pools
+
+log_note "check groupspace values while mounted"
+log_must eval "zfs groupspace $RACE_FS | grep $QGROUP | grep 100M"
+log_must eval "zfs groupspace $RACE_FS | grep $QGROUP | grep '50\\..*M'"
+
+log_must zfs unmount $RACE_FS
+log_note "check groupspace values while unmounted"
+log_must eval "zfs groupspace $RACE_FS | grep $QGROUP | grep 100M"
+log_must eval "zfs groupspace $RACE_FS | grep $QGROUP | grep '50\\..*M'"
+
+log_must zfs mount $RACE_FS
+log_note "check groupspace values after remount"
+log_must eval "zfs groupspace $RACE_FS | grep $QGROUP | grep 100M"
+log_must eval "zfs groupspace $RACE_FS | grep $QGROUP | grep '50\\..*M'"
+
+typeset -i i=0 ebusy=0
+while (( i < ITERS )); do
+	log_must zfs unmount $RACE_FS
+	zfs mount $RACE_FS &
+	typeset mpid=$!
+	zfs groupspace $RACE_FS >"$ERRFILE" 2>&1
+	typeset rc=$?
+	wait $mpid
+	if (( rc != 0 )) && grep -qi "busy" "$ERRFILE" 2>/dev/null; then
+		(( ebusy++ ))
+		log_note "Iteration $i: EBUSY: $(cat $ERRFILE)"
+	fi
+	rm -f "$ERRFILE"
+	(( i++ ))
+done
+
+(( ebusy > 0 )) && log_fail "EBUSY seen $ebusy/$ITERS times"
+
+log_pass "zfs groupspace shows correct quota values and does not EBUSY on mount race"
@@ -0,0 +1,100 @@
+#!/bin/ksh -p
+# SPDX-License-Identifier: CDDL-1.0
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
+# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or https://opensource.org/licenses/CDDL-1.0.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, include this CDDL HEADER in each
+# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright (c) 2026, Gluesys. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/userquota/userquota_common.kshlib
+
+#
+# DESCRIPTION:
+#	Verify 'zfs userspace' reports correct quota values whether the
+#	dataset is mounted or not, and does not fail with EBUSY when
+#	raced against 'zfs mount'.
+#
+# STRATEGY:
+#	1. Create a dataset with a userquota and write 50M of data.
+#	2. Check quota and used values while mounted.
+#	3. Unmount and verify the same values are still readable.
+#	4. Remount and re-verify values are consistent.
+#	5. Race 'zfs userspace' against 'zfs mount' 50 times; expect no EBUSY.
+#
+
+function cleanup
+{
+	zfs unmount $RACE_FS 2>/dev/null
+	datasetexists $RACE_FS && destroy_dataset $RACE_FS
+	rm -f "$ERRFILE" "$DATAFILE"
+}
+
+log_onexit cleanup
+
+typeset RACE_FS=$QFS/userspace_race
+typeset ERRFILE=/tmp/userspace_005_err.$$
+typeset DATAFILE
+typeset -i ITERS=50
+
+log_assert "zfs userspace shows correct quota values and does not EBUSY on mount race"
+
+log_must zfs create $RACE_FS
+log_must zfs set userquota@$QUSER1=100m $RACE_FS
+mkmount_writable $RACE_FS
+DATAFILE=$(get_prop mountpoint $RACE_FS)/userspace_005_data.$$
+log_must user_run $QUSER1 mkfile 50m $DATAFILE
+sync_all_pools
+
+log_note "check userspace values while mounted"
+log_must eval "zfs userspace $RACE_FS | grep $QUSER1 | grep 100M"
+log_must eval "zfs userspace $RACE_FS | grep $QUSER1 | grep '50\\..*M'"
+
+log_must zfs unmount $RACE_FS
+log_note "check userspace values while unmounted"
+log_must eval "zfs userspace $RACE_FS | grep $QUSER1 | grep 100M"
+log_must eval "zfs userspace $RACE_FS | grep $QUSER1 | grep '50\\..*M'"
+
+log_must zfs mount $RACE_FS
+log_note "check userspace values after remount"
+log_must eval "zfs userspace $RACE_FS | grep $QUSER1 | grep 100M"
+log_must eval "zfs userspace $RACE_FS | grep $QUSER1 | grep '50\\..*M'"
+
+typeset -i i=0 ebusy=0
+while (( i < ITERS )); do
+	log_must zfs unmount $RACE_FS
+	zfs mount $RACE_FS &
+	typeset mpid=$!
+	zfs userspace $RACE_FS >"$ERRFILE" 2>&1
+	typeset rc=$?
+	wait $mpid
+	if (( rc != 0 )) && grep -qi "busy" "$ERRFILE" 2>/dev/null; then
+		(( ebusy++ ))
+		log_note "Iteration $i: EBUSY: $(cat $ERRFILE)"
+	fi
+	rm -f "$ERRFILE"
+	(( i++ ))
+done
+
+(( ebusy > 0 )) && log_fail "EBUSY seen $ebusy/$ITERS times"
+
+log_pass "zfs userspace shows correct quota values and does not EBUSY on mount race"