[PATCH v2 1/3] fstests: fix call sites that used xfs_io directly

[PATCH v2 1/3] fstests: fix call sites that used xfs_io directly

[Amir Goldstein]

Convert those few remaining call sites to use the XFS_IO_PROG env var.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 common/quota      | 4 ++--
 common/rc         | 4 ++--
 tests/generic/043 | 2 +-
 tests/generic/044 | 4 ++--
 tests/generic/045 | 4 ++--
 tests/generic/046 | 4 ++--
 tests/generic/047 | 2 +-
 tests/generic/048 | 2 +-
 tests/generic/049 | 2 +-
 tests/generic/224 | 4 ++--
 tests/xfs/109     | 4 ++--
 tests/xfs/114     | 4 ++--
 tests/xfs/190     | 4 ++--
 tests/xfs/201     | 2 +-
 tests/xfs/229     | 2 +-
 tests/xfs/250     | 2 +-
 tests/xfs/291     | 2 +-
 17 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/common/quota b/common/quota
index fb2b2a0..678bc43 100644
--- a/common/quota
+++ b/common/quota
@@ -139,7 +139,7 @@ _file_as_id()
 
     parent=`dirname $1`
     if [ $3 = p ]; then
-	echo PARENT: xfs_io -r -c "chproj $2" -c "chattr +P" $parent >>$seqres.full
+	echo PARENT: $XFS_IO_PROG -r -c "chproj $2" -c "chattr +P" $parent >>$seqres.full
 	$XFS_IO_PROG -r -c "chproj $2" -c "chattr +P" $parent >>$seqres.full 2>&1
 	magik='$>'	# (irrelevent, above set projid-inherit-on-parent)
     elif [ $3 = u ]; then
@@ -165,7 +165,7 @@ EOF
 #	exec "dd if=/dev/zero of=$1 bs=$4 count=$5 >>$seqres.full 2>&1";
 
     if [ $3 = p ]; then
-	echo PARENT: xfs_io -r -c "chproj 0" -c "chattr -P" $parent >>$seqres.full
+	echo PARENT: $XFS_IO_PROG -r -c "chproj 0" -c "chattr -P" $parent >>$seqres.full
 	$XFS_IO_PROG -r -c "chproj 0" -c "chattr -P" $parent >>$seqres.full 2>&1
     fi
 }
diff --git a/common/rc b/common/rc
index c3da064..565ba67 100644
--- a/common/rc
+++ b/common/rc
@@ -3796,8 +3796,8 @@ init_rc()
 		exit 1
 	fi
 	# Figure out if we need to add -F ("foreign", deprecated) option to xfs_io
-	xfs_io -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
-	export XFS_IO_PROG="$XFS_IO_PROG -F"
+	$XFS_IO_PROG -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
+		export XFS_IO_PROG="$XFS_IO_PROG -F"
 
 	# xfs_copy doesn't work on v5 xfs yet without -d option
 	if [ "$FSTYP" == "xfs" ] && [[ $MKFS_OPTIONS =~ crc=1 ]]; then
diff --git a/tests/generic/043 b/tests/generic/043
index bd8eef8..b76a5aa 100755
--- a/tests/generic/043
+++ b/tests/generic/043
@@ -50,7 +50,7 @@ i=1;
 while [ $i -lt 1000 ]
 do
 	file=$SCRATCH_MNT/$i
-	xfs_io -f -c "pwrite -b 64k -S 0xff 0 64k" $file > /dev/null
+	$XFS_IO_PROG -f -c "pwrite -b 64k -S 0xff 0 64k" $file > /dev/null
 	if [ $? -ne 0 ]
 	then
 		echo error creating/writing file $file
diff --git a/tests/generic/044 b/tests/generic/044
index f46e828..0331baa 100755
--- a/tests/generic/044
+++ b/tests/generic/044
@@ -50,13 +50,13 @@ i=1;
 while [ $i -lt 1000 ]
 do
 	file=$SCRATCH_MNT/$i
-	xfs_io -f -c "pwrite -b 64k -S 0xff 0 64k" $file > /dev/null
+	$XFS_IO_PROG -f -c "pwrite -b 64k -S 0xff 0 64k" $file > /dev/null
 	if [ $? -ne 0 ]
 	then
 		echo error creating/writing file $file
 		exit
 	fi
-	xfs_io -c "truncate 64k" $file > /dev/null
+	$XFS_IO_PROG -c "truncate 64k" $file > /dev/null
 	if [ $? -ne 0 ]
 	then
 		echo error truncating file $file
diff --git a/tests/generic/045 b/tests/generic/045
index 4ec7650..874c955 100755
--- a/tests/generic/045
+++ b/tests/generic/045
@@ -50,13 +50,13 @@ i=1;
 while [ $i -lt 1000 ]
 do
 	file=$SCRATCH_MNT/$i
-	xfs_io -f -c "pwrite -b 64k -S 0xff 0 64k" $file > /dev/null
+	$XFS_IO_PROG -f -c "pwrite -b 64k -S 0xff 0 64k" $file > /dev/null
 	if [ $? -ne 0 ]
 	then
 		echo error creating/writing file $file
 		exit
 	fi
-	xfs_io -c "truncate 32k" $file > /dev/null
+	$XFS_IO_PROG -c "truncate 32k" $file > /dev/null
 	if [ $? -ne 0 ]
 	then
 		echo error truncating file $file
diff --git a/tests/generic/046 b/tests/generic/046
index 08f1137..5a894b8 100755
--- a/tests/generic/046
+++ b/tests/generic/046
@@ -50,13 +50,13 @@ i=1;
 while [ $i -lt 1000 ]
 do
 	file=$SCRATCH_MNT/$i
