xfs
[Top] [All Lists]

Re: [PATCH] Test to ensure that the EOFBLOCK_FL gets set/unset correctly

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH] Test to ensure that the EOFBLOCK_FL gets set/unset correctly.
From: Akshay Lal <akshaylal@xxxxxxxxxx>
Date: Fri, 27 Aug 2010 16:10:07 -0700
Cc: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=google.com; s=beta; t=1282950640; bh=30G1syo5QlJDp5jMdB9uSVDsfR4=; h=MIME-Version:Reply-To:In-Reply-To:References:From:Date:Message-ID: Subject:To:Cc:Content-Type:Content-Transfer-Encoding; b=qw6xnxSAV8BK1sH3xGS9wHdNxb8CzfGdMq/Tq/hCwJ8VXOjgku+maFshvccUMuRLr np0PvgA8sNNlZnodf5rmA==
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=beta; h=domainkey-signature:received:mime-version:received:reply-to :in-reply-to:references:from:date:message-id:subject:to:cc :content-type:content-transfer-encoding; bh=5y/nY/ZX+0hjxvG8MVgSgFCtNmMyzCFdCY43ATV5jFU=; b=NoS/KvfCxTRyuIcCnraHpB8aEe8oUCt+K3OF/R+hGH+O0JGYbDrl1w7IdLBWI8hpzh PisBapmGdQ4z9huHSWew==
Domainkey-signature: a=rsa-sha1; c=nofws; d=google.com; s=beta; h=mime-version:reply-to:in-reply-to:references:from:date:message-id :subject:to:cc:content-type:content-transfer-encoding; b=vNjqDL2ee+R8NPZF+3dQT6tMQw33ZF0XG6lybJWc0y4aafvpoRRl9WolF7sjc/Bfiu bSykFNLc2wafJN/cki+Q==
In-reply-to: <4C783305.5060506@xxxxxxxxxxx>
References: <1282941224-5805-1-git-send-email-alal@xxxxxxxxxx> <4C783305.5060506@xxxxxxxxxxx>
Reply-to: alal@xxxxxxxxxx
>> +# Test specific macros.
>> +BIT_NOT_SET=0   # inode flag - 0x40000 bit is set.
>> +BIT_SET=1       # inode flag - 0x400000 bit is not set.
>
> I'm thinking one of the values in the comment is not right :)
Duh. Thats silly of me.
Done.

>> +# Checks the state of the sample file in the filesystem and returns whether
>> +# the inode flag 0x400000 is set or not.
>
> I guess a little more comment here about why we do this only for ext4 might
> be good.
>
> Just to make it more obvious in the main test could you maybe call
> it something like _check_ext4_eof_flag() ?  If we ever need to do
> testing on other filesystems we can adjust it.
Done.


>> +        if ((${iflags} & 0x400000)); then
>> +                echo "EOFBLOCK_FL bit is set."
>
> This will cause the test to fail on anything but ext4 because
> it's expected in the output but only happens for ext4...
>
> You could redirect it into $seq.full, just to have it for analysis
> if the test ever fails.
I've added an else condition to exit the test with the error message
that the test is only
supported on ext4 filesystems.
I've also redirected all echo messages to $seq.full


>> +        # Run fsck on the fs. fsck -t ${FSTYP} -fn ${TEST_DEV}.
>> +        _check_test_fs
>
> Ah now I see why you didn't call it _check_ext4_eof_flag() ;)
>
> This may not be necessary, the harness runs fsck after each
> test anyway.  On a large filesystem this could take a pretty
> long time if you run fsck 6 times.  I'd just drop
> this and let the post-test-fsck catch any problems.
Removed.

---------------------------------------------------------------------------------------
Updated patch:
---------------------------------------------------------------------------------------

