[PATCH 1/3] Add new standard loop handling functions

[PATCH 1/3] Add new standard loop handling functions

[Tomas Racek]

Add _create_loop_device and _destroy_loop_device to uniformly handle
loopback devices.

Signed-off-by: Tomas Racek <tracek@redhat.com>
---
 common.rc | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/common.rc b/common.rc
index 966fc93..b5edd22 100644
--- a/common.rc
+++ b/common.rc
@@ -1804,6 +1804,21 @@ _test_batched_discard()
 	$FSTRIM_PROG ${1} &>/dev/null
 }
 
+_create_loop_device()
+{
+	file=$1
+	dev=`losetup -f`
+	losetup $dev $file || _fail "Cannot associate $file with $dev"
+	echo $dev
+}
+
+_destroy_loop_device()
+{
+	dev=$1
+	losetup -d $dev || _fail "Cannot destroy loop device $dev"
+}
+
+
 ################################################################################
 
 if [ "$iam" != new -a "$iam" != bench ]
-- 
1.7.11.7

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/3] Provide dumpe2fs via standard common.config interface

[Tomas Racek]

dumpe2fs can be now accessed via $DUMPE2FS, tests that require it can
check for its availability by _require_dumpe2fs function.

Signed-off-by: Tomas Racek <tracek@redhat.com>
---
 common.config | 1 +
 common.rc     | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/common.config b/common.config
index 57f505d..7174127 100644
--- a/common.config
+++ b/common.config
@@ -159,6 +159,7 @@ export KILLALL_PROG="`set_prog_path killall`"
 export INDENT_PROG="`set_prog_path indent`"
 export XFS_COPY_PROG="`set_prog_path xfs_copy`"
 export FSTRIM_PROG="`set_prog_path fstrim`"
+export DUMPE2FS_PROG="`set_prog_path dumpe2fs`"
 
 # Generate a comparable xfsprogs version number in the form of
 # major * 10000 + minor * 100 + release
diff --git a/common.rc b/common.rc
index b5edd22..6d2d5b9 100644
--- a/common.rc
+++ b/common.rc
@@ -1804,6 +1804,13 @@ _test_batched_discard()
 	$FSTRIM_PROG ${1} &>/dev/null
 }
 
+_require_dumpe2fs()
+{
+	if [ -z "$DUMPE2FS_PROG" ]; then
+		_notrun "This test requires dumpe2fs utility."
+	fi
+}
+
 _create_loop_device()
 {
 	file=$1
-- 
1.7.11.7

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/3] Provide dumpe2fs via standard common.config interface

[Rich Johnston]

On 10/18/2012 03:45 AM, Tomas Racek wrote:
> dumpe2fs can be now accessed via $DUMPE2FS, tests that require it can
> check for its availability by _require_dumpe2fs function.
>
> Signed-off-by: Tomas Racek <tracek@redhat.com>
> ---
>   common.config | 1 +
>   common.rc     | 7 +++++++
>   2 files changed, 8 insertions(+)
>

Tomas,
	Looks good.
--Rich

Reviewed-by: Rich Johnston <rjohnston@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/3] 289: Test that filesystem sends discard requests only on free sectors

[Tomas Racek]

This is done by comparing free sectors reported by some FS utility
(dumpe2fs/xfs_db) and actual discard commands sent to device obtained
via blk tracer in debugfs.

Currently supported FS are ext[34], xfs; device with discard support is
not required, the test creates loop device for this purpose.

Signed-off-by: Tomas Racek <tracek@redhat.com>
---
 289     | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 289.out |   5 ++
 group   |   1 +
 3 files changed, 175 insertions(+)
 create mode 100755 289
 create mode 100644 289.out