-	xfs_io -f -c "pwrite -b 32k -S 0xff 0 32k" $file > /dev/null
+	$XFS_IO_PROG -f -c "pwrite -b 32k -S 0xff 0 32k" $file > /dev/null
 	if [ $? -ne 0 ]
 	then
 		echo error creating/writing file $file
 		exit
 	fi
-	xfs_io -c "truncate 64k" $file > /dev/null
+	$XFS_IO_PROG -c "truncate 64k" $file > /dev/null
 	if [ $? -ne 0 ]
 	then
 		echo error truncating file $file
diff --git a/tests/generic/047 b/tests/generic/047
index b894ee6..631dc1e 100755
--- a/tests/generic/047
+++ b/tests/generic/047
@@ -81,7 +81,7 @@ i=1;
 while [ $i -lt 1000 ]
 do
 	file=$SCRATCH_MNT/$i
-	xfs_io -f -c "pwrite -b 32k -S 0xff 0 32k" -c "fsync" $file > /dev/null
+	$XFS_IO_PROG -f -c "pwrite -b 32k -S 0xff 0 32k" -c "fsync" $file > /dev/null
 	if [ $? -ne 0 ]
 	then
 		echo error creating/writing file $file
diff --git a/tests/generic/048 b/tests/generic/048
index 6f5f444..51d7efd 100755
--- a/tests/generic/048
+++ b/tests/generic/048
@@ -84,7 +84,7 @@ i=1;
 while [ $i -lt 1000 ]
 do
 	file=$SCRATCH_MNT/$i
-	xfs_io -f -c "pwrite -b 64k -S 0xff 0 10m" $file > /dev/null
+	$XFS_IO_PROG -f -c "pwrite -b 64k -S 0xff 0 10m" $file > /dev/null
 	if [ $? -ne 0 ]
 	then
 		echo error creating/writing file $file
diff --git a/tests/generic/049 b/tests/generic/049
index 320318e..1299242 100755
--- a/tests/generic/049
+++ b/tests/generic/049
@@ -81,7 +81,7 @@ i=1;
 while [ $i -lt 1000 ]
 do
 	file=$SCRATCH_MNT/$i
-	xfs_io -f -c "pwrite -b 32k -S 0xff 0 32k" $file > /dev/null
+	$XFS_IO_PROG -f -c "pwrite -b 32k -S 0xff 0 32k" $file > /dev/null
 	if [ $? -ne 0 ]
 	then
 		echo error creating/writing file $file
diff --git a/tests/generic/224 b/tests/generic/224
index 391d877..2c30a75 100755
--- a/tests/generic/224
+++ b/tests/generic/224
@@ -59,7 +59,7 @@ _scratch_mount >> $seqres.full 2>&1
 
 # set the reserved block pool to almost empty for XFS
 if [ "$FSTYP" = "xfs" ]; then
-	xfs_io -x -c "resblks 4" $SCRATCH_MNT >> $seqres.full 2>&1
+	$XFS_IO_PROG -x -c "resblks 4" $SCRATCH_MNT >> $seqres.full 2>&1
 fi
 
 FILES=1000
@@ -71,7 +71,7 @@ for i in `seq 0 1 $FILES`; do
 	# tripped over.
         (
 		sleep 5
-		xfs_io -f -c "truncate 10485760" $SCRATCH_MNT/testfile.$i
+		$XFS_IO_PROG -f -c "truncate 10485760" $SCRATCH_MNT/testfile.$i
 		dd if=/dev/zero of=$SCRATCH_MNT/testfile.$i bs=4k conv=notrunc
 	) > /dev/null 2>&1 &
 done
diff --git a/tests/xfs/109 b/tests/xfs/109
index ac20619..e0fdec3 100755
--- a/tests/xfs/109
+++ b/tests/xfs/109
@@ -50,7 +50,7 @@ populate()
 	i=0
 	while [ $i -le $files -a "X$faststart" = "X" ]; do
 		file=$SCRATCH_MNT/f$i
-		xfs_io -f -d -c 'pwrite -b 64k 0 64k' $file >/dev/null
+		$XFS_IO_PROG -f -d -c 'pwrite -b 64k 0 64k' $file >/dev/null
 		let i=$i+1
 	done
 
@@ -77,7 +77,7 @@ allocate()
 		{
 			j=0
 			while [ $j -lt 100 ]; do
-				xfs_io -f -c 'pwrite -b 64k 0 16m' $file \
+				$XFS_IO_PROG -f -c 'pwrite -b 64k 0 16m' $file \
 					>/dev/null 2>&1
 				rm $file
 				let j=$j+1
diff --git a/tests/xfs/114 b/tests/xfs/114
index 50cc71b..24474f7 100755
--- a/tests/xfs/114
+++ b/tests/xfs/114
@@ -44,7 +44,7 @@ _check_paths()
 	sync; sleep 1
 	echo ""
 	echo "Check parent"
-	if ! xfs_io -x -c 'parent -c' $SCRATCH_MNT | _filter_num; then
+	if ! $XFS_IO_PROG -x -c 'parent -c' $SCRATCH_MNT | _filter_num; then
 		exit 1
 	fi
 }
@@ -55,7 +55,7 @@ _print_names()
 	echo "Print out hardlink names for given path, $1"
 	echo ""
 
