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 <ezomori.nozomu@gmail.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #9256 
Closes #9285
This commit is contained in:
Brian Behlendorf
2019-09-10 13:42:30 -07:00
committed by GitHub
parent 562e1c0327
commit 25f06d677a
8 changed files with 109 additions and 23 deletions
+10 -5
View File
@@ -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);
+1 -1
View File
@@ -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);
+4 -15
View File
@@ -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