Illumos #3464

3464 zfs synctask code needs restructuring
Reviewed by: Dan Kimmel <[email protected]>
Reviewed by: Adam Leventhal <[email protected]>
Reviewed by: George Wilson <[email protected]>
Reviewed by: Christopher Siden <[email protected]>
Approved by: Garrett D'Amore <[email protected]>

References:
  https://www.illumos.org/issues/3464
  illumos/illumos-gate@3b2aab18808792cbd248a12f1edf139b89833c13

Ported-by: Tim Chase <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #1495

1 files changed, 80 insertions, 58 deletions

diff --git a/module/zfs/dsl_deleg.c b/module/zfs/dsl_deleg.c
index 48c261e63..d09e79f1c 100644
--- a/module/zfs/dsl_deleg.c
+++ b/module/zfs/dsl_deleg.c
@@ -147,28 +147,37 @@ dsl_deleg_can_unallow(char *ddname, nvlist_t *nvp, cred_t *cr)
 	return (0);
 }
 
+typedef struct dsl_deleg_arg {
+	const char *dda_name;
+	nvlist_t *dda_nvlist;
+} dsl_deleg_arg_t;
+
 static void
-dsl_deleg_set_sync(void *arg1, void *arg2, dmu_tx_t *tx)
+dsl_deleg_set_sync(void *arg, dmu_tx_t *tx)
 {
-	dsl_dir_t *dd = arg1;
-	nvlist_t *nvp = arg2;
-	objset_t *mos = dd->dd_pool->dp_meta_objset;
+	dsl_deleg_arg_t *dda = arg;
+	dsl_dir_t *dd;
+	dsl_pool_t *dp = dmu_tx_pool(tx);
+	objset_t *mos = dp->dp_meta_objset;
 	nvpair_t *whopair = NULL;
-	uint64_t zapobj = dd->dd_phys->dd_deleg_zapobj;
+	uint64_t zapobj;
+
+	VERIFY0(dsl_dir_hold(dp, dda->dda_name, FTAG, &dd, NULL));
 
+	zapobj = dd->dd_phys->dd_deleg_zapobj;
 	if (zapobj == 0) {
 		dmu_buf_will_dirty(dd->dd_dbuf, tx);
 		zapobj = dd->dd_phys->dd_deleg_zapobj = zap_create(mos,
 		    DMU_OT_DSL_PERMS, DMU_OT_NONE, 0, tx);
 	}
 
-	while ((whopair = nvlist_next_nvpair(nvp, whopair))) {
+	while ((whopair = nvlist_next_nvpair(dda->dda_nvlist, whopair))) {
 		const char *whokey = nvpair_name(whopair);
 		nvlist_t *perms;
 		nvpair_t *permpair = NULL;
 		uint64_t jumpobj;
 
-		VERIFY(nvpair_value_nvlist(whopair, &perms) == 0);
+		perms = fnvpair_value_nvlist(whopair);
 
 		if (zap_lookup(mos, zapobj, whokey, 8, 1, &jumpobj) != 0) {
 			jumpobj = zap_create_link(mos, DMU_OT_DSL_PERMS,
@@ -185,21 +194,27 @@ dsl_deleg_set_sync(void *arg1, void *arg2, dmu_tx_t *tx)
 			    "%s %s", whokey, perm);
 		}
 	}
+	dsl_dir_rele(dd, FTAG);
 }
 
 static void
-dsl_deleg_unset_sync(void *arg1, void *arg2, dmu_tx_t *tx)
+dsl_deleg_unset_sync(void *arg, dmu_tx_t *tx)
 {
-	dsl_dir_t *dd = arg1;
-	nvlist_t *nvp = arg2;
-	objset_t *mos = dd->dd_pool->dp_meta_objset;
+	dsl_deleg_arg_t *dda = arg;
+	dsl_dir_t *dd;
+	dsl_pool_t *dp = dmu_tx_pool(tx);
+	objset_t *mos = dp->dp_meta_objset;
 	nvpair_t *whopair = NULL;
-	uint64_t zapobj = dd->dd_phys->dd_deleg_zapobj;
+	uint64_t zapobj;
 
-	if (zapobj == 0)
+	VERIFY0(dsl_dir_hold(dp, dda->dda_name, FTAG, &dd, NULL));
+	zapobj = dd->dd_phys->dd_deleg_zapobj;
+	if (zapobj == 0) {
+		dsl_dir_rele(dd, FTAG);
 		return;
+	}
 
-	while ((whopair = nvlist_next_nvpair(nvp, whopair))) {
+	while ((whopair = nvlist_next_nvpair(dda->dda_nvlist, whopair))) {
 		const char *whokey = nvpair_name(whopair);
 		nvlist_t *perms;
 		nvpair_t *permpair = NULL;
@@ -234,35 +249,40 @@ dsl_deleg_unset_sync(void *arg1, void *arg2, dmu_tx_t *tx)
 			    "%s %s", whokey, perm);
 		}
 	}
+	dsl_dir_rele(dd, FTAG);
 }
 