-	xfs_io -x -c parent $1 | awk '/p_ino.*=/ {$3 = "inodeXXX"; print; next} {print}' 
+	$XFS_IO_PROG -x -c parent $1 | awk '/p_ino.*=/ {$3 = "inodeXXX"; print; next} {print}'
 }
 
 _test_create()
diff --git a/tests/xfs/190 b/tests/xfs/190
index 614a80c..d688216 100755
--- a/tests/xfs/190
+++ b/tests/xfs/190
@@ -62,8 +62,8 @@ dd if=/dev/zero of=$SCRATCH_MNT/$filename bs=1024k count=10 >> $seqres.full 2>&1
 echo Punching holes in file
 echo Punching holes in file >> $seqres.full
 for i in $holes ; do
-	echo xfs_io -c "unresvsp `echo $i |$SED_PROG 's/:/ /g'`" $SCRATCH_MNT/$filename >> $seqres.full
-	xfs_io -c "unresvsp `echo $i |$SED_PROG 's/:/ /g'`" $SCRATCH_MNT/$filename ;
+	echo $XFS_IO_PROG -c "unresvsp `echo $i |$SED_PROG 's/:/ /g'`" $SCRATCH_MNT/$filename >> $seqres.full
+	$XFS_IO_PROG -c "unresvsp `echo $i |$SED_PROG 's/:/ /g'`" $SCRATCH_MNT/$filename ;
 done
 
 echo Verifying holes are in the correct spots:
diff --git a/tests/xfs/201 b/tests/xfs/201
index ac8abf4..45dc42f 100755
--- a/tests/xfs/201
+++ b/tests/xfs/201
@@ -56,7 +56,7 @@ do_pwrite()
 	end=`expr $2 \* $min_align`
 	length=`expr $end - $offset`
 
-	xfs_io -d -f $file -c "pwrite $offset $length" >/dev/null
+	$XFS_IO_PROG -d -f $file -c "pwrite $offset $length" >/dev/null
 }
 
 _require_scratch
diff --git a/tests/xfs/229 b/tests/xfs/229
index b8fd914..0a42bcf 100755
--- a/tests/xfs/229
+++ b/tests/xfs/229
@@ -60,7 +60,7 @@ EXTSIZE="256k"
 mkdir ${TDIR}
 
 # Set the test directory extsize
-xfs_io -c "extsize ${EXTSIZE}" ${TDIR}
+$XFS_IO_PROG -c "extsize ${EXTSIZE}" ${TDIR}
 
 # Create a set of holey files
 echo "generating ${NFILES} files"
diff --git a/tests/xfs/250 b/tests/xfs/250
index 0cdc382..f807c5a 100755
--- a/tests/xfs/250
+++ b/tests/xfs/250
@@ -81,7 +81,7 @@ _test_loop()
 	mount -t xfs -o loop $LOOP_DEV $LOOP_MNT
 
 	echo "*** preallocate large file"
-	xfs_io -f -c "resvsp 0 $fsize" $LOOP_MNT/foo | _filter_io
+	$XFS_IO_PROG -f -c "resvsp 0 $fsize" $LOOP_MNT/foo | _filter_io
 
 	echo "*** unmount loop filesystem"
 	umount $LOOP_MNT > /dev/null 2>&1
diff --git a/tests/xfs/291 b/tests/xfs/291
index 808f333..c1cbe32 100755
--- a/tests/xfs/291
+++ b/tests/xfs/291
@@ -69,7 +69,7 @@ done
 sync
 
 # Soak up any remaining freespace
-xfs_io -f -c "pwrite 0 16m" -c "fsync" $SCRATCH_MNT/space_file.large >> $seqres.full 2>&1
+$XFS_IO_PROG -f -c "pwrite 0 16m" -c "fsync" $SCRATCH_MNT/space_file.large >> $seqres.full 2>&1
 
 # Take a look at freespace for any post-mortem on the test
 _scratch_unmount
-- 
2.7.4

[PATCH v2 2/3] fstests: strip quotes from "$XFS_IO_PROG" call sites

[Amir Goldstein]

The value of "$XFS_IO_PROG" may contain extra flags after the
binary path (e.g. -F), so it is wrong to use the variable inside
quotes in xfs_io execution call sites.

This bug surfaced while testing the new xfs_io -M flag.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 common/rc         |  6 +++---
 common/reflink    | 26 +++++++++++++-------------
 tests/generic/176 |  2 +-
 tests/xfs/328     |  2 +-
 4 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/common/rc b/common/rc
index 565ba67..a838750 100644
--- a/common/rc
+++ b/common/rc
@@ -90,7 +90,7 @@ _pwrite_byte() {
 	file="$4"
 	xfs_io_args="$5"
 
-	"$XFS_IO_PROG" $xfs_io_args -f -c "pwrite -S $pattern $offset $len" "$file"
+	$XFS_IO_PROG $xfs_io_args -f -c "pwrite -S $pattern $offset $len" "$file"
 }
 
 # mmap-write a byte into a range of a file
@@ -101,7 +101,7 @@ _mwrite_byte() {
 	mmap_len="$4"
 	file="$5"
 
-	"$XFS_IO_PROG" -f -c "mmap -rw 0 $mmap_len" -c "mwrite -S $pattern $offset $len" "$file"
+	$XFS_IO_PROG -f -c "mmap -rw 0 $mmap_len" -c "mwrite -S $pattern $offset $len" "$file"
 }
 
 # ls -l w/ selinux sometimes puts a dot at the end:
@@ -3437,7 +3437,7 @@ _xfs_bmapx_find() {
 	file="$1"
 	shift
 
-	"$XFS_IO_PROG" -c "bmap -${param}lpv" "$file" | grep -c "$@"
+	$XFS_IO_PROG -c "bmap -${param}lpv" "$file" | grep -c "$@"
 }
 
 _require_chattr()
