[PATCH 0/3] xfs: restore and enhance xfs indlen test

[PATCH 0/3] xfs: restore and enhance xfs indlen test

[Brian Foster]

Hi all,

This series is associated with the "buffered write and indlen fixes"
series recently posted to the XFS list. These patches move the original
test to the XFS directory as it now depends on an XFS-specific
mechanism, restore the effectiveness of the original test, and finally
enhance the test to reproduce the newly identified problems fixed by the
kernel patches. Thoughts, reviews, flames appreciated.

Brian

Brian Foster (3):
  xfstests: move generic indlen reservation test to xfs dir
  tests/xfs: update indlen res. test to use fail writes mechanism
  tests/xfs: update indlen res. test to include larger write pattern

 tests/generic/033     |  84 --------------------------------------
 tests/generic/033.out |   4 --
 tests/generic/group   |   1 -
 tests/xfs/289         | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/289.out     |   2 +
 tests/xfs/group       |   1 +
 6 files changed, 112 insertions(+), 89 deletions(-)
 delete mode 100755 tests/generic/033
 delete mode 100644 tests/generic/033.out
 create mode 100755 tests/xfs/289
 create mode 100644 tests/xfs/289.out

-- 
2.7.4

[PATCH 1/3] xfstests: move generic indlen reservation test to xfs dir

[Brian Foster]

This test was originally designed to reproduce the split indlen
reservation depletion problem in XFS. It was included as a generic test
simply because it had no hard dependencies on XFS or associated tools.

This test is no longer effective in its current form. Fixing it requires
use of XFS specific mechanisms. Therefore, move the test to the XFS
specific test directory. No other changes are made in this patch.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 tests/generic/033     | 84 ---------------------------------------------------
 tests/generic/033.out |  4 ---
 tests/generic/group   |  1 -
 tests/xfs/289         | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/289.out     |  4 +++
 tests/xfs/group       |  1 +
 6 files changed, 89 insertions(+), 89 deletions(-)
 delete mode 100755 tests/generic/033
 delete mode 100644 tests/generic/033.out
 create mode 100755 tests/xfs/289
 create mode 100644 tests/xfs/289.out

diff --git a/tests/generic/033 b/tests/generic/033
deleted file mode 100755
index 4f8bb92..0000000
--- a/tests/generic/033
+++ /dev/null
@@ -1,84 +0,0 @@
-#! /bin/bash
-# FS QA Test No. 033
-#
-# This test stresses indirect block reservation for delayed allocation extents.
-# XFS reserves extra blocks for deferred allocation of delalloc extents. These
-# reserved blocks can be divided among more extents than anticipated if the
-# original extent for which the blocks were reserved is split into multiple
-# delalloc extents. If this scenario repeats, eventually some extents are left
-# without any indirect block reservation whatsoever. This leads to assert
-# failures and possibly other problems in XFS.
-#
-#-----------------------------------------------------------------------
-# 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
-
-# real QA test starts here
-rm -f $seqres.full
-
-# Modify as appropriate.
-_supported_fs generic
-_supported_os Linux
-_require_scratch
-_require_xfs_io_command "fzero"
-
-_scratch_mkfs >/dev/null 2>&1
-_scratch_mount
-
-file=$SCRATCH_MNT/file.$seq
-bytes=$((64 * 1024))
-
-# create sequential delayed allocation
-$XFS_IO_PROG -f -c "pwrite 0 $bytes" $file >> $seqres.full 2>&1
-
-# Zero every other 4k range to split the larger delalloc extent into many more
-# smaller extents. Use zero instead of hole punch because the former does not
-# force writeback (and hence delalloc conversion). It can simply discard
-# delalloc blocks and convert the ranges to unwritten.
-endoff=$((bytes - 4096))
-for i in $(seq 0 8192 $endoff); do
-	$XFS_IO_PROG -c "fzero -k $i 4k" $file >> $seqres.full 2>&1
-done
-
-# now zero the opposite set to remove remaining delalloc extents
-for i in $(seq 4096 8192 $endoff); do
-	$XFS_IO_PROG -c "fzero -k $i 4k" $file >> $seqres.full 2>&1
-done
-
-_scratch_cycle_mount
-hexdump $file
-
-status=0
-exit
diff --git a/tests/generic/033.out b/tests/generic/033.out
deleted file mode 100644
index 419d831..0000000
--- a/tests/generic/033.out
+++ /dev/null
@@ -1,4 +0,0 @@
-QA output created by 033
-0000000 0000 0000 0000 0000 0000 0000 0000 0000
-*
-0010000
diff --git a/tests/generic/group b/tests/generic/group
index d0bc47d..98d57f3 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -35,7 +35,6 @@
 030 auto quick rw
 031 auto quick prealloc rw collapse
 032 auto quick rw