diff --git a/289 b/289
new file mode 100755
index 0000000..b6bffc1
--- /dev/null
+++ b/289
@@ -0,0 +1,169 @@
+#! /bin/bash
+# FS QA Test No. 289
+#
+# Test that filesystem sends discard requests only on free blocks
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2012 Red Hat, Inc., Tomas Racek <tracek@redhat.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+# creator
+owner=tracek@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=`mktemp -d`
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	$UMOUNT_PROG $loop_mnt
+	rm -rf $tmp
+	_destroy_loop_device $loop_dev
+	rm $img_file
+}
+
+mkfs_loop()
+{
+	if [ $FSTYP = "xfs" ]; then
+		MKFS_OPTIONS="-f $MKFS_OPTIONS"
+	fi
+
+	$MKFS_PROG -t $FSTYP $MKFS_OPTIONS $loop_dev &> /dev/null
+}
+
+get_block_size()
+{
+	case $FSTYP in
+	ext[34])
+		bsize=$($DUMPE2FS_PROG $loop_dev 2>&1 | sed -n '/^Block size/ s/.*: *\(.*\)/\1/p')
+	;;
+	xfs)
+		$UMOUNT_PROG $loop_mnt
+		bsize=$($XFS_DB_PROG -c "sb" -c"p" $loop_dev | sed -n 's/^blocksize = \([0-9]\+\).*/\1/p')
+		$MOUNT_PROG $loop_dev $loop_mnt
+	;;
+	esac
+}
+
+get_free_sectors()
+{
+	case $FSTYP in
+	ext[34])
+		size=1
+		clstsize=$($DUMPE2FS_PROG $loop_dev 2>/dev/null | sed '/Cluster size/!d; s/.*: *//')
+		if [ -n "$clstsize" ]; then
+			size=$((clstsize/bsize))
+		fi
+		$DUMPE2FS_PROG $loop_dev 2>/dev/null | sed '/  Free blocks/!d; s/.*: //; s/, /\n/g; /^$/d' | \
+			$AWK_PROG -v spb=$spb -v size=$size 'BEGIN{FS="-"}; {printf "%d-%d\n", spb * $1, (spb * $2 == 0)? spb * ($1 + size) - 1 : spb * ($2 + size) - 1}' > $free_sectors
+	;;
+	xfs)
+		$UMOUNT_PROG $loop_mnt
+		agblocks=$($XFS_DB_PROG -c"sb" -c "p" $loop_dev | sed -n 's/agblocks = \(.*\)$/\1/p')
+		$XFS_DB_PROG -c"freesp -d" $loop_dev | $AWK_PROG '{if($1 == "from") exit; else print}' | \
+			$AWK_PROG -v agb=$agblocks -v spb=$spb '{print spb * ($1 * agb + $2) "-" spb * ($1 * agb + $2 + $3) - 1}' | sort -n > $free_sectors
+		$MOUNT_PROG $loop_dev $loop_mnt
+	;;
+	esac
+}
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.config
+
+# real QA test starts here
+
+_supported_fs ext3 ext4 xfs
+_supported_os Linux
+_require_fstrim
+_require_dumpe2fs
+_require_fs_space $TEST_DIR 3145728
+
+debugfs=$($MOUNT_PROG | grep debugfs | cut -d " " -f3)
+[ -n $debugfs ] || _notrun "This test requires mounted debugfs"
+
+cat $debugfs/tracing/available_tracers | grep -q blk || _notrun "blk tracer is not available"
+
+img_file=$TEST_DIR/$$.fs
+$XFS_IO_PROG -f -c "truncate 3G" $img_file
+
+loop_dev=$(_create_loop_device $img_file)
+loop_base=$(basename $loop_dev)
+loop_mnt=$tmp/loop_mnt
+
+mkdir $loop_mnt
+
+mkfs_loop
+$MOUNT_PROG $loop_dev $loop_mnt
+
+echo -n "Generating garbage on loop..."
+for i in `seq 1 30`; do
+
+	mkdir $loop_mnt/$i
+	cp -r $here/* $loop_mnt/$i
+done
+find $loop_mnt -type f -print | $AWK_PROG 'BEGIN {srand()}; {if(rand() > 0.7) print $1;}' | xargs rm
+echo "done."
+
+ssize=`cat /sys/block/$loop_base/queue/hw_sector_size`
+get_block_size
+spb=$((bsize/ssize))
+
+free_sectors=$tmp/sectors
+trace=$tmp/trace
+
+echo -n "Obtaining free sectors from FS..."
+get_free_sectors
+echo "done."
+
+echo -n "Running fstrim & trace..."
+echo 1 > /sys/block/$loop_base/trace/enable
+echo blk > $debugfs/tracing/current_tracer
+echo > $debugfs/tracing/trace
+echo 1 > $debugfs/tracing/tracing_on
+
+$FSTRIM_PROG $loop_mnt > /dev/null
+
+cat $debugfs/tracing/trace | grep "\[fstrim\]"  | cut -d ":" -f2- |
+$AWK_PROG '$3 == "D"' |  		   # Filter discard operation
+$AWK_PROG '{print $4 "-" $4 + $6 - 1}' | # Transform (start, length) => (start, end)
+sort -n > $trace
+
+echo 0 > $debugfs/tracing/tracing_on
+echo 0 > /sys/block/$loop_base/trace/enable
+echo "done."
+
+echo -n "Comparing trace to free sectors..."
+while read line
+do
+	start=${line%-*}
+	end=${line#*-}
+	# Check if sectors obtained from trace can be matched with free sectors
+	if ! $AWK_PROG -v s=$start -v e=$end 'BEGIN {FS="-"}; {if ($1 <= s && e <= $2) found = 1}; END{if (found) exit 0; else exit 1}' $free_sectors
+	then
+		echo "Sectors $line are not marked as free by FS!"
+		exit
+	fi
+done <$trace
+echo "done."
+
+# success, all done
+status=0
+exit
diff --git a/289.out b/289.out
new file mode 100644
index 0000000..90465a6
--- /dev/null
+++ b/289.out
@@ -0,0 +1,5 @@
+QA output created by 289
+Generating garbage on loop...done.
+Obtaining free sectors from FS...done.
+Running fstrim & trace...done.
+Comparing trace to free sectors...done.
diff --git a/group b/group
index f52c5d5..ead4911 100644
--- a/group
+++ b/group
@@ -406,3 +406,4 @@ deprecated
 285 auto rw
 286 other
 287 auto dump quota quick
+289 auto trim
-- 
1.7.11.7

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/3] 289: Test that filesystem sends discard requests only on free sectors

[Rich Johnston]

On 10/18/2012 03:45 AM, Tomas Racek wrote:
> This is done by comparing free sectors reported by some FS utility
> (dumpe2fs/xfs_db) and actual discard commands sent to device obtained
> via blk tracer in debugfs.
>
> Currently supported FS are ext[34], xfs; device with discard support is
> not required, the test creates loop device for this purpose.
>
> Signed-off-by: Tomas Racek <tracek@redhat.com>
> ---
>   289     | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   289.out |   5 ++
>   group   |   1 +
>   3 files changed, 175 insertions(+)
>   create mode 100755 289
>   create mode 100644 289.out
>


This will not work for 3.x kernels, but they 2.6 and 3.x both use the 
same path.

> +debugfs=$($MOUNT_PROG | grep debugfs | cut -d " " -f3)
> +[ -n $debugfs ] || _notrun "This test requires mounted debugfs"

As you use $debugfs quite a bit from here on, suggest doing something like:

debugfs=/sys/kernel/debug
if [ ! -d $debugfs ]; then
    _notrun "This test requires mounted debugfs"
fi

Other than that it looks great.


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/3] 289: Test that filesystem sends discard requests only on free sectors

[Dave Chinner]

On Thu, Oct 18, 2012 at 10:45:46AM +0200, Tomas Racek wrote:
> This is done by comparing free sectors reported by some FS utility
> (dumpe2fs/xfs_db) and actual discard commands sent to device obtained
> via blk tracer in debugfs.
> 
> Currently supported FS are ext[34], xfs; device with discard support is
> not required, the test creates loop device for this purpose.
....
> +mkfs_loop()
> +{
> +	if [ $FSTYP = "xfs" ]; then
> +		MKFS_OPTIONS="-f $MKFS_OPTIONS"
> +	fi
> +
> +	$MKFS_PROG -t $FSTYP $MKFS_OPTIONS $loop_dev &> /dev/null

You don't need the "-t $FSTYP" here - the mkfs program is already
set to the specific filesytem type beign tested, anyway.

> +get_block_size()
> +{
> +	case $FSTYP in
> +	ext[34])
> +		bsize=$($DUMPE2FS_PROG $loop_dev 2>&1 | sed -n '/^Block size/ s/.*: *\(.*\)/\1/p')
> +	;;
> +	xfs)
> +		$UMOUNT_PROG $loop_mnt
> +		bsize=$($XFS_DB_PROG -c "sb" -c"p" $loop_dev | sed -n 's/^blocksize = \([0-9]\+\).*/\1/p')
> +		$MOUNT_PROG $loop_dev $loop_mnt