diff --git a/common/reflink b/common/reflink
index 1363971..8b34046 100644
--- a/common/reflink
+++ b/common/reflink
@@ -52,8 +52,8 @@ _require_test_reflink()
 	_require_xfs_io_command "reflink"
 
 	rm -rf "$TEST_DIR/file1" "$TEST_DIR/file2"
-	"$XFS_IO_PROG" -f -c "pwrite -S 0x61 0 65536" "$TEST_DIR/file1" > /dev/null
-	"$XFS_IO_PROG" -f -c "reflink $TEST_DIR/file1 0 0 65536" "$TEST_DIR/file2" > /dev/null
+	$XFS_IO_PROG -f -c "pwrite -S 0x61 0 65536" "$TEST_DIR/file1" > /dev/null
+	$XFS_IO_PROG -f -c "reflink $TEST_DIR/file1 0 0 65536" "$TEST_DIR/file2" > /dev/null
 	if [ ! -s "$TEST_DIR/file2" ]; then
 		rm -rf "$TEST_DIR/file1" "$TEST_DIR/file2"
 		_notrun "Reflink not supported by test filesystem type: $FSTYP"
@@ -69,8 +69,8 @@ _require_scratch_reflink()
 
 	_scratch_mkfs > /dev/null
 	_scratch_mount
-	"$XFS_IO_PROG" -f -c "pwrite -S 0x61 0 65536" "$SCRATCH_MNT/file1" > /dev/null
-	"$XFS_IO_PROG" -f -c "reflink $SCRATCH_MNT/file1 0 0 65536" "$SCRATCH_MNT/file2" > /dev/null
+	$XFS_IO_PROG -f -c "pwrite -S 0x61 0 65536" "$SCRATCH_MNT/file1" > /dev/null
+	$XFS_IO_PROG -f -c "reflink $SCRATCH_MNT/file1 0 0 65536" "$SCRATCH_MNT/file2" > /dev/null
 	if [ ! -s "$SCRATCH_MNT/file2" ]; then
 		_scratch_unmount
 		_notrun "Reflink not supported by scratch filesystem type: $FSTYP"
@@ -85,9 +85,9 @@ _require_test_dedupe()
 	_require_xfs_io_command "dedupe"
 
 	rm -rf "$TEST_DIR/file1" "$TEST_DIR/file2"
-	"$XFS_IO_PROG" -f -c "pwrite -S 0x61 0 65536" "$TEST_DIR/file1" > /dev/null
-	"$XFS_IO_PROG" -f -c "pwrite -S 0x61 0 65536" "$TEST_DIR/file2" > /dev/null
-	testio="$("$XFS_IO_PROG" -f -c "dedupe $TEST_DIR/file1 0 0 65536" "$TEST_DIR/file2" 2>&1)"
+	$XFS_IO_PROG -f -c "pwrite -S 0x61 0 65536" "$TEST_DIR/file1" > /dev/null
+	$XFS_IO_PROG -f -c "pwrite -S 0x61 0 65536" "$TEST_DIR/file2" > /dev/null
+	testio="$($XFS_IO_PROG -f -c "dedupe $TEST_DIR/file1 0 0 65536" "$TEST_DIR/file2" 2>&1)"
 	echo $testio | grep -q "Operation not supported" && \
 		_notrun "Dedupe not supported by test filesystem type: $FSTYP"
 	echo $testio | grep -q "Inappropriate ioctl for device" && \
@@ -105,9 +105,9 @@ _require_scratch_dedupe()
 
 	_scratch_mkfs > /dev/null
 	_scratch_mount
-	"$XFS_IO_PROG" -f -c "pwrite -S 0x61 0 65536" "$SCRATCH_MNT/file1" > /dev/null
-	"$XFS_IO_PROG" -f -c "pwrite -S 0x61 0 65536" "$SCRATCH_MNT/file2" > /dev/null
-	testio="$("$XFS_IO_PROG" -f -c "dedupe $SCRATCH_MNT/file1 0 0 65536" "$SCRATCH_MNT/file2" 2>&1)"
+	$XFS_IO_PROG -f -c "pwrite -S 0x61 0 65536" "$SCRATCH_MNT/file1" > /dev/null
+	$XFS_IO_PROG -f -c "pwrite -S 0x61 0 65536" "$SCRATCH_MNT/file2" > /dev/null
+	testio="$($XFS_IO_PROG -f -c "dedupe $SCRATCH_MNT/file1 0 0 65536" "$SCRATCH_MNT/file2" 2>&1)"
 	echo $testio | grep -q "Operation not supported" && \
 		_notrun "Dedupe not supported by test filesystem type: $FSTYP"
 	echo $testio | grep -q "Inappropriate ioctl for device" && \
@@ -161,7 +161,7 @@ _reflink() {
 	file1="$1"
 	file2="$2"
 
-	"$XFS_IO_PROG" -f -c "reflink $file1" "$file2"
+	$XFS_IO_PROG -f -c "reflink $file1" "$file2"
 }
 
 # Reflink some part of file1 into another part of file2
@@ -173,7 +173,7 @@ _reflink_range() {
 	len="$5"
 	xfs_io_args="$6"
 
-	"$XFS_IO_PROG" $xfs_io_args -f -c "reflink $file1 $offset1 $offset2 $len" "$file2"
+	$XFS_IO_PROG $xfs_io_args -f -c "reflink $file1 $offset1 $offset2 $len" "$file2"
 }
 
 # Dedupe some part of file1 into another part of file2
