[PATCH 0/5] xfstests: cleanups and new tests.

[PATCH 0/5] xfstests: cleanups and new tests.

[Dave Chinner]

Hi y'all,

The first two patches are minor cleanup patches. The first changes
some tests that can leave large deposits on the test fs when they
fail and that can lead to ENOSPC issues on small test devices. The
second is another test that shouldn't check the scratch device at
the end.

The remaining three patches are new generic tests that exercise data
corruption issues that we've tripped over in the past week on XFS.
The test cases have been isolated by Brian Foster and Eric Sandeen -
they deserve the credit for narrowing down the minimal tests to
expose the problems. I just added a few more corner cases and
packaged them for xfstests.

These failures manifest on block size smaller than page size
filesystems, and we have XFS fixes pending for these failures:

http://oss.sgi.com/pipermail/xfs/2014-September/038191.html
http://oss.sgi.com/pipermail/xfs/2014-September/038167.html

Ext4 folks, can you confirm that both the truncate tests fail on
ext4 with data corruption on 1k/2k filesystems? I'm pretty sure that
I have the corrupt hexdump outputs in the golden output files for
the tests and if I have that means ext4 is corrupting data in a
similar manner to XFS.  Eric is already looking at this but
could probably use some more help. ;)

-Dave.

[PATCH 1/5] generic: more tests should clean up TESTDIR on success

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

I'm getting enospc errors on a 4GB test device after a while of
running. Part of the issue is that many tests can't or don't clean
up previous failed runs when they start or if the run to success.
Hence while we want to slowly age the test filesystem, we don't
really want that aging to unintentionally run the filesystem out of
space. To that end:

$ sudo du -s /mnt/test/* | sort -nr |head -10
1929160 /mnt/test/fsfile
512000  /mnt/test/247.8133
512000  /mnt/test/247.4713
512000  /mnt/test/247.4488
466752  /mnt/test/fstest.9850.2
40000   /mnt/test/resv
29804   /mnt/test/fsstress.12144.1
26208   /mnt/test/populate_root
26208   /mnt/test/mnt
23216   /mnt/test/fsstress.4491.1

We can see that there are a few tests that using most of the space.
These are often left behind due to kernel failures during tests or
reboots while tests are in progress, so make sure that they at least
clean up such mess the next time they run.

Test generic/247, xfs/020 (fsfile) and generic/074 (fstest.$$.n)
are the worst offenders, so just target these to being with.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 tests/generic/074 | 21 ++++++++++-----------
 tests/generic/247 |  7 ++++++-
 tests/xfs/020     |  9 ++++++---
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/tests/generic/074 b/tests/generic/074
index df85d66..55264bd 100755
--- a/tests/generic/074
+++ b/tests/generic/074
@@ -33,20 +33,26 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _cleanup()
 {
-    cd /
-    rm -rf $TEST_DIR/fstest.$$.* $tmp.*
+    rm -rf $fstest_dir.* $tmp.*
 }
 
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
 
+_supported_fs generic
+_supported_os IRIX Linux
+_require_test
+
+rm -f $seqres.full
+fstest_dir=$TEST_DIR/fstest
+
 _do_test()
 {
     _n="$1"
     _param="$2"
 
-    out=$TEST_DIR/fstest.$$.$_n
+    out=$fstest_dir.$_n
     rm -rf $out
     if ! mkdir $out
     then
@@ -59,7 +65,7 @@ _do_test()
 	-e 's/-n [0-9][0-9]*/-n children/' \
 	-e 's/-l [0-9][0-9]*/-l loops/' \
 	-e 's/-f [0-9][0-9]*/-f files/'`
-    
+
     echo ""
     echo "-----------------------------------------------"
     echo "fstest.$_n : $_filter_param"
@@ -105,13 +111,6 @@ _process_args()
     done
 }
 
-# real QA test starts here
-rm -f $seqres.full
-
-_supported_fs generic
-_supported_os IRIX Linux
-_require_test
-
 #
 # set params
 # These params can take a while on different CPUs/OSs
diff --git a/tests/generic/247 b/tests/generic/247
index c8648a2..832ade1 100755
--- a/tests/generic/247
+++ b/tests/generic/247
@@ -48,7 +48,12 @@ _supported_fs generic
 _supported_os Linux
 _require_test
 
-testfile=$TEST_DIR/$seq.$$
+# this test leaves a 512MB file around if we abort the test during the run via a
+# reboot or kernel panic. Hence just name the file $seq so that we can always
+# clean up on the next run and not leave large stale files around on the testdir
+# that can lead to ENOSPC issues over time.
+testfile=$TEST_DIR/$seq
+rm -f $testfile
 
 loops=500
 iosize=1048576
diff --git a/tests/xfs/020 b/tests/xfs/020
index 957f3c4..dc305c1 100755
--- a/tests/xfs/020
+++ b/tests/xfs/020
@@ -37,7 +37,7 @@ _cleanup()
 {
     cd /
     rm -f $tmp.*
-    rm -f $TEST_DIR/fsfile
+    rm -f $fsfile
 }
 
 # get standard environment, filters and checks
@@ -51,8 +51,11 @@ _require_test
 
 echo "Silence is golden"
 
-$MKFS_PROG -t xfs -d size=60t,file,name=$TEST_DIR/fsfile >/dev/null
-$XFS_REPAIR_PROG -o ag_stride=32 -t 1 $TEST_DIR/fsfile >/dev/null 2>&1
+fsfile=$TEST_DIR/fsfile.$seq
+rm -f $fsfile
+
+$MKFS_PROG -t xfs -d size=60t,file,name=$fsfile >/dev/null
+$XFS_REPAIR_PROG -o ag_stride=32 -t 1 $fsfile >/dev/null 2>&1
 
 status=$?
 exit
-- 
2.0.0

Re: [PATCH 1/5] generic: more tests should clean up TESTDIR on success

[Eric Sandeen]

On 9/16/14 8:41 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> I'm getting enospc errors on a 4GB test device after a while of
> running. Part of the issue is that many tests can't or don't clean
> up previous failed runs when they start or if the run to success.
> Hence while we want to slowly age the test filesystem, we don't
> really want that aging to unintentionally run the filesystem out of
> space. To that end:
> 
> $ sudo du -s /mnt/test/* | sort -nr |head -10
> 1929160 /mnt/test/fsfile
> 512000  /mnt/test/247.8133
> 512000  /mnt/test/247.4713
> 512000  /mnt/test/247.4488
> 466752  /mnt/test/fstest.9850.2
> 40000   /mnt/test/resv
> 29804   /mnt/test/fsstress.12144.1
> 26208   /mnt/test/populate_root
> 26208   /mnt/test/mnt
> 23216   /mnt/test/fsstress.4491.1
> 
> We can see that there are a few tests that using most of the space.
> These are often left behind due to kernel failures during tests or
> reboots while tests are in progress, so make sure that they at least
> clean up such mess the next time they run.
> 
> Test generic/247, xfs/020 (fsfile) and generic/074 (fstest.$$.n)
> are the worst offenders, so just target these to being with.

Seems slightly random, but harmless.  :)

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

> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  tests/generic/074 | 21 ++++++++++-----------
>  tests/generic/247 |  7 ++++++-
>  tests/xfs/020     |  9 ++++++---
>  3 files changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/generic/074 b/tests/generic/074
> index df85d66..55264bd 100755
> --- a/tests/generic/074
> +++ b/tests/generic/074
> @@ -33,20 +33,26 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>  
>  _cleanup()
>  {
> -    cd /
> -    rm -rf $TEST_DIR/fstest.$$.* $tmp.*
> +    rm -rf $fstest_dir.* $tmp.*
>  }
>  
>  # get standard environment, filters and checks
>  . ./common/rc
>  . ./common/filter
>  
> +_supported_fs generic
> +_supported_os IRIX Linux
> +_require_test
> +
> +rm -f $seqres.full
> +fstest_dir=$TEST_DIR/fstest
> +
>  _do_test()
>  {
>      _n="$1"
>      _param="$2"
>  
> -    out=$TEST_DIR/fstest.$$.$_n
> +    out=$fstest_dir.$_n
>      rm -rf $out
>      if ! mkdir $out
>      then
> @@ -59,7 +65,7 @@ _do_test()
>  	-e 's/-n [0-9][0-9]*/-n children/' \
>  	-e 's/-l [0-9][0-9]*/-l loops/' \
>  	-e 's/-f [0-9][0-9]*/-f files/'`
> -    
> +
>      echo ""
>      echo "-----------------------------------------------"
>      echo "fstest.$_n : $_filter_param"
> @@ -105,13 +111,6 @@ _process_args()
>      done
>  }
>  
> -# real QA test starts here
> -rm -f $seqres.full
> -
> -_supported_fs generic
> -_supported_os IRIX Linux
> -_require_test
> -
>  #
>  # set params
>  # These params can take a while on different CPUs/OSs
> diff --git a/tests/generic/247 b/tests/generic/247
> index c8648a2..832ade1 100755
> --- a/tests/generic/247
> +++ b/tests/generic/247
> @@ -48,7 +48,12 @@ _supported_fs generic
>  _supported_os Linux
>  _require_test
>  
> -testfile=$TEST_DIR/$seq.$$
> +# this test leaves a 512MB file around if we abort the test during the run via a
> +# reboot or kernel panic. Hence just name the file $seq so that we can always
> +# clean up on the next run and not leave large stale files around on the testdir
> +# that can lead to ENOSPC issues over time.
> +testfile=$TEST_DIR/$seq
> +rm -f $testfile
>  
>  loops=500
>  iosize=1048576
> diff --git a/tests/xfs/020 b/tests/xfs/020
> index 957f3c4..dc305c1 100755
> --- a/tests/xfs/020
> +++ b/tests/xfs/020
> @@ -37,7 +37,7 @@ _cleanup()
>  {
>      cd /
>      rm -f $tmp.*
> -    rm -f $TEST_DIR/fsfile
> +    rm -f $fsfile
>  }
>  
>  # get standard environment, filters and checks
> @@ -51,8 +51,11 @@ _require_test
>  
>  echo "Silence is golden"
>  
> -$MKFS_PROG -t xfs -d size=60t,file,name=$TEST_DIR/fsfile >/dev/null
> -$XFS_REPAIR_PROG -o ag_stride=32 -t 1 $TEST_DIR/fsfile >/dev/null 2>&1
> +fsfile=$TEST_DIR/fsfile.$seq
> +rm -f $fsfile
> +
> +$MKFS_PROG -t xfs -d size=60t,file,name=$fsfile >/dev/null
> +$XFS_REPAIR_PROG -o ag_stride=32 -t 1 $fsfile >/dev/null 2>&1
>  
>  status=$?
>  exit
> 

