Z.I.A. disk open/close rework

Reworked how Z.I.A. opens and closes
disks when changing parameters, specifically
zia_provider and zia_disk/file_write.
The purpose is to remove redundant file/disk
openings when the provider has no use for them.

zia_get_provider() no longer opens vdevs
and the vdevs are now only opened in spa.c
whenever the provider is set AND either
zia_disk_write or zia_file_write is enabled.
Rather than calling the generic vdev_close/open(),
zia_open_vdevs() is called instead. Additionally,
the zia_open/close_vdevs() functions are now wrapped
with spa_vdev_state_enter() and spa_vdev_state_exit()
functions.

zia_close_vdevs() is not called whenever disabling
file_write and disk_write as it will eventually
be called whenever changing the provider or
freeing it by destroying the zpool.

Signed-off-by: Luke Beirne <[email protected]>

Author: Luke Beirne

include/sys/zia.h

@@ -114,7 +114,8 @@ void zia_prop_warn(boolean_t val, const char *name);
 int zia_init(void);
 int zia_fini(void);
 
-void *zia_get_provider(const char *name, vdev_t *vdev);
+void zia_open_vdevs(vdev_t *vd);
+void *zia_get_provider(const char *name);
 const char *zia_get_provider_name(void *provider);
 int zia_put_provider(void **provider, vdev_t *vdev);
 

module/zfs/spa.c

@@ -2180,6 +2180,11 @@ spa_unload(spa_t *spa)
 
 	bpobj_close(&spa->spa_deferred_bpobj);
 
+	if (zia_get_props(spa)->provider != NULL) {
+		zia_put_provider(&zia_get_props(spa)->provider,
+		    spa->spa_root_vdev);
+	}
+
 	spa_config_enter(spa, SCL_ALL, spa, RW_WRITER);
 
 	/*
@@ -2251,11 +2256,6 @@ spa_unload(spa_t *spa)
 	spa->spa_raidz_expand = NULL;
 	spa->spa_checkpoint_txg = 0;
 
-	if (zia_get_props(spa)->provider != NULL) {
-		zia_put_provider(&zia_get_props(spa)->provider,
-		    spa->spa_root_vdev);
-	}
-
 	spa_config_exit(spa, SCL_ALL, spa);
 }
 
@@ -9815,26 +9815,21 @@ spa_sync_props(void *arg, dmu_tx_t *tx)
 			if (zia_props->provider != NULL)
 				zia_put_provider(&zia_props->provider,
 				    spa->spa_root_vdev);
-			zia_props->provider = zia_get_provider(strval,
-			    spa->spa_root_vdev);
+			zia_props->provider = zia_get_provider(strval);
 			zia_props->can_offload = !!zia_props->provider;
 
 			/*
-			 * Dirty the configuration on vdevs as above.
-			 */
-			if (tx->tx_txg != TXG_INITIAL) {
-				vdev_config_dirty(spa->spa_root_vdev);
-				spa_async_request(spa, SPA_ASYNC_CONFIG_UPDATE);
-			}
-
-			/*
-			 * reopen devices so that provider is used
-			 * copied from zfs_ioc_pool_reopen
+			 * It is possible to enable disk_write or file_write
+			 * without passing a provider, or swapping one
+			 * provider with another while these flags are enabled.
+			 * In those cases, vdevs must be opened for the new
+			 * provider upon being passed.
+			 * The vdevs will be closed for the older provider
+			 * in zia_put_provider() first.
 			 */
-			spa_vdev_state_enter(spa, SCL_NONE);
-			vdev_close(spa->spa_root_vdev);
-			(void) vdev_open(spa->spa_root_vdev);
-			(void) spa_vdev_state_exit(spa, NULL, 0);
+			ASSERT3P(spa->spa_root_vdev, !=, NULL);
+			if (zia_props->disk_write || zia_props->file_write)
+				zia_open_vdevs(spa->spa_root_vdev);
 
 			spa_history_log_internal(spa, "set", tx,
 			    "%s=%s", nvpair_name(elem), strval);
@@ -9924,11 +9919,16 @@ spa_sync_props(void *arg, dmu_tx_t *tx)
 			zia_props->file_write =
 			    fnvpair_value_uint64(elem);
 