block size is returned by stat(1). No need for filesystem specific
methods to get this. e.g.:

$ stat -c %o foo
4096
$


> +get_free_sectors()
> +{
> +	case $FSTYP in
> +	ext[34])
> +		size=1
> +		clstsize=$($DUMPE2FS_PROG $loop_dev 2>/dev/null | sed '/Cluster size/!d; s/.*: *//')
> +		if [ -n "$clstsize" ]; then
> +			size=$((clstsize/bsize))
> +		fi
> +		$DUMPE2FS_PROG $loop_dev 2>/dev/null | sed '/  Free blocks/!d; s/.*: //; s/, /\n/g; /^$/d' | \
> +			$AWK_PROG -v spb=$spb -v size=$size 'BEGIN{FS="-"}; {printf "%d-%d\n", spb * $1, (spb * $2 == 0)? spb * ($1 + size) - 1 : spb * ($2 + size) - 1}' > $free_sectors

Lines are way too long, and this needs some comments to explain what
the line noise is calculatiing. And what is "$spb"?

> +	;;
> +	xfs)
> +		$UMOUNT_PROG $loop_mnt
> +		agblocks=$($XFS_DB_PROG -c"sb" -c "p" $loop_dev | sed -n 's/agblocks = \(.*\)$/\1/p')

