From 13d9a004fe533df8613888687650b1b0e272b67d Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Mon, 24 Oct 2016 13:28:58 -0700 Subject: [PATCH] Fix taskq creation failure in vdev_open_children() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit When creating and destroying pools in tight loop it's possible to exhaust the number of allowed threads on a system. This results in taskq_create() failling and a NULL dereference. Resolve the issue by falling back to opening the vdevs all synchronously. Reviewed-by: Denys Rtveliashvili Reviewed-by: HÃ¥kan Johansson Signed-off-by: Brian Behlendorf Closes zfsonlinux/spl#521 Closes #4637 --- config/user-commands.m4 | 1 + module/zfs/vdev.c | 3 + tests/runfiles/linux.run | 1 + tests/zfs-tests/include/commands.cfg.in | 1 + .../cli_root/zpool_create/Makefile.am | 1 + .../zpool_create/zpool_create_024_pos.ksh | 148 ++++++++++++++++++ 6 files changed, 155 insertions(+) create mode 100755 tests/zfs-tests/tests/functional/cli_root/zpool_create/zpool_create_024_pos.ksh diff --git a/config/user-commands.m4 b/config/user-commands.m4 index 6e9c3d103..d53bec4ff 100644 --- a/config/user-commands.m4 +++ b/config/user-commands.m4 @@ -89,6 +89,7 @@ AC_DEFUN([ZFS_AC_CONFIG_USER_COMMANDS_COMMON], [ AC_PATH_TOOL(USERADD, useradd, "/usr/sbin/useradd") AC_PATH_TOOL(USERDEL, userdel, "/usr/sbin/userdel") AC_PATH_TOOL(USERMOD, usermod, "/usr/sbin/usermod") + AC_PATH_TOOL(UUIDGEN, uuidgen, "") AC_PATH_TOOL(WAIT, wait, "wait") dnl # Builtin in bash AC_PATH_TOOL(WC, wc, "") ]) diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index 8a4d48a1d..e3a9da2d5 100644 --- a/module/zfs/vdev.c +++ b/module/zfs/vdev.c @@ -1179,12 +1179,15 @@ vdev_open_children(vdev_t *vd) * spa_namespace_lock */ if (vdev_uses_zvols(vd)) { +retry_sync: for (c = 0; c < children; c++) vd->vdev_child[c]->vdev_open_error = vdev_open(vd->vdev_child[c]); } else { tq = taskq_create("vdev_open", children, minclsyspri, children, children, TASKQ_PREPOPULATE); + if (tq == NULL) + goto retry_sync; for (c = 0; c < children; c++) VERIFY(taskq_dispatch(tq, vdev_open_child, diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run index 086ea2b27..f7ca9d5a7 100644 --- a/tests/runfiles/linux.run +++ b/tests/runfiles/linux.run @@ -248,6 +248,7 @@ tests = [ 'zpool_create_009_neg', 'zpool_create_010_neg', 'zpool_create_017_neg', 'zpool_create_018_pos', 'zpool_create_019_pos', 'zpool_create_021_pos', 'zpool_create_022_pos', 'zpool_create_023_neg', + 'zpool_create_024_pos', 'zpool_create_features_001_pos', 'zpool_create_features_002_pos', 'zpool_create_features_003_pos', 'zpool_create_features_004_neg'] diff --git a/tests/zfs-tests/include/commands.cfg.in b/tests/zfs-tests/include/commands.cfg.in index cf201c7e4..e7fb5ff13 100644 --- a/tests/zfs-tests/include/commands.cfg.in +++ b/tests/zfs-tests/include/commands.cfg.in @@ -121,6 +121,7 @@ export UNSHARE="@UNSHARE@" export USERADD="@USERADD@" export USERDEL="@USERDEL@" export USERMOD="@USERMOD@" +export UUIDGEN="@UUIDGEN@" export VMSTAT="@VMSTAT@" export WAIT="@WAIT@" export WC="@WC@" diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_create/Makefile.am b/tests/zfs-tests/tests/functional/cli_root/zpool_create/Makefile.am index 0530dec1a..a9ffbee03 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zpool_create/Makefile.am +++ b/tests/zfs-tests/tests/functional/cli_root/zpool_create/Makefile.am @@ -26,6 +26,7 @@ dist_pkgdata_SCRIPTS = \ zpool_create_021_pos.ksh \ zpool_create_022_pos.ksh \ zpool_create_023_neg.ksh \ + zpool_create_024_pos.ksh \ zpool_create_features_001_pos.ksh \ zpool_create_features_002_pos.ksh \ zpool_create_features_003_pos.ksh \ diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_create/zpool_create_024_pos.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_create/zpool_create_024_pos.ksh new file mode 100755 index 000000000..1fa22d6ca --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zpool_create/zpool_create_024_pos.ksh @@ -0,0 +1,148 @@ +#!/bin/ksh -p +# +# CDDL HEADER START +# +# The contents of this file are subject to the terms of the +# Common Development and Distribution License (the "License"). +# You may not use this file except in compliance with the License. +# +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE +# or http://www.opensolaris.org/os/licensing. +# See the License for the specific language governing permissions +# and limitations under the License. +# +# When distributing Covered Code, include this CDDL HEADER in each +# file and include the License file at usr/src/OPENSOLARIS.LICENSE. +# If applicable, add the following below this CDDL HEADER, with the +# fields enclosed by brackets "[]" replaced with your own identifying +# information: Portions Copyright [yyyy] [name of copyright owner] +# +# CDDL HEADER END +# + +# +# Copyright (c) 2016, Lawrence Livermore National Security, LLC. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/cli_root/zpool_create/zpool_create.cfg + +# +# DESCRIPTION: +# Many 'zpool create' and 'zpool destroy' must succeed concurrently. +# +# STRATEGY: +# 1. Create N process each of which create/destroy a pool M times. +# 2. Allow all process to run to completion. +# 3. Verify all pools and their vdevs were destroyed. +# + +verify_runnable "global" + +function cleanup +{ + if [[ -n "$child_pids" ]]; then + for wait_pid in $child_pids; do + $KILL $wait_pid 2>/dev/null + done + fi + + if [[ -n "$child_pools" ]]; then + for pool in $child_pools; do + typeset vdev0="$TEST_BASE_DIR/$pool-vdev0.img" + typeset vdev1="$TEST_BASE_DIR/$pool-vdev1.img" + + if poolexists $pool; then + destroy_pool $pool + fi + + $RM -f $vdev0 $vdev1 + done + fi +} + +log_onexit cleanup + +log_assert "Many 'zpool create' and 'zpool destroy' must succeed concurrently." + +child_pids="" +child_pools="" + +function zpool_stress +{ + typeset pool=$1 + typeset vdev0="$TEST_BASE_DIR/$pool-vdev0.img" + typeset vdev1="$TEST_BASE_DIR/$pool-vdev1.img" + typeset -i iters=$2 + typeset retry=10 + typeset j=0 + + $TRUNCATE -s $FILESIZE $vdev0 + $TRUNCATE -s $FILESIZE $vdev1 + + while [[ $j -lt $iters ]]; do + ((j = j + 1)) + $SLEEP 1 + + $ZPOOL create $pool $vdev0 $vdev1 + if [ $? -ne 0 ]; then + return 1; + fi + + # The 'zfs destroy' command is retried because it can + # transiently return EBUSY when blkid is concurrently + # probing new volumes and therefore has them open. + typeset k=0; + while [[ $k -lt $retry ]]; do + ((k = k + 1)) + + $ZPOOL destroy $pool + if [ $? -eq 0 ]; then + break; + elif [ $k -eq $retry ]; then + return 1; + fi + + $SLEEP 3 + done + done + + $RM -f $vdev0 $vdev1 + return 0 +} + +# 1. Create 128 process each of which create/destroy a pool 5 times. +typeset i=0 +while [[ $i -lt 128 ]]; do + typeset uuid=$($UUIDGEN | $CUT -c1-13) + + zpool_stress $TESTPOOL-$uuid 5 & + typeset pid=$! + + child_pids="$child_pids $pid" + child_pools="$child_pools $TESTPOOL-$uuid" + ((i = i + 1)) +done + +# 2. Allow all process to run to completion. +wait + +# 3. Verify all pools and their vdevs were destroyed. +for pool in $child_pools; do + typeset vdev0="$TEST_BASE_DIR/$pool-vdev0.img" + typeset vdev1="$TEST_BASE_DIR/$pool-vdev1.img" + + if poolexists $pool; then + log_fail "pool $pool exists" + fi + + if [ -e $vdev0 ]; then + log_fail "pool vdev $vdev0 exists" + fi + + if [ -e $vdev1 ]; then + log_fail "pool vdev $vdev1 exists" + fi +done + +log_pass "Many 'zpool create' and 'zpool destroy' must succeed concurrently." -- 2.40.0