@@ -185,7 +185,7 @@ _dedupe_range() {
 	len="$5"
 	xfs_io_args="$6"
 
-	"$XFS_IO_PROG" $xfs_io_args -f -c "dedupe $file1 $offset1 $offset2 $len" "$file2"
+	$XFS_IO_PROG $xfs_io_args -f -c "dedupe $file1 $offset1 $offset2 $len" "$file2"
 }
 
 # Create a file of interleaved unwritten and reflinked blocks
diff --git a/tests/generic/176 b/tests/generic/176
index b14255b..76e923a 100755
--- a/tests/generic/176
+++ b/tests/generic/176
@@ -75,7 +75,7 @@ test $fnr -lt 17 && _notrun "Insufficient space for stress test; would only crea
 
 echo "Create a many-block file"
 echo "creating $blocks_needed blocks..." >> "$seqres.full"
-"$XFS_IO_PROG" -f -c "pwrite -S 0x61 -b 4194304 0 $((2 ** (fnr + 1) * blksz))" "$testdir/file1" >> "$seqres.full"
+$XFS_IO_PROG -f -c "pwrite -S 0x61 -b 4194304 0 $((2 ** (fnr + 1) * blksz))" "$testdir/file1" >> "$seqres.full"
 echo "punching..." >> "$seqres.full"
 "$here/src/punch-alternating" "$testdir/file1" >> "$seqres.full"
 echo "...done" >> "$seqres.full"
diff --git a/tests/xfs/328 b/tests/xfs/328
index 33a0bef..8518456 100755
--- a/tests/xfs/328
+++ b/tests/xfs/328
@@ -80,7 +80,7 @@ bytes=$((blocks_needed * blksz))
 echo "Create a many-block file"
 echo "creating $blocks_needed blocks..." >> "$seqres.full"
 _pwrite_byte 0x62 0 $blksz $testdir/file0 >> $seqres.full
-"$XFS_IO_PROG" -f -c "pwrite -S 0x61 -b 4194304 0 $bytes" "$testdir/file1" >> "$seqres.full"
+$XFS_IO_PROG -f -c "pwrite -S 0x61 -b 4194304 0 $bytes" "$testdir/file1" >> "$seqres.full"
 echo "punching..." >> "$seqres.full"
 "$here/src/punch-alternating" "$testdir/file1" >> "$seqres.full"
 seq 0 2 $((2 ** (fnr + 1) )) | while read lblk; do
-- 
2.7.4

[PATCH v2 3/3] fstests: run xfs_io as multi threaded for 'quick' tests

[Amir Goldstein]

Try to run xfs_io for tests in group quick with command line
option -M which starts an idle thread before performing any io.

The purpose of this idle thread is to test io from a multi threaded
process. With single threaded process, the file table is not shared
and file structs are not reference counted.

In order to improve the chance of detecting file struct reference
leaks, we should run xfs_io commands with this option as much as
possible.

Analysis of the effect of xfs_io -M on tests runtime showed that
it may lead to slightly longer run times in extreme cases (e.g +3s
for generic/132), but has a negligable effect on runtime of tests
among the 'quick' group (worst case +0.3s for generic/130).

Therefore, we automatically add the -M flags only to tests in the
'quick' group.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 check     |  2 ++
 common/rc | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/check b/check
index 69341d8..e568598 100755
--- a/check
+++ b/check
@@ -574,6 +574,8 @@ for section in $HOST_OPTIONS_SECTIONS; do
 
 	    mkdir -p $RESULT_DIR
 
+	    export TEST_GROUPS=`grep $(basename $seqnum) "$SRC_DIR/$(dirname $seqnum)/group"`
+
 	    echo -n "$seqnum"
 
 		if $showme; then
diff --git a/common/rc b/common/rc
index a838750..7c478cf 100644
--- a/common/rc
+++ b/common/rc
@@ -3799,6 +3799,21 @@ init_rc()
 	$XFS_IO_PROG -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
 		export XFS_IO_PROG="$XFS_IO_PROG -F"
 
+	if echo $TEST_GROUPS | grep -q quick; then
+		# xfs_io -M flag runs xfs_io as multi threaded process
+		# in order to catch fdget/fdset reference leaks, because
+		# file structs are not reference counted in a single threaded
+		# process.
+		# Because reference counted fdget/fdset may lead to slightly
+		# longer run times in extreme cases (such as generic/132),
+		# we limit the use of -M flags to tests with short runtime,
+		# where the effect of the flag is negligable.
+		#
+		# Figure out if xfs_io supports the -M option
+		$XFS_IO_PROG -M -c quit 2>/dev/null && \
+			export XFS_IO_PROG="$XFS_IO_PROG -M"
+	fi
+
 	# xfs_copy doesn't work on v5 xfs yet without -d option
 	if [ "$FSTYP" == "xfs" ] && [[ $MKFS_OPTIONS =~ crc=1 ]]; then
 		export XFS_COPY_PROG="$XFS_COPY_PROG -d"
-- 
2.7.4

Re: [PATCH v2 3/3] fstests: run xfs_io as multi threaded for 'quick' tests

[Dave Chinner]