-033 auto quick rw zero
 034 auto quick metadata log
 035 auto quick
 036 auto aio rw stress
diff --git a/tests/xfs/289 b/tests/xfs/289
new file mode 100755
index 0000000..33cf060
--- /dev/null
+++ b/tests/xfs/289
@@ -0,0 +1,84 @@
+#! /bin/bash
+# FS QA Test No. 289
+#
+# This test stresses indirect block reservation for delayed allocation extents.
+# XFS reserves extra blocks for deferred allocation of delalloc extents. These
+# reserved blocks can be divided among more extents than anticipated if the
+# original extent for which the blocks were reserved is split into multiple
+# delalloc extents. If this scenario repeats, eventually some extents are left
+# without any indirect block reservation whatsoever. This leads to assert
+# failures and possibly other problems in XFS.
+#
+#-----------------------------------------------------------------------
+# 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
+
+# real QA test starts here
+rm -f $seqres.full
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+_require_xfs_io_command "fzero"
+
+_scratch_mkfs >/dev/null 2>&1
+_scratch_mount
+
+file=$SCRATCH_MNT/file.$seq
+bytes=$((64 * 1024))
+
+# create sequential delayed allocation
+$XFS_IO_PROG -f -c "pwrite 0 $bytes" $file >> $seqres.full 2>&1
+
+# Zero every other 4k range to split the larger delalloc extent into many more
+# smaller extents. Use zero instead of hole punch because the former does not
+# force writeback (and hence delalloc conversion). It can simply discard
+# delalloc blocks and convert the ranges to unwritten.
+endoff=$((bytes - 4096))
+for i in $(seq 0 8192 $endoff); do
+	$XFS_IO_PROG -c "fzero -k $i 4k" $file >> $seqres.full 2>&1
+done
+
+# now zero the opposite set to remove remaining delalloc extents
+for i in $(seq 4096 8192 $endoff); do
+	$XFS_IO_PROG -c "fzero -k $i 4k" $file >> $seqres.full 2>&1
+done
+
+_scratch_cycle_mount
+hexdump $file
+
+status=0
+exit
diff --git a/tests/xfs/289.out b/tests/xfs/289.out
new file mode 100644
index 0000000..bdcf195
--- /dev/null
+++ b/tests/xfs/289.out
@@ -0,0 +1,4 @@
+QA output created by 289
+0000000 0000 0000 0000 0000 0000 0000 0000 0000
+*
+0010000
diff --git a/tests/xfs/group b/tests/xfs/group
index e2dfae2..5c1ba16 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -280,6 +280,7 @@
 283 dump ioctl auto quick
 284 auto quick dump copy db mkfs repair
 287 auto dump quota quick
+289 auto quick rw zero
 290 auto rw prealloc quick ioctl zero
 291 auto repair
 292 auto mkfs quick
-- 
2.7.4

[PATCH 2/3] tests/xfs: update indlen res. test to use fail writes mechanism

[Brian Foster]

This test originally used zero range operations to reproduce problematic
indirect delalloc reservations on XFS. Zero range has since been updated
to include a pagecache flush before it updates extents, which means
delalloc extents are converted to real extents and indlen reservations
are drained before extents are modified. As a result, zero range is no
longer an effective tool to reproduce the original problem.

Instead, a debug mode failure injection mechanism has been added to XFS
to cause dropped writes and trigger the delalloc extent hole punch
behavior required to reproduce this problem. Update the test to use this
new mechanism.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 tests/xfs/289     | 28 +++++++++++++++++++---------
 tests/xfs/289.out |  4 +---
 tests/xfs/group   |  2 +-
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/tests/xfs/289 b/tests/xfs/289
index 33cf060..3aa53b9 100755
--- a/tests/xfs/289
+++ b/tests/xfs/289
@@ -44,6 +44,7 @@ _cleanup()
 
 # get standard environment, filters and checks
 . ./common/rc
+. ./common/punch
 
 # real QA test starts here
 rm -f $seqres.full
@@ -52,33 +53,42 @@ rm -f $seqres.full
 _supported_fs generic
 _supported_os Linux
 _require_scratch
-_require_xfs_io_command "fzero"
+_require_xfs_sysfs $(_short_dev $TEST_DEV)/drop_writes
 
 _scratch_mkfs >/dev/null 2>&1
 _scratch_mount
 
+sdev=$(_short_dev $SCRATCH_DEV)
 file=$SCRATCH_MNT/file.$seq
 bytes=$((64 * 1024))
 
 # create sequential delayed allocation
 $XFS_IO_PROG -f -c "pwrite 0 $bytes" $file >> $seqres.full 2>&1
 
