From ffb195c256f8a74a87c3834258ec90c513d66adb Mon Sep 17 00:00:00 2001 From: Olaf Faaland Date: Mon, 24 Jul 2017 08:48:28 -0700 Subject: [PATCH] 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 Signed-off-by: Olaf Faaland Closes #6394 --- module/zfs/mmp.c | 11 ++-- tests/runfiles/linux.run | 3 +- .../tests/functional/mmp/Makefile.am | 1 + .../functional/mmp/mmp_write_uberblocks.ksh | 60 +++++++++++++++++++ 4 files changed, 69 insertions(+), 6 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/mmp/mmp_write_uberblocks.ksh diff --git a/module/zfs/mmp.c b/module/zfs/mmp.c index ae5478a331..539b76826a 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 e556f1e8cc..842f6dd0c3 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 2399a6c377..75af0cdb71 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 0000000000..4a13dfa2c2 --- /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"