Patchwork xfstests: make defrag test 222 generic

login
register
mail settings
Submitter Eric Sandeen
Date March 12, 2013, 4:42 p.m.
Message ID <513F5B0A.4030405@redhat.com>
Download mbox | patch
Permalink /patch/227082/
State Not Applicable
Headers show

Comments

Eric Sandeen - March 12, 2013, 4:42 p.m.
test 222 is just a stress test defragging the test device:

# xfs_fsr QA tests
# run xfs_fsr over the test filesystem to give it a wide and varied set of
# inodes to try to defragment. This is effectively a crash/assert failure
# test looking for corruption induced by xfs_fsr runs.

There's no reason we can't do the same for other filesystems.

Define a new _defrag_dir() helper which just runs noisy/debug
dir defrag for ext4 and/or btrfs as well, and use that in 222
instead of hardcoded xfs_fsr.

Make 222 (and 218) generic; if no defrag is supported it just won't
run.

Change the comments at the top while we're at it.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---



--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba - March 15, 2013, 2:55 p.m.
On Tue, Mar 12, 2013 at 11:42:50AM -0500, Eric Sandeen wrote:
> Define a new _defrag_dir() helper which just runs noisy/debug
> dir defrag for ext4 and/or btrfs as well, and use that in 222
> instead of hardcoded xfs_fsr.

Dir defrag on btrfs does not recurse in the given directory to defrag
all files, but defragments the tree of the containing subvolume and the
whole extent tree (thus needs CAP_SYS_ADMIN).

This is known (and not very intuitive) behaviour, it would be better to
add a special parameter to defrag the root and/or the extent root and
pass it down to the ioctl via flags.

Until this is implemented I suggest to use a workaround via 'find':

 find $dir -print -execdir $DEFRAG_PROG '{}' +

Otherwise the test looks ok.

david

> +# Defrag a whole directory.  No checking as above, just run it, noisily
> +# output should be sent to $seq.full since it's very fs-specific
> +_defrag_dir()
> +{
> +    case "$FSTYP" in
> +    xfs)
> +	$DEFRAG_PROG -d -v $1
> +	;;
> +    ext4|ext4dev|btrfs)
       ext4|ext4dev)
> +	$DEFRAG_PROG -v $1
> +	;;
       btrfs)
        find $1 -print -execdir $DEFRAG_PROG '{}' +
        ;;
> +    *)
> +	$DEFRAG_PROG $1
> +	;;
> +    esac
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rich Johnston - March 15, 2013, 4:03 p.m.
On 03/15/2013 09:55 AM, David Sterba wrote:
> On Tue, Mar 12, 2013 at 11:42:50AM -0500, Eric Sandeen wrote:
>> Define a new _defrag_dir() helper which just runs noisy/debug
>> dir defrag for ext4 and/or btrfs as well, and use that in 222
>> instead of hardcoded xfs_fsr.
>
> Dir defrag on btrfs does not recurse in the given directory to defrag
> all files, but defragments the tree of the containing subvolume and the
> whole extent tree (thus needs CAP_SYS_ADMIN).
>
> This is known (and not very intuitive) behaviour, it would be better to
> add a special parameter to defrag the root and/or the extent root and
> pass it down to the ioctl via flags.
>
> Until this is implemented I suggest to use a workaround via 'find':
>
>   find $dir -print -execdir $DEFRAG_PROG '{}' +
>
> Otherwise the test looks ok.
>
> david

Eric, I will make this change at commit time if you agree?

David, can I put your name as the reviewer when I do?

Thanks
--Rich
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sandeen - March 15, 2013, 4:05 p.m.
On 3/15/13 11:03 AM, Rich Johnston wrote:
> On 03/15/2013 09:55 AM, David Sterba wrote:
>> On Tue, Mar 12, 2013 at 11:42:50AM -0500, Eric Sandeen wrote:
>>> Define a new _defrag_dir() helper which just runs noisy/debug
>>> dir defrag for ext4 and/or btrfs as well, and use that in 222
>>> instead of hardcoded xfs_fsr.
>>
>> Dir defrag on btrfs does not recurse in the given directory to defrag
>> all files, but defragments the tree of the containing subvolume and the
>> whole extent tree (thus needs CAP_SYS_ADMIN).
>>
>> This is known (and not very intuitive) behaviour, it would be better to
>> add a special parameter to defrag the root and/or the extent root and
>> pass it down to the ioctl via flags.
>>
>> Until this is implemented I suggest to use a workaround via 'find':
>>
>>   find $dir -print -execdir $DEFRAG_PROG '{}' +
>>
>> Otherwise the test looks ok.
>>
>> david
> 
> Eric, I will make this change at commit time if you agree?
> 
> David, can I put your name as the reviewer when I do?

I think that's big enough change I should send a V2. and
make a btrfs-special case in _defrag_dir.

-Eric

> Thanks
> --Rich
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rich Johnston - March 27, 2013, 2:53 p.m.
On 03/15/2013 11:05 AM, Eric Sandeen wrote:

>
> I think that's big enough change I should send a V2. and
> make a btrfs-special case in _defrag_dir.
>
> -Eric

Ping

Did I miss something, I know you are working on several things at once. :)

Thanks
--Rich
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/218 b/218
index 77782c6..e337bbf 100755
--- a/218
+++ b/218
@@ -44,7 +44,7 @@  _cleanup()
 . ./common.defrag
 
 # real QA test starts here
-_supported_fs xfs ext4 btrfs
+_supported_fs generic
 _supported_os Linux
 
 _setup_testdir
diff --git a/222 b/222
index df2672f..dc4d621 100755
--- a/222
+++ b/222
@@ -1,10 +1,10 @@ 
 #! /bin/bash
 # FS QA Test No. 222
 #
-# xfs_fsr QA tests
-# run xfs_fsr over the test filesystem to give it a wide and varied set of
+# defrag QA tests
+# run defrag over the test filesystem to give it a wide and varied set of
 # inodes to try to defragment. This is effectively a crash/assert failure
-# test looking for corruption induced by xfs_fsr runs.
+# test looking for corruption induced by defragmentation runs.
 #
 #-----------------------------------------------------------------------
 # Copyright (c) 2010 Dave Chinner.  All Rights Reserved.
@@ -44,14 +44,15 @@  trap "_cleanup ; exit \$status" 0 1 2 3 15
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.defrag
 
 # real QA test starts here
-_supported_fs xfs
+_supported_fs generic
 _supported_os Linux
 
-[ "$XFS_FSR_PROG" = "" ] && _notrun "xfs_fsr not found"
+_require_defrag
 
-xfs_fsr -d -v $TEST_DIR > $seq.full 2>&1
+_defrag_dir $TEST_DIR > $seq.full 2>&1
 
 echo "--- silence is golden ---"
 status=0 ; exit
diff --git a/common.defrag b/common.defrag
index 9c9eb9b..7736330 100644
--- a/common.defrag
+++ b/common.defrag
@@ -69,3 +69,19 @@  _defrag()
 	rm -f $1
 }
 
+# Defrag a whole directory.  No checking as above, just run it, noisily
+# output should be sent to $seq.full since it's very fs-specific
+_defrag_dir()
+{
+    case "$FSTYP" in
+    xfs)
+	$DEFRAG_PROG -d -v $1
+	;;
+    ext4|ext4dev|btrfs)
+	$DEFRAG_PROG -v $1
+	;;
+    *)
+	$DEFRAG_PROG $1
+	;;
+    esac
+}