Serialize ZTHR operations to eliminate races.

Signed-off-by: Serapheim Dimitropoulos <serapheim@delphix.com>
openzfs · Jan 10, 2019 · 8f8d9d3 · 8f8d9d3
1 parent 06f3fc2
commit 8f8d9d3
Show file tree

Hide file tree

Showing 7 changed files with 187 additions and 130 deletions.
diff --git a/include/sys/spa_checkpoint.h b/include/sys/spa_checkpoint.h
@@ -37,7 +37,7 @@ int spa_checkpoint(const char *);
 int spa_checkpoint_discard(const char *);
 
 boolean_t spa_checkpoint_discard_thread_check(void *, zthr_t *);
-int spa_checkpoint_discard_thread(void *, zthr_t *);
+void spa_checkpoint_discard_thread(void *, zthr_t *);
 
 int spa_checkpoint_get_stats(spa_t *, pool_checkpoint_stat_t *);
 

diff --git a/include/sys/zthr.h b/include/sys/zthr.h
@@ -14,42 +14,26 @@
  */
 
 /*
- * Copyright (c) 2017 by Delphix. All rights reserved.
+ * Copyright (c) 2017, 2018 by Delphix. All rights reserved.
  */
 
 #ifndef _SYS_ZTHR_H
 #define	_SYS_ZTHR_H
 
 typedef struct zthr zthr_t;
-typedef int (zthr_func_t)(void *, zthr_t *);
+typedef void (zthr_func_t)(void *, zthr_t *);
 typedef boolean_t (zthr_checkfunc_t)(void *, zthr_t *);
 
-struct zthr {
-	kthread_t	*zthr_thread;
-	kmutex_t	zthr_lock;
-	kcondvar_t	zthr_cv;
-	boolean_t	zthr_cancel;
-	hrtime_t	zthr_wait_time;
-
-	zthr_checkfunc_t	*zthr_checkfunc;
-	zthr_func_t	*zthr_func;
-	void		*zthr_arg;
-	int		zthr_rc;
-};
-
 extern zthr_t *zthr_create(zthr_checkfunc_t checkfunc,
     zthr_func_t *func, void *arg);
 extern zthr_t *zthr_create_timer(zthr_checkfunc_t *checkfunc,
     zthr_func_t *func, void *arg, hrtime_t nano_wait);
-
-extern void zthr_exit(zthr_t *t, int rc);
 extern void zthr_destroy(zthr_t *t);
 
 extern void zthr_wakeup(zthr_t *t);
-extern int zthr_cancel(zthr_t *t);
+extern void zthr_cancel(zthr_t *t);
 extern void zthr_resume(zthr_t *t);
 
 extern boolean_t zthr_iscancelled(zthr_t *t);
-extern boolean_t zthr_isrunning(zthr_t *t);
 
 #endif /* _SYS_ZTHR_H */
diff --git a/module/zfs/arc.c b/module/zfs/arc.c
@@ -5113,7 +5113,7 @@ arc_adjust_cb_check(void *arg, zthr_t *zthr)
  * from the ARC.
  */
 /* ARGSUSED */
-static int
+static void
 arc_adjust_cb(void *arg, zthr_t *zthr)
 {
 	uint64_t evicted = 0;
@@ -5147,8 +5147,6 @@ arc_adjust_cb(void *arg, zthr_t *zthr)
 	}
 	mutex_exit(&arc_adjust_lock);
 	spl_fstrans_unmark(cookie);
-
-	return (0);
 }
 
 /* ARGSUSED */
@@ -5190,7 +5188,7 @@ arc_reap_cb_check(void *arg, zthr_t *zthr)
  * to free more buffers.
  */
 /* ARGSUSED */
-static int
+static void
 arc_reap_cb(void *arg, zthr_t *zthr)
 {
 	int64_t free_memory;
@@ -5231,8 +5229,6 @@ arc_reap_cb(void *arg, zthr_t *zthr)
 		arc_reduce_target_size(to_free);
 	}
 	spl_fstrans_unmark(cookie);
-
-	return (0);
 }
 
 #ifdef _KERNEL

