From 5507ee535173b50482941305a940b45af74f971e Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 15 Nov 2023 11:40:06 -0800 Subject: [PATCH 1/3] Use device-mapper for per-mount test devices We don't directly mount the underlying devices for each mount because the kernel notices multiple mounts and doesn't setup a new super block for each. Previously the script used loopback devices to create the local shared block construct 'cause it was easy. This introduced corruption of blocks that saw concurrent read and write IOs. The buffered kernel file IO paths that loopback eventually degrades into by default (via splice) could have buffered readers copying out of pages without the page lock while writers modified the page. This manifest as occasional crc failure of blocks that we knowingly issue concurrent reads and writes to from multiple mounts (the quorum and super blocks). This changes the script to use device-mapper linear passthrough devices. Their IOs don't hit a caching layer and don't provide an opportunity to corrupt blocks. Signed-off-by: Zach Brown --- tests/README.md | 9 +++++---- tests/run-tests.sh | 31 +++++++++++++++++-------------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/tests/README.md b/tests/README.md index b61c6977..68f3b157 100644 --- a/tests/README.md +++ b/tests/README.md @@ -25,8 +25,9 @@ All options can be seen by running with -h. This script is built to test multi-node systems on one host by using different mounts of the same devices. The script creates a fake block device in front of each fs block device for each mount that will be -tested. Currently it will create free loop devices and will mount on -/mnt/test.[0-9]. +tested. It will create predictable device mapper devices and mounts +them on /mnt/test.N. These static device names and mount paths limit +the script to a single execution per host. All tests will be run by default. Particular tests can be included or excluded by providing test name regular expressions with the -I and -E @@ -104,8 +105,8 @@ used during the test. | Variable | Description | Origin | Example | | ---------------- | ------------------- | --------------- | ----------------- | -| T\_MB[0-9] | per-mount meta bdev | created per run | /dev/loop0 | -| T\_DB[0-9] | per-mount data bdev | created per run | /dev/loop1 | +| T\_MB[0-9] | per-mount meta bdev | created per run | /dev/mapper/\_scoutfs\_test\_meta\_[0-9] | +| T\_DB[0-9] | per-mount data bdev | created per run | /dev/mapper/\_scoutfs\_test\_data\_[0-9] | | T\_D[0-9] | per-mount test dir | made for test | /mnt/test.[0-9]/t | | T\_META\_DEVICE | main FS meta bdev | -M | /dev/vda | | T\_DATA\_DEVICE | main FS data bdev | -D | /dev/vdb | diff --git a/tests/run-tests.sh b/tests/run-tests.sh index 1202cada..2890f58a 100755 --- a/tests/run-tests.sh +++ b/tests/run-tests.sh @@ -326,16 +326,10 @@ unmount_all() { cmd wait $p done - # delete all temp meta devices - for dev in $(losetup --associated "$T_META_DEVICE" | cut -d : -f 1); do - if [ -e "$dev" ]; then - cmd losetup -d "$dev" - fi - done - # delete all temp data devices - for dev in $(losetup --associated "$T_DATA_DEVICE" | cut -d : -f 1); do - if [ -e "$dev" ]; then - cmd losetup -d "$dev" + # delete all temp devices + for dev in /dev/mapper/_scoutfs_test_*; do + if [ -b "$dev" ]; then + cmd dmsetup remove $dev fi done } @@ -434,6 +428,12 @@ $T_UTILS/fenced/scoutfs-fenced > "$T_FENCED_LOG" 2>&1 & fenced_pid=$! fenced_log "started fenced pid $fenced_pid in the background" +# setup dm tables +echo "0 $(blockdev --getsz $T_META_DEVICE) linear $T_META_DEVICE 0" > \ + $T_RESULTS/dmtable.meta +echo "0 $(blockdev --getsz $T_DATA_DEVICE) linear $T_DATA_DEVICE 0" > \ + $T_RESULTS/dmtable.data + # # mount concurrently so that a quorum is present to elect the leader and # start a server. @@ -442,10 +442,13 @@ msg "mounting $T_NR_MOUNTS mounts on meta $T_META_DEVICE data $T_DATA_DEVICE" pids="" for i in $(seq 0 $((T_NR_MOUNTS - 1))); do - meta_dev=$(losetup --find --show $T_META_DEVICE) - test -b "$meta_dev" || die "failed to create temp device $meta_dev" - data_dev=$(losetup --find --show $T_DATA_DEVICE) - test -b "$data_dev" || die "failed to create temp device $data_dev" + name="_scoutfs_test_meta_$i" + cmd dmsetup create "$name" --table "$(cat $T_RESULTS/dmtable.meta)" + meta_dev="/dev/mapper/$name" + + name="_scoutfs_test_data_$i" + cmd dmsetup create "$name" --table "$(cat $T_RESULTS/dmtable.data)" + data_dev="/dev/mapper/$name" dir="/mnt/test.$i" test -d "$dir" || cmd mkdir -p "$dir" From 2b94cd64687b23975f5613398acc15d5363778af Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 15 Nov 2023 13:20:09 -0800 Subject: [PATCH 2/3] Add loop module kernel message filter Now that we're not setting up per-mount loopback devices we can not have the loop module loaded until tests are running. Signed-off-by: Zach Brown --- tests/funcs/filter.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/funcs/filter.sh b/tests/funcs/filter.sh index 589eafe5..90496554 100644 --- a/tests/funcs/filter.sh +++ b/tests/funcs/filter.sh @@ -86,6 +86,7 @@ t_filter_dmesg() re="$re|scoutfs .* critical transaction commit failure.*" # change-devices causes loop device resizing + re="$re|loop: module loaded" re="$re|loop[0-9].* detected capacity change from.*" # ignore systemd-journal rotating From 03ab5cedb6ef3662dcbc2be8f578c449c25e950c Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 15 Nov 2023 13:58:08 -0800 Subject: [PATCH 3/3] clean up createmany-parallel-mounts test This test is trying to make sure that concurrent work isn't much, much, slower than individual work. It does this by timing creating a bunch of files in a dir on a mount and then timing doing the same in two mounts concurrently. But it messed it up the concurrency pretty badly. It had the concurrent createmany tasks creating files with a full path. That means that every create is trying to read all the parent directories. The way inode number allocation works means that one of the mounts is likely to be getting a write lock that includes a shared parent. This created a ton of cluster lock contention between the two tasks. Then it didn't sync the creates between phases. It could be accidentally recording the time it took to write out the dirty single creates as time taken during the parallel creates. By syncing between phases and having the createmany tasks create files relative to their per-mount directories we actually perform concurrent work and test that we're not creating contention outside of the task load. This became a problem as we switched from loopback devices to device mapper devices. The loopback writers were using buffered writes so we were masking the io cost of constantly invalidating and refilling the item cache by turning the reads into memory copies out of the page cache. While we're in here we actually clean up the created files and then use t_fail to fail the test while the files still exist so they can be examined. Signed-off-by: Zach Brown --- tests/golden/createmany-parallel-mounts | 1 + tests/tests/createmany-parallel-mounts.sh | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/golden/createmany-parallel-mounts b/tests/golden/createmany-parallel-mounts index e314a145..af8d7953 100644 --- a/tests/golden/createmany-parallel-mounts +++ b/tests/golden/createmany-parallel-mounts @@ -1,3 +1,4 @@ == measure initial createmany == measure initial createmany == measure two concurrent createmany runs +== cleanup diff --git a/tests/tests/createmany-parallel-mounts.sh b/tests/tests/createmany-parallel-mounts.sh index bfc786c0..f87cc899 100644 --- a/tests/tests/createmany-parallel-mounts.sh +++ b/tests/tests/createmany-parallel-mounts.sh @@ -7,9 +7,11 @@ t_require_mounts 2 COUNT=50000 -# Prep dirs for test. Each mount needs to make their own parent dir for -# the createmany run, otherwise both dirs will end up in the same inode -# group, causing updates to bounce that lock around. +# +# Prep dirs for test. We have per-directory inode number allocators so +# by putting each createmany in a per-mount dir they get their own inode +# number region and cluster locks. +# echo "== measure initial createmany" mkdir -p $T_D0/dir/0 mkdir $T_D1/dir/1 @@ -17,18 +19,20 @@ mkdir $T_D1/dir/1 echo "== measure initial createmany" START=$SECONDS createmany -o "$T_D0/file_" $COUNT >> $T_TMP.full +sync SINGLE=$((SECONDS - START)) echo single $SINGLE >> $T_TMP.full echo "== measure two concurrent createmany runs" START=$SECONDS -createmany -o $T_D0/dir/0/file $COUNT > /dev/null & +(cd $T_D0/dir/0; createmany -o ./file_ $COUNT > /dev/null) & pids="$!" -createmany -o $T_D1/dir/1/file $COUNT > /dev/null & +(cd $T_D1/dir/1; createmany -o ./file_ $COUNT > /dev/null) & pids="$pids $!" for p in $pids; do wait $p done +sync BOTH=$((SECONDS - START)) echo both $BOTH >> $T_TMP.full @@ -41,7 +45,10 @@ echo both $BOTH >> $T_TMP.full # synchronized operation. FACTOR=200 if [ "$BOTH" -gt $(($SINGLE*$FACTOR)) ]; then - echo "both createmany took $BOTH sec, more than $FACTOR x single $SINGLE sec" + t_fail "both createmany took $BOTH sec, more than $FACTOR x single $SINGLE sec" fi +echo "== cleanup" +find $T_D0/dir -delete + t_pass