On Sun, Nov 04, 2018 at 02:21:21PM -0600, Jayashree Mohan wrote:
> Hi Eryu and Filipe,
>
> I have incorporated the coding style suggested, and renamed the cleanup function. However, creating a clean fs image after each sub test is resulting in about 10-15s of additional overhead overall. I instead clean up the working directory and unmount. The time for the tests varies between 12-15 seconds.
>
> Please find the patch below
>
> —
>
> diff --git a/tests/generic/517-link b/tests/generic/517-link
> new file mode 100755
> index 0000000..ea5c5b7
> --- /dev/null
> +++ b/tests/generic/517-link
> @@ -0,0 +1,164 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2018 The University of Texas at Austin. All Rights Reserved.
> +#
> +# FS QA Test 517-link
> +#
> +# Test case created by CrashMonkey
> +#
> +# Test if we create a hard link to a file and persist either of the files, all the names persist.
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> + _cleanup_flakey
> + cd /
> + rm -f $tmp.*
> +}
> +
> +init_start_time=$(date +%s.%N)
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmflakey
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch_nocheck
> +_require_dm_target flakey
> +
> +# initialize scratch device
> +_scratch_mkfs >> $seqres.full 2>&1
> +_require_metadata_journaling $SCRATCH_DEV
> +_init_flakey
> +init_end_time=$(date +%s.%N)
> +init_time_elapsed=$(echo "$init_end_time - $init_start_time" | bc)
> +echo "Initialization time = $init_time_elapsed" >> $seqres.full
No timing inside the test, please. The harness times the test
execution.
> +stat_opt='-c "blocks: %b size: %s inode: %i links: %h"'
> +test_num=0
Don't number tests. If we add an extra test, it will completely
break the golden output.
> +total_time=0
> +
> +_clean_dir()
> +{
> + _mount_flakey
> + rm -rf $SCRATCH_MNT/*
> + sync
> + _unmount_flakey
> +}
Why not just _scratch_mkfs?
> +
> +# create a hard link $2 to file $1, and fsync $3, followed by power-cut
> +test_link()
> +{
> + test_num=$((test_num + 1))
> + local start_time=$(date +%s.%N)
> + local sibling=0
> + local before=""
> + local after=""
> + echo -ne "\n=== Test $test_num : link $1 $2 " | _filter_scratch
> + _mount_flakey
> +
Still whitespace damaged.
> + # now execute the workload
> + mkdir -p "${1%/*}" && mkdir -p "${2%/*}" && touch "$1"
One line each.
> + ln $1 $2
No checks for failure?
> +
> + if [ -z "$3" ]; then
> + echo -ne " with sync ===\n"
> + sync
> + before=`stat "$stat_opt" $1`
> + else
> + echo " with fsync $3 ===" | _filter_scratch
> +
> + # If the file being persisted is a sibling, create it first
> + if [ ! -f $3 ]; then
> + sibling=1
> + touch $3
> + fi
> +
> + $XFS_IO_PROG -c "fsync" $3
> +
> + if [ $sibling -ne 1 ]; then
> + before=`stat "$stat_opt" $1`
> + fi
> + fi
To tell the truth, I'd consider these two separate functions -
test_link_sync() and test_link_fsync(). More below.
> +
> + _flakey_drop_and_remount | tee -a $seqres.full
> +
> + if [ -f $1 ]; then
> + after=`stat "$stat_opt" $1`
> + fi
> +
> + if [ "$before" != "$after" ] && [ $sibling -ne 1 ]; then
> + echo "Before: $before" | tee -a $seqres.full
> + echo "After: $after" | tee -a $seqres.full
> + fi
Why tee all the output to $seqres.full? Once the timing info is
removed, it dones't contain anything extra that the normal output
file...
> +
> + _unmount_flakey
> + _check_scratch_fs $FLAKEY_DEV
> + [ $? -ne 0 ] && _fatal "fsck failed"
> + end_time=$(date +%s.%N)
> + time_elapsed=$(echo "$end_time - $start_time" | bc)
> + echo " Elapsed time : $time_elapsed" >> $seqres.full
> + total_time=$(echo "$total_time + $time_elapsed" | bc)
> + echo " Total time : $total_time" >> $seqres.full
> + _clean_dir
> +}
> +
> +# run the link test for different combinations
> +
> +test_start_time=$(date +%s.%N)
> +# Group 1: Both files within root directory
> +for file_name in $SCRATCH_MNT $SCRATCH_MNT/foo $SCRATCH_MNT/bar; do
> + test_link $SCRATCH_MNT/foo $SCRATCH_MNT/bar $file_name
> +done
> +test_link $SCRATCH_MNT/foo $SCRATCH_MNT/bar
> +
> +# Group 2: Create hard link in a sub directory
> +for file_name in $SCRATCH_MNT $SCRATCH_MNT/foo $SCRATCH_MNT/bar $SCRATCH_MNT/A $SCRATCH_MNT/A/bar $SCRATCH_MNT/A/foo; do
> + test_link $SCRATCH_MNT/foo $SCRATCH_MNT/A/bar $file_name
> +done
> +test_link $SCRATCH_MNT/foo $SCRATCH_MNT/A/bar
Lines too long.
Basically, this is a two dimensional array of filenames.
Make test_link_fsync() do:
src=$SCRATCH_MNT/$1
dst=$SCRATCH_MNT/$2
fsync=$SCRATCH_MNT/$3
And define your fsync filenames and link destinations names like so
(can't remember bash array notation, so paraphrasing):
# group 1: Both files within root directory
fnames[1]="./ foo bar"
dnames[1]="foo bar"
# group 2: Create hard link in a sub directory
fnames[2]="./ foo bar A A/bar A/foo"
dnames[2]="foo A/bar"
#g3
fnames[3]="./ foo bar A A/bar A/foo"
dnames[3]="A/foo bar"
#g4
fnames[4]="./ A A/bar A/foo"
dnames[4]="A/foo A/bar"
#g5
fnames[5]="./ foo bar A A/bar A/foo"
dnames[5]="bar A/bar"
#g6
fnames[6]="./ foo bar A A/bar A/foo"
dnames[6]="A/bar bar"
for ((i=1; i<=6; i++)); do
for f in $fnames[$i]; do
test_link_fsync $dnames[$i] $f
done
test_link_sync $dnames[1]
done
And all this shouty, shouty noise goes away. Much easier to read,
much simpler to maintain.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