[PATCH 2/5] check: more tests that shouldn't check the scratch device

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

xfs/200 leaves a dirty log as readonly filesystems don't write
unmount records to mark the log clean.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 tests/xfs/200 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/xfs/200 b/tests/xfs/200
index f4db64f..f0c4337 100755
--- a/tests/xfs/200
+++ b/tests/xfs/200
@@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fs xfs
 _supported_os Linux
 
-_require_scratch
+_require_scratch_nocheck
 
 _scratch_mkfs_xfs >/dev/null 2>&1
 
-- 
2.0.0

Re: [PATCH 2/5] check: more tests that shouldn't check the scratch device

[Eric Sandeen]

On 9/16/14 8:41 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> xfs/200 leaves a dirty log as readonly filesystems don't write
> unmount records to mark the log clean.

I wonder if it'd be better to just add a case that sets it RW again
and mounts/unmounts it, so we *can* check it after these gyrations?

But if for some reason you don't want to change the existing test,
this is probably ok too; if so,

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

> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  tests/xfs/200 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/xfs/200 b/tests/xfs/200
> index f4db64f..f0c4337 100755
> --- a/tests/xfs/200
> +++ b/tests/xfs/200
> @@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>  _supported_fs xfs
>  _supported_os Linux
>  
> -_require_scratch
> +_require_scratch_nocheck
>  
>  _scratch_mkfs_xfs >/dev/null 2>&1
>  
> 

[PATCH 3/5] generic: add mmap write vs truncate test

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

This test exposed a problem with mapped writes to the tail page of a
file in XFS. Hence make it a generic test so taht we can ensure that
all fielsystems handle the case correctly.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 tests/generic/029     | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/029.out |  65 ++++++++++++++++++++++++++
 tests/generic/group   |   1 +
 3 files changed, 189 insertions(+)
 create mode 100644 tests/generic/029
 create mode 100644 tests/generic/029.out

