From 1743c737f5ad6e2c6c429858b7c0f717ecb20954 Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Tue, 14 Jul 2020 15:21:57 -0400 Subject: [PATCH] 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 Reviewed-by: Ryan Moeller Signed-off-by: Alexander Motin Closes #10553 --- module/zfs/spa.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/module/zfs/spa.c b/module/zfs/spa.c index 6b60227d24..2783e9e297 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;