Simplify threads, mutexs, cvs and rwlocks

* Simplify threads, mutexs, cvs and rwlocks

* Update the zk_thread_create() function to use the same trick
  as Illumos.  Specifically, cast the new pthread_t to a void
  pointer and return that as the kthread_t *.  This avoids the
  issues associated with managing a wrapper structure and is
  safe as long as the callers never attempt to dereference it.

* Update all function prototypes passed to pthread_create() to
  match the expected prototype.  We were getting away this with
  before since the function were explicitly cast.

* Replaced direct zk_thread_create() calls with thread_create()
  for code consistency.  All consumers of libzpool now use the
  proper wrappers.

* The mutex_held() calls were converted to MUTEX_HELD().

* Removed all mutex_owner() calls and retired the interface.
  Instead use MUTEX_HELD() which provides the same information
  and allows the implementation details to be hidden.  In this
  case the use of the pthread_equals() function.

* The kthread_t, kmutex_t, krwlock_t, and krwlock_t types had
  any non essential fields removed.  In the case of kthread_t
  and kcondvar_t they could be directly typedef'd to pthread_t
  and pthread_cond_t respectively.

* Removed all extra ASSERTS from the thread, mutex, rwlock, and
  cv wrapper functions.  In practice, pthreads already provides
  the vast majority of checks as long as we check the return
  code.  Removing this code from our wrappers help readability.

* Added TS_JOINABLE state flag to pass to request a joinable rather
  than detached thread.  This isn't a standard thread_create() state
  but it's the least invasive way to pass this information and is
  only used by ztest.

TEST_ZTEST_TIMEOUT=3600

Chunwei Chen <[email protected]>
Reviewed-by: Tom Caputi <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #4547 
Closes #5503 
Closes #5523 
Closes #6377 
Closes #6495 

1 files changed, 59 insertions, 231 deletions

diff --git a/lib/libzpool/kernel.c b/lib/libzpool/kernel.c
index 3fc7337e8..c4b600d03 100644
--- a/lib/libzpool/kernel.c
+++ b/lib/libzpool/kernel.c
@@ -64,104 +64,29 @@ struct proc p0;
  * =========================================================================
  * threads
  * =========================================================================
+ *
+ * TS_STACK_MIN is dictated by the minimum allowed pthread stack size.  While
+ * TS_STACK_MAX is somewhat arbitrary, it was selected to be large enough for
+ * the expected stack depth while small enough to avoid exhausting address
+ * space with high thread counts.
  */
+#define	TS_STACK_MIN	MAX(PTHREAD_STACK_MIN, 32768)
+#define	TS_STACK_MAX	(256 * 1024)
 
-pthread_cond_t kthread_cond = PTHREAD_COND_INITIALIZER;
-pthread_mutex_t kthread_lock = PTHREAD_MUTEX_INITIALIZER;
-pthread_key_t kthread_key;
-int kthread_nr = 0;
-
-static void
-thread_init(void)
-{
-	kthread_t *kt;
-
-	VERIFY3S(pthread_key_create(&kthread_key, NULL), ==, 0);
-
-	/* Create entry for primary kthread */
-	kt = umem_zalloc(sizeof (kthread_t), UMEM_NOFAIL);
-	kt->t_tid = pthread_self();
-	kt->t_func = NULL;
-
-	VERIFY3S(pthread_setspecific(kthread_key, kt), ==, 0);
-
-	/* Only the main thread should be running at the moment */
-	ASSERT3S(kthread_nr, ==, 0);
-	kthread_nr = 1;
-}
-
-static void
-thread_fini(void)
-{
-	kthread_t *kt = curthread;
-
-	ASSERT(pthread_equal(kt->t_tid, pthread_self()));
-	ASSERT3P(kt->t_func, ==, NULL);
-
-	umem_free(kt, sizeof (kthread_t));
-
-	/* Wait for all threads to exit via thread_exit() */
-	VERIFY3S(pthread_mutex_lock(&kthread_lock), ==, 0);
-
-	kthread_nr--; /* Main thread is exiting */
-
-	while (kthread_nr > 0)
-		VERIFY0(pthread_cond_wait(&kthread_cond, &kthread_lock));
-
-	ASSERT3S(kthread_nr, ==, 0);
-	VERIFY3S(pthread_mutex_unlock(&kthread_lock), ==, 0);
-
-	VERIFY3S(pthread_key_delete(kthread_key), ==, 0);
-}
-
-kthread_t *
-zk_thread_current(void)
-{
-	kthread_t *kt = pthread_getspecific(kthread_key);
-
-	ASSERT3P(kt, !=, NULL);
-
-	return (kt);
-}
-
-void *
-zk_thread_helper(void *arg)
-{
-	kthread_t *kt = (kthread_t *)arg;
-
-	VERIFY3S(pthread_setspecific(kthread_key, kt), ==, 0);
-
-	VERIFY3S(pthread_mutex_lock(&kthread_lock), ==, 0);
-	kthread_nr++;
-	VERIFY3S(pthread_mutex_unlock(&kthread_lock), ==, 0);
-	(void) setpriority(PRIO_PROCESS, 0, kt->t_pri);
-
-	kt->t_tid = pthread_self();
-	((thread_func_arg_t)kt->t_func)(kt->t_arg);
-
-	/* Unreachable, thread must exit with thread_exit() */
-	abort();
-
-	return (NULL);
-}
-
+/*ARGSUSED*/
 kthread_t *