diff --git a/tests/generic/029 b/tests/generic/029
new file mode 100644
index 0000000..854794e
--- /dev/null
+++ b/tests/generic/029
@@ -0,0 +1,123 @@
+#! /bin/bash
+# FS QA Test No. generic/029
+#
+# Test mapped writes against truncate down/up to ensure we get the data
+# correctly written. This can expose data corruption bugs on filesystems where
+# the block size is smaller than the page size.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Red Hat, Inc.  All Rights Reserved.
+#
+# 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
+#-----------------------------------------------------------------------
+#
+
+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()
+{
+    cd /
+    rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os IRIX Linux
+_require_scratch
+
+testfile=$SCRATCH_MNT/testfile
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+
+# first case is just truncate down/truncate up to check that the mapped
+# write after the truncate up is correctly handled.
+$XFS_IO_PROG -t -f \
+-c "truncate 5120"              `# truncate     |                        |` \
+-c "pwrite -S 0x58 0 5120"      `# write        |XXXXXXXXXXXXXXXXXXXXXXXX|` \
+-c "mmap -rw 0 5120"            `# mmap         |                        |` \
+-c "mwrite -S 0x5a 2048 3072"   `# mwrite       |          ZZZZZZZZZZZZZZ|` \
+-c "truncate 2048"              `# truncate dn  |         |` \
+-c "truncate 5120"              `# truncate up  |                        |` \
+-c "mwrite -S 0x59 2048 3072"   `# mwrite       |          YYYYYYYYYYYYYY|` \
+-c "close"      \
+$testfile | _filter_xfs_io
+
+echo "==== Pre-Remount ==="
+hexdump -C $testfile
+_scratch_remount
+echo "==== Post-Remount =="
+hexdump -C $testfile
+
+rm -f $testfile
+sync
+
+# second case is to do a mwrite between the truncate to a block on the
+# same page we are truncating within the EOF. This checks that a mapped
+# write between truncate down and truncate up a further mapped
+# write to the same page into the new space doesn't result in data being lost.
+$XFS_IO_PROG -t -f \
+-c "truncate 5120"              `# truncate     |                        |` \
+-c "pwrite -S 0x58 0 5120"      `# write        |XXXXXXXXXXXXXXXXXXXXXXXX|` \
+-c "mmap -rw 0 5120"            `# mmap         |                        |` \
+-c "mwrite -S 0x5a 2048 3072"   `# mwrite       |          ZZZZZZZZZZZZZZ|` \
+-c "truncate 2048"              `# truncate dn  |         |` \
+-c "mwrite -S 0x57 1024 1024"   `# mwrite       |     WWWWW              |` \
+-c "truncate 5120"              `# truncate up  |                        |` \
+-c "mwrite -S 0x59 2048 3072"   `# mwrite       |          YYYYYYYYYYYYYY|` \
+-c "close"      \
+$testfile | _filter_xfs_io
+ 
+echo "==== Pre-Remount ==="
+hexdump -C $testfile
+_scratch_remount
+echo "==== Post-Remount =="
+hexdump -C $testfile
+
+# third case is the same as second case, just with non-1k aligned offsets and
+# sizes.
+$XFS_IO_PROG -t -f \
+-c "truncate 5121"              `# truncate     |                        |` \
+-c "pwrite -S 0x58 0 5121"      `# write        |XXXXXXXXXXXXXXXXXXXXXXXX|` \
+-c "mmap -rw 0 5121"            `# mmap         |                        |` \
+-c "mwrite -S 0x5a 2047 3071"   `# mwrite       |          ZZZZZZZZZZZZZZ|` \
+-c "truncate 2047"              `# truncate dn  |         |` \
+-c "mwrite -S 0x57 513 1025"    `# mwrite       |  WWWWW                 |` \
+-c "truncate 5121"              `# truncate up  |                        |` \
+-c "mwrite -S 0x59 2047 3071"   `# mwrite       |          YYYYYYYYYYYYYY|` \
+-c "close"      \
+$testfile | _filter_xfs_io
+ 
+echo "==== Pre-Remount ==="
+hexdump -C $testfile
+_scratch_remount
+echo "==== Post-Remount =="
+hexdump -C $testfile
+
+status=0
+exit
+
diff --git a/tests/generic/029.out b/tests/generic/029.out
new file mode 100644
index 0000000..457982c
--- /dev/null
+++ b/tests/generic/029.out
@@ -0,0 +1,65 @@
+QA output created by 029
+wrote 5120/5120 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+==== Pre-Remount ===
+00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
+*
+00000800  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
+*
+00001400
+==== Post-Remount ==
+00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
+*
+00000800  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
+*
+00001400
+wrote 5120/5120 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+==== Pre-Remount ===
+00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
+*
+00000400  57 57 57 57 57 57 57 57  57 57 57 57 57 57 57 57  |WWWWWWWWWWWWWWWW|
+*
+00000800  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
+*
+00001400
+==== Post-Remount ==
+00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
+*
+00000400  57 57 57 57 57 57 57 57  57 57 57 57 57 57 57 57  |WWWWWWWWWWWWWWWW|
+*
+00000800  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
+*
+00001400
+wrote 5121/5121 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+==== Pre-Remount ===
+00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
+*
+00000200  58 57 57 57 57 57 57 57  57 57 57 57 57 57 57 57  |XWWWWWWWWWWWWWWW|
+00000210  57 57 57 57 57 57 57 57  57 57 57 57 57 57 57 57  |WWWWWWWWWWWWWWWW|
+*
+00000600  57 57 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |WWXXXXXXXXXXXXXX|
+00000610  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
+*
+000007f0  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 59  |XXXXXXXXXXXXXXXY|
+00000800  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
+*
+000013f0  59 59 59 59 59 59 59 59  59 59 59 59 59 59 00 00  |YYYYYYYYYYYYYY..|
+00001400  00                                                |.|
+00001401
+==== Post-Remount ==
+00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
+*
+00000200  58 57 57 57 57 57 57 57  57 57 57 57 57 57 57 57  |XWWWWWWWWWWWWWWW|
+00000210  57 57 57 57 57 57 57 57  57 57 57 57 57 57 57 57  |WWWWWWWWWWWWWWWW|
+*
+00000600  57 57 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |WWXXXXXXXXXXXXXX|
+00000610  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
+*
+000007f0  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 59  |XXXXXXXXXXXXXXXY|
+00000800  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
+*
+000013f0  59 59 59 59 59 59 59 59  59 59 59 59 59 59 00 00  |YYYYYYYYYYYYYY..|
+00001400  00                                                |.|
+00001401
diff --git a/tests/generic/group b/tests/generic/group
index bdcfd9d..18c94de 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -31,6 +31,7 @@
 026 acl quick auto
 027 auto enospc
 028 auto quick
+029 auto quick rw
 053 acl repair auto quick
 062 attr udf auto quick
 068 other auto freeze dangerous stress
-- 
2.0.0

Re: [PATCH 3/5] generic: add mmap write vs truncate test

[Eric Sandeen]

On 9/16/14 8:41 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> This test exposed a problem with mapped writes to the tail page of a
> file in XFS. Hence make it a generic test so taht we can ensure that
> all fielsystems handle the case correctly.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

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

although I'd like to see "Pre-remount [123]" in the output to make
it a little easier to see which test failed (if only 2 test failed...)
- maybe that can be tidied up on commit.

Thanks,
-=Eric

> ---
>  tests/generic/029     | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/029.out |  65 ++++++++++++++++++++++++++
>  tests/generic/group   |   1 +
>  3 files changed, 189 insertions(+)
>  create mode 100644 tests/generic/029
>  create mode 100644 tests/generic/029.out
> 
> diff --git a/tests/generic/029 b/tests/generic/029
> new file mode 100644
> index 0000000..854794e
> --- /dev/null
> +++ b/tests/generic/029
> @@ -0,0 +1,123 @@
> +#! /bin/bash
> +# FS QA Test No. generic/029
> +#
> +# Test mapped writes against truncate down/up to ensure we get the data
> +# correctly written. This can expose data corruption bugs on filesystems where
> +# the block size is smaller than the page size.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2014 Red Hat, Inc.  All Rights Reserved.
> +#
> +# 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
> +#-----------------------------------------------------------------------
> +#
> +
> +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()
> +{
> +    cd /
> +    rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os IRIX Linux
> +_require_scratch
> +
> +testfile=$SCRATCH_MNT/testfile
> +
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount
> +
> +# first case is just truncate down/truncate up to check that the mapped
> +# write after the truncate up is correctly handled.
> +$XFS_IO_PROG -t -f \
> +-c "truncate 5120"              `# truncate     |                        |` \
> +-c "pwrite -S 0x58 0 5120"      `# write        |XXXXXXXXXXXXXXXXXXXXXXXX|` \
> +-c "mmap -rw 0 5120"            `# mmap         |                        |` \
> +-c "mwrite -S 0x5a 2048 3072"   `# mwrite       |          ZZZZZZZZZZZZZZ|` \
> +-c "truncate 2048"              `# truncate dn  |         |` \
> +-c "truncate 5120"              `# truncate up  |                        |` \
> +-c "mwrite -S 0x59 2048 3072"   `# mwrite       |          YYYYYYYYYYYYYY|` \
> +-c "close"      \
> +$testfile | _filter_xfs_io
> +
> +echo "==== Pre-Remount ==="
> +hexdump -C $testfile
> +_scratch_remount
> +echo "==== Post-Remount =="
> +hexdump -C $testfile
> +
> +rm -f $testfile
> +sync
> +
> +# second case is to do a mwrite between the truncate to a block on the
> +# same page we are truncating within the EOF. This checks that a mapped
> +# write between truncate down and truncate up a further mapped
> +# write to the same page into the new space doesn't result in data being lost.
> +$XFS_IO_PROG -t -f \
> +-c "truncate 5120"              `# truncate     |                        |` \
> +-c "pwrite -S 0x58 0 5120"      `# write        |XXXXXXXXXXXXXXXXXXXXXXXX|` \
> +-c "mmap -rw 0 5120"            `# mmap         |                        |` \
> +-c "mwrite -S 0x5a 2048 3072"   `# mwrite       |          ZZZZZZZZZZZZZZ|` \
> +-c "truncate 2048"              `# truncate dn  |         |` \
> +-c "mwrite -S 0x57 1024 1024"   `# mwrite       |     WWWWW              |` \
> +-c "truncate 5120"              `# truncate up  |                        |` \
> +-c "mwrite -S 0x59 2048 3072"   `# mwrite       |          YYYYYYYYYYYYYY|` \
> +-c "close"      \
> +$testfile | _filter_xfs_io
> + 
> +echo "==== Pre-Remount ==="
> +hexdump -C $testfile
> +_scratch_remount
> +echo "==== Post-Remount =="
> +hexdump -C $testfile
> +
> +# third case is the same as second case, just with non-1k aligned offsets and
> +# sizes.
> +$XFS_IO_PROG -t -f \
> +-c "truncate 5121"              `# truncate     |                        |` \
> +-c "pwrite -S 0x58 0 5121"      `# write        |XXXXXXXXXXXXXXXXXXXXXXXX|` \
> +-c "mmap -rw 0 5121"            `# mmap         |                        |` \
> +-c "mwrite -S 0x5a 2047 3071"   `# mwrite       |          ZZZZZZZZZZZZZZ|` \
> +-c "truncate 2047"              `# truncate dn  |         |` \
> +-c "mwrite -S 0x57 513 1025"    `# mwrite       |  WWWWW                 |` \
> +-c "truncate 5121"              `# truncate up  |                        |` \
> +-c "mwrite -S 0x59 2047 3071"   `# mwrite       |          YYYYYYYYYYYYYY|` \
> +-c "close"      \
> +$testfile | _filter_xfs_io
> + 
> +echo "==== Pre-Remount ==="
> +hexdump -C $testfile
> +_scratch_remount
> +echo "==== Post-Remount =="
> +hexdump -C $testfile
> +
> +status=0
> +exit
> +
> diff --git a/tests/generic/029.out b/tests/generic/029.out
> new file mode 100644
> index 0000000..457982c
> --- /dev/null
> +++ b/tests/generic/029.out
> @@ -0,0 +1,65 @@
> +QA output created by 029
> +wrote 5120/5120 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +==== Pre-Remount ===
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +00000800  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +00001400
> +==== Post-Remount ==
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +00000800  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +00001400
> +wrote 5120/5120 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +==== Pre-Remount ===
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +00000400  57 57 57 57 57 57 57 57  57 57 57 57 57 57 57 57  |WWWWWWWWWWWWWWWW|
> +*
> +00000800  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +00001400
> +==== Post-Remount ==
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +00000400  57 57 57 57 57 57 57 57  57 57 57 57 57 57 57 57  |WWWWWWWWWWWWWWWW|
> +*
> +00000800  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +00001400
> +wrote 5121/5121 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +==== Pre-Remount ===
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +00000200  58 57 57 57 57 57 57 57  57 57 57 57 57 57 57 57  |XWWWWWWWWWWWWWWW|
> +00000210  57 57 57 57 57 57 57 57  57 57 57 57 57 57 57 57  |WWWWWWWWWWWWWWWW|
> +*
> +00000600  57 57 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |WWXXXXXXXXXXXXXX|
> +00000610  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +000007f0  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 59  |XXXXXXXXXXXXXXXY|
> +00000800  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +000013f0  59 59 59 59 59 59 59 59  59 59 59 59 59 59 00 00  |YYYYYYYYYYYYYY..|
> +00001400  00                                                |.|
> +00001401
> +==== Post-Remount ==
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +00000200  58 57 57 57 57 57 57 57  57 57 57 57 57 57 57 57  |XWWWWWWWWWWWWWWW|
> +00000210  57 57 57 57 57 57 57 57  57 57 57 57 57 57 57 57  |WWWWWWWWWWWWWWWW|
> +*
> +00000600  57 57 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |WWXXXXXXXXXXXXXX|
> +00000610  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +000007f0  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 59  |XXXXXXXXXXXXXXXY|
> +00000800  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +000013f0  59 59 59 59 59 59 59 59  59 59 59 59 59 59 00 00  |YYYYYYYYYYYYYY..|
> +00001400  00                                                |.|
> +00001401
> diff --git a/tests/generic/group b/tests/generic/group
> index bdcfd9d..18c94de 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -31,6 +31,7 @@
>  026 acl quick auto
>  027 auto enospc
>  028 auto quick
> +029 auto quick rw
>  053 acl repair auto quick
>  062 attr udf auto quick
>  068 other auto freeze dangerous stress
> 

[PATCH 4/5] generic: add mmap write vs truncate/remap test

[Dave Chinner]

This test exposed a problem with mapped writes to the tail page of a
file in XFS and potentially ext4. Eric did all the hard work of
taking the bug report and generating the reproducable test case
on ext4, but I haven't been able to reproduce then problem on ext4.

Regardless, make it a generic test so that we can ensure that all
filesystems handle the case correctly.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 tests/generic/030     | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/030.out |  53 ++++++++++++++++++
 tests/generic/group   |   1 +
 3 files changed, 202 insertions(+)
 create mode 100644 tests/generic/030
 create mode 100644 tests/generic/030.out

diff --git a/tests/generic/030 b/tests/generic/030
new file mode 100644
index 0000000..9947b96
--- /dev/null
+++ b/tests/generic/030
@@ -0,0 +1,148 @@
+#! /bin/bash
+# FS QA Test No. generic/030
+#
+# Test mapped writes against remap+truncate down/up to ensure we get the data
+# correctly written. This can expose data corruption bugs on filesystems where
+# the block size is smaller than the page size.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Red Hat, Inc.  All Rights Reserved.
+#
+# 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
+#-----------------------------------------------------------------------
+#
+
+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()
+{
+    cd /
+    rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os IRIX Linux
+_require_scratch
+_require_xfs_io_command "mremap"
+
+testfile=$SCRATCH_MNT/testfile
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+
+# first case is just truncate down/truncate up to check that the mapped
+# write after the truncate up is correctly handled.
+$XFS_IO_PROG -t -f \
+-c "truncate 5017k"             `# truncate     |       |` \
+-c "pwrite -S 0x58 0 5017k"     `# write        |X...XXX|` \
+-c "mmap -rw 0 5017k"           `# mmap         |       |` \
+-c "truncate 5020k"             `# truncate up  |                        |` \
+-c "mremap -m 5020k"            `# mremap up    |                        |` \
+-c "mwrite -S 0x57 5017k 3k"    `# mwrite       |        WWWWWWWWWWWWWWWW|` \
+-c "mremap 5017k "              `# mremap dn    |       |` \
+-c "truncate 5017k"             `# mremap dn    |       |` \
+-c "truncate 5020k"             `# truncate up  |                        |` \
+-c "mremap -m 5020k"            `# mremap up    |                        |` \
+-c "mwrite -S 0x59 5017k 3k"    `# mwrite       |        YYYYYYYYYYYYYYYY|` \
+-c "close"      \
+$testfile | _filter_xfs_io
+
+echo "==== Pre-Remount ==="
+hexdump -C $testfile
+_scratch_remount
+echo "==== Post-Remount =="
+hexdump -C $testfile
+
+rm -f $testfile
+sync
+
+# second case is to do a mwrite between the truncate to a block on the
+# same page we are truncating within the EOF. This checks that a mapped
+# write between truncate down and truncate up a further mapped
+# write to the same page into the new space doesn't result in data being lost.
+$XFS_IO_PROG -t -f \
+-c "truncate 5017k"             `# truncate     |       |` \
+-c "pwrite -S 0x58 0 5017k"     `# write        |X...XXX|` \
+-c "mmap -rw 0 5017k"           `# mmap         |       |` \
+-c "truncate 5020k"             `# truncate up  |                        |` \
+-c "mremap -m 5020k"            `# mremap up    |                        |` \
+-c "mwrite -S 0x57 5017k 3k"    `# mwrite       |        WWWWWWWWWWWWWWWW|` \
+-c "mremap 5017k "              `# mremap dn    |       |` \
+-c "truncate 5017k"             `# mremap dn    |       |` \
+-c "mwrite -S 0x5a 5016k 1k"    `# mwrite       |    ZZZ                 |` \
+-c "truncate 5020k"             `# truncate up  |                        |` \
+-c "mremap -m 5020k"            `# mremap up    |                        |` \
+-c "mwrite -S 0x59 5017k 3k"    `# mwrite       |        YYYYYYYYYYYYYYYY|` \
+-c "close"      \
+$testfile | _filter_xfs_io
+
+echo "==== Pre-Remount ==="
+hexdump -C $testfile
+_scratch_remount
+echo "==== Post-Remount =="
+hexdump -C $testfile
+
+# third case is the same as the first, but this time on unaligned byte
+# boundaries rather than block boundaries. This mimics the exact mmap write
+# patterns of the application that exposed the bug in the first place, and
+# so is somewhat more complex and has repeated operations in it. 
+$XFS_IO_PROG -t -f \
+-c "truncate 5136912" \
+-c "pwrite -S 0x58 0 5136912" \
+-c "mmap -rw 0 5136912" \
+-c "mremap 5136912" \
+-c "truncate 5136912" \
+-c "truncate 5139720" \
+-c "mremap -m 5139720" \
+-c "mwrite -S 0 5136912 2808" \
+-c "mwrite -S 0 5136912 2808" \
+-c "mwrite -S 0 5136912 2808" \
+-c "mremap 5136912 " \
+-c "truncate 5136912" \
+-c "truncate 5139720" \
+-c "mremap -m 5139720" \
+-c "mwrite -S 0 5136912 2808" \
+-c "mwrite -S 0 5136912 2808" \
+-c "mwrite -S 0x59 5136912 2808" \
+-c "truncate 5140480" \
+-c "mremap 5140480" \
+-c "msync -s 0 5140480" \
+-c "mremap 5139720" \
+-c "munmap" \
+-c "close" \
+$testfile | _filter_xfs_io
+
+echo "==== Pre-Remount ==="
+hexdump -C $testfile
+_scratch_remount
+echo "==== Post-Remount =="
+hexdump -C $testfile
+
+status=0
+exit
+
diff --git a/tests/generic/030.out b/tests/generic/030.out
new file mode 100644
index 0000000..20f6561
--- /dev/null
+++ b/tests/generic/030.out
@@ -0,0 +1,53 @@
+QA output created by 030
+wrote 5137408/5137408 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+==== Pre-Remount ===
+00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
+*
+004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
+*
+004e7000
+==== Post-Remount ==
+00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
+*
+004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
+*
+004e7000
+wrote 5137408/5137408 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+==== Pre-Remount ===
+00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
+*
+004e6000  5a 5a 5a 5a 5a 5a 5a 5a  5a 5a 5a 5a 5a 5a 5a 5a  |ZZZZZZZZZZZZZZZZ|
+*
+004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
+*
+004e7000
+==== Post-Remount ==
+00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
+*
+004e6000  5a 5a 5a 5a 5a 5a 5a 5a  5a 5a 5a 5a 5a 5a 5a 5a  |ZZZZZZZZZZZZZZZZ|
+*
+004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
+*
+004e7000
+wrote 5136912/5136912 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+==== Pre-Remount ===
+00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
+*
+004e6210  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
+*
+004e6d00  59 59 59 59 59 59 59 59  00 00 00 00 00 00 00 00  |YYYYYYYY........|
+004e6d10  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+*
+004e7000
+==== Post-Remount ==
+00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
+*
+004e6210  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
+*
+004e6d00  59 59 59 59 59 59 59 59  00 00 00 00 00 00 00 00  |YYYYYYYY........|
+004e6d10  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+*
+004e7000
diff --git a/tests/generic/group b/tests/generic/group
index 18c94de..ba1c913 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -32,6 +32,7 @@
 027 auto enospc
 028 auto quick
 029 auto quick rw
+030 auto quick rw
 053 acl repair auto quick
 062 attr udf auto quick
 068 other auto freeze dangerous stress
-- 
2.0.0

Re: [PATCH 4/5] generic: add mmap write vs truncate/remap test

[Eric Sandeen]

On 9/16/14 8:41 PM, Dave Chinner wrote:
> This test exposed a problem with mapped writes to the tail page of a
> file in XFS and potentially ext4. Eric did all the hard work of
> taking the bug report and generating the reproducable test case
> on ext4, but I haven't been able to reproduce then problem on ext4.

Hm, not even w/ the last test case below, on the byte boundaries?

FWIW, I think I can simplify that one - the repeated ops aren't
necessary.  Probably no need to carry that along if they aren't
relevant to the bug.

I'll look at this more closely tomorrow, thanks for encapsulating
this in an xfstest.

-Eric

> Regardless, make it a generic test so that we can ensure that all
> filesystems handle the case correctly.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  tests/generic/030     | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/030.out |  53 ++++++++++++++++++
>  tests/generic/group   |   1 +
>  3 files changed, 202 insertions(+)
>  create mode 100644 tests/generic/030
>  create mode 100644 tests/generic/030.out
> 
> diff --git a/tests/generic/030 b/tests/generic/030
> new file mode 100644
> index 0000000..9947b96
> --- /dev/null
> +++ b/tests/generic/030
> @@ -0,0 +1,148 @@
> +#! /bin/bash
> +# FS QA Test No. generic/030
> +#
> +# Test mapped writes against remap+truncate down/up to ensure we get the data
> +# correctly written. This can expose data corruption bugs on filesystems where
> +# the block size is smaller than the page size.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2014 Red Hat, Inc.  All Rights Reserved.
> +#
> +# 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
> +#-----------------------------------------------------------------------
> +#
> +
> +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()
> +{
> +    cd /
> +    rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os IRIX Linux
> +_require_scratch
> +_require_xfs_io_command "mremap"
> +
> +testfile=$SCRATCH_MNT/testfile
> +
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount
> +
> +# first case is just truncate down/truncate up to check that the mapped
> +# write after the truncate up is correctly handled.
> +$XFS_IO_PROG -t -f \
> +-c "truncate 5017k"             `# truncate     |       |` \
> +-c "pwrite -S 0x58 0 5017k"     `# write        |X...XXX|` \
> +-c "mmap -rw 0 5017k"           `# mmap         |       |` \
> +-c "truncate 5020k"             `# truncate up  |                        |` \
> +-c "mremap -m 5020k"            `# mremap up    |                        |` \
> +-c "mwrite -S 0x57 5017k 3k"    `# mwrite       |        WWWWWWWWWWWWWWWW|` \
> +-c "mremap 5017k "              `# mremap dn    |       |` \
> +-c "truncate 5017k"             `# mremap dn    |       |` \
> +-c "truncate 5020k"             `# truncate up  |                        |` \
> +-c "mremap -m 5020k"            `# mremap up    |                        |` \
> +-c "mwrite -S 0x59 5017k 3k"    `# mwrite       |        YYYYYYYYYYYYYYYY|` \
> +-c "close"      \
> +$testfile | _filter_xfs_io
> +
> +echo "==== Pre-Remount ==="
> +hexdump -C $testfile
> +_scratch_remount
> +echo "==== Post-Remount =="
> +hexdump -C $testfile
> +
> +rm -f $testfile
> +sync
> +
> +# second case is to do a mwrite between the truncate to a block on the
> +# same page we are truncating within the EOF. This checks that a mapped
> +# write between truncate down and truncate up a further mapped
> +# write to the same page into the new space doesn't result in data being lost.
> +$XFS_IO_PROG -t -f \
> +-c "truncate 5017k"             `# truncate     |       |` \
> +-c "pwrite -S 0x58 0 5017k"     `# write        |X...XXX|` \
> +-c "mmap -rw 0 5017k"           `# mmap         |       |` \
> +-c "truncate 5020k"             `# truncate up  |                        |` \
> +-c "mremap -m 5020k"            `# mremap up    |                        |` \
> +-c "mwrite -S 0x57 5017k 3k"    `# mwrite       |        WWWWWWWWWWWWWWWW|` \
> +-c "mremap 5017k "              `# mremap dn    |       |` \
> +-c "truncate 5017k"             `# mremap dn    |       |` \
> +-c "mwrite -S 0x5a 5016k 1k"    `# mwrite       |    ZZZ                 |` \
> +-c "truncate 5020k"             `# truncate up  |                        |` \
> +-c "mremap -m 5020k"            `# mremap up    |                        |` \
> +-c "mwrite -S 0x59 5017k 3k"    `# mwrite       |        YYYYYYYYYYYYYYYY|` \
> +-c "close"      \
> +$testfile | _filter_xfs_io
> +
> +echo "==== Pre-Remount ==="
> +hexdump -C $testfile
> +_scratch_remount
> +echo "==== Post-Remount =="
> +hexdump -C $testfile
> +
> +# third case is the same as the first, but this time on unaligned byte
> +# boundaries rather than block boundaries. This mimics the exact mmap write
> +# patterns of the application that exposed the bug in the first place, and
> +# so is somewhat more complex and has repeated operations in it. 
> +$XFS_IO_PROG -t -f \
> +-c "truncate 5136912" \
> +-c "pwrite -S 0x58 0 5136912" \
> +-c "mmap -rw 0 5136912" \
> +-c "mremap 5136912" \
> +-c "truncate 5136912" \
> +-c "truncate 5139720" \
> +-c "mremap -m 5139720" \
> +-c "mwrite -S 0 5136912 2808" \
> +-c "mwrite -S 0 5136912 2808" \
> +-c "mwrite -S 0 5136912 2808" \
> +-c "mremap 5136912 " \
> +-c "truncate 5136912" \
> +-c "truncate 5139720" \
> +-c "mremap -m 5139720" \
> +-c "mwrite -S 0 5136912 2808" \
> +-c "mwrite -S 0 5136912 2808" \
> +-c "mwrite -S 0x59 5136912 2808" \
> +-c "truncate 5140480" \
> +-c "mremap 5140480" \
> +-c "msync -s 0 5140480" \
> +-c "mremap 5139720" \
> +-c "munmap" \
> +-c "close" \
> +$testfile | _filter_xfs_io
> +
> +echo "==== Pre-Remount ==="
> +hexdump -C $testfile
> +_scratch_remount
> +echo "==== Post-Remount =="
> +hexdump -C $testfile
> +
> +status=0
> +exit
> +
> diff --git a/tests/generic/030.out b/tests/generic/030.out
> new file mode 100644
> index 0000000..20f6561
> --- /dev/null
> +++ b/tests/generic/030.out
> @@ -0,0 +1,53 @@
> +QA output created by 030
> +wrote 5137408/5137408 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +==== Pre-Remount ===
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +004e7000
> +==== Post-Remount ==
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +004e7000
> +wrote 5137408/5137408 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +==== Pre-Remount ===
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +004e6000  5a 5a 5a 5a 5a 5a 5a 5a  5a 5a 5a 5a 5a 5a 5a 5a  |ZZZZZZZZZZZZZZZZ|
> +*
> +004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +004e7000
> +==== Post-Remount ==
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +004e6000  5a 5a 5a 5a 5a 5a 5a 5a  5a 5a 5a 5a 5a 5a 5a 5a  |ZZZZZZZZZZZZZZZZ|
> +*
> +004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +004e7000
> +wrote 5136912/5136912 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +==== Pre-Remount ===
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +004e6210  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +004e6d00  59 59 59 59 59 59 59 59  00 00 00 00 00 00 00 00  |YYYYYYYY........|
> +004e6d10  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> +*
> +004e7000
> +==== Post-Remount ==
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +004e6210  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +004e6d00  59 59 59 59 59 59 59 59  00 00 00 00 00 00 00 00  |YYYYYYYY........|
> +004e6d10  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> +*
> +004e7000
> diff --git a/tests/generic/group b/tests/generic/group
> index 18c94de..ba1c913 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -32,6 +32,7 @@
>  027 auto enospc
>  028 auto quick
>  029 auto quick rw
> +030 auto quick rw
>  053 acl repair auto quick
>  062 attr udf auto quick
>  068 other auto freeze dangerous stress
> 

Re: [PATCH 4/5] generic: add mmap write vs truncate/remap test

[Dave Chinner]

On Tue, Sep 16, 2014 at 11:32:11PM -0500, Eric Sandeen wrote:
> On 9/16/14 8:41 PM, Dave Chinner wrote:
> > This test exposed a problem with mapped writes to the tail page of a
> > file in XFS and potentially ext4. Eric did all the hard work of
> > taking the bug report and generating the reproducable test case
> > on ext4, but I haven't been able to reproduce then problem on ext4.
> 
> Hm, not even w/ the last test case below, on the byte boundaries?

Sorry, I forgot to rewrite that once I got the byte boundary test
from you. It does, indeed, fail on ext4 as does the previous
mmap/truncate test:

$ sudo MKFS_OPTIONS="-b 1024" ./check generic/029 generic/03[01]
FSTYP         -- ext4
PLATFORM      -- Linux/x86_64 test4 3.17.0-rc4-dgc+
MKFS_OPTIONS  -- -b 1024 /dev/ram1
MOUNT_OPTIONS -- -o acl,user_xattr /dev/ram1 /mnt/scr

generic/029 2s ... - output mismatch (see /home/dave/src/xfstests-dev/results//generic/029.out.bad)
    --- tests/generic/029.out   2014-09-17 10:33:15.000000000 +1000
    +++ /home/dave/src/xfstests-dev/results//generic/029.out.bad        2014-09-17 14:50:02.000000000 +1000
    @@ -10,7 +10,9 @@
     ==== Post-Remount ==
     00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
     *
    -00000800  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
    +00000800  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
    +*
    +00001000  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
    ...
    (Run 'diff -u tests/generic/029.out /home/dave/src/xfstests-dev/results//generic/029.out.bad'  to see the entire diff)
generic/030 1s ... - output mismatch (see /home/dave/src/xfstests-dev/results//generic/030.out.bad)
    --- tests/generic/030.out   2014-09-17 11:13:55.000000000 +1000
    +++ /home/dave/src/xfstests-dev/results//generic/030.out.bad        2014-09-17 14:50:03.000000000 +1000
    @@ -10,7 +10,7 @@
     ==== Post-Remount ==
     00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
     *
    -004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
    +004e6400  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
     *
     004e7000
    ...
    (Run 'diff -u tests/generic/030.out /home/dave/src/xfstests-dev/results//generic/030.out.bad'  to see the entire diff)
generic/031 2s ... 2s
Ran: generic/029 generic/030 generic/031
Failures: generic/029 generic/030
Failed 2 of 3 tests


> FWIW, I think I can simplify that one - the repeated ops aren't
> necessary.  Probably no need to carry that along if they aren't
> relevant to the bug.
> 
> I'll look at this more closely tomorrow, thanks for encapsulating
> this in an xfstest.

Can you send any updates as delta patches to this one?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCH 4.5/5] generic: tidy up mmap write vs truncate/remap test

[Eric Sandeen]

Some minor changes to 030 as sent, to:

1) differentiate which tests failed in the output hunks, and
2) remove extraneous operations from the last test.

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

diff --git a/tests/generic/030 b/tests/generic/030
old mode 100644
new mode 100755
index 9947b96..8bb50bd
--- a/tests/generic/030
+++ b/tests/generic/030
@@ -72,10 +72,10 @@ $XFS_IO_PROG -t -f \
 -c "close"      \
 $testfile | _filter_xfs_io
 
-echo "==== Pre-Remount ==="
+echo "==== Pre-Remount 1 ==="
 hexdump -C $testfile
 _scratch_remount
-echo "==== Post-Remount =="
+echo "==== Post-Remount 1 =="
 hexdump -C $testfile
 
 rm -f $testfile
@@ -101,10 +101,10 @@ $XFS_IO_PROG -t -f \
 -c "close"      \
 $testfile | _filter_xfs_io
 
-echo "==== Pre-Remount ==="
+echo "==== Pre-Remount 2 ==="
 hexdump -C $testfile
 _scratch_remount
-echo "==== Post-Remount =="
+echo "==== Post-Remount 2 =="
 hexdump -C $testfile
 
 # third case is the same as the first, but this time on unaligned byte
@@ -115,32 +115,21 @@ $XFS_IO_PROG -t -f \
 -c "truncate 5136912" \
 -c "pwrite -S 0x58 0 5136912" \
 -c "mmap -rw 0 5136912" \
--c "mremap 5136912" \
--c "truncate 5136912" \
 -c "truncate 5139720" \
 -c "mremap -m 5139720" \
 -c "mwrite -S 0 5136912 2808" \
--c "mwrite -S 0 5136912 2808" \
--c "mwrite -S 0 5136912 2808" \
 -c "mremap 5136912 " \
 -c "truncate 5136912" \
 -c "truncate 5139720" \
 -c "mremap -m 5139720" \
--c "mwrite -S 0 5136912 2808" \
--c "mwrite -S 0 5136912 2808" \
 -c "mwrite -S 0x59 5136912 2808" \
--c "truncate 5140480" \
--c "mremap 5140480" \
--c "msync -s 0 5140480" \
--c "mremap 5139720" \
--c "munmap" \
 -c "close" \
 $testfile | _filter_xfs_io
 
-echo "==== Pre-Remount ==="
+echo "==== Pre-Remount 3 ==="
 hexdump -C $testfile
 _scratch_remount
-echo "==== Post-Remount =="
+echo "==== Post-Remount 3 =="
 hexdump -C $testfile
 
 status=0
diff --git a/tests/generic/030.out b/tests/generic/030.out
index 20f6561..b3a17e5 100644
--- a/tests/generic/030.out
+++ b/tests/generic/030.out
@@ -1,13 +1,13 @@
 QA output created by 030
 wrote 5137408/5137408 bytes at offset 0
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-==== Pre-Remount ===
+==== Pre-Remount 1 ===
 00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
 *
 004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
 *
 004e7000
-==== Post-Remount ==
+==== Post-Remount 1 ==
 00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
 *
 004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
@@ -15,7 +15,7 @@ XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 004e7000
 wrote 5137408/5137408 bytes at offset 0
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-==== Pre-Remount ===
+==== Pre-Remount 2 ===
 00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
 *
 004e6000  5a 5a 5a 5a 5a 5a 5a 5a  5a 5a 5a 5a 5a 5a 5a 5a  |ZZZZZZZZZZZZZZZZ|
@@ -23,7 +23,7 @@ XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
 *
 004e7000
-==== Post-Remount ==
+==== Post-Remount 2 ==
 00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
 *
 004e6000  5a 5a 5a 5a 5a 5a 5a 5a  5a 5a 5a 5a 5a 5a 5a 5a  |ZZZZZZZZZZZZZZZZ|
@@ -33,7 +33,7 @@ XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 004e7000
 wrote 5136912/5136912 bytes at offset 0
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-==== Pre-Remount ===
+==== Pre-Remount 3 ===
 00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
 *
 004e6210  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
@@ -42,7 +42,7 @@ XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 004e6d10  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
 *
 004e7000
-==== Post-Remount ==
+==== Post-Remount 3 ==
 00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
 *
 004e6210  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|

Re: [PATCH 4/5] generic: add mmap write vs truncate/remap test

[Eric Sandeen]

On 9/16/14 8:41 PM, Dave Chinner wrote:
> This test exposed a problem with mapped writes to the tail page of a
> file in XFS and potentially ext4. Eric did all the hard work of
> taking the bug report and generating the reproducable test case
> on ext4, but I haven't been able to reproduce then problem on ext4.
> 
> Regardless, make it a generic test so that we can ensure that all
> filesystems handle the case correctly.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Fine as it is; I've also sent a cleanup patch which you can fold in,
or merge separately, as you wish.

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

> ---
>  tests/generic/030     | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/030.out |  53 ++++++++++++++++++
>  tests/generic/group   |   1 +
>  3 files changed, 202 insertions(+)
>  create mode 100644 tests/generic/030
>  create mode 100644 tests/generic/030.out
> 
> diff --git a/tests/generic/030 b/tests/generic/030
> new file mode 100644
> index 0000000..9947b96
> --- /dev/null
> +++ b/tests/generic/030
> @@ -0,0 +1,148 @@
> +#! /bin/bash
> +# FS QA Test No. generic/030
> +#
> +# Test mapped writes against remap+truncate down/up to ensure we get the data
> +# correctly written. This can expose data corruption bugs on filesystems where
> +# the block size is smaller than the page size.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2014 Red Hat, Inc.  All Rights Reserved.
> +#
> +# 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
> +#-----------------------------------------------------------------------
> +#
> +
> +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()
> +{
> +    cd /
> +    rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os IRIX Linux
> +_require_scratch
> +_require_xfs_io_command "mremap"
> +
> +testfile=$SCRATCH_MNT/testfile
> +
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount
> +
> +# first case is just truncate down/truncate up to check that the mapped
> +# write after the truncate up is correctly handled.
> +$XFS_IO_PROG -t -f \
> +-c "truncate 5017k"             `# truncate     |       |` \
> +-c "pwrite -S 0x58 0 5017k"     `# write        |X...XXX|` \
> +-c "mmap -rw 0 5017k"           `# mmap         |       |` \
> +-c "truncate 5020k"             `# truncate up  |                        |` \
> +-c "mremap -m 5020k"            `# mremap up    |                        |` \
> +-c "mwrite -S 0x57 5017k 3k"    `# mwrite       |        WWWWWWWWWWWWWWWW|` \
> +-c "mremap 5017k "              `# mremap dn    |       |` \
> +-c "truncate 5017k"             `# mremap dn    |       |` \
> +-c "truncate 5020k"             `# truncate up  |                        |` \
> +-c "mremap -m 5020k"            `# mremap up    |                        |` \
> +-c "mwrite -S 0x59 5017k 3k"    `# mwrite       |        YYYYYYYYYYYYYYYY|` \
> +-c "close"      \
> +$testfile | _filter_xfs_io
> +
> +echo "==== Pre-Remount ==="
> +hexdump -C $testfile
> +_scratch_remount
> +echo "==== Post-Remount =="
> +hexdump -C $testfile
> +
> +rm -f $testfile
> +sync
> +
> +# second case is to do a mwrite between the truncate to a block on the
> +# same page we are truncating within the EOF. This checks that a mapped
> +# write between truncate down and truncate up a further mapped
> +# write to the same page into the new space doesn't result in data being lost.
> +$XFS_IO_PROG -t -f \
> +-c "truncate 5017k"             `# truncate     |       |` \
> +-c "pwrite -S 0x58 0 5017k"     `# write        |X...XXX|` \
> +-c "mmap -rw 0 5017k"           `# mmap         |       |` \
> +-c "truncate 5020k"             `# truncate up  |                        |` \
> +-c "mremap -m 5020k"            `# mremap up    |                        |` \
> +-c "mwrite -S 0x57 5017k 3k"    `# mwrite       |        WWWWWWWWWWWWWWWW|` \
> +-c "mremap 5017k "              `# mremap dn    |       |` \
> +-c "truncate 5017k"             `# mremap dn    |       |` \
> +-c "mwrite -S 0x5a 5016k 1k"    `# mwrite       |    ZZZ                 |` \
> +-c "truncate 5020k"             `# truncate up  |                        |` \
> +-c "mremap -m 5020k"            `# mremap up    |                        |` \
> +-c "mwrite -S 0x59 5017k 3k"    `# mwrite       |        YYYYYYYYYYYYYYYY|` \
> +-c "close"      \
> +$testfile | _filter_xfs_io
> +
> +echo "==== Pre-Remount ==="
> +hexdump -C $testfile
> +_scratch_remount
> +echo "==== Post-Remount =="
> +hexdump -C $testfile
> +
> +# third case is the same as the first, but this time on unaligned byte
> +# boundaries rather than block boundaries. This mimics the exact mmap write
> +# patterns of the application that exposed the bug in the first place, and
> +# so is somewhat more complex and has repeated operations in it. 
> +$XFS_IO_PROG -t -f \
> +-c "truncate 5136912" \
> +-c "pwrite -S 0x58 0 5136912" \
> +-c "mmap -rw 0 5136912" \
> +-c "mremap 5136912" \
> +-c "truncate 5136912" \
> +-c "truncate 5139720" \
> +-c "mremap -m 5139720" \
> +-c "mwrite -S 0 5136912 2808" \
> +-c "mwrite -S 0 5136912 2808" \
> +-c "mwrite -S 0 5136912 2808" \
> +-c "mremap 5136912 " \
> +-c "truncate 5136912" \
> +-c "truncate 5139720" \
> +-c "mremap -m 5139720" \
> +-c "mwrite -S 0 5136912 2808" \
> +-c "mwrite -S 0 5136912 2808" \
> +-c "mwrite -S 0x59 5136912 2808" \
> +-c "truncate 5140480" \
> +-c "mremap 5140480" \
> +-c "msync -s 0 5140480" \
> +-c "mremap 5139720" \
> +-c "munmap" \
> +-c "close" \
> +$testfile | _filter_xfs_io
> +
> +echo "==== Pre-Remount ==="
> +hexdump -C $testfile
> +_scratch_remount
> +echo "==== Post-Remount =="
> +hexdump -C $testfile
> +
> +status=0
> +exit
> +
> diff --git a/tests/generic/030.out b/tests/generic/030.out
> new file mode 100644
> index 0000000..20f6561
> --- /dev/null
> +++ b/tests/generic/030.out
> @@ -0,0 +1,53 @@
> +QA output created by 030
> +wrote 5137408/5137408 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +==== Pre-Remount ===
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +004e7000
> +==== Post-Remount ==
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +004e7000
> +wrote 5137408/5137408 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +==== Pre-Remount ===
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +004e6000  5a 5a 5a 5a 5a 5a 5a 5a  5a 5a 5a 5a 5a 5a 5a 5a  |ZZZZZZZZZZZZZZZZ|
> +*
> +004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +004e7000
> +==== Post-Remount ==
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +004e6000  5a 5a 5a 5a 5a 5a 5a 5a  5a 5a 5a 5a 5a 5a 5a 5a  |ZZZZZZZZZZZZZZZZ|
> +*
> +004e6400  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +004e7000
> +wrote 5136912/5136912 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +==== Pre-Remount ===
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +004e6210  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +004e6d00  59 59 59 59 59 59 59 59  00 00 00 00 00 00 00 00  |YYYYYYYY........|
> +004e6d10  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> +*
> +004e7000
> +==== Post-Remount ==
> +00000000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
> +*
> +004e6210  59 59 59 59 59 59 59 59  59 59 59 59 59 59 59 59  |YYYYYYYYYYYYYYYY|
> +*
> +004e6d00  59 59 59 59 59 59 59 59  00 00 00 00 00 00 00 00  |YYYYYYYY........|
> +004e6d10  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> +*
> +004e7000
> diff --git a/tests/generic/group b/tests/generic/group
> index 18c94de..ba1c913 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -32,6 +32,7 @@
>  027 auto enospc
>  028 auto quick
>  029 auto quick rw
> +030 auto quick rw
>  053 acl repair auto quick
>  062 attr udf auto quick
>  068 other auto freeze dangerous stress
> 

Re: [PATCH 4/5] generic: add mmap write vs truncate/remap test

[Eric Sandeen]

On 9/16/14 8:41 PM, Dave Chinner wrote:
> This test exposed a problem with mapped writes to the tail page of a
> file in XFS and potentially ext4. Eric did all the hard work of
> taking the bug report and generating the reproducable test case
> on ext4, but I haven't been able to reproduce then problem on ext4.
> 
> Regardless, make it a generic test so that we can ensure that all
> filesystems handle the case correctly.

Oof, kermit on #xfs points out that even this is sufficient to show
a problem:

 mkfs.ext4 -b 1024 -F empty.img
  mount -o loop empty.img mnt
  sync

xfs_io -f -t \
-c            "pwrite               0          0x210  "      \
-c            "mmap      -rw        0          0xd08  "      \
-c            "mwrite    -S         0x50       0x210  0xaf8"  \
-c            "truncate                        0x1000"  \
mnt/testfile

  echo "==== Pre-Remount ==="
  hexdump -C mnt/testfile | tail -n 8
  umount mnt/
  mount -o loop empty.img mnt
  echo "==== Post-Remount =="
  hexdump -C mnt/testfile | tail -n 8



i'm not sure about the testcase - mmapping beyond EOF? - but seeing data
change after remount looks scary regardless.

==== Pre-Remount ===
00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000210  50 50 50 50 50 50 50 50  50 50 50 50 50 50 50 50  |PPPPPPPPPPPPPPPP|
*
00000d00  50 50 50 50 50 50 50 50  00 00 00 00 00 00 00 00  |PPPPPPPP........|
00000d10  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00001000
==== Post-Remount ==
00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000210  50 50 50 50 50 50 50 50  50 50 50 50 50 50 50 50  |PPPPPPPPPPPPPPPP|
*
00000400  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00001000

-Eric

Re: [PATCH 4/5] generic: add mmap write vs truncate/remap test

[Dave Chinner]

On Fri, Sep 19, 2014 at 07:17:20PM -0500, Eric Sandeen wrote:
> On 9/16/14 8:41 PM, Dave Chinner wrote:
> > This test exposed a problem with mapped writes to the tail page of a
> > file in XFS and potentially ext4. Eric did all the hard work of
> > taking the bug report and generating the reproducable test case
> > on ext4, but I haven't been able to reproduce then problem on ext4.
> > 
> > Regardless, make it a generic test so that we can ensure that all
> > filesystems handle the case correctly.
> 
> Oof, kermit on #xfs points out that even this is sufficient to show
> a problem:
> 
>  mkfs.ext4 -b 1024 -F empty.img
>   mount -o loop empty.img mnt
>   sync
> 
> xfs_io -f -t \
> -c            "pwrite               0          0x210  "      \
> -c            "mmap      -rw        0          0xd08  "      \
> -c            "mwrite    -S         0x50       0x210  0xaf8"  \
> -c            "truncate                        0x1000"  \
> mnt/testfile

That's supposed to SIGBUS. From the mmap man page:

SIGBUS	Attempted  access to a portion of the buffer that does not
	correspond to the file (for example, beyond the end of the
	file, including the case where another process has truncated
	the file).

I'm not sure that can be fixed in the filesystem, though, because
after a page fault inside the valid file region we can't prevent
mmap from writing beyond EOF in the same page. It's one of those "we
can't really do anything sane here" interface problems.

However, the truncate up is supposed to leave the newly exposed
regions full of zeros and not expose stale data from beyond the old
EOF. XFS results in the correct output which is:

00000000  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd |................|
*
00000210  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
*
00001000

Because it zeros allocated space between the old EOF and new EOF on
truncate up.

What is really interesting is this addition:

xfs_io -f -t \
-c            "pwrite               0          0x210  "      \
-c            "mmap      -rw        0          0xd08  "      \
-c            "mwrite    -S         0x50       0x210  0xaf8"  \
-c            "fsync" \
-c            "truncate                        0x1000"  \
mnt/testfile

Causes the ext4 data corruption goes away, probably because the act
of writing the page zeroes the tail blocks beyond EOF before writing
them. XFS has code to specifically do this in xfs_vm_writepage, and
I'm pretty sure we got that from ext4. So in the absence of ext4
zeroing on truncate up, I suspect it needs to write the tail page
at the old EOF on truncate up just like we have needed to add to
XFS to solve the other problems...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCH 5/5] generic: add write vs fcollapse test

[Dave Chinner]

This test exposed a problem with XFS where it failed to write back a
partial page correctly during a fcollapse operation. This left a
stray dirty buffer on the page, and hence invalidation of the page
then failed of the fcollapse returned an EBUSY error.

Make this a generic test so that we can ensure that all filesystems
handle the case correctly. Test case originally worked out and
written by Brian Foster.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 tests/generic/031     | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/031.out | 19 ++++++++++++++
 tests/generic/group   |  1 +
 3 files changed, 93 insertions(+)
 create mode 100644 tests/generic/031
 create mode 100644 tests/generic/031.out

diff --git a/tests/generic/031 b/tests/generic/031
new file mode 100644
index 0000000..7d615c0
--- /dev/null
+++ b/tests/generic/031
@@ -0,0 +1,73 @@
+#! /bin/bash
+# FS QA Test No. generic/031
+#
+# Test non-aligned writes against fcollapse to ensure that partial pages are
+# correctly written and aren't left behind causing invalidation or data
+# corruption issues.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Red Hat, Inc.  All Rights Reserved.
+#
+# 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
+#-----------------------------------------------------------------------
+#
+
+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()
+{
+    cd /
+    rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os IRIX Linux
+_require_scratch
+_require_xfs_io_command "fcollapse"
+
+testfile=$SCRATCH_MNT/testfile
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+
+$XFS_IO_PROG -f \
+	-c "pwrite 185332 55756" \
+	-c "fcollapse 28672 40960" \
+	-c "pwrite 133228 63394" \
+	-c "fcollapse 0 4096" \
+$testfile | _filter_xfs_io
+
+echo "==== Pre-Remount ==="
+hexdump -C $testfile
+_scratch_remount
+echo "==== Post-Remount =="
+hexdump -C $testfile
+
+status=0
+exit
+
diff --git a/tests/generic/031.out b/tests/generic/031.out
new file mode 100644
index 0000000..194bfa4
--- /dev/null
+++ b/tests/generic/031.out
@@ -0,0 +1,19 @@
+QA output created by 031
+wrote 55756/55756 bytes at offset 185332
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 63394/63394 bytes at offset 133228
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+==== Pre-Remount ===
+00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+*
+0001f860  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
+0001f870  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
+*
+0002fdc0
+==== Post-Remount ==
+00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+*
+0001f860  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
+0001f870  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
+*
+0002fdc0
diff --git a/tests/generic/group b/tests/generic/group
index ba1c913..dd6db3b 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -33,6 +33,7 @@
 028 auto quick
 029 auto quick rw
 030 auto quick rw
+031 auto quick prealloc rw
 053 acl repair auto quick
 062 attr udf auto quick
 068 other auto freeze dangerous stress
-- 
2.0.0

Re: [PATCH 5/5] generic: add write vs fcollapse test

[Brian Foster]

On Wed, Sep 17, 2014 at 11:41:53AM +1000, Dave Chinner wrote:
> This test exposed a problem with XFS where it failed to write back a
> partial page correctly during a fcollapse operation. This left a
> stray dirty buffer on the page, and hence invalidation of the page
> then failed of the fcollapse returned an EBUSY error.
> 
> Make this a generic test so that we can ensure that all filesystems
> handle the case correctly. Test case originally worked out and
> written by Brian Foster.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  tests/generic/031     | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/031.out | 19 ++++++++++++++
>  tests/generic/group   |  1 +
>  3 files changed, 93 insertions(+)
>  create mode 100644 tests/generic/031
>  create mode 100644 tests/generic/031.out
> 
> diff --git a/tests/generic/031 b/tests/generic/031
> new file mode 100644
> index 0000000..7d615c0
> --- /dev/null
> +++ b/tests/generic/031
> @@ -0,0 +1,73 @@
> +#! /bin/bash
> +# FS QA Test No. generic/031
> +#
> +# Test non-aligned writes against fcollapse to ensure that partial pages are
> +# correctly written and aren't left behind causing invalidation or data
> +# corruption issues.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2014 Red Hat, Inc.  All Rights Reserved.
> +#
> +# 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
> +#-----------------------------------------------------------------------
> +#
> +
> +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()
> +{
> +    cd /
> +    rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os IRIX Linux
> +_require_scratch
> +_require_xfs_io_command "fcollapse"
> +
> +testfile=$SCRATCH_MNT/testfile
> +
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount
> +
> +$XFS_IO_PROG -f \
> +	-c "pwrite 185332 55756" \
> +	-c "fcollapse 28672 40960" \
> +	-c "pwrite 133228 63394" \
> +	-c "fcollapse 0 4096" \
> +$testfile | _filter_xfs_io
> +

A comment (or per-line comments as in the other tests) would be good
here to explain what's going on. E.g.:

# This sequence of operations exploits a known failure to handle partial
# page writeback on sub page sized fsb filesystems. This occurs when a
# page has a non-contiguous mix of dirty and clean blocks (e.g., dirty
# block, clean block, dirty block, ...).
#
# The first write and collapse creates a dirty range in the file,
# flushes and truncates the file down to an unaligned boundary. The
# truncate implicit in the collapse dirties a block somewhere in the 2nd
# half of the new EOF page. The second write creates more dirty data,
# but specifically only writes to the first few bytes of the EOF page.
# This and the previous truncate creates the mixed page state described
# above.
#
# The final collapse attempts to flush and invalidate the entire cached
# set for the file. If the writeback of the mixed page does not
# complete, the invalidate fails with -EBUSY upon hitting a dirty page
# and aborts the collapse.
$XFS_IO_PROG -f \
	-c "pwrite 185332 55756"	# write extend file
	-c "fcollapse 28672 40960"	# collapse to unaligned boundary
	-c "pwrite 133228 63394"	# dirty first part of new eof page
	-c "fcollapse 0 4096"		# try a collapse
$testfile | _filter_xfs_io

> +echo "==== Pre-Remount ==="
> +hexdump -C $testfile
> +_scratch_remount
> +echo "==== Post-Remount =="
> +hexdump -C $testfile
> +

Note that this test is a collapse failure moreso than the data
corruption error (e.g., collapse returns EBUSY), though the reason for
that occurrence (data sync failure) is certainly a data corruption
issue. The hexdump/remount checks are fine, I just want to clarify that
they aren't necessary and they will probably just reflect the fact that
the collapse failed. I think the comment is sufficient to provide that
context and avoid any confusion.

Other than that this looks pretty good, thanks for knocking these tests
out.

Brian

> +status=0
> +exit
> +
> diff --git a/tests/generic/031.out b/tests/generic/031.out
> new file mode 100644
> index 0000000..194bfa4
> --- /dev/null
> +++ b/tests/generic/031.out
> @@ -0,0 +1,19 @@
> +QA output created by 031
> +wrote 55756/55756 bytes at offset 185332
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +wrote 63394/63394 bytes at offset 133228
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +==== Pre-Remount ===
> +00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> +*
> +0001f860  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
> +0001f870  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
> +*
> +0002fdc0
> +==== Post-Remount ==
> +00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> +*
> +0001f860  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
> +0001f870  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
> +*
> +0002fdc0
> diff --git a/tests/generic/group b/tests/generic/group
> index ba1c913..dd6db3b 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -33,6 +33,7 @@
>  028 auto quick
>  029 auto quick rw
>  030 auto quick rw
> +031 auto quick prealloc rw
>  053 acl repair auto quick
>  062 attr udf auto quick
>  068 other auto freeze dangerous stress
> -- 
> 2.0.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] generic: add write vs fcollapse test

[Dave Chinner]

On Wed, Sep 17, 2014 at 08:40:30AM -0400, Brian Foster wrote:
> On Wed, Sep 17, 2014 at 11:41:53AM +1000, Dave Chinner wrote:
> > This test exposed a problem with XFS where it failed to write back a
> > partial page correctly during a fcollapse operation. This left a
> > stray dirty buffer on the page, and hence invalidation of the page
> > then failed of the fcollapse returned an EBUSY error.
> > 
> > Make this a generic test so that we can ensure that all filesystems
> > handle the case correctly. Test case originally worked out and
> > written by Brian Foster.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> >  tests/generic/031     | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/031.out | 19 ++++++++++++++
> >  tests/generic/group   |  1 +
> >  3 files changed, 93 insertions(+)
> >  create mode 100644 tests/generic/031
> >  create mode 100644 tests/generic/031.out
> > 
> > diff --git a/tests/generic/031 b/tests/generic/031
> > new file mode 100644
> > index 0000000..7d615c0
> > --- /dev/null
> > +++ b/tests/generic/031
> > @@ -0,0 +1,73 @@
> > +#! /bin/bash
> > +# FS QA Test No. generic/031
> > +#
> > +# Test non-aligned writes against fcollapse to ensure that partial pages are
> > +# correctly written and aren't left behind causing invalidation or data
> > +# corruption issues.

Note the test already documents what the problem it is trying to
expose in it's description....

......
> > +$XFS_IO_PROG -f \
> > +	-c "pwrite 185332 55756" \
> > +	-c "fcollapse 28672 40960" \
> > +	-c "pwrite 133228 63394" \
> > +	-c "fcollapse 0 4096" \
> > +$testfile | _filter_xfs_io
> > +
> 
> A comment (or per-line comments as in the other tests) would be good
> here to explain what's going on. E.g.:
> 
> # This sequence of operations exploits a known failure to handle partial
> # page writeback on sub page sized fsb filesystems. This occurs when a
> # page has a non-contiguous mix of dirty and clean blocks (e.g., dirty
> # block, clean block, dirty block, ...).
> #
> # The first write and collapse creates a dirty range in the file,
> # flushes and truncates the file down to an unaligned boundary. The
> # truncate implicit in the collapse dirties a block somewhere in the 2nd
> # half of the new EOF page. The second write creates more dirty data,
> # but specifically only writes to the first few bytes of the EOF page.
> # This and the previous truncate creates the mixed page state described
> # above.
> #
> # The final collapse attempts to flush and invalidate the entire cached
> # set for the file. If the writeback of the mixed page does not
> # complete, the invalidate fails with -EBUSY upon hitting a dirty page
> # and aborts the collapse.

Details of how XFS failed really aren't relevant to the test case -
that belongs in the commit message for the XFS fix.  The test case
simply needs to document is what condition the test is supposed to
be exercising. That's done in the intial test description, so
there's no need to duplicate it again.

> $XFS_IO_PROG -f \
> 	-c "pwrite 185332 55756"	# write extend file
> 	-c "fcollapse 28672 40960"	# collapse to unaligned boundary
> 	-c "pwrite 133228 63394"	# dirty first part of new eof page
> 	-c "fcollapse 0 4096"		# try a collapse
> $testfile | _filter_xfs_io

That's about as much as is necessary, I think.

> > +echo "==== Pre-Remount ==="
> > +hexdump -C $testfile
> > +_scratch_remount
> > +echo "==== Post-Remount =="
> > +hexdump -C $testfile
> > +
> 
> Note that this test is a collapse failure moreso than the data
> corruption error (e.g., collapse returns EBUSY), though the reason
> for that occurrence (data sync failure) is certainly a data
> corruption issue. The hexdump/remount checks are fine, I just want
> to clarify that they aren't necessary and they will probably just
> reflect the fact that the collapse failed. I think the comment is
> sufficient to provide that context and avoid any confusion.

Again, this focuses on the specific XFS failure. How XFS failed is
mostly irrelevant - we know that ext4 had a similar problem that
showed up as data corruption and not a syscall failure. Further, now
that we've fixed the XFS problem that lead to syscall failure we
still need to check that we are actually writing the data correctly.

Simply put: a test doesn't care what the failure is, it just needs to
verify the functionality is operating correctly. And for a data
manipulation operation, checking that data isn't corrupted is a
pretty important check to make. ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com