On Sun, Oct 16, 2016 at 01:53:42PM +0300, Amir Goldstein wrote:
> Try to run xfs_io for tests in group quick with command line
> option -M which starts an idle thread before performing any io.
> 
> The purpose of this idle thread is to test io from a multi threaded
> process. With single threaded process, the file table is not shared
> and file structs are not reference counted.
> 
> In order to improve the chance of detecting file struct reference
> leaks, we should run xfs_io commands with this option as much as
> possible.
> 
> Analysis of the effect of xfs_io -M on tests runtime showed that
> it may lead to slightly longer run times in extreme cases (e.g +3s
> for generic/132), but has a negligable effect on runtime of tests
> among the 'quick' group (worst case +0.3s for generic/130).
> 
> Therefore, we automatically add the -M flags only to tests in the
> 'quick' group.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  check     |  2 ++
>  common/rc | 15 +++++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/check b/check
> index 69341d8..e568598 100755
> --- a/check
> +++ b/check
> @@ -574,6 +574,8 @@ for section in $HOST_OPTIONS_SECTIONS; do
>  
>  	    mkdir -p $RESULT_DIR
>  
> +	    export TEST_GROUPS=`grep $(basename $seqnum) "$SRC_DIR/$(dirname $seqnum)/group"`
> +

Do we really want to go down this path of changing behaviours
of utilities based on what group  they belong to? It means we can no
longer look at the test code and recreate the command line without
having to work out what group context the test is running under. And
how do we tell that it's correct and we don't inadvertantly break
it? I ask this because.....

>  	    echo -n "$seqnum"
>  
>  		if $showme; then
> diff --git a/common/rc b/common/rc
> index a838750..7c478cf 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3799,6 +3799,21 @@ init_rc()
>  	$XFS_IO_PROG -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
>  		export XFS_IO_PROG="$XFS_IO_PROG -F"
>  
> +	if echo $TEST_GROUPS | grep -q quick; then
> +		# xfs_io -M flag runs xfs_io as multi threaded process
> +		# in order to catch fdget/fdset reference leaks, because
> +		# file structs are not reference counted in a single threaded
> +		# process.
> +		# Because reference counted fdget/fdset may lead to slightly
> +		# longer run times in extreme cases (such as generic/132),
> +		# we limit the use of -M flags to tests with short runtime,
> +		# where the effect of the flag is negligable.
> +		#
> +		# Figure out if xfs_io supports the -M option
> +		$XFS_IO_PROG -M -c quit 2>/dev/null && \
> +			export XFS_IO_PROG="$XFS_IO_PROG -M"
> +	fi
> +

.... I can't see how this works - init_rc is
called and sets $XFS_IO_PROG long before TEST_GROUPS is set and
exported. How did you verify that the correct xfs_io command lines
were being issued?

IMO, either turn it on for everything if it is supported, or make it
an explicit command line option to enable. A couple of seconds of
extra runtime here and there means nothing for a typical auto test
run which can take hours to run....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v2 3/3] fstests: run xfs_io as multi threaded for 'quick' tests

[Amir Goldstein]

On Mon, Oct 17, 2016 at 12:46 AM, Dave Chinner <david@fromorbit.com> wrote:
> On Sun, Oct 16, 2016 at 01:53:42PM +0300, Amir Goldstein wrote:
>> Try to run xfs_io for tests in group quick with command line
>> option -M which starts an idle thread before performing any io.
>>
>> The purpose of this idle thread is to test io from a multi threaded
>> process. With single threaded process, the file table is not shared
>> and file structs are not reference counted.
>>
>> In order to improve the chance of detecting file struct reference
>> leaks, we should run xfs_io commands with this option as much as
>> possible.
>>
>> Analysis of the effect of xfs_io -M on tests runtime showed that
>> it may lead to slightly longer run times in extreme cases (e.g +3s
>> for generic/132), but has a negligable effect on runtime of tests
>> among the 'quick' group (worst case +0.3s for generic/130).
>>
>> Therefore, we automatically add the -M flags only to tests in the
>> 'quick' group.
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>>  check     |  2 ++
>>  common/rc | 15 +++++++++++++++
>>  2 files changed, 17 insertions(+)
>>
>> diff --git a/check b/check
>> index 69341d8..e568598 100755
>> --- a/check
>> +++ b/check
>> @@ -574,6 +574,8 @@ for section in $HOST_OPTIONS_SECTIONS; do
>>
>>           mkdir -p $RESULT_DIR
>>
>> +         export TEST_GROUPS=`grep $(basename $seqnum) "$SRC_DIR/$(dirname $seqnum)/group"`
>> +
>
> Do we really want to go down this path of changing behaviours
> of utilities based on what group  they belong to? It means we can no
> longer look at the test code and recreate the command line without
> having to work out what group context the test is running under. And
> how do we tell that it's correct and we don't inadvertantly break
> it? I ask this because.....
>
>>           echo -n "$seqnum"
>>
>>               if $showme; then
>> diff --git a/common/rc b/common/rc
>> index a838750..7c478cf 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -3799,6 +3799,21 @@ init_rc()
>>       $XFS_IO_PROG -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
>>               export XFS_IO_PROG="$XFS_IO_PROG -F"
>>
>> +     if echo $TEST_GROUPS | grep -q quick; then
>> +             # xfs_io -M flag runs xfs_io as multi threaded process
>> +             # in order to catch fdget/fdset reference leaks, because
>> +             # file structs are not reference counted in a single threaded
>> +             # process.
>> +             # Because reference counted fdget/fdset may lead to slightly
>> +             # longer run times in extreme cases (such as generic/132),
>> +             # we limit the use of -M flags to tests with short runtime,
>> +             # where the effect of the flag is negligable.
>> +             #
>> +             # Figure out if xfs_io supports the -M option
>> +             $XFS_IO_PROG -M -c quit 2>/dev/null && \
>> +                     export XFS_IO_PROG="$XFS_IO_PROG -M"
>> +     fi
>> +
>
> .... I can't see how this works - init_rc is
> called and sets $XFS_IO_PROG long before TEST_GROUPS is set and
> exported.