xfs_info | mkfs_filter > /dev/null 2> $tmp.info
. $tmp.info

and the number of blocks in an AG is now in $agsize.

> +		$XFS_DB_PROG -c"freesp -d" $loop_dev
>
> | $AWK_PROG '{if($1 == "from") exit; else print}' | \
> +			$AWK_PROG -v agb=$agblocks -v spb=$spb '{print spb * ($1 * agb + $2) "-" spb * ($1 * agb + $2 + $3) - 1}' | sort -n > $free_sectors
> +		$MOUNT_PROG $loop_dev $loop_mnt

These need to be line wrapped to fit within roughly 80 columns.
Also, some comments about that all this line noise is calculating
would be handy for us to determine if it is correct and reliable...


> +	;;
>
>
> +	esac
> +}
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.config
> +
> +# real QA test starts here
> +
> +_supported_fs ext3 ext4 xfs
> +_supported_os Linux
> +_require_fstrim
> +_require_dumpe2fs

Not for XFS.

> +_require_fs_space $TEST_DIR 3145728

So requires 3GB of space? That will fail on all my test devices.
If you need this much space, then please use the scratch device.

> +
> +debugfs=$($MOUNT_PROG | grep debugfs | cut -d " " -f3)
> +[ -n $debugfs ] || _notrun "This test requires mounted debugfs"
> +
> +cat $debugfs/tracing/available_tracers | grep -q blk || _notrun "blk tracer is not available"

_require_tracer
_require_blk_tracer

> +free_sectors=$tmp/sectors
> +trace=$tmp/trace
> +
> +echo -n "Obtaining free sectors from FS..."
> +get_free_sectors
> +echo "done."
> +
> +echo -n "Running fstrim & trace..."
> +echo 1 > /sys/block/$loop_base/trace/enable
> +echo blk > $debugfs/tracing/current_tracer
> +echo > $debugfs/tracing/trace
> +echo 1 > $debugfs/tracing/tracing_on

This trace+trim operations set should be done in a function...

> +
> +$FSTRIM_PROG $loop_mnt > /dev/null
> +
> +cat $debugfs/tracing/trace | grep "\[fstrim\]"  | cut -d ":" -f2- |
> +$AWK_PROG '$3 == "D"' |  		   # Filter discard operation
> +$AWK_PROG '{print $4 "-" $4 + $6 - 1}' | # Transform (start, length) => (start, end)
> +sort -n > $trace

The moment the trace output format changes, this will break.  Also,
just cat-ing from $debugfs/tracing/trace is unreliable and can skip
events (because it is slow) and if the trace buffer overflows,
events will also be missed. Hence I don't think this is a reliable
way to capture where free space is supposed to be in the underlying
file.

Better, IMO, is to preallocate the image file, then run fiemap on it
after the fact to find all the holes that were punched by the
discard operations. That way you actually test the loop device
discard implementation, as well as confirming that fstrim only
discards free space.

With this, you don't even need to to dump free space from the
filesystem prior to issuing fstrim - all you need to do is confirm
that there is a hole in the file covering each free space extent.

Cheers,

Dave.

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/3] Add new standard loop handling functions

[Rich Johnston]

On 10/18/2012 03:45 AM, Tomas Racek wrote:
> Add _create_loop_device and _destroy_loop_device to uniformly handle
> loopback devices.
>
> Signed-off-by: Tomas Racek <tracek@redhat.com>
> ---
>   common.rc | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>

Tomas,
	Looks good.
--Rich

Reviewed-by: Rich Johnston <rjohnston@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/3] Add new standard loop handling functions

[Dave Chinner]

On Thu, Oct 18, 2012 at 10:45:44AM +0200, Tomas Racek wrote:
> Add _create_loop_device and _destroy_loop_device to uniformly handle
> loopback devices.
> 
> Signed-off-by: Tomas Racek <tracek@redhat.com>
> ---
>  common.rc | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/common.rc b/common.rc
> index 966fc93..b5edd22 100644
> --- a/common.rc
> +++ b/common.rc
> @@ -1804,6 +1804,21 @@ _test_batched_discard()
>  	$FSTRIM_PROG ${1} &>/dev/null
>  }
>  
> +_create_loop_device()
> +{
> +	file=$1
> +	dev=`losetup -f`
> +	losetup $dev $file || _fail "Cannot associate $file with $dev"
> +	echo $dev
> +}
> +
> +_destroy_loop_device()
> +{
> +	dev=$1
> +	losetup -d $dev || _fail "Cannot destroy loop device $dev"
> +}
> +
> +
>  ################################################################################

Perhaps a followup patch that converts existing loop device users to
use this interface is appropriate?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs