diff mbox series

[v2] tests/xfs: test COW writeback failure when overlapping non-shared blocks

Message ID 20211025130053.8343-1-bfoster@redhat.com (mailing list archive)
State New, archived
Headers show
Series [v2] tests/xfs: test COW writeback failure when overlapping non-shared blocks | expand

Commit Message

Brian Foster Oct. 25, 2021, 1 p.m. UTC 
Test that COW writeback that overlaps non-shared delalloc blocks
does not leave around stale delalloc blocks on I/O failure. This
triggers assert failures and free space accounting corruption on
XFS.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---

v2:
- Explicitly set COW extent size hint.
- Move to tests/xfs.
- Various minor cleanups.
v1: https://lore.kernel.org/fstests/20211021163959.1887011-1-bfoster@redhat.com/

 tests/xfs/999     | 62 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/999.out |  2 ++
 2 files changed, 64 insertions(+)
 create mode 100755 tests/xfs/999
 create mode 100644 tests/xfs/999.out

Comments

Darrick J. Wong Oct. 25, 2021, 4:10 p.m. UTC | #1 
On Mon, Oct 25, 2021 at 09:00:53AM -0400, Brian Foster wrote:
> Test that COW writeback that overlaps non-shared delalloc blocks
> does not leave around stale delalloc blocks on I/O failure. This
> triggers assert failures and free space accounting corruption on
> XFS.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>

LGTM.  Thanks for the patch + reproducer!

At some point this test ought to grow a link to the upstream fix patch,
which is currently in the 5.16 merge branch, e.g.:

# Regression test for kernel commit:
#
# 5ca5916b6bc9 ("xfs: punch out data fork delalloc blocks on COW
# writeback failure")

...but as Sunday afternoon came and went with neither -rc7 nor a final
release being tagged, I'm not sure when that commit will appear
upstream.  It's entirely possible that Linus is sitting in the dark
right now, since I came back from my long weekend to a noticeable
amount of downed trees around town.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
> 
> v2:
> - Explicitly set COW extent size hint.
> - Move to tests/xfs.
> - Various minor cleanups.
> v1: https://lore.kernel.org/fstests/20211021163959.1887011-1-bfoster@redhat.com/
> 
>  tests/xfs/999     | 62 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/999.out |  2 ++
>  2 files changed, 64 insertions(+)
>  create mode 100755 tests/xfs/999
>  create mode 100644 tests/xfs/999.out
> 
> diff --git a/tests/xfs/999 b/tests/xfs/999
> new file mode 100755
> index 00000000..f27972bc
> --- /dev/null
> +++ b/tests/xfs/999
> @@ -0,0 +1,62 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Red Hat, Inc.  All Rights Reserved.
> +#
> +# FS QA Test 999
> +#
> +# Test that COW writeback that overlaps non-shared delalloc blocks does not
> +# leave around stale delalloc blocks on I/O failure. This triggers assert
> +# failures and free space accounting corruption on XFS.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick clone
> +
> +_cleanup()
> +{
> +	_cleanup_flakey
> +	cd /
> +	rm -r -f $tmp.*
> +}
> +
> +# Import common functions.
> +. ./common/reflink
> +. ./common/dmflakey
> +
> +# real QA test starts here
> +_supported_fs xfs
> +_require_scratch_reflink
> +_require_cp_reflink
> +_require_xfs_io_command "cowextsize"
> +_require_flakey_with_error_writes
> +
> +_scratch_mkfs >> $seqres.full
> +_init_flakey
> +_mount_flakey
> +
> +blksz=$(_get_file_block_size $SCRATCH_MNT)
> +
> +# Set the COW extent size hint to guarantee COW fork preallocation occurs over a
> +# bordering block offset.
> +$XFS_IO_PROG -c "cowextsize $((blksz * 2))" $SCRATCH_MNT >> $seqres.full
> +
> +# create two files that share a single block
> +$XFS_IO_PROG -fc "pwrite $blksz $blksz" $SCRATCH_MNT/file1 >> $seqres.full
> +_cp_reflink $SCRATCH_MNT/file1 $SCRATCH_MNT/file2
> +
> +# Perform a buffered write across the shared and non-shared blocks. On XFS, this
> +# creates a COW fork extent that covers the shared block as well as the just
> +# created non-shared delalloc block. Fail the writeback to verify that all
> +# delayed allocation is cleaned up properly.
> +_load_flakey_table $FLAKEY_ERROR_WRITES
> +$XFS_IO_PROG -c "pwrite 0 $((blksz * 2))" \
> +	-c fsync $SCRATCH_MNT/file2 >> $seqres.full
> +_load_flakey_table $FLAKEY_ALLOW_WRITES
> +
> +# Try a post-fail reflink and then unmount. Both of these are known to produce
> +# errors and/or assert failures on XFS if we trip over a stale delalloc block.
> +_cp_reflink $SCRATCH_MNT/file2 $SCRATCH_MNT/file3
> +_unmount_flakey
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/999.out b/tests/xfs/999.out
> new file mode 100644
> index 00000000..88b69c4c
> --- /dev/null
> +++ b/tests/xfs/999.out
> @@ -0,0 +1,2 @@
> +QA output created by 999
> +fsync: Input/output error
> -- 
> 2.31.1
>
Eryu Guan Oct. 31, 2021, 3:20 p.m. UTC | #2 
On Mon, Oct 25, 2021 at 09:10:22AM -0700, Darrick J. Wong wrote:
> On Mon, Oct 25, 2021 at 09:00:53AM -0400, Brian Foster wrote:
> > Test that COW writeback that overlaps non-shared delalloc blocks
> > does not leave around stale delalloc blocks on I/O failure. This
> > triggers assert failures and free space accounting corruption on
> > XFS.
> > 
> > Signed-off-by: Brian Foster <bfoster@redhat.com>
> 
> LGTM.  Thanks for the patch + reproducer!
> 
> At some point this test ought to grow a link to the upstream fix patch,
> which is currently in the 5.16 merge branch, e.g.:
> 
> # Regression test for kernel commit:
> #
> # 5ca5916b6bc9 ("xfs: punch out data fork delalloc blocks on COW
> # writeback failure")

And this test triggers an ASSERT (though it won't crash the kernel
unless XFS_ASSERT_FATAL is set), I'd like wait for the fix to land so we
could have the correct commit ID by then.

> 
> ...but as Sunday afternoon came and went with neither -rc7 nor a final
> release being tagged, I'm not sure when that commit will appear
> upstream.  It's entirely possible that Linus is sitting in the dark
> right now, since I came back from my long weekend to a noticeable
> amount of downed trees around town.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>

Thanks for the review, and for all the other reviews!

Eryu
Darrick J. Wong Nov. 3, 2021, 4:10 p.m. UTC | #3 
On Sun, Oct 31, 2021 at 11:20:04PM +0800, Eryu Guan wrote:
> On Mon, Oct 25, 2021 at 09:10:22AM -0700, Darrick J. Wong wrote:
> > On Mon, Oct 25, 2021 at 09:00:53AM -0400, Brian Foster wrote:
> > > Test that COW writeback that overlaps non-shared delalloc blocks
> > > does not leave around stale delalloc blocks on I/O failure. This
> > > triggers assert failures and free space accounting corruption on
> > > XFS.
> > > 
> > > Signed-off-by: Brian Foster <bfoster@redhat.com>
> > 
> > LGTM.  Thanks for the patch + reproducer!
> > 
> > At some point this test ought to grow a link to the upstream fix patch,
> > which is currently in the 5.16 merge branch, e.g.:
> > 
> > # Regression test for kernel commit:
> > #
> > # 5ca5916b6bc9 ("xfs: punch out data fork delalloc blocks on COW
> > # writeback failure")
> 
> And this test triggers an ASSERT (though it won't crash the kernel
> unless XFS_ASSERT_FATAL is set), I'd like wait for the fix to land so we
> could have the correct commit ID by then.

...and now that the commit is in Linus' tree, please resubmit this test
with the kernel commit annotation (the commit id is unchanged). :)

--D