Correct, but common/rc is then included again from every single test,
so XFS_IO_PROG is adjusted to add the -M/i flag per test after
TEST_GROUPS is set for a specific test in ./check.
I saw the -F flag code and figured that's a good place to add -M/i as well.
With a hunch, I would say that if -F where to be added, it where to be
added twice.

> How did you verify that the correct xfs_io command lines
> were being issued?
>

Both by adding debug print and by monitoring ps

> IMO, either turn it on for everything if it is supported, or make it
> an explicit command line option to enable.

If I make the default off, then not enough people will test for reference leaks.
Since leaks may be hard to find, because they may only be on rare error path,
the motivation for a test suit IMO is to enforce this option as much
as possible.

OTOH, if everyone, always run with -i, then the fast path fdget is
never tested with
xfs_io. fast path will get tested with other tools used by tests
though, so maybe
that is not such a big concern.

> A couple of seconds of
> extra runtime here and there means nothing for a typical auto test
> run which can take hours to run....
>

In that case, I think we should turn the flag always if supported.
After all, even it tests take longer to run that's exactly the sort of penalty
that should be acceptable to pay for running with extra debugging options
enabled.

As an extra, I suggested we could turn the flag off for people running the
test with non debug kernel, or say, without CONFIG_PROVE_LOCKING.
If there are people doing that, and I imagine there are, they may care more
about test runtime and less about detecting leaks?

> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com

Re: [PATCH v2 3/3] fstests: run xfs_io as multi threaded for 'quick' tests

[Amir Goldstein]

On Mon, Oct 17, 2016 at 10:01 AM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Mon, Oct 17, 2016 at 12:46 AM, Dave Chinner <david@fromorbit.com> wrote:
>> On Sun, Oct 16, 2016 at 01:53:42PM +0300, Amir Goldstein wrote:
>>> Try to run xfs_io for tests in group quick with command line
>>> option -M which starts an idle thread before performing any io.
>>>
>>> The purpose of this idle thread is to test io from a multi threaded
>>> process. With single threaded process, the file table is not shared
>>> and file structs are not reference counted.
>>>
>>> In order to improve the chance of detecting file struct reference
>>> leaks, we should run xfs_io commands with this option as much as
>>> possible.
>>>
>>> Analysis of the effect of xfs_io -M on tests runtime showed that
>>> it may lead to slightly longer run times in extreme cases (e.g +3s
>>> for generic/132), but has a negligable effect on runtime of tests
>>> among the 'quick' group (worst case +0.3s for generic/130).
>>>
>>> Therefore, we automatically add the -M flags only to tests in the
>>> 'quick' group.
>>>
>>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>>> ---

...

>> A couple of seconds of
>> extra runtime here and there means nothing for a typical auto test
>> run which can take hours to run....
>>
>

Just to follow up on the extra runtime.
I did one auto test run at 1h42m without -i flag
and then auto test run at 1h45m with -i flag
I did not repeat several runs to reduce noise.
If anyone thinks that difference is too expensive please shout.

Amir.

Re: [PATCH v2 3/3] fstests: run xfs_io as multi threaded for 'quick' tests

[Eryu Guan]

