Skip to content

Commit

Permalink
Re-work management of the auto trim taskq...
Browse files Browse the repository at this point in the history
... and related locking

Don't take the spa_auto_trim_lock in the sync path: The auto trim taskq
is now stopped and started in response to changes in the "autotrim"
property asynchronously by the spa async task.  If the spa_auto-trim_lock
is taken in the sync task, the system can deadlock
as follows:

	Non-sync context task: Acquires spa_auto_trim_lock via
	    spa_vdev_enter() or some other path.
	Sync task: syncing a change to the "autotrim" property attempts
	    to take spa_auto_trim_lock and blocks.
	Non-sync context task: blocks attempting to take a spa config
	    lock which won't be released until the sync task completes.
	Deadlock.

Also, since the auto trim taskq is now started asynchronously, it
may not yet be ready yet when the sync task calls spa_auto_trim().
Modified spa_auto_trim so it will simply return if the taskq is not
available yet (it will do its thing on the next pass).

Also, avoid starting auto trim taskqs for the "import$" pseudo-pool.

Also, don't attempt to create the taskq if a spa unload is in-progress.
  • Loading branch information
dweeezil committed Jan 18, 2019
1 parent 3dc6024 commit 1eda737
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 13 deletions.
2 changes: 2 additions & 0 deletions include/sys/spa.h
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,8 @@ extern int spa_scan_get_stats(spa_t *spa, pool_scan_stat_t *ps);
#define SPA_ASYNC_REMOVE_STOP 0x80
#define SPA_ASYNC_INITIALIZE_RESTART 0x100
#define SPA_ASYNC_MAN_TRIM_TASKQ_DESTROY 0x200
#define SPA_ASYNC_AUTO_TRIM_TASKQ_CREATE 0x400
#define SPA_ASYNC_AUTO_TRIM_TASKQ_DESTROY 0x800

/*
* Controls the behavior of spa_vdev_remove().
Expand Down
42 changes: 29 additions & 13 deletions module/zfs/spa.c
Original file line number Diff line number Diff line change
Expand Up @@ -3541,7 +3541,8 @@ spa_ld_get_props(spa_t *spa)

mutex_enter(&spa->spa_auto_trim_lock);
spa_prop_find(spa, ZPOOL_PROP_AUTOTRIM, &spa->spa_auto_trim);
if (spa->spa_auto_trim == SPA_AUTO_TRIM_ON)
if (spa->spa_auto_trim == SPA_AUTO_TRIM_ON &&
strcmp(spa->spa_name, TRYIMPORT_NAME) != 0)
spa_auto_trim_taskq_create(spa);
mutex_exit(&spa->spa_auto_trim_lock);

Expand Down Expand Up @@ -5306,7 +5307,8 @@ spa_create(const char *pool, nvlist_t *nvroot, nvlist_t *props,

mutex_enter(&spa->spa_auto_trim_lock);
spa->spa_auto_trim = zpool_prop_default_numeric(ZPOOL_PROP_AUTOTRIM);
if (spa->spa_auto_trim == SPA_AUTO_TRIM_ON)
if (spa->spa_auto_trim == SPA_AUTO_TRIM_ON &&
strcmp(spa->spa_name, TRYIMPORT_NAME) != 0)
spa_auto_trim_taskq_create(spa);
mutex_exit(&spa->spa_auto_trim_lock);

Expand Down Expand Up @@ -7250,11 +7252,27 @@ spa_async_thread(void *arg)
mutex_exit(&spa_namespace_lock);
}

/*
* Trim taskq management.
*/
if (tasks & SPA_ASYNC_MAN_TRIM_TASKQ_DESTROY) {
mutex_enter(&spa->spa_man_trim_lock);
spa_man_trim_taskq_destroy(spa);
mutex_exit(&spa->spa_man_trim_lock);
}
if (tasks & SPA_ASYNC_AUTO_TRIM_TASKQ_CREATE) {
mutex_enter(&spa->spa_auto_trim_lock);
if (spa->spa_auto_trim_taskq == NULL)
spa_auto_trim_taskq_create(spa);
mutex_exit(&spa->spa_auto_trim_lock);
}
if (tasks & SPA_ASYNC_AUTO_TRIM_TASKQ_DESTROY) {
mutex_enter(&spa->spa_auto_trim_lock);
if (spa->spa_auto_trim_taskq != NULL)
spa_auto_trim_taskq_destroy(spa);
mutex_exit(&spa->spa_auto_trim_lock);
}


/*
* Let the world know that we're done.
Expand Down Expand Up @@ -7768,16 +7786,9 @@ spa_sync_props(void *arg, dmu_tx_t *tx)
spa->spa_force_trim = intval;
break;
case ZPOOL_PROP_AUTOTRIM:
mutex_enter(&spa->spa_auto_trim_lock);
if (intval != spa->spa_auto_trim) {
spa->spa_auto_trim = intval;
if (intval != 0)
spa_auto_trim_taskq_create(spa);
else
spa_auto_trim_taskq_destroy(
spa);
}
mutex_exit(&spa->spa_auto_trim_lock);
spa_async_request(spa, intval ?
SPA_ASYNC_AUTO_TRIM_TASKQ_CREATE :
SPA_ASYNC_AUTO_TRIM_TASKQ_DESTROY);
break;
case ZPOOL_PROP_AUTOEXPAND:
spa->spa_autoexpand = intval;
Expand Down Expand Up @@ -8509,7 +8520,6 @@ spa_auto_trim(spa_t *spa, uint64_t txg)
{
ASSERT(spa_config_held(spa, SCL_CONFIG, RW_READER) == SCL_CONFIG);
ASSERT(!MUTEX_HELD(&spa->spa_auto_trim_lock));
ASSERT(spa->spa_auto_trim_taskq != NULL);

/*
* Another pool management task might be currently prevented from
Expand All @@ -8519,6 +8529,12 @@ spa_auto_trim(spa_t *spa, uint64_t txg)
if (!mutex_tryenter(&spa->spa_auto_trim_lock))
return;

/* Async start-up of the auto trim taskq may not yet have completed */
if (spa->spa_auto_trim_taskq == NULL) {
mutex_exit(&spa->spa_auto_trim_lock);
return;
}

/* Count the number of auto trim threads which will be launched below */
/* spa->spa_num_auto_trimming += spa->spa_root_vdev->vdev_children; */
for (uint64_t i = 0; i < spa->spa_root_vdev->vdev_children; i++) {
Expand Down
4 changes: 4 additions & 0 deletions module/zfs/spa_misc.c
Original file line number Diff line number Diff line change
Expand Up @@ -2601,6 +2601,10 @@ spa_auto_trim_taskq_create(spa_t *spa)
{
char *name = kmem_alloc(MAXPATHLEN, KM_SLEEP);

/* Don't create the taskq if the pool is unloading */
if (spa->spa_sync_on == B_FALSE)
return;

ASSERT(MUTEX_HELD(&spa->spa_auto_trim_lock));
ASSERT(spa->spa_auto_trim_taskq == NULL);
(void) snprintf(name, MAXPATHLEN, "z_atrim_%s", spa->spa_name);
Expand Down

0 comments on commit 1eda737

Please sign in to comment.