-			/* reopen devices so that provider is used */
-			spa_vdev_state_enter(spa, SCL_NONE);
-			vdev_close(spa->spa_root_vdev);
-			(void) vdev_open(spa->spa_root_vdev);
-			(void) spa_vdev_state_exit(spa, NULL, 0);
+			/*
+			 * Only open vdevs for the provider if one
+			 * has already been instantiated and if the
+			 * passed value to file_write is 1, not 0
+			 * to disable it.
+			 */
+			ASSERT3P(spa->spa_root_vdev, !=, NULL);
+			if (zia_props->provider && zia_props->file_write) {
+				zia_open_vdevs(spa->spa_root_vdev);
+			}
 
 			zia_prop_warn(zia_props->file_write,
 			    "File Write");
@@ -9937,11 +9937,13 @@ spa_sync_props(void *arg, dmu_tx_t *tx)
 			zia_props->disk_write =
 			    fnvpair_value_uint64(elem);
 
-			/* reopen devices so that provider is used */
-			spa_vdev_state_enter(spa, SCL_NONE);
-			vdev_close(spa->spa_root_vdev);
-			(void) vdev_open(spa->spa_root_vdev);
-			(void) spa_vdev_state_exit(spa, NULL, 0);
+			/*
+			 * Check for the same reason as file_write.
+			 */
+			ASSERT3P(spa->spa_root_vdev, !=, NULL);
+			if (zia_props->provider && zia_props->disk_write) {
+				zia_open_vdevs(spa->spa_root_vdev);
+			}
 
 			zia_prop_warn(zia_props->disk_write,
 			    "Disk Write");

module/zfs/zia.c

@@ -295,7 +295,7 @@ zia_fini(void)
 
 #ifdef ZIA
 /* recursively find all leaf vdevs and open them */
-static void zia_open_vdevs(vdev_t *vd) {
+static void zia_open_vdevs_impl(vdev_t *vd) {
 	vdev_ops_t *ops = vd->vdev_ops;
 	if (ops->vdev_op_leaf) {
 		ASSERT(!vd->vdev_zia_handle);
@@ -309,23 +309,34 @@ static void zia_open_vdevs(vdev_t *vd) {
 			}
 #ifdef _KERNEL
 			else if (memcmp(ops->vdev_op_type, "disk", 4) == 0) {
-				/* first member is struct block_device * */
-				void *disk = vd->vdev_tsd;
-				zia_disk_open(vd, vd->vdev_path, disk);
+				zia_disk_open(vd, vd->vdev_path,
+				    get_bdev(vd->vdev_tsd));
 			}
 #endif
 		}
 	} else {
 		for (uint64_t i = 0; i < vd->vdev_children; i++) {
 			vdev_t *child = vd->vdev_child[i];
-			zia_open_vdevs(child);
+			zia_open_vdevs_impl(child);
 		}
 	}
 }
 #endif
 
+void zia_open_vdevs(vdev_t *vd) {
+#ifdef ZIA
+	if (!vd)
+		return;
+
+	spa_vdev_state_enter(vd->vdev_spa, SCL_NONE);
+	zia_open_vdevs_impl(vd);
+	(void) spa_vdev_state_exit(vd->vdev_spa, NULL, 0);
+#endif
+	(void) vd;
+}
+
 void *
-zia_get_provider(const char *name, vdev_t *vdev)
+zia_get_provider(const char *name)
 {
 #ifdef ZIA
 	if (!dpusm) {
@@ -339,13 +350,9 @@ zia_get_provider(const char *name, vdev_t *vdev)
 	    name, provider);
 #endif
 
-	/* set up Z.I.A. for existing vdevs */
-	if (vdev) {
-		zia_open_vdevs(vdev);
-	}
 	return (provider);
 #else
-	(void) name; (void) vdev;
+	(void) name;
 
 #ifdef _KERNEL
 	printk("Z.I.A. not available. Cannot obtain handle to providers.\n");
@@ -407,7 +414,9 @@ zia_put_provider(void **provider, vdev_t *vdev)
 	 * make sure the vdevs don't keep pointing to the invalid provider
 	 */
 	if (vdev) {
+		spa_vdev_state_enter(vdev->vdev_spa, SCL_NONE);
 		zia_close_vdevs(vdev);
+		(void) spa_vdev_state_exit(vdev->vdev_spa, NULL, 0);
 	}
 
 #ifdef _KERNEL
@@ -1639,8 +1648,10 @@ zia_disk_open(vdev_t *vdev, const char *path,
 		return (ZIA_ERROR);
 	}
 
-	void *provider = zia_get_props(vdev->vdev_spa)->provider;
-	if (!dpusm || !provider) {
+	zia_props_t *props = zia_get_props(vdev->vdev_spa);
+	void *provider = props->provider;
+
+	if (!dpusm || !provider || props->disk_write != 1) {
 		return (ZIA_FALLBACK);
 	}