On Mon, Oct 17, 2016 at 10:01:19AM +0300, Amir Goldstein wrote:
> On Mon, Oct 17, 2016 at 12:46 AM, Dave Chinner <david@fromorbit.com> wrote:
> > On Sun, Oct 16, 2016 at 01:53:42PM +0300, Amir Goldstein wrote:
> >> Try to run xfs_io for tests in group quick with command line
> >> option -M which starts an idle thread before performing any io.
> >>
> >> The purpose of this idle thread is to test io from a multi threaded
> >> process. With single threaded process, the file table is not shared
> >> and file structs are not reference counted.
> >>
> >> In order to improve the chance of detecting file struct reference
> >> leaks, we should run xfs_io commands with this option as much as
> >> possible.
> >>
> >> Analysis of the effect of xfs_io -M on tests runtime showed that
> >> it may lead to slightly longer run times in extreme cases (e.g +3s
> >> for generic/132), but has a negligable effect on runtime of tests
> >> among the 'quick' group (worst case +0.3s for generic/130).
> >>
> >> Therefore, we automatically add the -M flags only to tests in the
> >> 'quick' group.
> >>
> >> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >> ---
> >>  check     |  2 ++
> >>  common/rc | 15 +++++++++++++++
> >>  2 files changed, 17 insertions(+)
> >>
> >> diff --git a/check b/check
> >> index 69341d8..e568598 100755
> >> --- a/check
> >> +++ b/check
> >> @@ -574,6 +574,8 @@ for section in $HOST_OPTIONS_SECTIONS; do
> >>
> >>           mkdir -p $RESULT_DIR
> >>
> >> +         export TEST_GROUPS=`grep $(basename $seqnum) "$SRC_DIR/$(dirname $seqnum)/group"`
> >> +
> >
> > Do we really want to go down this path of changing behaviours
> > of utilities based on what group  they belong to? It means we can no
> > longer look at the test code and recreate the command line without
> > having to work out what group context the test is running under. And
> > how do we tell that it's correct and we don't inadvertantly break
> > it? I ask this because.....
> >
> >>           echo -n "$seqnum"
> >>
> >>               if $showme; then
> >> diff --git a/common/rc b/common/rc
> >> index a838750..7c478cf 100644
> >> --- a/common/rc
> >> +++ b/common/rc
> >> @@ -3799,6 +3799,21 @@ init_rc()
> >>       $XFS_IO_PROG -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
> >>               export XFS_IO_PROG="$XFS_IO_PROG -F"
> >>
> >> +     if echo $TEST_GROUPS | grep -q quick; then
> >> +             # xfs_io -M flag runs xfs_io as multi threaded process
> >> +             # in order to catch fdget/fdset reference leaks, because
> >> +             # file structs are not reference counted in a single threaded
> >> +             # process.
> >> +             # Because reference counted fdget/fdset may lead to slightly
> >> +             # longer run times in extreme cases (such as generic/132),
> >> +             # we limit the use of -M flags to tests with short runtime,
> >> +             # where the effect of the flag is negligable.
> >> +             #
> >> +             # Figure out if xfs_io supports the -M option
> >> +             $XFS_IO_PROG -M -c quit 2>/dev/null && \
> >> +                     export XFS_IO_PROG="$XFS_IO_PROG -M"
> >> +     fi
> >> +
> >
> > .... I can't see how this works - init_rc is
> > called and sets $XFS_IO_PROG long before TEST_GROUPS is set and
> > exported.
> 
> Correct, but common/rc is then included again from every single test,
> so XFS_IO_PROG is adjusted to add the -M/i flag per test after
> TEST_GROUPS is set for a specific test in ./check.
> I saw the -F flag code and figured that's a good place to add -M/i as well.
> With a hunch, I would say that if -F where to be added, it where to be
> added twice.
> 
> > How did you verify that the correct xfs_io command lines
> > were being issued?
> >
> 
> Both by adding debug print and by monitoring ps
> 
> > IMO, either turn it on for everything if it is supported, or make it
> > an explicit command line option to enable.
> 
> If I make the default off, then not enough people will test for reference leaks.
> Since leaks may be hard to find, because they may only be on rare error path,
> the motivation for a test suit IMO is to enforce this option as much
> as possible.
> 
> OTOH, if everyone, always run with -i, then the fast path fdget is
> never tested with
> xfs_io. fast path will get tested with other tools used by tests
> though, so maybe
> that is not such a big concern.

[Sorry for the late response, I'm travelling this week and the network
connection is not as good as I expected.]

I'm still having concerns about losing test coverage by enabling "-i" by
default. How about adding an command line option to disable it? So at
least we could have a way to turn it off. Or is it completely impossible
to lose any test coverage?

Thanks,
Eryu

Re: [PATCH v2 3/3] fstests: run xfs_io as multi threaded for 'quick' tests

[Amir Goldstein]

On Thu, Oct 20, 2016 at 5:25 PM, Eryu Guan <eguan@redhat.com> wrote:
> On Mon, Oct 17, 2016 at 10:01:19AM +0300, Amir Goldstein wrote:

...

>
> I'm still having concerns about losing test coverage by enabling "-i" by
> default. How about adding an command line option to disable it?

I could take the easy way and add a command line option to satisfy
concerns, but we all know that 99% of the time, nobody is going to use it,
so we will be just silencing out concerns instead of addressing them.

> So at
> least we could have a way to turn it off. Or is it completely impossible
> to lose any test coverage?

I would not argue that it is completely impossible to loose test coverage
but I would argue that there is low probability of loosing *interesting* test
coverage.

Here is the argument I am making:
A file reference leak can be anywhere, (e.g in EXDEV error path of clone
file range ioctl) so we SHOULD have -i test coverage for as many APIs
as possible with as many arguments as possible.
OTOH, the difference between slowpath and fastpath of fdget()/fdput()
for single/multi threaded process is quite invariant to the specific API,
so I do not see the value in test coverage of fastpath to all APIs.

Anyway, I'd be happy to add a command line option or env var if that is
the preference of folks, but then I would really appreciate a suggestion
for option name or var name, because it is going to be quite hard to
find an intuitive name for this test variant.

Thanks,
Amir.

Re: [PATCH v2 3/3] fstests: run xfs_io as multi threaded for 'quick' tests

[Eryu Guan]

On Thu, Oct 20, 2016 at 09:27:03PM +0300, Amir Goldstein wrote:
> On Thu, Oct 20, 2016 at 5:25 PM, Eryu Guan <eguan@redhat.com> wrote:
> > On Mon, Oct 17, 2016 at 10:01:19AM +0300, Amir Goldstein wrote:
> 
> ...
> 
> >
> > I'm still having concerns about losing test coverage by enabling "-i" by
> > default. How about adding an command line option to disable it?
> 
> I could take the easy way and add a command line option to satisfy
> concerns, but we all know that 99% of the time, nobody is going to use it,
> so we will be just silencing out concerns instead of addressing them.
> 
> > So at
> > least we could have a way to turn it off. Or is it completely impossible
> > to lose any test coverage?
> 
> I would not argue that it is completely impossible to loose test coverage
> but I would argue that there is low probability of loosing *interesting* test
> coverage.
> 
> Here is the argument I am making:
> A file reference leak can be anywhere, (e.g in EXDEV error path of clone
> file range ioctl) so we SHOULD have -i test coverage for as many APIs
> as possible with as many arguments as possible.
> OTOH, the difference between slowpath and fastpath of fdget()/fdput()
> for single/multi threaded process is quite invariant to the specific API,
> so I do not see the value in test coverage of fastpath to all APIs.

Then I'll take it as it is, and we can always add a new option to turn
it off when we find it necessary :) Thanks!

Eryu