>From f1e8df788e4b93ae33e01c48a859f2b21c425357 Mon Sep 17 00:00:00 2001
From: Akshay Lal <alal@xxxxxxxxxx>
Date: Fri, 27 Aug 2010 13:14:18 -0700
Subject: [PATCH] Test to ensure that the EOFBLOCK_FL gets set/unset correctly.
 As found by Theodore Ts'o:
 If a 128K file is falloc'ed using the KEEP_SIZE flag, and then
 write exactly 128K, the EOFBLOCK_FL doesn't get cleared correctly.
 This forces e2fsck to complain about that inode.

Bug reference:
http://thread.gmane.org/gmane.comp.file-systems.ext4/20682
---
 243     |  157 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 243.out |   13 +++++
 group   |    1 +
 3 files changed, 171 insertions(+), 0 deletions(-)
 create mode 100644 243
 create mode 100644 243.out

diff --git a/243 b/243
new file mode 100644
index 0000000..804513f
--- /dev/null
+++ b/243
@@ -0,0 +1,157 @@
+#! /bin/bash
+# FS QA Test No. 243
+#
+# Test to ensure that the EOFBLOCK_FL gets set/unset correctly.
+#
+# As found by Theodore Ts'o:
+# If a 128K file is falloc'ed using the KEEP_SIZE flag, and then
+# write exactly 128K, the EOFBLOCK_FL doesn't get cleared correctly.
+# This forces e2fsck to complain about that inode.
+#
+# Ref: http://thread.gmane.org/gmane.comp.file-systems.ext4/20682
+#
+# creator
+owner=alal@xxxxxxxxxx
+
+seq=`basename $0`
+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
+
+# Test specific macros.
+BIT_NOT_SET=0   # inode flag - 0x40000 bit is not set.
+BIT_SET=1       # inode flag - 0x400000 bit is set.
+
+# Generic test cleanup function.
+_cleanup()
+{
+   cd /
+   rm -f $tmp.*
+}
+
+# Ext4 uses the EOFBLOCKS_FL bit when fallocating blocks with KEEP_SIZE
+# enabled. The only time this bit should be set is when extending the allocated
+# blocks further than what the i_size represents. In the situations wherein the
+# i_size covers all allocated blocks, this bit should be cleared.
+
+# Checks the state of the sample file in the filesystem and returns whether
+# the inode flag 0x400000 is set or not.
+_check_ext4_eof_flag()
+{
+   bit_set=1
+
+   # Check whether EOFBLOCK_FL is set.
+   if [ "${FSTYP}" == "ext4" ]; then
+        # Unmount the ${TEST_DEV}
+        umount ${TEST_DEV}
+
+        # Run debugfs to gather file_parameters - specifically iflags.
+        file_params=`debugfs ${TEST_DEV} -R "stat ${1}" 2>&1 | grep -e Flags:`
+        iflags=${file_params#*Flags: }
+
+        # Ensure that the iflags value was parsed correctly.
+        if [ -z ${iflags} ]; then
+                echo "iFlags value was not parsed successfully." >> $seq.full
+                status=1
+                exit ${status}
+        fi
+
+        # Check if EOFBLOCKS_FL is set.
+        if ((${iflags} & 0x400000)); then
+                echo "EOFBLOCK_FL bit is set." >> $seq.full
+                bit_set=1
+        else
+                echo "EOFBLOCK_FL bit is not set." >> $seq.full
+                bit_set=0
+        fi
+
+        # Check current bit state to expected value.
+        if [ ${bit_set} -ne ${2} ]; then
+                echo "Error: Current bit state incorrect." >> $seq.full
+                exit ${status}
+        fi
+
+        # Mount the ${TEST_DEV}
+        mount ${TEST_DEV} -t ${FSTYP} ${TEST_DIR}
+   else
+     echo "Only EXT4 filesystems supported." >> $seq.full
+     status=1
+     exit ${status}
+  fi
+}
+
+# Get standard environment, filters and checks.
+. ./common.rc
+. ./common.filter
+
+# Prerequisites for the test run.
+_supported_fs generic
+_supported_os Linux
+_require_xfs_io_falloc
+
+# Real QA test starts here.
+rm -f $seq.full
+
+# Begin test cases.
+
+# Using FALLOC_FL_KEEP_SIZE
+# Test 1: Fallocate 40960 bytes and write 4096 bytes (buffered io).
+echo "Test 1: Fallocate 40960 bytes and write 4096 bytes (buffered io)." \
+    >> $seq.full
+${XFS_IO_PROG} -F -f                    \
+  -c 'falloc -k 0 40960'                \
+  -c 'pwrite 0 4096'                    \
+  ${TEST_DIR}/test_1 | _filter_xfs_io_unique
+_check_ext4_eof_flag test_1 ${BIT_SET}
+
+# Test 2: Fallocate 40960 bytes and write 4096 bytes (direct io).
+echo "Test 2: Fallocate 40960 bytes and write 4096 bytes (direct io)." \
+    >> $seq.full
+${XFS_IO_PROG} -F -f -d                 \
+  -c 'falloc -k 0 40960'                \
+  -c 'pwrite 0 4096'                    \
+  ${TEST_DIR}/test_2 | _filter_xfs_io_unique
+_check_ext4_eof_flag test_2 ${BIT_SET}
+
+# Test 3: Fallocate 40960 bytes and write 40960 bytes (buffered io).
+echo "Test 3: Fallocate 40960 bytes and write 40960 bytes (buffered io)." \
+    >> $seq.full
+${XFS_IO_PROG} -F -f                    \
+  -c 'falloc -k 0 40960'                \
+  -c 'pwrite 0 40960'                   \
+  ${TEST_DIR}/test_3 | _filter_xfs_io_unique
+_check_ext4_eof_flag test_3 ${BIT_NOT_SET}
+
+# Test 4: Fallocate 40960 bytes and write 40960 bytes (direct io).
+echo "Test 4: Fallocate 40960 bytes and write 40960 bytes (direct io)." \
+    >> $seq.full
+${XFS_IO_PROG} -F -f -d                 \
+  -c 'falloc -k 0 40960'                \
+  -c 'pwrite 0 40960'                   \
+  ${TEST_DIR}/test_4 | _filter_xfs_io_unique
+_check_ext4_eof_flag test_4 ${BIT_NOT_SET}
+
+# Test 5: Fallocate 128k, seek 256k and write 4k block (buffered io).
+echo "Test 5: Fallocate 128k, seek 256k and write 4k block (buffered io)." \
+    >> $seq.full
+${XFS_IO_PROG} -F -f                    \
+  -c 'falloc -k 0 128k'                 \
+  -c 'pwrite 256k 4k'                   \
+  ${TEST_DIR}/test_5 | _filter_xfs_io_unique
+_check_ext4_eof_flag test_5 ${BIT_NOT_SET}
+
+# Test 6: Fallocate 128k, seek to 256k and write a 4k block (direct io).
+echo "Test 6: Fallocate 128k, seek to 256k and write a 4k block (direct io)." \
+    >> $seq.full
+${XFS_IO_PROG} -F -f -d                 \
+  -c 'falloc -k 0 128k'                 \
+  -c 'pwrite 256k 4k'                   \
+  ${TEST_DIR}/test_6 | _filter_xfs_io_unique
+_check_ext4_eof_flag test_6 ${BIT_NOT_SET}
+
+
+status=0
+exit ${status}
diff --git a/243.out b/243.out
new file mode 100644
index 0000000..290a005
--- /dev/null
+++ b/243.out
@@ -0,0 +1,13 @@
+QA output created by 243
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 40960/40960 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 40960/40960 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 262144
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 262144
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/group b/group
index ff16bb3..e6dab13 100644
--- a/group
+++ b/group
@@ -356,3 +356,4 @@ deprecated
 240 auto aio quick rw
 241 auto
 242 auto quick prealloc
+243 auto quick prealloc
-- 
1.7.1

<Prev in Thread] Current Thread [Next in Thread>