> > 
> > ...but as Sunday afternoon came and went with neither -rc7 nor a final
> > release being tagged, I'm not sure when that commit will appear
> > upstream.  It's entirely possible that Linus is sitting in the dark
> > right now, since I came back from my long weekend to a noticeable
> > amount of downed trees around town.
> > 
> > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> 
> Thanks for the review, and for all the other reviews!
> 
> Eryu
Darrick J. Wong Nov. 17, 2021, 6:01 p.m. UTC | #4 
On Mon, Oct 25, 2021 at 09:00:53AM -0400, Brian Foster wrote:
> Test that COW writeback that overlaps non-shared delalloc blocks
> does not leave around stale delalloc blocks on I/O failure. This
> triggers assert failures and free space accounting corruption on
> XFS.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
> 
> v2:
> - Explicitly set COW extent size hint.
> - Move to tests/xfs.
> - Various minor cleanups.
> v1: https://lore.kernel.org/fstests/20211021163959.1887011-1-bfoster@redhat.com/
> 
>  tests/xfs/999     | 62 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/999.out |  2 ++
>  2 files changed, 64 insertions(+)
>  create mode 100755 tests/xfs/999
>  create mode 100644 tests/xfs/999.out
> 
> diff --git a/tests/xfs/999 b/tests/xfs/999
> new file mode 100755
> index 00000000..f27972bc
> --- /dev/null
> +++ b/tests/xfs/999
> @@ -0,0 +1,62 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Red Hat, Inc.  All Rights Reserved.
> +#
> +# FS QA Test 999
> +#
> +# Test that COW writeback that overlaps non-shared delalloc blocks does not
> +# leave around stale delalloc blocks on I/O failure. This triggers assert
> +# failures and free space accounting corruption on XFS.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick clone
> +
> +_cleanup()
> +{
> +	_cleanup_flakey
> +	cd /
> +	rm -r -f $tmp.*
> +}
> +
> +# Import common functions.
> +. ./common/reflink
> +. ./common/dmflakey
> +
> +# real QA test starts here
> +_supported_fs xfs
> +_require_scratch_reflink
> +_require_cp_reflink
> +_require_xfs_io_command "cowextsize"
> +_require_flakey_with_error_writes
> +
> +_scratch_mkfs >> $seqres.full
> +_init_flakey
> +_mount_flakey
> +
> +blksz=$(_get_file_block_size $SCRATCH_MNT)
> +
> +# Set the COW extent size hint to guarantee COW fork preallocation occurs over a
> +# bordering block offset.
> +$XFS_IO_PROG -c "cowextsize $((blksz * 2))" $SCRATCH_MNT >> $seqres.full
> +
> +# create two files that share a single block
> +$XFS_IO_PROG -fc "pwrite $blksz $blksz" $SCRATCH_MNT/file1 >> $seqres.full
> +_cp_reflink $SCRATCH_MNT/file1 $SCRATCH_MNT/file2
> +
> +# Perform a buffered write across the shared and non-shared blocks. On XFS, this
> +# creates a COW fork extent that covers the shared block as well as the just
> +# created non-shared delalloc block. Fail the writeback to verify that all
> +# delayed allocation is cleaned up properly.
> +_load_flakey_table $FLAKEY_ERROR_WRITES
> +$XFS_IO_PROG -c "pwrite 0 $((blksz * 2))" \
> +	-c fsync $SCRATCH_MNT/file2 >> $seqres.full
> +_load_flakey_table $FLAKEY_ALLOW_WRITES

Hmm.  So I've been running this test in my djwong-dev tree and hit this
last night:

--- xfs/999.out
+++ xfs/999.out.bad
@@ -1,2 +1,3 @@
 QA output created by 999
-fsync: Input/output error
+stat: Input/output error
+cp: failed to access '/opt/file3': Input/output error

Digging into the kernel log, I see this happen:

[10240.821719] XFS (dm-0): Mounting V5 Filesystem
[10240.855461] XFS (dm-0): Ending clean mount
[10240.857030] XFS (dm-0): Quotacheck needed: Please wait.
[10240.860095] XFS (dm-0): Quotacheck: Done.
[10240.977055] XFS (dm-0): log I/O error -5
[10240.977459] XFS (dm-0): Log I/O Error (0x2) detected at xlog_ioend_work+0x5f/0xb0 [xfs] (fs/xfs/xfs_log.c:1377).  Shutting down filesystem.
[10240.978682] XFS (dm-0): Please unmount the filesystem and rectify the problem(s)
[10241.044886] XFS (dm-0): Unmounting Filesystem

I guess the log tried to checkpoint for the brief window where the
flakey table was enabled, and shut down the whole fs?  I don't have any
good ideas for how to solve this, though.

Hm.  What if you did something like:

$XFS_IO_PROG -c 'pwrite...' $SCRATCH_MNT/file2
_load_flakey_table $FLAKEY_ERROR_WRITES
$XFS_IO_PROG -c 'sync_range -wa' $SCRATCH_MNT/file2
+_load_flakey_table $FLAKEY_ALLOW_WRITES

to constrain the window in which disk write will fail?  Seeing as s_f_r
doesn't actually tell the fs to flush its own metadata or anything.

(Yikes, did I finally find a use for sync_file_range??)

--D