-# Zero every other 4k range to split the larger delalloc extent into many more
-# smaller extents. Use zero instead of hole punch because the former does not
-# force writeback (and hence delalloc conversion). It can simply discard
-# delalloc blocks and convert the ranges to unwritten.
+# Enable write drops. All buffered writes are dropped from this point on.
+echo 1 > /sys/fs/xfs/$sdev/drop_writes
+
+# Write every other 4k range to split the larger delalloc extent into many more
+# smaller extents. Use pwrite because with write failures enabled, all
+# preexisting delalloc blocks in the range of the I/O are tossed without
+# discretion. This allows manipulation of the delalloc extent without conversion
+# to real blocks (and thus releasing the indirect reservation).
 endoff=$((bytes - 4096))
 for i in $(seq 0 8192 $endoff); do
-	$XFS_IO_PROG -c "fzero -k $i 4k" $file >> $seqres.full 2>&1
+	$XFS_IO_PROG -c "pwrite $i 4k" $file >> $seqres.full 2>&1
 done
 
-# now zero the opposite set to remove remaining delalloc extents
+# now pwrite the opposite set to remove remaining delalloc extents
 for i in $(seq 4096 8192 $endoff); do
-	$XFS_IO_PROG -c "fzero -k $i 4k" $file >> $seqres.full 2>&1
+	$XFS_IO_PROG -c "pwrite $i 4k" $file >> $seqres.full 2>&1
 done
 
+echo 0 > /sys/fs/xfs/$sdev/drop_writes
+
+echo "Silence is golden."
+
 _scratch_cycle_mount
-hexdump $file
+$XFS_IO_PROG -c 'bmap -vp' $file | _filter_bmap
 
 status=0
 exit
diff --git a/tests/xfs/289.out b/tests/xfs/289.out
index bdcf195..72e60f9 100644
--- a/tests/xfs/289.out
+++ b/tests/xfs/289.out
@@ -1,4 +1,2 @@
 QA output created by 289
-0000000 0000 0000 0000 0000 0000 0000 0000 0000
-*
-0010000
+Silence is golden.
diff --git a/tests/xfs/group b/tests/xfs/group
index 5c1ba16..7c188fe 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -280,7 +280,7 @@
 283 dump ioctl auto quick
 284 auto quick dump copy db mkfs repair
 287 auto dump quota quick
-289 auto quick rw zero
+289 auto quick rw
 290 auto rw prealloc quick ioctl zero
 291 auto repair
 292 auto mkfs quick
-- 
2.7.4

[PATCH 3/3] tests/xfs: update indlen res. test to include larger write pattern

[Brian Foster]

The indirect blocks reservation test originally reproduced a problem
with smaller delalloc extents being split into separate extents with
insufficient indlen blocks. This was ultimately resolved by allowing to
borrow blocks from the freed extent.

Since then, similar problems have been reproduced when larger delalloc
extents are repeatedly split and merged with new writes. These repeated
splits exposed a problem in the old indlen reservation split algorithm
when dealing with extents that are already under-reserved from previous
splits. This resulted in unfair distribution of existing reservation and
fairly large delalloc extents without any indlen reservation whatsoever.

Enhance the original indlen reservation test to include a pattern that
reproduces this behavior.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 tests/xfs/289 | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/tests/xfs/289 b/tests/xfs/289
index 3aa53b9..5839a24 100755
--- a/tests/xfs/289
+++ b/tests/xfs/289
@@ -85,10 +85,25 @@ done
 
 echo 0 > /sys/fs/xfs/$sdev/drop_writes
 
-echo "Silence is golden."
-
 _scratch_cycle_mount
 $XFS_IO_PROG -c 'bmap -vp' $file | _filter_bmap
 
+# Now test a buffered write workload with larger extents. Write a 100m extent,
+# split it at the 3/4 mark, then write another 100m extent that is contiguous
+# with the 1/4 portion of the split extent. Repeat several times. This pattern
+# is known to prematurely exhaust indirect reservations and cause warnings and
+# assert failures.
+rm -f $file
+for offset in $(seq 0 100 500); do
+	$XFS_IO_PROG -fc "pwrite ${offset}m 100m" $file >> $seqres.full 2>&1
+
+	punchoffset=$((offset + 75))
+	echo 1 > /sys/fs/xfs/$sdev/drop_writes
+	$XFS_IO_PROG -c "pwrite ${punchoffset}m 4k" $file >> $seqres.full 2>&1
+	echo 0 > /sys/fs/xfs/$sdev/drop_writes
+done
+
+echo "Silence is golden."
+
 status=0
 exit
-- 
2.7.4

Re: [PATCH 0/3] xfs: restore and enhance xfs indlen test

[Eryu Guan]