diff --git a/module/zfs/spa.c b/module/zfs/spa.c
@@ -1477,13 +1477,11 @@ spa_unload(spa_t *spa)
 	}
 
 	if (spa->spa_condense_zthr != NULL) {
-		ASSERT(!zthr_isrunning(spa->spa_condense_zthr));
 		zthr_destroy(spa->spa_condense_zthr);
 		spa->spa_condense_zthr = NULL;
 	}
 
 	if (spa->spa_checkpoint_discard_zthr != NULL) {
-		ASSERT(!zthr_isrunning(spa->spa_checkpoint_discard_zthr));
 		zthr_destroy(spa->spa_checkpoint_discard_zthr);
 		spa->spa_checkpoint_discard_zthr = NULL;
 	}
@@ -7047,12 +7045,12 @@ spa_async_suspend(spa_t *spa)
 	spa_vdev_remove_suspend(spa);
 
 	zthr_t *condense_thread = spa->spa_condense_zthr;
-	if (condense_thread != NULL && zthr_isrunning(condense_thread))
-		VERIFY0(zthr_cancel(condense_thread));
+	if (condense_thread != NULL)
+		zthr_cancel(condense_thread);
 
 	zthr_t *discard_thread = spa->spa_checkpoint_discard_zthr;
-	if (discard_thread != NULL && zthr_isrunning(discard_thread))
-		VERIFY0(zthr_cancel(discard_thread));
+	if (discard_thread != NULL)
+		zthr_cancel(discard_thread);
 }
 
 void
@@ -7065,11 +7063,11 @@ spa_async_resume(spa_t *spa)
 	spa_restart_removal(spa);
 
 	zthr_t *condense_thread = spa->spa_condense_zthr;
-	if (condense_thread != NULL && !zthr_isrunning(condense_thread))
+	if (condense_thread != NULL)
 		zthr_resume(condense_thread);
 
 	zthr_t *discard_thread = spa->spa_checkpoint_discard_zthr;
-	if (discard_thread != NULL && !zthr_isrunning(discard_thread))
+	if (discard_thread != NULL)
 		zthr_resume(discard_thread);
 }
 

diff --git a/module/zfs/spa_checkpoint.c b/module/zfs/spa_checkpoint.c
@@ -393,7 +393,7 @@ spa_checkpoint_discard_thread_check(void *arg, zthr_t *zthr)
 	return (B_TRUE);
 }
 
-int
+void
 spa_checkpoint_discard_thread(void *arg, zthr_t *zthr)
 {
 	spa_t *spa = arg;
@@ -408,7 +408,7 @@ spa_checkpoint_discard_thread(void *arg, zthr_t *zthr)
 			dmu_buf_t **dbp;
 
 			if (zthr_iscancelled(zthr))
-				return (0);
+				return;
 
 			ASSERT3P(vd->vdev_ops, !=, &vdev_indirect_ops);
 
@@ -445,8 +445,6 @@ spa_checkpoint_discard_thread(void *arg, zthr_t *zthr)
 	VERIFY0(dsl_sync_task(spa->spa_name, NULL,
 	    spa_checkpoint_discard_complete_sync, spa,
 	    0, ZFS_SPACE_CHECK_NONE));
-
-	return (0);
 }
 
 

diff --git a/module/zfs/vdev_indirect.c b/module/zfs/vdev_indirect.c
@@ -647,7 +647,7 @@ spa_condense_indirect_thread_check(void *arg, zthr_t *zthr)
 }
 
 /* ARGSUSED */
-static int
+static void
 spa_condense_indirect_thread(void *arg, zthr_t *zthr)
 {
 	spa_t *spa = arg;
@@ -744,13 +744,11 @@ spa_condense_indirect_thread(void *arg, zthr_t *zthr)
 	 * shutting down.
 	 */
 	if (zthr_iscancelled(zthr))
-		return (0);
+		return;
 
 	VERIFY0(dsl_sync_task(spa_name(spa), NULL,
 	    spa_condense_indirect_complete_sync, sci, 0,
 	    ZFS_SPACE_CHECK_EXTRA_RESERVED));
-
-	return (0);
 }
 
 /*