> +
> +# Try a post-fail reflink and then unmount. Both of these are known to produce
> +# errors and/or assert failures on XFS if we trip over a stale delalloc block.
> +_cp_reflink $SCRATCH_MNT/file2 $SCRATCH_MNT/file3
> +_unmount_flakey
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/999.out b/tests/xfs/999.out
> new file mode 100644
> index 00000000..88b69c4c
> --- /dev/null
> +++ b/tests/xfs/999.out
> @@ -0,0 +1,2 @@
> +QA output created by 999
> +fsync: Input/output error
> -- 
> 2.31.1
>
Brian Foster Dec. 17, 2021, 3:19 p.m. UTC | #5 
On Wed, Nov 17, 2021 at 10:01:05AM -0800, Darrick J. Wong wrote:
> On Mon, Oct 25, 2021 at 09:00:53AM -0400, Brian Foster wrote:
> > Test that COW writeback that overlaps non-shared delalloc blocks
> > does not leave around stale delalloc blocks on I/O failure. This
> > triggers assert failures and free space accounting corruption on
> > XFS.
> > 
> > Signed-off-by: Brian Foster <bfoster@redhat.com>
> > ---
> > 
> > v2:
> > - Explicitly set COW extent size hint.
> > - Move to tests/xfs.
> > - Various minor cleanups.
> > v1: https://lore.kernel.org/fstests/20211021163959.1887011-1-bfoster@redhat.com/
> > 
> >  tests/xfs/999     | 62 +++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/999.out |  2 ++
> >  2 files changed, 64 insertions(+)
> >  create mode 100755 tests/xfs/999
> >  create mode 100644 tests/xfs/999.out
> > 
> > diff --git a/tests/xfs/999 b/tests/xfs/999
> > new file mode 100755
> > index 00000000..f27972bc
> > --- /dev/null
> > +++ b/tests/xfs/999
> > @@ -0,0 +1,62 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2021 Red Hat, Inc.  All Rights Reserved.
> > +#
> > +# FS QA Test 999
> > +#
> > +# Test that COW writeback that overlaps non-shared delalloc blocks does not
> > +# leave around stale delalloc blocks on I/O failure. This triggers assert
> > +# failures and free space accounting corruption on XFS.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick clone
> > +
> > +_cleanup()
> > +{
> > +	_cleanup_flakey
> > +	cd /
> > +	rm -r -f $tmp.*
> > +}
> > +
> > +# Import common functions.
> > +. ./common/reflink
> > +. ./common/dmflakey
> > +
> > +# real QA test starts here
> > +_supported_fs xfs
> > +_require_scratch_reflink
> > +_require_cp_reflink
> > +_require_xfs_io_command "cowextsize"
> > +_require_flakey_with_error_writes
> > +
> > +_scratch_mkfs >> $seqres.full
> > +_init_flakey
> > +_mount_flakey
> > +
> > +blksz=$(_get_file_block_size $SCRATCH_MNT)
> > +
> > +# Set the COW extent size hint to guarantee COW fork preallocation occurs over a
> > +# bordering block offset.
> > +$XFS_IO_PROG -c "cowextsize $((blksz * 2))" $SCRATCH_MNT >> $seqres.full
> > +
> > +# create two files that share a single block
> > +$XFS_IO_PROG -fc "pwrite $blksz $blksz" $SCRATCH_MNT/file1 >> $seqres.full
> > +_cp_reflink $SCRATCH_MNT/file1 $SCRATCH_MNT/file2
> > +
> > +# Perform a buffered write across the shared and non-shared blocks. On XFS, this
> > +# creates a COW fork extent that covers the shared block as well as the just
> > +# created non-shared delalloc block. Fail the writeback to verify that all
> > +# delayed allocation is cleaned up properly.
> > +_load_flakey_table $FLAKEY_ERROR_WRITES
> > +$XFS_IO_PROG -c "pwrite 0 $((blksz * 2))" \
> > +	-c fsync $SCRATCH_MNT/file2 >> $seqres.full
> > +_load_flakey_table $FLAKEY_ALLOW_WRITES
> 
> Hmm.  So I've been running this test in my djwong-dev tree and hit this
> last night:
> 
> --- xfs/999.out
> +++ xfs/999.out.bad
> @@ -1,2 +1,3 @@
>  QA output created by 999
> -fsync: Input/output error
> +stat: Input/output error
> +cp: failed to access '/opt/file3': Input/output error
> 
> Digging into the kernel log, I see this happen:
> 
> [10240.821719] XFS (dm-0): Mounting V5 Filesystem
> [10240.855461] XFS (dm-0): Ending clean mount
> [10240.857030] XFS (dm-0): Quotacheck needed: Please wait.
> [10240.860095] XFS (dm-0): Quotacheck: Done.
> [10240.977055] XFS (dm-0): log I/O error -5
> [10240.977459] XFS (dm-0): Log I/O Error (0x2) detected at xlog_ioend_work+0x5f/0xb0 [xfs] (fs/xfs/xfs_log.c:1377).  Shutting down filesystem.
> [10240.978682] XFS (dm-0): Please unmount the filesystem and rectify the problem(s)
> [10241.044886] XFS (dm-0): Unmounting Filesystem
> 
> I guess the log tried to checkpoint for the brief window where the
> flakey table was enabled, and shut down the whole fs?  I don't have any
> good ideas for how to solve this, though.
> 
> Hm.  What if you did something like:
> 
> $XFS_IO_PROG -c 'pwrite...' $SCRATCH_MNT/file2
> _load_flakey_table $FLAKEY_ERROR_WRITES
> $XFS_IO_PROG -c 'sync_range -wa' $SCRATCH_MNT/file2
> +_load_flakey_table $FLAKEY_ALLOW_WRITES
> 

