zfsonlinux issue openzfs#3681 - lock order inversion between zvol_ope…

…n() and

dsl_pool_sync()...zvol_rename_minors()

Create/destroy per-pool zvol asynchroous work taskqs in
spa_activate()/spa_deactivate(), remove explicit calls to
create/destroy taskqs at pool create/import/export/destroy.
Remove the zvol taskq list and rwlock in zvol.c

include/sys/spa.h

@@ -801,6 +801,7 @@ extern uint64_t spa_version(spa_t *spa);
 extern boolean_t spa_deflate(spa_t *spa);
 extern metaslab_class_t *spa_normal_class(spa_t *spa);
 extern metaslab_class_t *spa_log_class(spa_t *spa);
+extern taskq_t *spa_zvol_tq(spa_t *spa);
 extern void spa_evicting_os_register(spa_t *, objset_t *os);
 extern void spa_evicting_os_deregister(spa_t *, objset_t *os);
 extern void spa_evicting_os_wait(spa_t *spa);

include/sys/spa_impl.h

@@ -252,7 +252,8 @@ struct spa {
 	uint64_t	spa_deadman_synctime;	/* deadman expiration timer */
 	uint64_t	spa_errata;		/* errata issues detected */
 	spa_stats_t	spa_stats;		/* assorted spa statistics */
-
+	/* taskq for asynchronous zvol-minor-related work */
+	taskq_t		*spa_zvol_tq;
 	/*
 	 * spa_refcount & spa_config_lock must be the last elements
 	 * because refcount_t changes size based on compilation options.

include/sys/zvol.h

@@ -32,8 +32,6 @@
 #define	ZVOL_ZAP_OBJ		2ULL
 
 typedef enum {
-	ZVOL_ASYNC_CREATE_TASKQ,
-	ZVOL_ASYNC_REMOVE_TASKQ,
 	ZVOL_ASYNC_CREATE_MINORS,
 	ZVOL_ASYNC_REMOVE_MINORS,
 	ZVOL_ASYNC_REMOVE_MINOR,
@@ -57,8 +55,6 @@ extern int zvol_check_volblocksize(const char *name, uint64_t volblocksize);
 extern int zvol_get_stats(objset_t *os, nvlist_t *nv);
 extern boolean_t zvol_is_zvol(const char *);
 extern void zvol_create_cb(objset_t *os, void *arg, cred_t *cr, dmu_tx_t *tx);
-extern void zvol_async_create_taskq(const char *pool);
-extern void zvol_async_remove_taskq(const char *pool);
 extern void zvol_async_create_minors(const char *name);
 extern void zvol_async_remove_minor(const char *name);
 extern void zvol_async_remove_minors(const char *name);

module/zfs/spa.c

@@ -1131,6 +1131,19 @@ spa_activate(spa_t *spa, int mode)
 	avl_create(&spa->spa_errlist_last,
 	    spa_error_entry_compare, sizeof (spa_error_entry_t),
 	    offsetof(spa_error_entry_t, se_avl));
+
+	{
+		char tqname[MAXNAMELEN];
+		char *pool = spa_name(spa);
+		snprintf(tqname, MAXNAMELEN, "zvol_async/%s", pool);
+		spa->spa_zvol_tq = taskq_create(tqname, 1, maxclsyspri, 1, INT_MAX,
+			TASKQ_PREPOPULATE);
+		if (spa->spa_zvol_tq == NULL) {
+			cmn_err(CE_WARN,
+				"ZFS: failed to create async work taskq for pool %s\n",
+				pool);
+		}
+	}
 }
 
 /*
@@ -1147,6 +1160,15 @@ spa_deactivate(spa_t *spa)
 	ASSERT(spa->spa_async_zio_root == NULL);
 	ASSERT(spa->spa_state != POOL_STATE_UNINITIALIZED);
 
+	if (spa->spa_zvol_tq) {
+#ifdef _KERNEL
+		/* remove all the minors for the pool */
+		zvol_async_remove_minors(spa_name(spa));
+#endif
+		/* wait for all the tasks to be completed, then destroy the taskq */
+		taskq_destroy(spa->spa_zvol_tq);
+	}
+
 	spa_evicting_os_wait(spa);
 
 	txg_list_destroy(&spa->spa_vdev_txg_list);
@@ -3084,11 +3106,8 @@ spa_open_common(const char *pool, spa_t **spapp, void *tag, nvlist_t *nvpolicy,
 	}
 
 #ifdef _KERNEL
-	if (firstopen) {
-		char *name = spa_name(spa);
-		zvol_async_create_taskq(name);
-		zvol_async_create_minors(name);
-	}
+	if (firstopen)
+		zvol_async_create_minors(spa_name(spa));
 #endif
 
 	*spapp = spa;
@@ -4211,7 +4230,6 @@ spa_import(char *pool, nvlist_t *config, nvlist_t *props, uint64_t flags)
 	spa_history_log_version(spa, "import");
 
 #ifdef _KERNEL
-	zvol_async_create_taskq(pool);
 	zvol_async_create_minors(pool);
 #endif
 

module/zfs/spa_misc.c

