Message ID | 9a6764237b900f40e563d8dee2853f1430245b74.1736496620.git.nirjhar.roy.lists@gmail.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
Series | Add support for central fsconfig and -q <n> unconditional loop | expand |
On Fri, Jan 10, 2025 at 09:10:28AM +0000, Nirjhar Roy (IBM) wrote: > This adds support to pick and use any existing FS config from > configs/<fstype>/<config>. e.g. > > configs/xfs/1k > configs/xfs/4k > configs/ext4/4k > configs/ext4/64k > > This should help us maintain and test different fs test > configurations from a central place. We also hope that > this will be useful for both developers and testers to > look into what is being actively maintained and tested > by FS Maintainers. I haven't been using the current in-place configs in kvm-xfstests and gce-xfstests because there are number of things that my setup can support that xfstests native config doesn't support (and becuase my system predates the FS config setup). I don't mind just using my own custom setup, but if we can keep feature parity, perhaps someday I can switch over to xfstests's system. This might also make it easier for people to more easily test using the same setup as the FS maintainers, regardless of which test running infrastructure they are using. A) A way of specifying the minimum device size needed for the TEST and SCRATCH device. Using a smaller file system size reduces test run time, and if you are paying for cloud test infrastructure, the size of the Google Persistent Disk or Amazon Elastic Block Storage has a direct impact on the cost per test, which in turn impacts how many tests we can afford to run. But for certain test configurations, such as using a larger block size, or using bigalloc, a larger test device size might be needed in order for tests to be able to run correctly. B) A way of specifying test exclusions, both at a global level, or on a per-fs, or on a per-configuration basis. It should also be possible to specify the kernel version being tested, and so that certain test exclusions might only be done when testing LTS kernels (for example): #if LINUX_VERSION_CODE < KERNEL_VERSION(6,6,30) // This test failure is fixed by commit 631426ba1d45q ("mm/madvise: // make MADV_POPULATE_(READ|WRITE) handle VM_FAULT_RETRY properly"), // which first landed in v6.9, and was backported to 6.6.30 as commit // 631426ba1d45. Unfortunately, it's too involved to backport it and its // dependencies to the 6.1 or earlier LTS kernels generic/743 #endif C) A way to run shell functions to do setups for testing things like overlayfs, or nfs (where I may be starting separate VM's for the NFS server, or needing to find the IP address for the NFS server running the appropriate kernel under test, which either been the same as the kernel under test, or which might be some standard server version, such as a RHEL or SLES kernel), as part of the setup and teardown of a particular test configuration. D) I also have a really nice scheme for specifying a mkfs configuration for testing LTS kernels, since I use the same test appliance for testing upstream and LTS/product kernels, and the latest mkfs.xfs will produce a file system image that isn't supported by a 5.15 LTS kernel. Product teams might also want to run tests using the mkfs configuration for that era kernel, even if a 6.1 kernel can support a file system created using the latest version of xfsprogs or e2fsprogs. Doing this is a bit non-trivial due to a misfeature of how mkfs.xfs works, but I have a workaround that some might find useful here: https://github.com/tytso/xfstests-bld/commit/f62433b74146e6ecacdeace306828c6c7510c4a6 Note that this might also be useful for xfstests, where specific xfstests scripts have to handle cases where mkfs.xfs might unexpectedly fail due to an unfortunate combination between the test-specific _scratch_mkfs options, and the global MKFS_OPTIONS. This never happens with ext4, due to how mkfs.ext4 handles conflicting command-line options, but it *is* a problem with mkfs.xfs. If you think merging an 150 line shell script library is easier than trying to get consensus from within the xfs community, here's a technical workaround to what might be charitably described as a disagreement between the xfs architects and the needs of the testing community. :-) If we're going to have critical mass, maybe this is something that's worth discussing at the upcoming LSF/MM? As I said, I'm happy having this be an exclusive feature in gce-xfstests and kvm-xfstests, but perhaps it would make sense to uplevel some of this feature into xfstests so that more people can take advantage of it, and to make it easier for us to share test configs across test teams and upstream developers? Cheers, - Ted
On 10/1/25 17:10, Nirjhar Roy (IBM) wrote: > This adds support to pick and use any existing FS config from > configs/<fstype>/<config>. e.g. > > configs/xfs/1k > configs/xfs/4k > configs/ext4/4k > configs/ext4/64k > > This should help us maintain and test different fs test > configurations from a central place. We also hope that > this will be useful for both developers and testers to > look into what is being actively maintained and tested > by FS Maintainers. > > When we will have fsconfigs set, then will be another subdirectory created > in results/<section>. For example let's look at the following: > > The directory tree structure on running > sudo ./check -q 2 -R xunit-quiet -c xfs/4k,configs/xfs/1k selftest/001 selftest/007 > The -c option check makes sense to me. Is it possible to get this feature implemented first while the -q option is still under discussion? That said, I have a suggestion for the -c option— Global config variables should be overridden by file system-specific config files. For example, if configs/localhost.config contains: MKFS_OPTIONS="--sectorsize 64K" but configs/<fstype>/some_config sets: MKFS_OPTIONS="" then the value from configs/<fstype>/some_config should take priority. I ran some tests with btrfs, and I don’t see this behavior happening yet. Thanks, Anand
Anand Jain <anand.jain@oracle.com> writes: > On 10/1/25 17:10, Nirjhar Roy (IBM) wrote: >> This adds support to pick and use any existing FS config from >> configs/<fstype>/<config>. e.g. >> >> configs/xfs/1k >> configs/xfs/4k >> configs/ext4/4k >> configs/ext4/64k >> >> This should help us maintain and test different fs test >> configurations from a central place. We also hope that >> this will be useful for both developers and testers to >> look into what is being actively maintained and tested >> by FS Maintainers. >> >> When we will have fsconfigs set, then will be another subdirectory created >> in results/<section>. For example let's look at the following: >> >> The directory tree structure on running >> sudo ./check -q 2 -R xunit-quiet -c xfs/4k,configs/xfs/1k selftest/001 selftest/007 >> > > > The -c option check makes sense to me. Is it possible to get this > feature implemented first while the -q option is still under discussion? Hi Anand, Thanks for trying the patches. The design of -c option is still under discussion [1]. But it will be helpful if you could help us understand your reasons for finding -c option useful :) [1]: https://lore.kernel.org/linux-fsdevel/Z55RXUKB5O5l8QjM@li-dc0c254c-257c-11b2-a85c-98b6c1322444.ibm.com/ > > That said, I have a suggestion for the -c option— > Global config variables should be overridden by file system-specific > config files. > > For example, if configs/localhost.config contains: > > MKFS_OPTIONS="--sectorsize 64K" > > but configs/<fstype>/some_config sets: > > MKFS_OPTIONS="" > > then the value from configs/<fstype>/some_config should take priority. > > I ran some tests with btrfs, and I don’t see this behavior happening yet. I think that was intentional. I guess the reasoning was, we don't want to break use cases for folks who still wanted to use local.config file option. However, in the new proposed design [2] we are thinking of having 1 large config per filesystem. e.g. configs/btrfs/config.btrfs which will define all of the relevant sections e.g. btrfs_4k, btrfs_64k, ... Then on invokking "make", it will generate a single large fs config file i.e. configs/.all-sections-configs which will club all filesystems section configs together. Now when someone invokes check script with different -s options, it will first look into local.config file, if local.config not found, then it will look into configs/.all-sections-configs to get the relevant section defines. This hopefully should address all the other concerns which were raised on the current central fs config design. [2]: https://lore.kernel.org/linux-fsdevel/87plj0hp7e.fsf@gmail.com/ -ritesh > > Thanks, Anand
On 2/3/25 01:30, Ritesh Harjani (IBM) wrote: > Anand Jain <anand.jain@oracle.com> writes: > >> On 10/1/25 17:10, Nirjhar Roy (IBM) wrote: >>> This adds support to pick and use any existing FS config from >>> configs/<fstype>/<config>. e.g. >>> >>> configs/xfs/1k >>> configs/xfs/4k >>> configs/ext4/4k >>> configs/ext4/64k >>> >>> This should help us maintain and test different fs test >>> configurations from a central place. We also hope that >>> this will be useful for both developers and testers to >>> look into what is being actively maintained and tested >>> by FS Maintainers. >>> >>> When we will have fsconfigs set, then will be another subdirectory created >>> in results/<section>. For example let's look at the following: >>> >>> The directory tree structure on running >>> sudo ./check -q 2 -R xunit-quiet -c xfs/4k,configs/xfs/1k selftest/001 selftest/007 >>> >> >> >> The -c option check makes sense to me. Is it possible to get this >> feature implemented first while the -q option is still under discussion? > > Hi Anand, > > Thanks for trying the patches. The design of -c option is still under > discussion [1]. But it will be helpful if you could help us understand > your reasons for finding -c option useful :) > Reason is exactly same as you mentioned and I copied it here: --- 1. Testers and other FS developers can know what is being actively tested and maintained by FS Maintainers. --- > [1]: https://lore.kernel.org/linux-fsdevel/Z55RXUKB5O5l8QjM@li-dc0c254c-257c-11b2-a85c-98b6c1322444.ibm.com/ > >> >> That said, I have a suggestion for the -c option— >> Global config variables should be overridden by file system-specific >> config files. >> >> For example, if configs/localhost.config contains: >> >> MKFS_OPTIONS="--sectorsize 64K" >> >> but configs/<fstype>/some_config sets: >> >> MKFS_OPTIONS="" >> >> then the value from configs/<fstype>/some_config should take priority. >> >> I ran some tests with btrfs, and I don’t see this behavior happening yet. > > I think that was intentional. I guess the reasoning was, we don't want to > break use cases for folks who still wanted to use local.config file > option. > Ok. > However, in the new proposed design [2] we are thinking of having 1 > large config per filesystem. e.g. configs/btrfs/config.btrfs which will > define all of the relevant sections e.g. btrfs_4k, btrfs_64k, ... Then > on invokking "make", it will generate a single large fs config file i.e. > configs/.all-sections-configs which will club all filesystems section > configs together. > > Now when someone invokes check script with different -s options, it will > first look into local.config file, if local.config not found, then it > will look into configs/.all-sections-configs to get the relevant section > defines. > > This hopefully should address all the other concerns which were raised > on the current central fs config design. > > [2]: https://lore.kernel.org/linux-fsdevel/87plj0hp7e.fsf@gmail.com/ > I think it’s making things a bit more complicated than needed. True. We’d probably be better off with one big config file. No need for <fstype> directories under configs. configs/<fstype>.config should work just fine. I’m not fond of the need to run make; it seems excessive. The way to run it is simple and works well: $ ./check -c configs/btrfs.config $ ./check -c configs/btrfs.config -s <section1> ... $ ./check -c configs/btrfs.config -S <exclude-section1> ... These steps already work fine: $ cp configs/btrfs.config configs/$(hostname).config $ ./check -s <section> .. (Patch containing configs/btrfs.config is already in the ML). Also, a --dry-run option to check the config before running would be super helpful. Thanks, Anand > -ritesh > >> >> Thanks, Anand
diff --git a/README.config-sections b/README.config-sections index a42d9d7b..0511b707 100644 --- a/README.config-sections +++ b/README.config-sections @@ -108,10 +108,22 @@ MKFS_OPTIONS="-q -F -b1024" [ext4_nojournal] MKFS_OPTIONS="-q -F -b4096 -O ^has_journal" +# we could also do like: FS_CONFIG_OPTION=ext4/1k +[ext4_1k_bs_fsconfig] +FS_CONFIG_OPTION=configs/ext4/1k + +# we could also do like: FS_CONFIG_OPTION=ext4/nojournal +[ext4_nojournal_fsconfig] +FS_CONFIG_OPTION=configs/ext4/nojournal + [xfs_filesystem] MKFS_OPTIONS="-f" FSTYP=xfs +# we could also do like: FS_CONFIG_OPTION=xfs/4k +[xfs_4k_bs_fsconfig] +FS_CONFIG_OPTION=configs/xfs/4k + [ext3_filesystem] FSTYP=ext3 MOUNT_OPTIONS="-o noatime" diff --git a/check b/check index f7028836..90aa94ba 100755 --- a/check +++ b/check @@ -80,6 +80,7 @@ check options -I <n> iterate the test list <n> times, but stops iterating further in case of any test failure -d dump test output to stdout -b brief test summary + -c use fs configs from configs/<fstype>/<config> e.g. configs/xfs/4k -R fmt[,fmt] generate report in formats specified. Supported formats: xunit, xunit-quiet --large-fs optimise scratch device for large filesystems -s section run only specified section from config file @@ -116,6 +117,14 @@ excluded from the list of tests to run from that test dir. external_file argument is a path to a single file containing a list of tests to exclude in the form of <test dir>/<test name>. +-c can be used in various formats as mentioned below. We can also set the +FS_CONFIG_OPTION environment variable in the local.config file and get the +same functionality. e.g. + 1) FS_CONFIG_OPTION=xfs/4k + 2) FS_CONFIG_OPTION=configs/xfs/4k +Please note that cmdline will take precedence if FS_CONFIG_OPTION is +passed in both local.config file and via cmdline. + examples: check xfs/001 check -g quick @@ -123,6 +132,9 @@ examples: check -x stress xfs/* check -X .exclude -g auto check -E ~/.xfstests.exclude + check -c xfs/4k,ext4/1k + check -c xfs/4k -c configs/ext4/1k + check -c $(echo configs/xfs/* | tr ' ' ',') -q 5 tests/selftest/007 ' exit 1 } @@ -312,6 +324,8 @@ while [ $# -gt 0 ]; do ;; -s) RUN_SECTION="$RUN_SECTION $2"; shift ;; -S) EXCLUDE_SECTION="$EXCLUDE_SECTION $2"; shift ;; + -c) CMD_FS_CONFIG_OPTION="$CMD_FS_CONFIG_OPTION$2,"; + export OPTIONS_HAVE_FSCONFIGS=true; shift ;; -l) diff="diff" ;; -udiff) diff="$diff -u" ;; @@ -458,7 +472,7 @@ _wipe_counters() _global_log() { echo "$1" >> $check.log - if $OPTIONS_HAVE_SECTIONS; then + if options_have_sections_or_fs_configs; then echo "$1" >> ${REPORT_DIR}/check.log fi } @@ -513,7 +527,7 @@ _wrapup() }' \ | sort -n >$tmp.out mv $tmp.out $check.time - if $OPTIONS_HAVE_SECTIONS; then + if options_have_sections_or_fs_configs; then cp $check.time ${REPORT_DIR}/check.time fi fi @@ -524,9 +538,14 @@ _wrapup() echo "SECTION -- $section" >>$tmp.summary echo "=========================" >>$tmp.summary - _global_log "SECTION -- $section" _global_log "=========================" >>$tmp.summary + + if [[ -n $FS_CONFIG_OPTION ]]; then + echo "FSCONFIG -- $FS_CONFIG_OPTION" >> $tmp.summary + _global_log "FSCONFIG -- $FS_CONFIG_OPTION" + fi + _global_log "$(_display_test_configuration)" if ((${#try[*]} > 0)); then local test_aggr_stats=$(_print_list loop_test_stats_per_section) @@ -541,7 +560,7 @@ _wrapup() fi $interrupt && echo "Interrupted!" | tee -a $check.log - if $OPTIONS_HAVE_SECTIONS; then + if options_have_sections_or_fs_configs; then $interrupt && echo "Interrupted!" | tee -a \ ${REPORT_DIR}/check.log fi @@ -592,7 +611,7 @@ _wrapup() sum_bad=`expr $sum_bad + ${#bad[*]}` _wipe_counters - if ! $OPTIONS_HAVE_SECTIONS; then + if ! options_have_sections_or_fs_configs; then rm -f $tmp.* fi } @@ -773,7 +792,7 @@ _detect_kmemleak _prepare_test_list fstests_start_time="$(date +"%F %T")" -if $OPTIONS_HAVE_SECTIONS; then +if options_have_sections_or_fs_configs; then trap "_summary; exit \$status" 0 1 2 3 15 else trap "_wrapup; exit \$status" 0 1 2 3 15 @@ -782,6 +801,7 @@ fi function run_section() { local section=$1 skip + local fsconfig=$2 OLD_FSTYP=$FSTYP OLD_TEST_FS_MOUNT_OPTS=$TEST_FS_MOUNT_OPTS @@ -814,7 +834,10 @@ function run_section() fi fi - get_next_config $section + # FS_CONFIG_OPTION (if provided) will only be set after get_next_config + # has run which will export the FS_CONFIG_OPTION, so that we can use it + # later + get_next_config $section $fsconfig _canonicalize_devices mkdir -p $RESULT_BASE @@ -828,6 +851,10 @@ function run_section() echo "SECTION -- $section" fi + if $OPTIONS_HAVE_FSCONFIGS; then + echo "FSCONFIG -- $FS_CONFIG_OPTION" + fi + sect_start=`_wallclock` if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then echo "RECREATING -- $FSTYP on $TEST_DEV" @@ -849,7 +876,12 @@ function run_section() # TEST_DEV has been recreated, previous FSTYP derived from # TEST_DEV could be changed, source common/rc again with # correct FSTYP to get FSTYP specific configs, e.g. common/xfs + # sourcing common/rc can re-set FS_CONFIG_OPTION to the value in + # local.config file(if present). So store and restore the value of + # FS_CONFIG_OPTION which is there currently at this point. + local fs_config_tmp="$FS_CONFIG_OPTION" . common/rc + export FS_CONFIG_OPTION="$fs_config_tmp" _prepare_test_list elif [ "$OLD_TEST_FS_MOUNT_OPTS" != "$TEST_FS_MOUNT_OPTS" ]; then _test_unmount 2> /dev/null @@ -933,10 +965,14 @@ function run_section() # we don't include the tests/ directory in the name output. export seqnum=${seq#$SRC_DIR/} group=${seqnum%%/*} + + REPORT_DIR="$RESULT_BASE" if $OPTIONS_HAVE_SECTIONS; then - REPORT_DIR="$RESULT_BASE/$section" - else - REPORT_DIR="$RESULT_BASE" + REPORT_DIR="$REPORT_DIR/$section" + fi + if $OPTIONS_HAVE_FSCONFIGS; then + local fsconfig_subdir="${FS_CONFIG_OPTION//configs/''}" + REPORT_DIR="$REPORT_DIR/$fsconfig_subdir" fi export RESULT_DIR="$REPORT_DIR/$group" seqres="$REPORT_DIR/$seqnum" @@ -1151,14 +1187,28 @@ function run_section() _scratch_unmount 2> /dev/null } +prepare_fs_config_list() +{ + IFS=',' read -r -a CMD_FS_CONFIG_OPTION_LIST <<< "$CMD_FS_CONFIG_OPTION" + CMD_FS_OPTION_LIST_LEN=${#CMD_FS_CONFIG_OPTION_LIST[@]} + if [ $CMD_FS_OPTION_LIST_LEN -eq 0 ]; then + # Create an empty fsconfig "" to pass to run_section in iters loop + CMD_FS_CONFIG_OPTION_LIST=("") + fi +} + +prepare_fs_config_list + for ((iters = 0; iters < $iterations; iters++)) do for section in $HOST_OPTIONS_SECTIONS; do - run_section $section - if [ "$sum_bad" != 0 ] && [ "$istop" = true ]; then - interrupt=false - status=`expr $sum_bad != 0` - exit - fi + for fsconfig in "${CMD_FS_CONFIG_OPTION_LIST[@]}"; do + run_section $section $fsconfig + if [ "$sum_bad" != 0 ] && [ "$istop" = true ]; then + interrupt=false + status=`expr $sum_bad != 0` + exit + fi + done done done diff --git a/common/config b/common/config index 5f86fc2c..901dc1e8 100644 --- a/common/config +++ b/common/config @@ -343,6 +343,11 @@ if [ -x /usr/sbin/selinuxenabled ] && /usr/sbin/selinuxenabled; then export SELINUX_MOUNT_OPTIONS fi +options_have_sections_or_fs_configs() +{ + $OPTIONS_HAVE_SECTIONS || $OPTIONS_HAVE_FSCONFIGS +} + _common_mount_opts() { case $FSTYP in @@ -783,8 +788,32 @@ parse_config_section() { | sed -n -e "/^\[$SECTION\]/,/^\s*\[/{/^[^#].*\=.*/p;}"` } +parse_fs_config_option() { + local fs_config_option + if ! $OPTIONS_HAVE_FSCONFIGS; then + return 0 + fi + + if [ -f "$1" ]; then + fs_config_option=$1 + elif [ -f configs/"$1" ]; then + fs_config_option=configs/"$1" + else + echo "Invalid FS_CONFIG_OPTION $1" + exit 1 + fi + + eval `sed -e 's/[[:space:]]*\=[[:space:]]*/=/g' \ + -e 's/#.*$//' \ + -e 's/[[:space:]]*$//' \ + -e 's/^[[:space:]]*//' \ + -e "s/^\([^=]*\)=\"\?'\?\([^\"']*\)\"\?'\?$/export \1=\"\2\"/" \ + < $fs_config_option` +} + get_next_config() { - if [ ! -z "$CONFIG_INCLUDED" ] && ! $OPTIONS_HAVE_SECTIONS; then + local cmd_fs_config_option=$2 + if [[ $CONFIG_INCLUDED && ! options_have_sections_or_fs_configs ]]; then return 0 fi @@ -817,6 +846,23 @@ get_next_config() { fi parse_config_section $1 + + # FS_CONFIG_OPTION can be set either via cmdline -c xfs/4k (through + # CMD_FS_CONFIG_OPTION) or can be set in section config file + # FS_CONFIG_OPTION=xfs/4k. + # Please note that FS_CONFIG_OPTION in the local.config file can be + # overridden by the value passed through the command line. + if [[ -n "$cmd_fs_config_option" ]]; then + export FS_CONFIG_OPTION=$cmd_fs_config_option + fi + + if [[ -n "$FS_CONFIG_OPTION" ]]; then + export OPTIONS_HAVE_FSCONFIGS=true + parse_fs_config_option $FS_CONFIG_OPTION + else + export OPTIONS_HAVE_FSCONFIGS=false + fi + if [ ! -z "$OLD_FSTYP" ] && [ $OLD_FSTYP != $FSTYP ]; then [ -z "$MOUNT_OPTIONS" ] && _mount_opts [ -z "$TEST_FS_MOUNT_OPTS" ] && _test_mount_opts @@ -900,7 +946,8 @@ get_next_config() { } if [ -z "$CONFIG_INCLUDED" ]; then - get_next_config `echo $HOST_OPTIONS_SECTIONS | cut -f1 -d" "` + get_next_config `echo $HOST_OPTIONS_SECTIONS | cut -f1 -d" "` \ + `echo $CMD_FS_CONFIG_OPTION | cut -f1 -d","` export CONFIG_INCLUDED=true # Autodetect fs type based on what's on $TEST_DEV unless it's been set diff --git a/common/report b/common/report index 7128bbeb..8723c510 100644 --- a/common/report +++ b/common/report @@ -12,7 +12,7 @@ REPORT_ENV_LIST=("SECTION" "FSTYP" "PLATFORM" "MKFS_OPTIONS" "MOUNT_OPTIONS" \ # Variables that are captured in the report /if/ they are set. REPORT_ENV_LIST_OPT=("TAPE_DEV" "RMT_TAPE_DEV" "FSSTRESS_AVOID" "FSX_AVOID" "KCONFIG_PATH" "PERF_CONFIGNAME" "MIN_FSSIZE" - "IDMAPPED_MOUNTS") + "IDMAPPED_MOUNTS" "FS_CONFIG_OPTION") encode_xml() {