I notice that sync_range doesn't combine options like that even though
the underlying flags can be combined for the syscall. That aside, I
suspect a combination of an fsync on the earlier reflink command and a
couple sync_range calls here (to achieve the intended behavior above) is
probably the best option to try and avoid this. That seems to preserve
the intended behavior of the test and I don't see any spurious failures
in a couple hundred or so iterations.. (granted I don't know how
reproducible that was in the first place).

Brian

> to constrain the window in which disk write will fail?  Seeing as s_f_r
> doesn't actually tell the fs to flush its own metadata or anything.
> 
> (Yikes, did I finally find a use for sync_file_range??)
> 
> --D
> 
> > +
> > +# Try a post-fail reflink and then unmount. Both of these are known to produce
> > +# errors and/or assert failures on XFS if we trip over a stale delalloc block.
> > +_cp_reflink $SCRATCH_MNT/file2 $SCRATCH_MNT/file3
> > +_unmount_flakey
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/xfs/999.out b/tests/xfs/999.out
> > new file mode 100644
> > index 00000000..88b69c4c
> > --- /dev/null
> > +++ b/tests/xfs/999.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 999
> > +fsync: Input/output error
> > -- 
> > 2.31.1
> > 
>
diff mbox series

Patch

diff --git a/tests/xfs/999 b/tests/xfs/999
new file mode 100755
index 00000000..f27972bc
--- /dev/null
+++ b/tests/xfs/999
@@ -0,0 +1,62 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Red Hat, Inc.  All Rights Reserved.
+#
+# FS QA Test 999
+#
+# Test that COW writeback that overlaps non-shared delalloc blocks does not
+# leave around stale delalloc blocks on I/O failure. This triggers assert
+# failures and free space accounting corruption on XFS.
+#
+. ./common/preamble
+_begin_fstest auto quick clone
+
+_cleanup()
+{
+	_cleanup_flakey
+	cd /
+	rm -r -f $tmp.*
+}
+
+# Import common functions.
+. ./common/reflink
+. ./common/dmflakey
+
+# real QA test starts here
+_supported_fs xfs
+_require_scratch_reflink
+_require_cp_reflink
+_require_xfs_io_command "cowextsize"
+_require_flakey_with_error_writes
+
+_scratch_mkfs >> $seqres.full
+_init_flakey
+_mount_flakey
+
+blksz=$(_get_file_block_size $SCRATCH_MNT)
+
+# Set the COW extent size hint to guarantee COW fork preallocation occurs over a
+# bordering block offset.
+$XFS_IO_PROG -c "cowextsize $((blksz * 2))" $SCRATCH_MNT >> $seqres.full
+
+# create two files that share a single block
+$XFS_IO_PROG -fc "pwrite $blksz $blksz" $SCRATCH_MNT/file1 >> $seqres.full
+_cp_reflink $SCRATCH_MNT/file1 $SCRATCH_MNT/file2
+
+# Perform a buffered write across the shared and non-shared blocks. On XFS, this
+# creates a COW fork extent that covers the shared block as well as the just
+# created non-shared delalloc block. Fail the writeback to verify that all
+# delayed allocation is cleaned up properly.
+_load_flakey_table $FLAKEY_ERROR_WRITES
+$XFS_IO_PROG -c "pwrite 0 $((blksz * 2))" \
+	-c fsync $SCRATCH_MNT/file2 >> $seqres.full
+_load_flakey_table $FLAKEY_ALLOW_WRITES
+
+# Try a post-fail reflink and then unmount. Both of these are known to produce
+# errors and/or assert failures on XFS if we trip over a stale delalloc block.
+_cp_reflink $SCRATCH_MNT/file2 $SCRATCH_MNT/file3
+_unmount_flakey
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/999.out b/tests/xfs/999.out
new file mode 100644
index 00000000..88b69c4c
--- /dev/null
+++ b/tests/xfs/999.out
@@ -0,0 +1,2 @@ 
+QA output created by 999
+fsync: Input/output error