-zk_thread_create(caddr_t stk, size_t stksize, thread_func_t func, void *arg,
-    uint64_t len, proc_t *pp, int state, pri_t pri, int detachstate)
+zk_thread_create(void (*func)(void *), void *arg, size_t stksize, int state)
 {
-	kthread_t *kt;
 	pthread_attr_t attr;
+	pthread_t tid;
 	char *stkstr;
+	int detachstate = PTHREAD_CREATE_DETACHED;
 
-	ASSERT0(state & ~TS_RUN);
-	ASSERT0(len);
+	VERIFY0(pthread_attr_init(&attr));
 
-	kt = umem_zalloc(sizeof (kthread_t), UMEM_NOFAIL);
-	kt->t_func = func;
-	kt->t_arg = arg;
-	kt->t_pri = pri;
+	if (state & TS_JOINABLE)
+		detachstate = PTHREAD_CREATE_JOINABLE;
 
-	VERIFY0(pthread_attr_init(&attr));
 	VERIFY0(pthread_attr_setdetachstate(&attr, detachstate));
 
 	/*
@@ -183,6 +108,7 @@ zk_thread_create(caddr_t stk, size_t stksize, thread_func_t func, void *arg,
 
 	VERIFY3S(stksize, >, 0);
 	stksize = P2ROUNDUP(MAX(stksize, TS_STACK_MIN), PAGESIZE);
+
 	/*
 	 * If this ever fails, it may be because the stack size is not a
 	 * multiple of system page size.
@@ -190,36 +116,10 @@ zk_thread_create(caddr_t stk, size_t stksize, thread_func_t func, void *arg,
 	VERIFY0(pthread_attr_setstacksize(&attr, stksize));
 	VERIFY0(pthread_attr_setguardsize(&attr, PAGESIZE));
 
-	VERIFY0(pthread_create(&kt->t_tid, &attr, &zk_thread_helper, kt));
+	VERIFY0(pthread_create(&tid, &attr, (void *(*)(void *))func, arg));
 	VERIFY0(pthread_attr_destroy(&attr));
 
-	return (kt);
-}
-
-void
-zk_thread_exit(void)
-{
-	kthread_t *kt = curthread;
-
-	ASSERT(pthread_equal(kt->t_tid, pthread_self()));
-
-	umem_free(kt, sizeof (kthread_t));
-
-	VERIFY0(pthread_mutex_lock(&kthread_lock));
-	kthread_nr--;
-	VERIFY0(pthread_mutex_unlock(&kthread_lock));
-
-	VERIFY0(pthread_cond_broadcast(&kthread_cond));
-	pthread_exit((void *)TS_MAGIC);
-}
-
-void
-zk_thread_join(kt_did_t tid)
-{
-	void *ret;
-
-	pthread_join((pthread_t)tid, &ret);
-	VERIFY3P(ret, ==, (void *)TS_MAGIC);
+	return ((void *)(uintptr_t)tid);
 }
 
 /*
@@ -291,46 +191,34 @@ kstat_set_raw_ops(kstat_t *ksp,
 void
 mutex_init(kmutex_t *mp, char *name, int type, void *cookie)
 {
-	ASSERT3S(type, ==, MUTEX_DEFAULT);
-	ASSERT3P(cookie, ==, NULL);
-	mp->m_owner = MTX_INIT;
-	mp->m_magic = MTX_MAGIC;
-	VERIFY3S(pthread_mutex_init(&mp->m_lock, NULL), ==, 0);
+	VERIFY0(pthread_mutex_init(&mp->m_lock, NULL));
+	memset(&mp->m_owner, 0, sizeof (pthread_t));
 }
 
 void
 mutex_destroy(kmutex_t *mp)
 {
-	ASSERT3U(mp->m_magic, ==, MTX_MAGIC);
-	ASSERT3P(mp->m_owner, ==, MTX_INIT);
-	ASSERT0(pthread_mutex_destroy(&(mp)->m_lock));
-	mp->m_owner = MTX_DEST;
-	mp->m_magic = 0;
+	VERIFY0(pthread_mutex_destroy(&mp->m_lock));
 }
 
 void
 mutex_enter(kmutex_t *mp)
 {
-	ASSERT3U(mp->m_magic, ==, MTX_MAGIC);
-	ASSERT3P(mp->m_owner, !=, MTX_DEST);
-	ASSERT3P(mp->m_owner, !=, curthread);
-	VERIFY3S(pthread_mutex_lock(&mp->m_lock), ==, 0);
-	ASSERT3P(mp->m_owner, ==, MTX_INIT);
-	mp->m_owner = curthread;
+	VERIFY0(pthread_mutex_lock(&mp->m_lock));
+	mp->m_owner = pthread_self();
 }
 
 int
 mutex_tryenter(kmutex_t *mp)
 {
-	int err;
-	ASSERT3U(mp->m_magic, ==, MTX_MAGIC);
-	ASSERT3P(mp->m_owner, !=, MTX_DEST);
-	if (0 == (err = pthread_mutex_trylock(&mp->m_lock))) {
-		ASSERT3P(mp->m_owner, ==, MTX_INIT);
-		mp->m_owner = curthread;
+	int error;
+
+	error = pthread_mutex_trylock(&mp->m_lock);
+	if (error == 0) {
+		mp->m_owner = pthread_self();
 		return (1);
 	} else {
-		VERIFY3S(err, ==, EBUSY);
+		VERIFY3S(error, ==, EBUSY);
 		return (0);
 	}
 }
@@ -338,23 +226,8 @@ mutex_tryenter(kmutex_t *mp)
 void
 mutex_exit(kmutex_t *mp)
 {
-	ASSERT3U(mp->m_magic, ==, MTX_MAGIC);
-	ASSERT3P(mutex_owner(mp), ==, curthread);
-	mp->m_owner = MTX_INIT;
-	VERIFY3S(pthread_mutex_unlock(&mp->m_lock), ==, 0);
-}
-
-void *
-mutex_owner(kmutex_t *mp)
-{
-	ASSERT3U(mp->m_magic, ==, MTX_MAGIC);
-	return (mp->m_owner);
-}
-
-int
-mutex_held(kmutex_t *mp)
-{
-	return (mp->m_owner == curthread);
+	memset(&mp->m_owner, 0, sizeof (pthread_t));
+	VERIFY0(pthread_mutex_unlock(&mp->m_lock));
 }
 
 /*
@@ -366,89 +239,60 @@ mutex_held(kmutex_t *mp)
 void
 rw_init(krwlock_t *rwlp, char *name, int type, void *arg)
 {
-	ASSERT3S(type, ==, RW_DEFAULT);
-	ASSERT3P(arg, ==, NULL);
-	VERIFY3S(pthread_rwlock_init(&rwlp->rw_lock, NULL), ==, 0);
-	rwlp->rw_owner = RW_INIT;
-	rwlp->rw_wr_owner = RW_INIT;
+	VERIFY0(pthread_rwlock_init(&rwlp->rw_lock, NULL));
 	rwlp->rw_readers = 0;
-	rwlp->rw_magic = RW_MAGIC;
+	rwlp->rw_owner = 0;
 }
 
 void
 rw_destroy(krwlock_t *rwlp)
 {
-	ASSERT3U(rwlp->rw_magic, ==, RW_MAGIC);
-	ASSERT(rwlp->rw_readers == 0 && rwlp->rw_wr_owner == RW_INIT);
-	VERIFY3S(pthread_rwlock_destroy(&rwlp->rw_lock), ==, 0);
-	rwlp->rw_magic = 0;
+	VERIFY0(pthread_rwlock_destroy(&rwlp->rw_lock));
 }
 
 void
 rw_enter(krwlock_t *rwlp, krw_t rw)
 {
-	ASSERT3U(rwlp->rw_magic, ==, RW_MAGIC);
-	ASSERT3P(rwlp->rw_owner, !=, curthread);
-	ASSERT3P(rwlp->rw_wr_owner, !=, curthread);
-
 	if (rw == RW_READER) {
-		VERIFY3S(pthread_rwlock_rdlock(&rwlp->rw_lock), ==, 0);
-		ASSERT3P(rwlp->rw_wr_owner, ==, RW_INIT);
-
+		VERIFY0(pthread_rwlock_rdlock(&rwlp->rw_lock));
 		atomic_inc_uint(&rwlp->rw_readers);
 	} else {
-		VERIFY3S(pthread_rwlock_wrlock(&rwlp->rw_lock), ==, 0);
-		ASSERT3P(rwlp->rw_wr_owner, ==, RW_INIT);
-		ASSERT3U(rwlp->rw_readers, ==, 0);
-
-		rwlp->rw_wr_owner = curthread;
+		VERIFY0(pthread_rwlock_wrlock(&rwlp->rw_lock));
+		rwlp->rw_owner = pthread_self();
 	}
-
-	rwlp->rw_owner = curthread;
 }
 
 void
 rw_exit(krwlock_t *rwlp)
 {
-	ASSERT3U(rwlp->rw_magic, ==, RW_MAGIC);
-	ASSERT(RW_LOCK_HELD(rwlp));
-
 	if (RW_READ_HELD(rwlp))
 		atomic_dec_uint(&rwlp->rw_readers);
 	else
-		rwlp->rw_wr_owner = RW_INIT;
+		rwlp->rw_owner = 0;
 
-	rwlp->rw_owner = RW_INIT;
-	VERIFY3S(pthread_rwlock_unlock(&rwlp->rw_lock), ==, 0);
+	VERIFY0(pthread_rwlock_unlock(&rwlp->rw_lock));
 }
 
 int
 rw_tryenter(krwlock_t *rwlp, krw_t rw)
 {
-	int rv;
-
-	ASSERT3U(rwlp->rw_magic, ==, RW_MAGIC);
+	int error;
 
 	if (rw == RW_READER)
-		rv = pthread_rwlock_tryrdlock(&rwlp->rw_lock);
+		error = pthread_rwlock_tryrdlock(&rwlp->rw_lock);
 	else
-		rv = pthread_rwlock_trywrlock(&rwlp->rw_lock);
-
-	if (rv == 0) {
-		ASSERT3P(rwlp->rw_wr_owner, ==, RW_INIT);
+		error = pthread_rwlock_trywrlock(&rwlp->rw_lock);
 
+	if (error == 0) {
 		if (rw == RW_READER)
 			atomic_inc_uint(&rwlp->rw_readers);
-		else {
-			ASSERT3U(rwlp->rw_readers, ==, 0);
-			rwlp->rw_wr_owner = curthread;
-		}
+		else
+			rwlp->rw_owner = pthread_self();
 
-		rwlp->rw_owner = curthread;
 		return (1);
 	}
 
-	VERIFY3S(rv, ==, EBUSY);
+	VERIFY3S(error, ==, EBUSY);
 
 	return (0);
 }
@@ -456,8 +300,6 @@ rw_tryenter(krwlock_t *rwlp, krw_t rw)
 int
 rw_tryupgrade(krwlock_t *rwlp)
 {
-	ASSERT3U(rwlp->rw_magic, ==, RW_MAGIC);
-
 	return (0);
 }
 
@@ -470,27 +312,21 @@ rw_tryupgrade(krwlock_t *rwlp)
 void
 cv_init(kcondvar_t *cv, char *name, int type, void *arg)
 {
-	ASSERT3S(type, ==, CV_DEFAULT);
-	cv->cv_magic = CV_MAGIC;
-	VERIFY0(pthread_cond_init(&cv->cv, NULL));
+	VERIFY0(pthread_cond_init(cv, NULL));
 }
 
 void
 cv_destroy(kcondvar_t *cv)
 {
-	ASSERT3U(cv->cv_magic, ==, CV_MAGIC);
-	VERIFY0(pthread_cond_destroy(&cv->cv));
-	cv->cv_magic = 0;
+	VERIFY0(pthread_cond_destroy(cv));
 }
 
 void
 cv_wait(kcondvar_t *cv, kmutex_t *mp)
 {
-	ASSERT3U(cv->cv_magic, ==, CV_MAGIC);
-	ASSERT3P(mutex_owner(mp), ==, curthread);
-	mp->m_owner = MTX_INIT;
-	VERIFY0(pthread_cond_wait(&cv->cv, &mp->m_lock));
-	mp->m_owner = curthread;
+	memset(&mp->m_owner, 0, sizeof (pthread_t));
+	VERIFY0(pthread_cond_wait(cv, &mp->m_lock));
+	mp->m_owner = pthread_self();
 }
 
 clock_t
@@ -501,8 +337,6 @@ cv_timedwait(kcondvar_t *cv, kmutex_t *mp, clock_t abstime)
 	timestruc_t ts;
 	clock_t delta;
 
-	ASSERT3U(cv->cv_magic, ==, CV_MAGIC);
-
 	delta = abstime - ddi_get_lbolt();
 	if (delta <= 0)
 		return (-1);
@@ -516,10 +350,9 @@ cv_timedwait(kcondvar_t *cv, kmutex_t *mp, clock_t abstime)
 		ts.tv_nsec -= NANOSEC;
 	}
 
-	ASSERT3P(mutex_owner(mp), ==, curthread);
-	mp->m_owner = MTX_INIT;
-	error = pthread_cond_timedwait(&cv->cv, &mp->m_lock, &ts);
-	mp->m_owner = curthread;
+	memset(&mp->m_owner, 0, sizeof (pthread_t));
+	error = pthread_cond_timedwait(cv, &mp->m_lock, &ts);
+	mp->m_owner = pthread_self();
 
 	if (error == ETIMEDOUT)
 		return (-1);
@@ -548,7 +381,7 @@ cv_timedwait_hires(kcondvar_t *cv, kmutex_t *mp, hrtime_t tim, hrtime_t res,
 	if (delta <= 0)
 		return (-1);
 
-	VERIFY(gettimeofday(&tv, NULL) == 0);
+	VERIFY0(gettimeofday(&tv, NULL));
 
 	ts.tv_sec = tv.tv_sec + delta / NANOSEC;
 	ts.tv_nsec = tv.tv_usec * NSEC_PER_USEC + (delta % NANOSEC);
@@ -557,10 +390,9 @@ cv_timedwait_hires(kcondvar_t *cv, kmutex_t *mp, hrtime_t tim, hrtime_t res,
 		ts.tv_nsec -= NANOSEC;
 	}
 
-	ASSERT(mutex_owner(mp) == curthread);
-	mp->m_owner = MTX_INIT;
-	error = pthread_cond_timedwait(&cv->cv, &mp->m_lock, &ts);
-	mp->m_owner = curthread;
+	memset(&mp->m_owner, 0, sizeof (pthread_t));
+	error = pthread_cond_timedwait(cv, &mp->m_lock, &ts);
+	mp->m_owner = pthread_self();
 
 	if (error == ETIMEDOUT)
 		return (-1);
@@ -573,15 +405,13 @@ cv_timedwait_hires(kcondvar_t *cv, kmutex_t *mp, hrtime_t tim, hrtime_t res,
 void
 cv_signal(kcondvar_t *cv)
 {
-	ASSERT3U(cv->cv_magic, ==, CV_MAGIC);
-	VERIFY0(pthread_cond_signal(&cv->cv));
+	VERIFY0(pthread_cond_signal(cv));
 }
 
 void
 cv_broadcast(kcondvar_t *cv)
 {
-	ASSERT3U(cv->cv_magic, ==, CV_MAGIC);
-	VERIFY0(pthread_cond_broadcast(&cv->cv));
+	VERIFY0(pthread_cond_broadcast(cv));
 }
 
 /*
@@ -1188,7 +1018,6 @@ kernel_init(int mode)
 
 	VERIFY0(uname(&hw_utsname));
 
-	thread_init();
 	system_taskq_init();
 	icp_init();
 
@@ -1207,7 +1036,6 @@ kernel_fini(void)
 
 	icp_fini();
 	system_taskq_fini();
-	thread_fini();
 
 	random_fini();
 }