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 <[email protected]>
Signed-off-by: Olaf Faaland <[email protected]>
Closes #6394

4 files changed, 69 insertions, 6 deletions

diff --git a/module/zfs/mmp.c b/module/zfs/mmp.c
index ae5478a33..539b76826 100644
--- 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 e556f1e8c..842f6dd0c 100644
--- 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 2399a6c37..75af0cdb7 100644
--- 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
index 000000000..4a13dfa2c
--- /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"