Fix LOR between dp_config_rwlock and spa_props_lock

Our QE team during automated API testing hit deadlock in ZFS, caused
by lock order reversal.  From one side dsl_sync_task_sync() locks
dp_config_rwlock as writer and calls spa_sync_props(), which waits
for spa_props_lock.  From another spa_prop_get() locks spa_props_lock
and then calls dsl_pool_config_enter(), trying to lock dp_config_rwlock
as reader.

This patch makes spa_prop_get() lock dp_config_rwlock before
spa_props_lock, making the order consistent.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Ryan Moeller <ryan@ixsystems.com>
Signed-off-by: Alexander Motin <mav@FreeBSD.org>
Closes #10553
This commit is contained in:
Alexander Motin 2020-07-14 15:21:57 -04:00 committed by GitHub
parent 5f72109e5b
commit 1743c737f5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 7 additions and 11 deletions

View File

@ -414,12 +414,15 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
objset_t *mos = spa->spa_meta_objset; objset_t *mos = spa->spa_meta_objset;
zap_cursor_t zc; zap_cursor_t zc;
zap_attribute_t za; zap_attribute_t za;
dsl_pool_t *dp;
int err; int err;
err = nvlist_alloc(nvp, NV_UNIQUE_NAME, KM_SLEEP); err = nvlist_alloc(nvp, NV_UNIQUE_NAME, KM_SLEEP);
if (err) if (err)
return (err); return (err);
dp = spa_get_dsl(spa);
dsl_pool_config_enter(dp, FTAG);
mutex_enter(&spa->spa_props_lock); mutex_enter(&spa->spa_props_lock);
/* /*
@ -428,10 +431,8 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
spa_prop_get_config(spa, nvp); spa_prop_get_config(spa, nvp);
/* If no pool property object, no more prop to get. */ /* If no pool property object, no more prop to get. */
if (mos == NULL || spa->spa_pool_props_object == 0) { if (mos == NULL || spa->spa_pool_props_object == 0)
mutex_exit(&spa->spa_props_lock);
goto out; goto out;
}
/* /*
* Get properties from the MOS pool property object. * Get properties from the MOS pool property object.
@ -455,23 +456,17 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
src = ZPROP_SRC_LOCAL; src = ZPROP_SRC_LOCAL;
if (prop == ZPOOL_PROP_BOOTFS) { if (prop == ZPOOL_PROP_BOOTFS) {
dsl_pool_t *dp;
dsl_dataset_t *ds = NULL; dsl_dataset_t *ds = NULL;
dp = spa_get_dsl(spa);
dsl_pool_config_enter(dp, FTAG);
err = dsl_dataset_hold_obj(dp, err = dsl_dataset_hold_obj(dp,
za.za_first_integer, FTAG, &ds); za.za_first_integer, FTAG, &ds);
if (err != 0) { if (err != 0)
dsl_pool_config_exit(dp, FTAG);
break; break;
}
strval = kmem_alloc(ZFS_MAX_DATASET_NAME_LEN, strval = kmem_alloc(ZFS_MAX_DATASET_NAME_LEN,
KM_SLEEP); KM_SLEEP);
dsl_dataset_name(ds, strval); dsl_dataset_name(ds, strval);
dsl_dataset_rele(ds, FTAG); dsl_dataset_rele(ds, FTAG);
dsl_pool_config_exit(dp, FTAG);
} else { } else {
strval = NULL; strval = NULL;
intval = za.za_first_integer; intval = za.za_first_integer;
@ -502,8 +497,9 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
} }
} }
zap_cursor_fini(&zc); zap_cursor_fini(&zc);
mutex_exit(&spa->spa_props_lock);
out: out:
mutex_exit(&spa->spa_props_lock);
dsl_pool_config_exit(dp, FTAG);
if (err && err != ENOENT) { if (err && err != ENOENT) {
nvlist_free(*nvp); nvlist_free(*nvp);
*nvp = NULL; *nvp = NULL;