diff options
| author | Alexander Motin <[email protected]> | 2020-07-14 15:21:57 -0400 |
|---|---|---|
| committer | GitHub <[email protected]> | 2020-07-14 12:21:57 -0700 |
| commit | 1743c737f5ad6e2c6c429858b7c0f717ecb20954 (patch) | |
| tree | 87da399170b38d8197ede250343e509c7f943b2e | |
| parent | 5f72109e5bd84124bd6dd9877fae3fa9ae7ee348 (diff) | |
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 <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Closes #10553
| -rw-r--r-- | module/zfs/spa.c | 18 |
1 files changed, 7 insertions, 11 deletions
diff --git a/module/zfs/spa.c b/module/zfs/spa.c index 6b60227d2..2783e9e29 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -414,12 +414,15 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp) objset_t *mos = spa->spa_meta_objset; zap_cursor_t zc; zap_attribute_t za; + dsl_pool_t *dp; int err; err = nvlist_alloc(nvp, NV_UNIQUE_NAME, KM_SLEEP); if (err) return (err); + dp = spa_get_dsl(spa); + dsl_pool_config_enter(dp, FTAG); 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); /* If no pool property object, no more prop to get. */ - if (mos == NULL || spa->spa_pool_props_object == 0) { - mutex_exit(&spa->spa_props_lock); + if (mos == NULL || spa->spa_pool_props_object == 0) goto out; - } /* * 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; if (prop == ZPOOL_PROP_BOOTFS) { - dsl_pool_t *dp; dsl_dataset_t *ds = NULL; - dp = spa_get_dsl(spa); - dsl_pool_config_enter(dp, FTAG); err = dsl_dataset_hold_obj(dp, za.za_first_integer, FTAG, &ds); - if (err != 0) { - dsl_pool_config_exit(dp, FTAG); + if (err != 0) break; - } strval = kmem_alloc(ZFS_MAX_DATASET_NAME_LEN, KM_SLEEP); dsl_dataset_name(ds, strval); dsl_dataset_rele(ds, FTAG); - dsl_pool_config_exit(dp, FTAG); } else { strval = NULL; intval = za.za_first_integer; @@ -502,8 +497,9 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp) } } zap_cursor_fini(&zc); - mutex_exit(&spa->spa_props_lock); out: + mutex_exit(&spa->spa_props_lock); + dsl_pool_config_exit(dp, FTAG); if (err && err != ENOENT) { nvlist_free(*nvp); *nvp = NULL; |