]> granicus.if.org Git - zfs/commitdiff
Fix /etc/hostid on root pool deadlock
authorBrian Behlendorf <behlendorf1@llnl.gov>
Tue, 10 Sep 2019 20:42:30 +0000 (13:42 -0700)
committerGitHub <noreply@github.com>
Tue, 10 Sep 2019 20:42:30 +0000 (13:42 -0700)
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

include/sys/spa.h
include/sys/spa_impl.h
module/zfs/spa.c
module/zfs/spa_config.c
module/zfs/spa_misc.c
tests/runfiles/linux.run
tests/zfs-tests/tests/functional/mmp/Makefile.am
tests/zfs-tests/tests/functional/mmp/mmp_hostid.ksh [new file with mode: 0755]

index 49432684346ed5e1d9e1b5b502fb9f5ed156d6a6..8323662f604cdb11a4bbbe174a6085470e1a4239 100644 (file)
@@ -1136,7 +1136,7 @@ extern void spa_set_missing_tvds(spa_t *spa, uint64_t missing);
 extern boolean_t spa_top_vdevs_spacemap_addressable(spa_t *spa);
 extern uint64_t spa_total_metaslabs(spa_t *spa);
 extern boolean_t spa_multihost(spa_t *spa);
-extern unsigned long spa_get_hostid(void);
+extern uint32_t spa_get_hostid(spa_t *spa);
 extern void spa_activate_allocation_classes(spa_t *, dmu_tx_t *);
 extern boolean_t spa_livelist_delete_check(spa_t *spa);
 
index 503600c8c3e1901536643ea342cb105832ec8e6f..71b07405cd58988066356c8e7aac4f32c87f6d6e 100644 (file)
@@ -411,6 +411,7 @@ struct spa {
        mmp_thread_t    spa_mmp;                /* multihost mmp thread */
        list_t          spa_leaf_list;          /* list of leaf vdevs */
        uint64_t        spa_leaf_list_gen;      /* track leaf_list changes */
+       uint32_t        spa_hostid;             /* cached system hostid */
 
        /*
         * spa_refcount & spa_config_lock must be the last elements
index 0e2cd31b12051129d9caaf6ac6de06595de5ad24..56c6dd8cd19c3e8cb5e13d9cf57cfa7b4c6a296b 100644 (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);
index 43da79dc3f1382242748b7875e9c67ef25566754..de1ad21da6dea2cd77711807766035b76c9a9064 100644 (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);
index a18f9604ac2fb04f5b251aa0949df3c45be7391f..1ee110b54a9ef027bf47ce3037c1737ae43410f9 100644 (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
index d05ff61202abfe16d9a61fe36feef6e60e19e412..182e6137aae14228616e879511494876a08501f5 100644 (file)
@@ -657,7 +657,7 @@ tags = ['functional', 'mmap']
 tests = ['mmp_on_thread', 'mmp_on_uberblocks', 'mmp_on_off', 'mmp_interval',
     'mmp_active_import', 'mmp_inactive_import', 'mmp_exported_import',
     'mmp_write_uberblocks', 'mmp_reset_interval', 'multihost_history',
-    'mmp_on_zdb', 'mmp_write_distribution']
+    'mmp_on_zdb', 'mmp_write_distribution', 'mmp_hostid']
 tags = ['functional', 'mmp']
 
 [tests/functional/mount]
index e39a0a5aac8ea0ad8f7bab9791f2c796ab73df5a..2848fd4ce6925c50e570180ecdae46bdedced9d0 100644 (file)
@@ -12,6 +12,7 @@ dist_pkgdata_SCRIPTS = \
        mmp_reset_interval.ksh \
        mmp_on_zdb.ksh \
        mmp_write_distribution.ksh \
+       mmp_hostid.ksh \
        setup.ksh \
        cleanup.ksh
 
diff --git a/tests/zfs-tests/tests/functional/mmp/mmp_hostid.ksh b/tests/zfs-tests/tests/functional/mmp/mmp_hostid.ksh
new file mode 100755 (executable)
index 0000000..b492b10
--- /dev/null
@@ -0,0 +1,90 @@
+#!/bin/ksh -p
+#
+# CDDL HEADER START
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright (c) 2019 by Lawrence Livermore National Security, LLC.
+#
+
+# DESCRIPTION:
+#      Verify the hostid file can reside on a ZFS dataset.
+#
+# STRATEGY:
+#      1. Create a non-redundant pool
+#      2. Create an 'etc' dataset containing a valid hostid file
+#      3. Create a file so the pool will have some contents
+#      4. Verify multihost cannot be enabled until the /etc/hostid is linked
+#      5. Verify vdevs may be attached and detached
+#      6. Verify normal, cache, log and special vdevs can be added
+#      7. Verify normal, cache, and log vdevs can be removed
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/mmp/mmp.cfg
+. $STF_SUITE/tests/functional/mmp/mmp.kshlib
+
+verify_runnable "both"
+
+function cleanup
+{
+       default_cleanup_noexit
+       log_must rm $MMP_DIR/file.{0,1,2,3,4,5}
+       log_must rmdir $MMP_DIR
+       log_must mmp_clear_hostid
+}
+
+log_assert "Verify hostid file can reside on a ZFS dataset"
+log_onexit cleanup
+
+log_must mkdir -p $MMP_DIR
+log_must truncate -s $MINVDEVSIZE $MMP_DIR/file.{0,1,2,3,4,5}
+
+# 1. Create a non-redundant pool
+log_must zpool create $MMP_POOL $MMP_DIR/file.0
+
+# 2. Create an 'etc' dataset containing a valid hostid file; caching is
+#    disabled on the dataset to force the hostid to be read from disk.
+log_must zfs create -o primarycache=none -o secondarycache=none $MMP_POOL/etc
+mntpnt_etc=$(get_prop mountpoint $MMP_POOL/etc)
+log_must mmp_set_hostid $HOSTID1
+log_must mv $HOSTID_FILE $mntpnt_etc/hostid
+
+# 3. Create a file so the pool will have some contents
+log_must zfs create $MMP_POOL/fs
+mntpnt_fs=$(get_prop mountpoint $MMP_POOL/fs)
+log_must mkfile 1M $fs_mntpnt/file
+
+# 4. Verify multihost cannot be enabled until the /etc/hostid is linked
+log_mustnot zpool set multihost=on $MMP_POOL
+log_must ln -s $mntpnt_etc/hostid $HOSTID_FILE
+log_must zpool set multihost=on $MMP_POOL
+
+# 5. Verify vdevs may be attached and detached
+log_must zpool attach $MMP_POOL $MMP_DIR/file.0 $MMP_DIR/file.1
+log_must zpool detach $MMP_POOL $MMP_DIR/file.1
+
+# 6. Verify normal, cache, log and special vdevs can be added
+log_must zpool add $MMP_POOL $MMP_DIR/file.1
+log_must zpool add $MMP_POOL $MMP_DIR/file.2
+log_must zpool add $MMP_POOL cache $MMP_DIR/file.3
+log_must zpool add $MMP_POOL log $MMP_DIR/file.4
+log_must zpool add $MMP_POOL special $MMP_DIR/file.5
+
+# 7. Verify normal, cache, and log vdevs can be removed
+log_must zpool remove $MMP_POOL $MMP_DIR/file.2
+log_must zpool remove $MMP_POOL $MMP_DIR/file.3
+log_must zpool remove $MMP_POOL $MMP_DIR/file.4
+
+log_pass "Verify hostid file can reside on a ZFS dataset."