race condition between spa async threads and export

[sdimitro]

@shartse recently hit a ztest failure where the deadman timed out. The problem was a race condition between multiple threads doing attempting to export the pool and hitting a deadlock suspending and resuming the SPA's async thread and zthrs. The specific scenario from Sara's core is the following:

There are 3 threads (A,B,C) that just entered spa_export_common() and one zthr (thread D - which in Sara's case was the livelist zthr but could just as easily have been a different one).

Timeline:

• Threads A, B, and C started racing in spa_async_suspend().
• Thread A suspended all async threads and grabbed the spa_namespace_lock.
• Thread B had a no-op pretty much in spa_async_suspend() and got stuck waiting for Thread A to drop the spa_namespace_lock in spa_export_common().
• Thread A seeing that there are still references to the pool, executes one of the error code paths and resumes all the async threads through spa_async_resume(), dropping the spa_namespace_lock and exiting.
• Thread B then grabs the spa_namespace_lock, hits the same error and is about the enter spa_async_resume().
• Thread C enters spa_async_suspend() calling zthr_cancel() in one of the zthrs, grabbing the zthr_request_lock and later wait on cv_wait() for zthr_cv to be broadcasted.
• Thread D (the zthr), the thread that is supposed to broadcast zthr_cv, is stuck in dsl_sync_task()/spa_open_common() waiting for the spa_namespace_lock to be dropped by thread B.
• Thread B spa_async_resume() which calls zthr_resume() for thread D, at which point it gets stuck waiting to grab the zthr_request_lock mutex that is held by thread C in zthr_cancel.

Resulting Deadlock:
Thread B holds the spa_namespace_lock and is stuck waiting for the zthr_request_lock.
Thread C holds the zthr_request_lock and is stuck waiting for zthr_cv to receive a signal.
Thread D is stuck waiting for the spa_namespace_lock and it cannot signal zthr_cv.

Relevant Code paths:

spa_export_common -
... <cropped>...
	/*
	 * Put a hold on the pool, drop the namespace lock, stop async tasks,
	 * reacquire the namespace lock, and see if we can export.
	 */
	spa_open_ref(spa, FTAG);
	mutex_exit(&spa_namespace_lock);
	spa_async_suspend(spa);
... <cropped> ...
	mutex_enter(&spa_namespace_lock);
... <cropped> ...
	/*
	 * A pool cannot be exported or destroyed if there are active
	 * references.  If we are resetting a pool, allow references by
	 * fault injection handlers.
	 */
	if (!spa_refcount_zero(spa) ||
	    (spa->spa_inject_ref != 0 &&
	    new_state != POOL_STATE_UNINITIALIZED)) {
		spa_async_resume(spa);
		mutex_exit(&spa_namespace_lock);
		return (SET_ERROR(EBUSY));
	}
... <cropped> ...

zthr_cancel -
	mutex_enter(&t->zthr_request_lock);
	mutex_enter(&t->zthr_state_lock);
... <cropped> ...
	if (t->zthr_thread != NULL) {
... <cropped> ...
		while (t->zthr_thread != NULL)
			cv_wait(&t->zthr_cv, &t->zthr_state_lock);
... <cropped> ...
	}
	mutex_exit(&t->zthr_state_lock);
	mutex_exit(&t->zthr_request_lock);

zthr_resume -
	mutex_enter(&t->zthr_request_lock);
	mutex_enter(&t->zthr_state_lock);
... <cropped> ...
	if (t->zthr_thread == NULL) {
		t->zthr_thread = thread_create(NULL, 0, zthr_procedure, t,
		    0, &p0, TS_RUN, minclsyspri);
	}
	mutex_exit(&t->zthr_state_lock);
	mutex_exit(&t->zthr_request_lock);
}

Even though in Sara's scenario it was a livelist open-context thread, since this has not been upstreamed yet, I have the snippet of another zthr that could just as easily have caused this.

spa_condense_indirect_thread -
... <cropped> ...
	VERIFY0(dsl_sync_task(spa_name(spa), NULL,
	    spa_condense_indirect_complete_sync, sci, 0,
	    ZFS_SPACE_CHECK_EXTRA_RESERVED));

There are a couple of potential solutions here. After discussing it with Matt, we figured that serializing most of spa_export_common() by bailing early if there are other threads in that codepath may be the best solution in making export more stable overall.