On Thu, Feb 09, 2017 at 02:43:41PM -0500, Brian Foster wrote:
> Hi all,
> 
> This series is associated with the "buffered write and indlen fixes"
> series recently posted to the XFS list. These patches move the original
> test to the XFS directory as it now depends on an XFS-specific
> mechanism, restore the effectiveness of the original test, and finally
> enhance the test to reproduce the newly identified problems fixed by the
> kernel patches. Thoughts, reviews, flames appreciated.

I think it's still worth keeping the original fzero tests in generic, as
a generic function/regression test, though it's not effective to
reproduce the original bug. And maybe it's still working and effective
on some distro kernels? How about just adding a new test for XFS?

Thanks,
Eryu

Re: [PATCH 0/3] xfs: restore and enhance xfs indlen test

[Brian Foster]

On Fri, Feb 10, 2017 at 03:15:19PM +0800, Eryu Guan wrote:
> On Thu, Feb 09, 2017 at 02:43:41PM -0500, Brian Foster wrote:
> > Hi all,
> > 
> > This series is associated with the "buffered write and indlen fixes"
> > series recently posted to the XFS list. These patches move the original
> > test to the XFS directory as it now depends on an XFS-specific
> > mechanism, restore the effectiveness of the original test, and finally
> > enhance the test to reproduce the newly identified problems fixed by the
> > kernel patches. Thoughts, reviews, flames appreciated.
> 
> I think it's still worth keeping the original fzero tests in generic, as
> a generic function/regression test, though it's not effective to
> reproduce the original bug. And maybe it's still working and effective
> on some distro kernels? How about just adding a new test for XFS?
> 

Sure, in that case I guess I can just copy generic/033 rather than move
it and then squash these down into a single patch. Any other comments on
the resulting test before I post an update?

Brian

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

Re: [PATCH 0/3] xfs: restore and enhance xfs indlen test

[Darrick J. Wong]

On Fri, Feb 10, 2017 at 08:58:30AM -0500, Brian Foster wrote:
> On Fri, Feb 10, 2017 at 03:15:19PM +0800, Eryu Guan wrote:
> > On Thu, Feb 09, 2017 at 02:43:41PM -0500, Brian Foster wrote:
> > > Hi all,
> > > 
> > > This series is associated with the "buffered write and indlen fixes"
> > > series recently posted to the XFS list. These patches move the original
> > > test to the XFS directory as it now depends on an XFS-specific
> > > mechanism, restore the effectiveness of the original test, and finally
> > > enhance the test to reproduce the newly identified problems fixed by the
> > > kernel patches. Thoughts, reviews, flames appreciated.
> > 
> > I think it's still worth keeping the original fzero tests in generic, as
> > a generic function/regression test, though it's not effective to
> > reproduce the original bug. And maybe it's still working and effective
> > on some distro kernels? How about just adding a new test for XFS?
> > 
> 
> Sure, in that case I guess I can just copy generic/033 rather than move
> it and then squash these down into a single patch. Any other comments on
> the resulting test before I post an update?

/me thinks the new test ought to _notrun if the fail_writes file doesn't
show up... that said /me is sorta braindead after a lot of travel
yesterday and could've missed it already being there. :(

--D

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

Re: [PATCH 0/3] xfs: restore and enhance xfs indlen test

[Brian Foster]

On Fri, Feb 10, 2017 at 08:25:23AM -0800, Darrick J. Wong wrote:
> On Fri, Feb 10, 2017 at 08:58:30AM -0500, Brian Foster wrote:
> > On Fri, Feb 10, 2017 at 03:15:19PM +0800, Eryu Guan wrote:
> > > On Thu, Feb 09, 2017 at 02:43:41PM -0500, Brian Foster wrote:
> > > > Hi all,
> > > > 
> > > > This series is associated with the "buffered write and indlen fixes"
> > > > series recently posted to the XFS list. These patches move the original
> > > > test to the XFS directory as it now depends on an XFS-specific
> > > > mechanism, restore the effectiveness of the original test, and finally
> > > > enhance the test to reproduce the newly identified problems fixed by the
> > > > kernel patches. Thoughts, reviews, flames appreciated.
> > > 
> > > I think it's still worth keeping the original fzero tests in generic, as
> > > a generic function/regression test, though it's not effective to
> > > reproduce the original bug. And maybe it's still working and effective
> > > on some distro kernels? How about just adding a new test for XFS?
> > > 
> > 
> > Sure, in that case I guess I can just copy generic/033 rather than move
> > it and then squash these down into a single patch. Any other comments on
> > the resulting test before I post an update?
> 
> /me thinks the new test ought to _notrun if the fail_writes file doesn't
> show up... that said /me is sorta braindead after a lot of travel
> yesterday and could've missed it already being there. :(
> 

Yep, there's a '_require_xfs_sysfs ...' in there that skips the test as
appropriate.

Brian

> --D
> 
> > 
> > Brian
> > 
> > > Thanks,
> > > Eryu
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html