From 25f06d677a81a65ca98fa3d725ab5031a4864104 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Tue, 10 Sep 2019 13:42:30 -0700 Subject: Fix /etc/hostid on root pool deadlock Accidentally introduced by dc04a8c which now takes the SCL_VDEV lock as a reader in zfs_blkptr_verify(). A deadlock can occur if the /etc/hostid file resides on a dataset in the same pool. This is because reading the /etc/hostid file may occur while the caller is holding the SCL_VDEV lock as a writer. For example, to perform a `zpool attach` as shown in the abbreviated stack below. To resolve the issue we cache the system's hostid when initializing the spa_t, or when modifying the multihost property. The cached value is then relied upon for subsequent accesses. Call Trace: spa_config_enter+0x1e8/0x350 [zfs] zfs_blkptr_verify+0x33c/0x4f0 [zfs] <--- trying read lock zio_read+0x6c/0x140 [zfs] ... vfs_read+0xfc/0x1e0 kernel_read+0x50/0x90 ... spa_get_hostid+0x1c/0x38 [zfs] spa_config_generate+0x1a0/0x610 [zfs] vdev_label_init+0xa0/0xc80 [zfs] vdev_create+0x98/0xe0 [zfs] spa_vdev_attach+0x14c/0xb40 [zfs] <--- grabbed write lock Reviewed-by: loli10K Signed-off-by: Brian Behlendorf Closes #9256 Closes #9285 --- module/zfs/spa.c | 15 ++++++++++----- module/zfs/spa_config.c | 2 +- module/zfs/spa_misc.c | 19 ++++--------------- 3 files changed, 15 insertions(+), 21 deletions(-) (limited to 'module') diff --git a/module/zfs/spa.c b/module/zfs/spa.c index 0e2cd31b1..56c6dd8cd 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -589,8 +589,13 @@ spa_prop_validate(spa_t *spa, nvlist_t *props) if (!error && intval > 1) error = SET_ERROR(EINVAL); - if (!error && !spa_get_hostid()) - error = SET_ERROR(ENOTSUP); + if (!error) { + uint32_t hostid = zone_get_hostid(NULL); + if (hostid) + spa->spa_hostid = hostid; + else + error = SET_ERROR(ENOTSUP); + } break; @@ -2970,7 +2975,7 @@ spa_activity_check_required(spa_t *spa, uberblock_t *ub, nvlist_t *label, if (nvlist_exists(label, ZPOOL_CONFIG_HOSTID)) hostid = fnvlist_lookup_uint64(label, ZPOOL_CONFIG_HOSTID); - if (hostid == spa_get_hostid()) + if (hostid == spa_get_hostid(spa)) return (B_FALSE); /* @@ -3489,7 +3494,7 @@ spa_ld_select_uberblock(spa_t *spa, spa_import_type_t type) spa->spa_config); if (activity_check) { if (ub->ub_mmp_magic == MMP_MAGIC && ub->ub_mmp_delay && - spa_get_hostid() == 0) { + spa_get_hostid(spa) == 0) { nvlist_free(label); fnvlist_add_uint64(spa->spa_load_info, ZPOOL_CONFIG_MMP_STATE, MMP_STATE_NO_HOSTID); @@ -4176,7 +4181,7 @@ spa_ld_load_vdev_metadata(spa_t *spa) * be imported when the system hostid is zero. The exception to * this rule is zdb which is always allowed to access pools. */ - if (spa_multihost(spa) && spa_get_hostid() == 0 && + if (spa_multihost(spa) && spa_get_hostid(spa) == 0 && (spa->spa_import_flags & ZFS_IMPORT_SKIP_MMP) == 0) { fnvlist_add_uint64(spa->spa_load_info, ZPOOL_CONFIG_MMP_STATE, MMP_STATE_NO_HOSTID); diff --git a/module/zfs/spa_config.c b/module/zfs/spa_config.c index 43da79dc3..de1ad21da 100644 --- a/module/zfs/spa_config.c +++ b/module/zfs/spa_config.c @@ -457,7 +457,7 @@ spa_config_generate(spa_t *spa, vdev_t *vd, uint64_t txg, int getstats) fnvlist_add_string(config, ZPOOL_CONFIG_COMMENT, spa->spa_comment); - hostid = spa_get_hostid(); + hostid = spa_get_hostid(spa); if (hostid != 0) fnvlist_add_uint64(config, ZPOOL_CONFIG_HOSTID, hostid); fnvlist_add_string(config, ZPOOL_CONFIG_HOSTNAME, utsname()->nodename); diff --git a/module/zfs/spa_misc.c b/module/zfs/spa_misc.c index a18f9604a..1ee110b54 100644 --- a/module/zfs/spa_misc.c +++ b/module/zfs/spa_misc.c @@ -668,6 +668,7 @@ spa_add(const char *name, nvlist_t *config, const char *altroot) spa->spa_proc = &p0; spa->spa_proc_state = SPA_PROC_NONE; spa->spa_trust_config = B_TRUE; + spa->spa_hostid = zone_get_hostid(NULL); spa->spa_deadman_synctime = MSEC2NSEC(zfs_deadman_synctime_ms); spa->spa_deadman_ziotime = MSEC2NSEC(zfs_deadman_ziotime_ms); @@ -2560,22 +2561,10 @@ spa_multihost(spa_t *spa) return (spa->spa_multihost ? B_TRUE : B_FALSE); } -unsigned long -spa_get_hostid(void) +uint32_t +spa_get_hostid(spa_t *spa) { - unsigned long myhostid; - -#ifdef _KERNEL - myhostid = zone_get_hostid(NULL); -#else /* _KERNEL */ - /* - * We're emulating the system's hostid in userland, so - * we can't use zone_get_hostid(). - */ - (void) ddi_strtoul(hw_serial, NULL, 10, &myhostid); -#endif /* _KERNEL */ - - return (myhostid); + return (spa->spa_hostid); } boolean_t -- cgit v1.2.3