@@ -1658,6 +1658,12 @@ spa_log_class(spa_t *spa)
 	return (spa->spa_log_class);
 }
 
+taskq_t *
+spa_zvol_tq(spa_t *spa)
+{
+	return (spa->spa_zvol_tq);
+}
+
 void
 spa_evicting_os_register(spa_t *spa, objset_t *os)
 {

module/zfs/zfs_ioctl.c

@@ -1485,12 +1485,6 @@ zfs_ioc_pool_create(zfs_cmd_t *zc)
 	    ZPROP_SRC_LOCAL, rootprops, NULL)) != 0)
 		(void) spa_destroy(zc->zc_name);
 
-	/*
-	 * Create a taskq for asynchronous zvol-related work in this pool
-	 */
-	if (!error)
-		zvol_async_create_taskq(zc->zc_name);
-
 pool_props_bad:
 	nvlist_free(rootprops);
 	nvlist_free(zplprops);
@@ -1506,10 +1500,8 @@ zfs_ioc_pool_destroy(zfs_cmd_t *zc)
 	int error;
 	zfs_log_history(zc);
 	error = spa_destroy(zc->zc_name);
-	if (error == 0) {
+	if (error == 0)
 		zvol_async_remove_minors(zc->zc_name);
-		zvol_async_remove_taskq(zc->zc_name);
-	}
 	return (error);
 }
 
@@ -1561,10 +1553,9 @@ zfs_ioc_pool_export(zfs_cmd_t *zc)
 
 	zfs_log_history(zc);
 	error = spa_export(zc->zc_name, NULL, force, hardforce);
-	if (error == 0) {
+	if (error == 0)
 		zvol_async_remove_minors(zc->zc_name);
-		zvol_async_remove_taskq(zc->zc_name);
-	}
+
 	return (error);
 }
 

module/zfs/zvol.c

@@ -53,20 +53,6 @@ zvol_create_minors_cb(const char *dsname, void *arg);
 static int
 zvol_create_snap_minor_cb(const char *dsname, void *arg);
 