-int
-dsl_deleg_set(const char *ddname, nvlist_t *nvp, boolean_t unset)
+static int
+dsl_deleg_check(void *arg, dmu_tx_t *tx)
 {
+	dsl_deleg_arg_t *dda = arg;
 	dsl_dir_t *dd;
 	int error;
-	nvpair_t *whopair = NULL;
-	int blocks_modified = 0;
 
-	error = dsl_dir_open(ddname, FTAG, &dd, NULL);
-	if (error)
-		return (error);
-
-	if (spa_version(dmu_objset_spa(dd->dd_pool->dp_meta_objset)) <
+	if (spa_version(dmu_tx_pool(tx)->dp_spa) <
 	    SPA_VERSION_DELEGATED_PERMS) {
-		dsl_dir_close(dd, FTAG);
 		return (ENOTSUP);
 	}
 
-	while ((whopair = nvlist_next_nvpair(nvp, whopair)))
-		blocks_modified++;
+	error = dsl_dir_hold(dmu_tx_pool(tx), dda->dda_name, FTAG, &dd, NULL);
+	if (error == 0)
+		dsl_dir_rele(dd, FTAG);
+	return (error);
+}
 
-	error = dsl_sync_task_do(dd->dd_pool, NULL,
-	    unset ? dsl_deleg_unset_sync : dsl_deleg_set_sync,
-	    dd, nvp, blocks_modified);
-	dsl_dir_close(dd, FTAG);
+int
+dsl_deleg_set(const char *ddname, nvlist_t *nvp, boolean_t unset)
+{
+	dsl_deleg_arg_t dda;
 
-	return (error);
+	/* nvp must already have been verified to be valid */
+
+	dda.dda_name = ddname;
+	dda.dda_nvlist = nvp;
+
+	return (dsl_sync_task(ddname, dsl_deleg_check,
+	    unset ? dsl_deleg_unset_sync : dsl_deleg_set_sync,
+	    &dda, fnvlist_num_pairs(nvp)));
 }
 
 /*
@@ -293,9 +313,15 @@ dsl_deleg_get(const char *ddname, nvlist_t **nvp)
 	zap_attribute_t *baseza, *za;
 	char *source;
 
-	error = dsl_dir_open(ddname, FTAG, &startdd, NULL);
-	if (error)
+	error = dsl_pool_hold(ddname, FTAG, &dp);
+	if (error != 0)
+		return (error);
+
+	error = dsl_dir_hold(dp, ddname, FTAG, &startdd, NULL);
+	if (error != 0) {
+		dsl_pool_rele(dp, FTAG);
 		return (error);
+	}
 
 	dp = startdd->dd_pool;
 	mos = dp->dp_meta_objset;
@@ -307,20 +333,16 @@ dsl_deleg_get(const char *ddname, nvlist_t **nvp)
 	source = kmem_alloc(MAXNAMELEN + strlen(MOS_DIR_NAME) + 1, KM_SLEEP);
 	VERIFY(nvlist_alloc(nvp, NV_UNIQUE_NAME, KM_SLEEP) == 0);
 
-	rw_enter(&dp->dp_config_rwlock, RW_READER);
 	for (dd = startdd; dd != NULL; dd = dd->dd_parent) {
 		nvlist_t *sp_nvp;
 		uint64_t n;
 
-		if (dd->dd_phys->dd_deleg_zapobj &&
-		    (zap_count(mos, dd->dd_phys->dd_deleg_zapobj,
-		    &n) == 0) && n) {
-			VERIFY(nvlist_alloc(&sp_nvp,
-			    NV_UNIQUE_NAME, KM_SLEEP) == 0);
-		} else {
+		if (dd->dd_phys->dd_deleg_zapobj == 0 ||
+		    zap_count(mos, dd->dd_phys->dd_deleg_zapobj, &n) != 0 ||
+		    n == 0)
 			continue;
-		}
 
+		sp_nvp = fnvlist_alloc();
 		for (zap_cursor_init(basezc, mos,
 		    dd->dd_phys->dd_deleg_zapobj);
 		    zap_cursor_retrieve(basezc, baseza) == 0;
@@ -330,27 +352,23 @@ dsl_deleg_get(const char *ddname, nvlist_t **nvp)
 			ASSERT(baseza->za_integer_length == 8);
 			ASSERT(baseza->za_num_integers == 1);
 
-			VERIFY(nvlist_alloc(&perms_nvp,
-			    NV_UNIQUE_NAME, KM_SLEEP) == 0);
+			perms_nvp = fnvlist_alloc();
 			for (zap_cursor_init(zc, mos, baseza->za_first_integer);
 			    zap_cursor_retrieve(zc, za) == 0;
 			    zap_cursor_advance(zc)) {
-				VERIFY(nvlist_add_boolean(perms_nvp,
-				    za->za_name) == 0);
+				fnvlist_add_boolean(perms_nvp, za->za_name);
 			}
 			zap_cursor_fini(zc);
-			VERIFY(nvlist_add_nvlist(sp_nvp, baseza->za_name,
-			    perms_nvp) == 0);
-			nvlist_free(perms_nvp);
+			fnvlist_add_nvlist(sp_nvp, baseza->za_name, perms_nvp);
+			fnvlist_free(perms_nvp);
 		}
 
 		zap_cursor_fini(basezc);
 
 		dsl_dir_name(dd, source);
-		VERIFY(nvlist_add_nvlist(*nvp, source, sp_nvp) == 0);
+		fnvlist_add_nvlist(*nvp, source, sp_nvp);
 		nvlist_free(sp_nvp);
 	}
-	rw_exit(&dp->dp_config_rwlock);
 
 	kmem_free(source, MAXNAMELEN + strlen(MOS_DIR_NAME) + 1);
 	kmem_free(baseza, sizeof(zap_attribute_t));
@@ -358,7 +376,8 @@ dsl_deleg_get(const char *ddname, nvlist_t **nvp)
 	kmem_free(za, sizeof(zap_attribute_t));
 	kmem_free(zc, sizeof(zap_cursor_t));
 
-	dsl_dir_close(startdd, FTAG);
+	dsl_dir_rele(startdd, FTAG);
+	dsl_pool_rele(dp, FTAG);
 	return (0);
 }
 
@@ -564,7 +583,7 @@ dsl_deleg_access_impl(dsl_dataset_t *ds, const char *perm, cred_t *cr)
 	avl_create(&permsets, perm_set_compare, sizeof (perm_set_t),
 	    offsetof(perm_set_t, p_node));
 
-	rw_enter(&dp->dp_config_rwlock, RW_READER);
+	ASSERT(dsl_pool_config_held(dp));
 	for (dd = ds->ds_dir; dd != NULL; dd = dd->dd_parent,
 	    checkflag = ZFS_DELEG_DESCENDENT) {
 		uint64_t zapobj;
@@ -625,7 +644,6 @@ again:
 	}
 	error = EPERM;
 success:
-	rw_exit(&dp->dp_config_rwlock);
 
 	cookie = NULL;
 	while ((setnode = avl_destroy_nodes(&permsets, &cookie)) != NULL)
@@ -637,15 +655,19 @@ success:
 int
 dsl_deleg_access(const char *dsname, const char *perm, cred_t *cr)
 {
+	dsl_pool_t *dp;
 	dsl_dataset_t *ds;
 	int error;
 
-	error = dsl_dataset_hold(dsname, FTAG, &ds);
-	if (error)
+	error = dsl_pool_hold(dsname, FTAG, &dp);
+	if (error != 0)
 		return (error);
-
-	error = dsl_deleg_access_impl(ds, perm, cr);
-	dsl_dataset_rele(ds, FTAG);
+	error = dsl_dataset_hold(dp, dsname, FTAG, &ds);
+	if (error == 0) {
+		error = dsl_deleg_access_impl(ds, perm, cr);
+		dsl_dataset_rele(ds, FTAG);
+	}
+	dsl_pool_rele(dp, FTAG);
 
 	return (error);
 }