Release SCL_STATE in map_write_done()

The config lock must be held for the duration of the MMP write.
Since the I/Os are executed via map_nowait(), the done function
is the only place where we know the write has completed.

Since SCL_STATE is taken as reader, overlapping I/Os do not
create a deadlock.  The refcount is simply increased when new
I/Os are queued and decreased when I/Os complete.

Test case added which exercises the probe IO call path to
verify the fix and prevent a regression.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Closes #6394

diff --git a/module/zfs/mmp.c b/module/zfs/mmp.c
index ae5478a3313102c0de6258450dba3bdc168a5d77..539b76826a865f256b56bef19d7c4abdcee602c3 100644 (file)
--- a/module/zfs/mmp.c
+++ b/module/zfs/mmp.c
@@ -287,6 +287,7 @@ mmp_write_done(zio_t *zio)
 
 unlock:
        mutex_exit(&mts->mmp_io_lock);
+       spa_config_exit(spa, SCL_STATE, FTAG);
 
        abd_free(zio->io_abd);
 }
@@ -322,9 +323,12 @@ mmp_write_uberblock(spa_t *spa)
        int label;
        uint64_t offset;
 
+       spa_config_enter(spa, SCL_STATE, FTAG, RW_READER);
        vd = vdev_random_leaf(spa);
-       if (vd == NULL || !vdev_writeable(vd))
+       if (vd == NULL || !vdev_writeable(vd)) {
+               spa_config_exit(spa, SCL_STATE, FTAG);
                return;
+       }
 
        mutex_enter(&mmp->mmp_io_lock);
 
@@ -437,11 +441,8 @@ mmp_thread(spa_t *spa)
                        zio_suspend(spa, NULL);
                }
 
-               if (multihost) {
-                       spa_config_enter(spa, SCL_STATE, FTAG, RW_READER);
+               if (multihost)
                        mmp_write_uberblock(spa);
-                       spa_config_exit(spa, SCL_STATE, FTAG);
-               }
 
                CALLB_CPR_SAFE_BEGIN(&cpr);
                (void) cv_timedwait_sig(&mmp->mmp_thread_cv,

diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run
index e556f1e8cc1dd45bd474682417d1a9c60350e428..842f6dd0c3b9976fb5c3d0a896f13ee108264e58 100644 (file)
--- a/tests/runfiles/linux.run
+++ b/tests/runfiles/linux.run
@@ -412,7 +412,8 @@ tests = ['mmap_write_001_pos', 'mmap_read_001_pos']
 
 [tests/functional/mmp]
 tests = ['mmp_on_thread', 'mmp_on_uberblocks', 'mmp_on_off', 'mmp_interval',
-    'mmp_active_import', 'mmp_inactive_import', 'mmp_exported_import']
+    'mmp_active_import', 'mmp_inactive_import', 'mmp_exported_import',
+    'mmp_write_uberblocks']
 
 [tests/functional/mount]
 tests = ['umount_001', 'umountall_001']

diff --git a/tests/zfs-tests/tests/functional/mmp/Makefile.am b/tests/zfs-tests/tests/functional/mmp/Makefile.am
index 2399a6c377d2174c7146d316680c5d361631cd99..75af0cdb71485b9b583614b4a4d3cba0c666f1d9 100644 (file)
--- a/tests/zfs-tests/tests/functional/mmp/Makefile.am
+++ b/tests/zfs-tests/tests/functional/mmp/Makefile.am
@@ -7,6 +7,7 @@ dist_pkgdata_SCRIPTS = \
        mmp_active_import.ksh \
        mmp_inactive_import.ksh \
        mmp_exported_import.ksh \
+       mmp_write_uberblocks.ksh \
        setup.ksh \
        cleanup.ksh \
        mmp.kshlib \

diff --git a/tests/zfs-tests/tests/functional/mmp/mmp_write_uberblocks.ksh b/tests/zfs-tests/tests/functional/mmp/mmp_write_uberblocks.ksh
new file mode 100755 (executable)
index 0000000..4a13dfa
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/mmp/mmp_write_uberblocks.ksh
@@ -0,0 +1,60 @@
+#!/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) 2017 by Lawrence Livermore National Security, LLC.
+#
+
+# DESCRIPTION:
+#      Verify MMP behaves correctly when failing to write uberblocks.
+#
+# STRATEGY:
+#      1. Create a mirrored pool and enable multihost
+#      2. Inject a 50% failure rate when writing uberblocks to a device
+#      3. Delay briefly for additional MMP writes to complete
+#      4. Verify the failed uberblock writes did not prevent MMP updates
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/mmp/mmp.cfg
+. $STF_SUITE/tests/functional/mmp/mmp.kshlib
+
+verify_runnable "both"
+
+function cleanup
+{
+       zinject -c all
+       default_cleanup_noexit
+       log_must mmp_clear_hostid
+}
+
+log_assert "mmp behaves correctly when failing to write uberblocks."
+log_onexit cleanup
+
+log_must mmp_set_hostid $HOSTID1
+default_mirror_setup_noexit $DISKS
+log_must zpool set multihost=on $TESTPOOL
+log_must zinject -d ${DISK[0]} -e io -T write -f 50 $TESTPOOL -L uber
+
+prev_count=$(wc -l /proc/spl/kstat/zfs/$TESTPOOL/multihost | cut -f1 -d' ')
+log_must sleep 3
+curr_count=$(wc -l /proc/spl/kstat/zfs/$TESTPOOL/multihost | cut -f1 -d' ')
+
+if [ $curr_count -eq $prev_count ]; then
+       log_fail "mmp writes did not occur when uberblock IO errors injected"
+fi
+
+log_pass "mmp correctly wrote uberblocks when IO errors injected"