Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 29 Jul 2026 17:50:02 +0000
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 9a2c9345c435 - releng/15.0 - Make zvol_set_common() block until the operation has completed
Message-ID:  <6a6a3d4a.3b83d.7b45b486@gitrepo.freebsd.org>

index | next in thread | raw e-mail

The branch releng/15.0 has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=9a2c9345c435208ef73869b67fb4d9bf3097243a

commit 9a2c9345c435208ef73869b67fb4d9bf3097243a
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2026-02-02 01:55:04 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2026-07-28 15:19:38 +0000

    Make zvol_set_common() block until the operation has completed
    
    This is motivated by a FreeBSD AIO test case which create a zvol with -o
    volmode=dev, then immediately tries to open the zvol device file.  The
    open occasionally fails with ENOENT.
    
    When a zvol is created without the volmode setting, zvol_create_minors()
    blocks until the task is finished, at which point OS-dependent code will
    have created a device file.  However, zvol_set_common() may cause the
    device file to be destroyed and re-created, at least on FreeBSD, if the
    voltype switches from GEOM to DEV.  In this case, we do not block
    waiting for the operation to finish, causing the test failure.
    
    Fix the problem by making zvol_set_common() block until the operation
    has finished.  In FreeBSD zvol code, use g_waitidle() to block until
    asynchronous GEOM operations are done.  This fixes a secondary race
    where zvol_os_remove_minor() does not block until the zvol device file
    is removed, and the subsequent zvol_os_create_minor() fails because the
    (to-be-destroyed) device file already exists.
    
    Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
    Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
    Signed-off-by: Mark Johnston <markj@FreeBSD.org>
    Closes #18191
    
    Approved by:    so
    Security:       FreeBSD-EN-26:19.zfs
    
    (cherry picked from commit d736868672cb3df197d2ddcfef29ac14edf4ee20)
    (cherry picked from commit f6745503ded7cc938a52d6916277da12c5b31d4f)
---
 .../openzfs/module/os/freebsd/zfs/zvol_os.c        |  4 +-
 sys/contrib/openzfs/module/zfs/zvol.c              | 46 ++++++++++++++++++++--
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c b/sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c
index ef245fba1088..68f905124c49 100644
--- a/sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c
+++ b/sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c
@@ -918,7 +918,7 @@ retry:
 		return (SET_ERROR(ENXIO));
 
 	mutex_enter(&zv->zv_state_lock);
-	if (zv->zv_zso->zso_dying || zv->zv_flags & ZVOL_REMOVING) {
+	if (zv->zv_flags & ZVOL_REMOVING || zv->zv_zso->zso_dying) {
 		err = SET_ERROR(ENXIO);
 		goto out_locked;
 	}
@@ -1425,6 +1425,7 @@ zvol_os_remove_minor(zvol_state_t *zv)
 		pp->private = NULL;
 		g_wither_geom(pp->geom, ENXIO);
 		g_topology_unlock();
+		g_waitidle(curthread);
 	} else if (zv->zv_volmode == ZFS_VOLMODE_DEV) {
 		struct zvol_state_dev *zsd = &zso->zso_dev;
 		struct cdev *dev = zsd->zsd_cdev;
@@ -1554,6 +1555,7 @@ out_dmu_objset_disown:
 		g_error_provider(zv->zv_zso->zso_geom.zsg_provider, 0);
 		/* geom was locked inside zvol_alloc() function */
 		g_topology_unlock();
+		g_waitidle(curthread);
 	}
 out_doi:
 	kmem_free(doi, sizeof (dmu_object_info_t));
