From 467cf22b99e0bb90635bc35b42ce7dcb9b051b61 Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Thu, 11 Feb 2010 15:26:24 -0600 Subject: [PATCH] b=21881 mdt_num_threads tuning Fixes to mdt_handler.c so that it honors mdt_num_threads in the same manner as ost_handler.c does for oss_num_threads. Enhancements to the test framework and new tests (53a and b, to check for thread tunable behavior) for conf-sanity are included. i=adilger@sun.com i=robert.read@sun.com --- lustre/include/lustre_net.h | 6 -- lustre/mdt/mdt_handler.c | 54 +++++++++------- lustre/tests/conf-sanity.sh | 96 +++++++++++++++++++++++++++ lustre/tests/functions.sh | 144 +++++++++++++++++++++++++++++++++++++++++ lustre/tests/rpc.sh | 11 ++-- lustre/tests/test-framework.sh | 87 ++++++++++++++++++------- 6 files changed, 342 insertions(+), 56 deletions(-) diff --git a/lustre/include/lustre_net.h b/lustre/include/lustre_net.h index c5f3b56..b91459f 100644 --- a/lustre/include/lustre_net.h +++ b/lustre/include/lustre_net.h @@ -118,12 +118,6 @@ #define MDT_NUM_THREADS max(min_t(unsigned long, MDT_MAX_THREADS, \ cfs_num_physpages >> (25 - CFS_PAGE_SHIFT)), \ 2UL) -#define FLD_NUM_THREADS max(min_t(unsigned long, MDT_MAX_THREADS, \ - cfs_num_physpages >> (25 - CFS_PAGE_SHIFT)), \ - 2UL) -#define SEQ_NUM_THREADS max(min_t(unsigned long, MDT_MAX_THREADS, \ - cfs_num_physpages >> (25 - CFS_PAGE_SHIFT)), \ - 2UL) /* Absolute limits */ #define MDS_THREADS_MIN 2 diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c index 56fec6a..ff17a56 100644 --- a/lustre/mdt/mdt_handler.c +++ b/lustre/mdt/mdt_handler.c @@ -99,7 +99,9 @@ ldlm_mode_t mdt_dlm_lock_modes[] = { /* * Initialized in mdt_mod_init(). */ -unsigned long mdt_num_threads; +static unsigned long mdt_num_threads; +static unsigned long mdt_min_threads; +static unsigned long mdt_max_threads; /* ptlrpc request handler for MDT. All handlers are * grouped into several slices - struct mdt_opc_slice, @@ -3883,9 +3885,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m) * We'd like to have a mechanism to set this on a per-device * basis, but alas... */ - .psc_min_threads = min(max(mdt_num_threads, MDT_MIN_THREADS), - MDT_MAX_THREADS), - .psc_max_threads = MDT_MAX_THREADS, + .psc_min_threads = mdt_min_threads, + .psc_max_threads = mdt_max_threads, .psc_ctx_tags = LCT_MD_THREAD }; @@ -3916,9 +3917,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m) .psc_req_portal = MDS_READPAGE_PORTAL, .psc_rep_portal = MDC_REPLY_PORTAL, .psc_watchdog_factor = MDT_SERVICE_WATCHDOG_FACTOR, - .psc_min_threads = min(max(mdt_num_threads, MDT_MIN_THREADS), - MDT_MAX_THREADS), - .psc_max_threads = MDT_MAX_THREADS, + .psc_min_threads = mdt_min_threads, + .psc_max_threads = mdt_max_threads, .psc_ctx_tags = LCT_MD_THREAD }; m->mdt_readpage_service = @@ -3944,9 +3944,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m) .psc_req_portal = MDS_SETATTR_PORTAL, .psc_rep_portal = MDC_REPLY_PORTAL, .psc_watchdog_factor = MDT_SERVICE_WATCHDOG_FACTOR, - .psc_min_threads = min(max(mdt_num_threads, MDT_MIN_THREADS), - MDT_MAX_THREADS), - .psc_max_threads = MDT_MAX_THREADS, + .psc_min_threads = mdt_min_threads, + .psc_max_threads = mdt_max_threads, .psc_ctx_tags = LCT_MD_THREAD }; @@ -3975,8 +3974,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m) .psc_req_portal = SEQ_CONTROLLER_PORTAL, .psc_rep_portal = MDC_REPLY_PORTAL, .psc_watchdog_factor = MDT_SERVICE_WATCHDOG_FACTOR, - .psc_min_threads = SEQ_NUM_THREADS, - .psc_max_threads = SEQ_NUM_THREADS, + .psc_min_threads = mdt_min_threads, + .psc_max_threads = mdt_max_threads, .psc_ctx_tags = LCT_MD_THREAD|LCT_DT_THREAD }; @@ -4004,8 +4003,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m) .psc_req_portal = SEQ_METADATA_PORTAL, .psc_rep_portal = MDC_REPLY_PORTAL, .psc_watchdog_factor = MDT_SERVICE_WATCHDOG_FACTOR, - .psc_min_threads = SEQ_NUM_THREADS, - .psc_max_threads = SEQ_NUM_THREADS, + .psc_min_threads = mdt_min_threads, + .psc_max_threads = mdt_max_threads, .psc_ctx_tags = LCT_MD_THREAD|LCT_DT_THREAD }; @@ -4036,8 +4035,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m) .psc_req_portal = SEQ_DATA_PORTAL, .psc_rep_portal = OSC_REPLY_PORTAL, .psc_watchdog_factor = MDT_SERVICE_WATCHDOG_FACTOR, - .psc_min_threads = SEQ_NUM_THREADS, - .psc_max_threads = SEQ_NUM_THREADS, + .psc_min_threads = mdt_min_threads, + .psc_max_threads = mdt_max_threads, .psc_ctx_tags = LCT_MD_THREAD|LCT_DT_THREAD }; @@ -4063,8 +4062,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m) .psc_req_portal = FLD_REQUEST_PORTAL, .psc_rep_portal = MDC_REPLY_PORTAL, .psc_watchdog_factor = MDT_SERVICE_WATCHDOG_FACTOR, - .psc_min_threads = FLD_NUM_THREADS, - .psc_max_threads = FLD_NUM_THREADS, + .psc_min_threads = mdt_min_threads, + .psc_max_threads = mdt_max_threads, .psc_ctx_tags = LCT_DT_THREAD|LCT_MD_THREAD }; @@ -4093,9 +4092,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m) .psc_req_portal = MDS_MDS_PORTAL, .psc_rep_portal = MDC_REPLY_PORTAL, .psc_watchdog_factor = MDT_SERVICE_WATCHDOG_FACTOR, - .psc_min_threads = min(max(mdt_num_threads, MDT_MIN_THREADS), - MDT_MAX_THREADS), - .psc_max_threads = MDT_MAX_THREADS, + .psc_min_threads = mdt_min_threads, + .psc_max_threads = mdt_max_threads, .psc_ctx_tags = LCT_MD_THREAD }; m->mdt_xmds_service = @@ -5769,7 +5767,19 @@ static int __init mdt_mod_init(void) llo_local_obj_register(&mdt_last_recv); - mdt_num_threads = MDT_NUM_THREADS; + if (mdt_num_threads > 0) { + if (mdt_num_threads > MDT_MAX_THREADS) + mdt_num_threads = MDT_MAX_THREADS; + if (mdt_num_threads < MDT_MIN_THREADS) + mdt_num_threads = MDT_MIN_THREADS; + mdt_max_threads = mdt_min_threads = mdt_num_threads; + } else { + mdt_max_threads = MDT_MAX_THREADS; + mdt_min_threads = MDT_MIN_THREADS; + if (mdt_min_threads < MDT_NUM_THREADS) + mdt_min_threads = MDT_NUM_THREADS; + } + lprocfs_mdt_init_vars(&lvars); rc = class_register_type(&mdt_obd_device_ops, NULL, lvars.module_vars, LUSTRE_MDT_NAME, diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh index 199c73c9..1281d3f 100644 --- a/lustre/tests/conf-sanity.sh +++ b/lustre/tests/conf-sanity.sh @@ -2212,6 +2212,102 @@ test_52() { } run_test 52 "check recovering objects from lost+found" +# Checks threads_min/max/started for some service +# +# Arguments: service name (OST or MDT), facet (e.g., ost1, $SINGLEMDS), and a +# parameter pattern prefix like 'ost.*.ost'. +thread_sanity() { + local modname=$1 + local facet=$2 + local parampat=$3 + local opts=$4 + local tmin + local tmin2 + local tmax + local tmax2 + local tstarted + local paramp + local msg="Insane $modname thread counts" + shift 4 + + setup + check_mount || return 41 + + # We need to expand $parampat, but it may match multiple parameters, so + # we'll pick the first one + if ! paramp=$(do_facet $facet "lctl get_param -N ${parampat}.threads_min"|head -1); then + error "Couldn't expand ${parampat}.threads_min parameter name" + return 22 + fi + + # Remove the .threads_min part + paramp=${paramp%.threads_min} + + # Check for sanity in defaults + tmin=$(do_facet $facet "lctl get_param -n ${paramp}.threads_min" || echo 0) + tmax=$(do_facet $facet "lctl get_param -n ${paramp}.threads_max" || echo 0) + tstarted=$(do_facet $facet "lctl get_param -n ${paramp}.threads_started" || echo 0) + lassert 23 "$msg (PDSH problems?)" '(($tstarted && $tmin && $tmax))' || return $? + lassert 24 "$msg" '(($tstarted >= $tmin && $tstarted <= tmax ))' || return $? + + # Check that we can lower min/max + do_facet $facet "lctl set_param ${paramp}.threads_min=$((tmin - 1))" + do_facet $facet "lctl set_param ${paramp}.threads_max=$((tmax - 10))" + tmin2=$(do_facet $facet "lctl get_param -n ${paramp}.threads_min" || echo 0) + tmax2=$(do_facet $facet "lctl get_param -n ${paramp}.threads_max" || echo 0) + lassert 25 "$msg" '(($tmin2 == ($tmin - 1) && $tmax2 == ($tmax -10)))' || return $? + + # Check that we can set min/max to the same value + do_facet $facet "lctl set_param ${paramp}.threads_min=$tmin" + do_facet $facet "lctl set_param ${paramp}.threads_max=$tmin" + tmin2=$(do_facet $facet "lctl get_param -n ${paramp}.threads_min" || echo 0) + tmax2=$(do_facet $facet "lctl get_param -n ${paramp}.threads_max" || echo 0) + lassert 26 "$msg" '(($tmin2 == $tmin && $tmax2 == $tmin))' || return $? + + # Check that we can't set max < min + do_facet $facet "lctl set_param ${paramp}.threads_max=$((tmin - 1))" + tmin2=$(do_facet $facet "lctl get_param -n ${paramp}.threads_min" || echo 0) + tmax2=$(do_facet $facet "lctl get_param -n ${paramp}.threads_max" || echo 0) + lassert 27 "$msg" '(($tmin <= $tmax2))' || return $? + + # We need to ensure that we get the module options desired; to do this + # we set LOAD_MODULES_REMOTE=true and we call setmodopts below. + LOAD_MODULES_REMOTE=true + cleanup + local oldvalue + setmodopts -a $modname "$opts" oldvalue + + load_modules + setup + check_mount || return 41 + + # Restore previous setting of MODOPTS_* + setmodopts $modname "$oldvalue" + + # Check that $opts took + tmin=$(do_facet $facet "lctl get_param -n ${paramp}.threads_min") + tmax=$(do_facet $facet "lctl get_param -n ${paramp}.threads_max") + tstarted=$(do_facet $facet "lctl get_param -n ${paramp}.threads_started") + lassert 28 "$msg" '(($tstarted == $tmin && $tstarted == $tmax ))' || return $? + cleanup + + # Workaround a YALA bug where YALA expects that modules will remain + # loaded on the servers + LOAD_MODULES_REMOTE=false + setup + cleanup +} + +test_53a() { + thread_sanity OST ost1 'ost.*.ost' 'oss_num_threads=64' +} +run_test 53a "check OSS thread count params" + +test_53b() { + thread_sanity MDT $SINGLEMDS 'mdt.*.*.' 'mdt_num_threads=64' +} +run_test 53b "check MDT thread count params" + cleanup_gss equals_msg `basename $0`: test complete [ -f "$TESTSUITELOG" ] && cat $TESTSUITELOG && grep -q FAIL $TESTSUITELOG && exit 1 || true diff --git a/lustre/tests/functions.sh b/lustre/tests/functions.sh index 1dd9ac5..9442145 100644 --- a/lustre/tests/functions.sh +++ b/lustre/tests/functions.sh @@ -13,6 +13,150 @@ assert_env() { [ $failed ] && exit 1 || true } +# lrepl - Lustre test Read-Eval-Print Loop. +# +# This function implements a REPL for the Lustre test framework. It +# doesn't exec an actual shell because the user may want to inspect +# variables and use functions from the test framework. +lrepl() { + local line + local rawline + local prompt + + cat < ' rawline + do + line="$line"$'\n'"$rawline" + case "$rawline" in + # We could check for here documents by matching + # against *<<*, but who cares. + *\\) continue;; + *) break;; + esac + done + ;; + *) line=$rawline + esac + + case "$line" in + *\\) break;; + esac + + # Finally! Time to eval. + eval "$line" + done + + echo $'\n\tExiting interactive shell...\n' + return 0 +} + +# lassert - Lustre test framework assert +# +# Arguments: failure code, failure message, expression/statement +# +# lassert evaluates the expression given, and, if false, calls +# error() to trigger test failure. If REPL_ON_LASSERT is true then +# lassert will call lrepl() to give the user an interactive shell. +# If the REPL sets retcode=0 then the assertion failure will be +# ignored. +lassert() { + local retcode=$1 + local msg=$2 + shift 2 + + echo "checking $* ($(eval echo \""$*"\"))..." + eval "$@" && return 0; + + if ${REPL_ON_LASSERT:-false}; then + echo "Assertion $retcode failed: $* (expanded: $(eval echo \""$*"\")) +$msg" + lrepl + fi + + error "Assertion $retcode failed: $* (expanded: $(eval echo \""$*"\")) +$msg" + return $retcode +} + +# setmodopts- set module options for subsequent calls to load_modules +# +# Usage: setmodopts module_name new_value [var_in_which_to_save_old_value] +# setmodopts -a module_name new_value [var_in_which_to_save_old_value] +# +# In the second usage the new value is appended to the old. +setmodopts() { + local _append=false + + if [ "$1" = -a ]; then + _append=true + shift + fi + + local _var=MODOPTS_$1 + local _newvalue=$2 + local _savevar=$3 + local _oldvalue + + # Dynamic naming of variables is a pain in bash. In ksh93 we could + # write "nameref opts_var=${modname}_MODOPTS" then assign directly + # to opts_var. Associative arrays would also help, alternatively. + # Alas, we're stuck with eval until all distros move to a more recent + # version of bash. Fortunately we don't need to eval unset and export. + + if [ -z "$_newvalue" ]; then + unset $_var + return 0 + fi + + _oldvalue=${!var} + $_append && _newvalue="$_oldvalue $_newvalue" + export $_var="$_newvalue" + echo setmodopts: ${_var}=${_newvalue} + + [ -n "$_savevar" ] && eval $_savevar=\""$_oldvalue"\" +} + echoerr () { echo "$@" 1>&2 ; } signaled() { diff --git a/lustre/tests/rpc.sh b/lustre/tests/rpc.sh index e1127d2..d2914b6 100755 --- a/lustre/tests/rpc.sh +++ b/lustre/tests/rpc.sh @@ -9,11 +9,12 @@ if [ ! -f $LUSTRE/tests/rpc.sh ]; then fi . $LUSTRE/tests/test-framework.sh -init_test_env $@ +init_test_env . ${CONFIG:=$LUSTRE/tests/cfg/$NAME.sh} -cmd=$1 -shift -$cmd $@ +# Reset the trap on ERR set by the framework. Noticing this failure is the +# framework's job. +trap ERR -exit $? +# Execute the command +"$@" diff --git a/lustre/tests/test-framework.sh b/lustre/tests/test-framework.sh index 1f4a50c..be0a9ad 100644 --- a/lustre/tests/test-framework.sh +++ b/lustre/tests/test-framework.sh @@ -34,6 +34,9 @@ if [ -f "$EXCEPT_LIST_FILE" ]; then . $EXCEPT_LIST_FILE fi +[ -z "$MODPROBECONF" -a -f /etc/modprobe.conf ] && MODPROBECONF=/etc/modprobe.conf +[ -z "$MODPROBECONF" -a -f /etc/modprobe.d/Lustre ] && MODPROBECONF=/etc/modprobe.d/Lustre + assert_DIR () { local failed="" [[ $DIR/ = $MOUNT/* ]] || \ @@ -230,27 +233,64 @@ module_loaded () { /sbin/lsmod | grep -q $1 } +# Load a module on the system where this is running. +# +# Synopsis: load_module module_name [module arguments for insmod/modprobe] +# +# If module arguments are not given but MODOPTS_ is set, then its value +# will be used as the arguments. Otherwise arguments will be obtained from +# /etc/modprobe.conf, from /etc/modprobe.d/Lustre, or else none will be used. +# load_module() { + local optvar EXT=".ko" module=$1 shift BASE=`basename $module $EXT` + # If no module arguments were passed, get them from $MODOPTS_, else from + # modprobe.conf + if [ $# -eq 0 ]; then + # $MODOPTS_; we could use associative arrays, but that's not in + # Bash until 4.x, so we resort to eval. + optvar="MODOPTS_$(basename $module | tr a-z A-Z)" + eval set -- \$$optvar + if [ $# -eq 0 -a -n "$MODPROBECONF" ]; then + # Nothing in $MODOPTS_; try modprobe.conf + set -- $(grep "^options\\s*\<${module}\>" $MODPROBECONF) + # Get rid of "options $module" + (($# > 0)) && shift 2 + + # Ensure we have accept=all for lnet + if [ $module = lnet ]; then + # OK, this is a bit wordy... + local arg accept_all_present=false + for arg in "$@"; do + [ "$arg" = accept=all ] && accept_all_present=true + done + $accept_all_present || set -- "$@" accept=all + fi + fi + fi + + [ $# -gt 0 ] && echo "${module} options: '$*'" + module_loaded ${BASE} && return + # Note that insmod will ignore anything in modprobe.conf, which is why we're + # passing options on the command-line. if [ "$BASE" == "lnet_selftest" ] && \ [ -f ${LUSTRE}/../lnet/selftest/${module}${EXT} ]; then insmod ${LUSTRE}/../lnet/selftest/${module}${EXT} - elif [ -f ${LUSTRE}/${module}${EXT} ]; then - insmod ${LUSTRE}/${module}${EXT} $@ + insmod ${LUSTRE}/${module}${EXT} "$@" else # must be testing a "make install" or "rpm" installation # note failed to load ptlrpc_gss is considered not fatal if [ "$BASE" == "ptlrpc_gss" ]; then - modprobe $BASE $@ 2>/dev/null || echo "gss/krb5 is not supported" + modprobe $BASE "$@" 2>/dev/null || echo "gss/krb5 is not supported" else - modprobe $BASE $@ + modprobe $BASE "$@" fi fi } @@ -258,10 +298,12 @@ load_module() { load_modules_local() { if [ -n "$MODPROBE" ]; then # use modprobe - return 0 + echo "Using modprobe to load modules" + return 0 fi if [ "$HAVE_MODULES" = true ]; then - # we already loaded + # we already loaded + echo "Modules already loaded" return 0 fi HAVE_MODULES=true @@ -270,15 +312,7 @@ load_modules_local() { load_module ../libcfs/libcfs/libcfs [ "$PTLDEBUG" ] && lctl set_param debug="$PTLDEBUG" [ "$SUBSYSTEM" ] && lctl set_param subsystem_debug="${SUBSYSTEM# }" - local MODPROBECONF= - [ -f /etc/modprobe.conf ] && MODPROBECONF=/etc/modprobe.conf - [ ! "$MODPROBECONF" -a -d /etc/modprobe.d ] && MODPROBECONF=/etc/modprobe.d/Lustre - [ -z "$LNETOPTS" -a "$MODPROBECONF" ] && \ - LNETOPTS=$(awk '/^options lnet/ { print $0}' $MODPROBECONF | sed 's/^options lnet //g') - echo $LNETOPTS | grep -q "accept=all" || LNETOPTS="$LNETOPTS accept=all"; - echo "lnet options: '$LNETOPTS'" - # note that insmod will ignore anything in modprobe.conf - load_module ../lnet/lnet/lnet $LNETOPTS + load_module ../lnet/lnet/lnet LNETLND=${LNETLND:-"socklnd/ksocklnd"} load_module ../lnet/klnds/$LNETLND load_module lvfs/lvfs @@ -1505,6 +1539,17 @@ single_local_node () { [ "$1" = "$HOSTNAME" ] } +# Outputs environment variable assignments that should be passed to remote nodes +get_env_vars() { + local var + local value + + for var in ${!MODOPTS_*}; do + value=${!var} + echo "${var}=\"$value\"" + done +} + do_nodes() { local verbose=false # do not stripe off hostname if verbose, bug 19215 @@ -1537,9 +1582,9 @@ do_nodes() { fi if $verbose ; then - $myPDSH $rnodes "(PATH=\$PATH:$RLUSTRE/utils:$RLUSTRE/tests:/sbin:/usr/sbin; cd $RPWD; LUSTRE=\"$RLUSTRE\" sh -c \"$@\")" + $myPDSH $rnodes "(PATH=\$PATH:$RLUSTRE/utils:$RLUSTRE/tests:/sbin:/usr/sbin; cd $RPWD; LUSTRE=\"$RLUSTRE\" $(get_env_vars) sh -c \"$@\")" else - $myPDSH $rnodes "(PATH=\$PATH:$RLUSTRE/utils:$RLUSTRE/tests:/sbin:/usr/sbin; cd $RPWD; LUSTRE=\"$RLUSTRE\" sh -c \"$@\")" | sed -re "s/\w+:\s//g" + $myPDSH $rnodes "(PATH=\$PATH:$RLUSTRE/utils:$RLUSTRE/tests:/sbin:/usr/sbin; cd $RPWD; LUSTRE=\"$RLUSTRE\" $(get_env_vars) sh -c \"$@\")" | sed -re "s/\w+:\s//g" fi return ${PIPESTATUS[0]} } @@ -2429,15 +2474,11 @@ error_noexit() { error() { error_noexit "$@" - if $FAIL_ON_ERROR; then - reset_fail_loc - exit 1 - fi + exit 1 } error_exit() { - error_noexit "$@" - exit 1 + error "$@" } # use only if we are ignoring failures for this test, bugno required. -- 1.8.3.1