-/*
- * Perform device minor related actions in taskqs, one taskq
- * (one worker thread) per pool
- */
-#include <sys/rwlock.h>
-static list_t zvol_async_taskq_list;
-static krwlock_t zvol_async_taskq_lock;
-
-typedef struct zvol_taskq {
-	const char zt_pool[MAXNAMELEN];
-	taskq_t *zt_tq;
-	list_node_t zt_next;
-} zvol_taskq_t;
-
 /* zvol_create_minors_cb argument */
 typedef struct {
 	const char *name;
@@ -1662,11 +1648,6 @@ zvol_init(void)
 
 	mutex_init(&zvol_state_lock, NULL, MUTEX_DEFAULT, NULL);
 
-	list_create(&zvol_async_taskq_list, sizeof (zvol_taskq_t),
-	    offsetof(zvol_taskq_t, zt_next));
-
-	rw_init(&zvol_async_taskq_lock, NULL, RW_DEFAULT, NULL);
-
 	error = register_blkdev(zvol_major, ZVOL_DRIVER);
 	if (error) {
 		printk(KERN_INFO "ZFS: register_blkdev() failed %d\n", error);
@@ -1679,9 +1660,6 @@ zvol_init(void)
 	return (0);
 
 out:
-	rw_destroy(&zvol_async_taskq_lock);
-	list_destroy(&zvol_async_taskq_list);
-
 	mutex_destroy(&zvol_state_lock);
 	list_destroy(&zvol_state_list);
 
@@ -1691,24 +1669,7 @@ zvol_init(void)
 void
 zvol_fini(void)
 {
-	zvol_taskq_t *zt = NULL;
-
-	/*
-	 * cleanup async taskqs
-	 * this step performs all the tasks on the queues first
-	 */
-	rw_enter(&zvol_async_taskq_lock, RW_WRITER);
-	while ((zt = list_remove_head(&zvol_async_taskq_list))) {
-		taskq_destroy(zt->zt_tq);
-		kmem_free(zt, sizeof (zvol_taskq_t));
-	}
-	rw_exit(&zvol_async_taskq_lock);
-
-	/* cleanup the list and the lock */
-	list_destroy(&zvol_async_taskq_list);
-	rw_destroy(&zvol_async_taskq_lock);
-
-	/* remove minors synchronously */
+	/* remove stray minors synchronously */
 	zvol_remove_minors(NULL);
 	blk_unregister_region(MKDEV(zvol_major, 0), 1UL << MINORBITS);
 	unregister_blkdev(zvol_major, ZVOL_DRIVER);
@@ -1717,80 +1678,6 @@ zvol_fini(void)
 }
 
 
-/*
- * Add/remove taskq per pool, create/remove/rename minor(s)
- */
-
-/* Create task for the pool, performed at pool create and import */
-static int
-zvol_create_taskq(const char *pool)
-{
-	zvol_taskq_t *zt, *ezt;
-
-	zt = kmem_zalloc(sizeof (zvol_taskq_t), KM_PUSHPAGE);
-
-	/* Use one thread to assure ordered completion of requests */
-	zt->zt_tq = taskq_create("zvol_at", 1, maxclsyspri, 1, INT_MAX,
-	    TASKQ_PREPOPULATE);
-	if (zt->zt_tq == NULL) {
-		kmem_free(zt, sizeof (zvol_taskq_t));
-		printk(KERN_INFO
-		    "ZFS: failed to create async work taskq for pool %s\n",
-		    pool);
-		return (-ENOMEM);
-	}
-
-	strlcpy((char *)zt->zt_pool, (char *)pool, MAXNAMELEN);
-
-	rw_enter(&zvol_async_taskq_lock, RW_WRITER);
-	/* check if the queue already exists */
-	for (ezt = list_head(&zvol_async_taskq_list); ezt != NULL;
-	    ezt = list_next(&zvol_async_taskq_list, ezt)) {
-		if (strncmp(ezt->zt_pool, pool, MAXNAMELEN) == 0) {
-			break;
-		}
-	}
-	if (ezt == NULL)
-		list_insert_head(&zvol_async_taskq_list, zt);
-	rw_exit(&zvol_async_taskq_lock);
-
-	if (ezt) {
-		printk(KERN_INFO
-		    "ZFS: async work taskq for pool %s already exists\n", pool);
-		taskq_destroy(zt->zt_tq);
-		kmem_free(zt, sizeof (zvol_taskq_t));
-	}
-
-	return (0);
-}
-
-/* Remove task for the pool, performed at pool export and destroy */
-static int
-zvol_remove_taskq(const char *pool)
-{
-	zvol_taskq_t *zt;
-
-	rw_enter(&zvol_async_taskq_lock, RW_WRITER);
-	for (zt = list_head(&zvol_async_taskq_list); zt != NULL;
-	    zt = list_next(&zvol_async_taskq_list, zt)) {
-		if (strncmp(zt->zt_pool, pool, MAXNAMELEN) == 0) {
-			list_remove(&zvol_async_taskq_list, zt);
-			break;
-		}
-	}
-	rw_exit(&zvol_async_taskq_lock);
-
-	/* This may have to wait for completion of the jobs on the queue */
-	if (zt) {
-		taskq_destroy(zt->zt_tq);
-		kmem_free(zt, sizeof (zvol_taskq_t));
-		return (0);
-	}
-
-	printk(KERN_INFO "ZFS: async work taskq for pool %s not found\n", pool);
-	return (-ENOENT);
-}
-
 /* The worker thread function performed asynchronously */
 static void
 _zvol_async_task(void *param)
@@ -1829,23 +1716,23 @@ static int
 zvol_dispatch_taskq(zvol_async_arg_t *arg)
 {
 	int error = 0;
-	zvol_taskq_t *zt;
+	spa_t *spa;
+	taskq_t *tq;
 
-	rw_enter(&zvol_async_taskq_lock, RW_READER);
-	for (zt = list_head(&zvol_async_taskq_list); zt != NULL;
-	    zt = list_next(&zvol_async_taskq_list, zt)) {
-		if (strncmp(zt->zt_pool, arg->pool, MAXNAMELEN) == 0)
-			break;
-	}
-	if (zt) {
-		error = taskq_dispatch(zt->zt_tq, _zvol_async_task,
-		    arg, TQ_SLEEP);
+	error = spa_open(arg->pool, &spa, FTAG);
+	if (error != 0)
+		return (error);
+
+	tq = spa_zvol_tq(spa);
+	if (tq) {
+		error = taskq_dispatch(tq, _zvol_async_task, arg, TQ_SLEEP);
 	} else {
 		printk(KERN_INFO "ZFS: async work taskq for pool %s not found;"
 		    " failed to dispatch work op %d\n", arg->pool, arg->op);
 		error = -ENOENT;
 	}
-	rw_exit(&zvol_async_taskq_lock);
+
+	spa_close(spa, FTAG);
 
 	return (error);
 }
@@ -1860,12 +1747,6 @@ zvol_async_dispatch(zvol_async_op_t op, const char *name1, const char *name2,
 	zvol_async_arg_t *arg;
 
 	switch (op) {
-	case ZVOL_ASYNC_CREATE_TASKQ:
-		error = zvol_create_taskq(name1);
-		break;
-	case ZVOL_ASYNC_REMOVE_TASKQ:
-		error = zvol_remove_taskq(name1);
-		break;
 	case ZVOL_ASYNC_CREATE_MINORS:
 	case ZVOL_ASYNC_REMOVE_MINORS:
 	case ZVOL_ASYNC_REMOVE_MINOR:
@@ -1903,18 +1784,6 @@ zvol_async_dispatch(zvol_async_op_t op, const char *name1, const char *name2,
 /*
  * The external interfaces wrapping zvol_async_dispatch()
  */
-void
-zvol_async_create_taskq(const char *pool)
-{
-	zvol_async_dispatch(ZVOL_ASYNC_CREATE_TASKQ, pool, NULL, 0);
-}
-
-void
-zvol_async_remove_taskq(const char *pool)
-{
-	zvol_async_dispatch(ZVOL_ASYNC_REMOVE_TASKQ, pool, NULL, 0);
-}
-
 void
 zvol_async_create_minors(const char *name)
 {