diff --git a/sys/contrib/openzfs/module/zfs/zvol.c b/sys/contrib/openzfs/module/zfs/zvol.c
index 407758641580..b6d6f9db2597 100644
--- a/sys/contrib/openzfs/module/zfs/zvol.c
+++ b/sys/contrib/openzfs/module/zfs/zvol.c
@@ -1936,6 +1936,10 @@ typedef struct zvol_set_prop_int_arg {
 	uint64_t zsda_value;
 	zprop_source_t zsda_source;
 	zfs_prop_t zsda_prop;
+	taskqid_t zsda_taskqid;
+	boolean_t zsda_dispatched;
+	kmutex_t zsda_lock;
+	kcondvar_t zsda_cv;
 } zvol_set_prop_int_arg_t;
 
 /*
@@ -1966,6 +1970,7 @@ zvol_set_common_sync_cb(dsl_pool_t *dp, dsl_dataset_t *ds, void *arg)
 	char dsname[ZFS_MAX_DATASET_NAME_LEN];
 	zvol_task_t *task;
 	uint64_t prop;
+	taskqid_t id;
 
 	const char *prop_name = zfs_prop_to_name(zsda->zsda_prop);
 	dsl_dataset_name(ds, dsname);
@@ -1984,8 +1989,12 @@ zvol_set_common_sync_cb(dsl_pool_t *dp, dsl_dataset_t *ds, void *arg)
 	}
 	task->zt_value = prop;
 	strlcpy(task->zt_name1, dsname, sizeof (task->zt_name1));
-	(void) taskq_dispatch(dp->dp_spa->spa_zvol_taskq, zvol_task_cb,
-	    task, TQ_SLEEP);
+	id = taskq_dispatch(dp->dp_spa->spa_zvol_taskq, zvol_task_cb, task,
+	    TQ_SLEEP);
+	mutex_enter(&zsda->zsda_lock);
+	if (id != TASKQID_INVALID && id > zsda->zsda_taskqid)
+		zsda->zsda_taskqid = id;
+	mutex_exit(&zsda->zsda_lock);
 	return (0);
 }
 
@@ -2018,6 +2027,11 @@ zvol_set_common_sync(void *arg, dmu_tx_t *tx)
 	dmu_objset_find_dp(dp, dd->dd_object, zvol_set_common_sync_cb,
 	    zsda, DS_FIND_CHILDREN);
 
+	mutex_enter(&zsda->zsda_lock);
+	zsda->zsda_dispatched = TRUE;
+	cv_broadcast(&zsda->zsda_cv);
+	mutex_exit(&zsda->zsda_lock);
+
 	dsl_dir_rele(dd, FTAG);
 }
 
@@ -2026,14 +2040,38 @@ zvol_set_common(const char *ddname, zfs_prop_t prop, zprop_source_t source,
     uint64_t val)
 {
 	zvol_set_prop_int_arg_t zsda;
+	spa_t *spa;
+	int error;
 
 	zsda.zsda_name = ddname;
 	zsda.zsda_source = source;
 	zsda.zsda_value = val;
 	zsda.zsda_prop = prop;
+	zsda.zsda_taskqid = TASKQID_INVALID;
+	zsda.zsda_dispatched = FALSE;
+	mutex_init(&zsda.zsda_lock, NULL, MUTEX_DEFAULT, NULL);
+	cv_init(&zsda.zsda_cv, NULL, CV_DEFAULT, NULL);
 
-	return (dsl_sync_task(ddname, zvol_set_common_check,
-	    zvol_set_common_sync, &zsda, 0, ZFS_SPACE_CHECK_NONE));
+	error = spa_open(ddname, &spa, FTAG);
+	if (error != 0)
+		goto out;
+	error = dsl_sync_task(ddname, zvol_set_common_check,
+	    zvol_set_common_sync, &zsda, 0, ZFS_SPACE_CHECK_NONE);
+	if (error == 0) {
+		mutex_enter(&zsda.zsda_lock);
+		while (!zsda.zsda_dispatched)
+			cv_wait(&zsda.zsda_cv, &zsda.zsda_lock);
+		mutex_exit(&zsda.zsda_lock);
+
+		if (zsda.zsda_taskqid != TASKQID_INVALID)
+			taskq_wait_outstanding(spa->spa_zvol_taskq,
+			    zsda.zsda_taskqid);
+	}
+	spa_close(spa, FTAG);
+out:
+	cv_destroy(&zsda.zsda_cv);
+	mutex_destroy(&zsda.zsda_lock);
+	return (error);
 }
 
 void


home | help

Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a6a3d4a.3b83d.7b45b486>