diff mbox series

[4/4] fstests: test xfs swapext log replay

Message ID 20220420083653.1031631-5-zlang@redhat.com (mailing list archive)
State New, archived
Headers show
Series several long time unmerged patches from zlang | expand

Commit Message

Zorro Lang April 20, 2022, 8:36 a.m. UTC 
If an inode had been in btree format and had a data fork owner change
logged (XFS_ILOG_DOWNER), after changing the format to non-btree, will
hit an ASSERT in xfs_recover_inode_owner_change() which enforces that
if XFS_ILOG_[AD]OWNER is set.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---

Hi,

3+ years past, this test is still failed on latest upstream linux kernel,
as we talked below:
https://patchwork.kernel.org/project/fstests/patch/20181223141721.5318-1-zlang@redhat.com/

I think it's time to bring it back to talk again. If it's a case issue, I'll fix.
If it's a bug, means this case is good to merge.

Thanks,
Zorro

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

Comments

Darrick J. Wong April 20, 2022, 6:08 p.m. UTC | #1 
On Wed, Apr 20, 2022 at 04:36:53PM +0800, Zorro Lang wrote:
> If an inode had been in btree format and had a data fork owner change
> logged (XFS_ILOG_DOWNER), after changing the format to non-btree, will
> hit an ASSERT in xfs_recover_inode_owner_change() which enforces that
> if XFS_ILOG_[AD]OWNER is set.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
> 
> Hi,
> 
> 3+ years past, this test is still failed on latest upstream linux kernel,
> as we talked below:
> https://patchwork.kernel.org/project/fstests/patch/20181223141721.5318-1-zlang@redhat.com/
> 
> I think it's time to bring it back to talk again. If it's a case issue, I'll fix.
> If it's a bug, means this case is good to merge.

Uhoh.  So ... did you write this as a regression test for dc1baa715bbf
and then discovered that it uncovered another problem?

> Thanks,
> Zorro
> 
>  tests/xfs/999     | 58 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/999.out |  2 ++
>  2 files changed, 60 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..b1d58671
> --- /dev/null
> +++ b/tests/xfs/999
> @@ -0,0 +1,58 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Red Hat Inc.  All Rights Reserved.
> +#
> +# FS QA Test 999
> +#
> +# If an inode had been in btree format and had a data fork owner change
> +# logged, after changing the format to non-btree, will hit an ASSERT or
> +# fs corruption.
> +# This case trys to cover: dc1baa715bbf ("xfs: do not log/recover swapext
> +# extent owner changes for deleted inodes")
> +#
> +. ./common/preamble
> +_begin_fstest auto quick fsr
> +
> +# Import common functions.
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs generic
> +_require_scratch
> +_scratch_mkfs_xfs | _filter_mkfs 2>$tmp.mkfs >/dev/null
> +. $tmp.mkfs
> +
> +_scratch_mount
> +localfile=$SCRATCH_MNT/fragfile
> +
> +# Try to create a file with 1024 * (3 blocks + 1 hole):
> +# +----------+--------+-------+----------+--------+
> +# | 3 blocks | 1 hole |  ...  | 3 blocks | 1 hole |
> +# +----------+--------+-------+----------+--------+
> +#
> +# The number of extents we can get maybe more or less than 1024, this method
> +# just to get a btree inode format.
> +filesize=$((dbsize * 1024 * 4))
> +for i in `seq $filesize -$dbsize 0`; do
> +	if [ $((i % (3 * dbsize))) -eq 0 ]; then
> +		continue
> +	fi
> +	$XFS_IO_PROG -f -d -c "pwrite $i $dbsize" $localfile >> $seqres.full
> +done

I wonder if you could use what _scratch_xfs_populate does to create
S_IFREG.FMT_BTREE instead of open-coding it, but I bet this test
predates that... :)

Anyway, this looks fine but I want to go try it to see what happens.

--D

> +
> +# Make a data fork owner change log
> +$XFS_FSR_PROG -v -d $localfile >> $seqres.full 2>&1
> +
> +# Truncate the file to 0, and change the inode format to extent, then shutdown
> +# the fs to keep the XFS_ILOG_DOWNER flag
> +$XFS_IO_PROG -t -x -c "pwrite 0 $dbsize" \
> +	     -c "fsync" \
> +	     -c "shutdown" $localfile >> $seqres.full
> +
> +# Cycle mount, to replay the log
> +_scratch_cycle_mount
> +
> +echo "Silence is golden"
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/999.out b/tests/xfs/999.out
> new file mode 100644
> index 00000000..3b276ca8
> --- /dev/null
> +++ b/tests/xfs/999.out
> @@ -0,0 +1,2 @@
> +QA output created by 999
> +Silence is golden
> -- 
> 2.31.1
>
Darrick J. Wong April 20, 2022, 10:06 p.m. UTC | #2 
On Wed, Apr 20, 2022 at 04:36:53PM +0800, Zorro Lang wrote:
> If an inode had been in btree format and had a data fork owner change
> logged (XFS_ILOG_DOWNER), after changing the format to non-btree, will
> hit an ASSERT in xfs_recover_inode_owner_change() which enforces that
> if XFS_ILOG_[AD]OWNER is set.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
> 
> Hi,
> 
> 3+ years past, this test is still failed on latest upstream linux kernel,
> as we talked below:
> https://patchwork.kernel.org/project/fstests/patch/20181223141721.5318-1-zlang@redhat.com/
> 
> I think it's time to bring it back to talk again. If it's a case issue, I'll fix.
> If it's a bug, means this case is good to merge.
> 
> Thanks,
> Zorro
> 
>  tests/xfs/999     | 58 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/999.out |  2 ++
>  2 files changed, 60 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..b1d58671
> --- /dev/null
> +++ b/tests/xfs/999
> @@ -0,0 +1,58 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Red Hat Inc.  All Rights Reserved.
> +#
> +# FS QA Test 999
> +#
> +# If an inode had been in btree format and had a data fork owner change
> +# logged, after changing the format to non-btree, will hit an ASSERT or
> +# fs corruption.
> +# This case trys to cover: dc1baa715bbf ("xfs: do not log/recover swapext
> +# extent owner changes for deleted inodes")
> +#
> +. ./common/preamble
> +_begin_fstest auto quick fsr
> +
> +# Import common functions.
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs generic
> +_require_scratch
> +_scratch_mkfs_xfs | _filter_mkfs 2>$tmp.mkfs >/dev/null
> +. $tmp.mkfs
> +
> +_scratch_mount
> +localfile=$SCRATCH_MNT/fragfile
> +
> +# Try to create a file with 1024 * (3 blocks + 1 hole):
> +# +----------+--------+-------+----------+--------+
> +# | 3 blocks | 1 hole |  ...  | 3 blocks | 1 hole |
> +# +----------+--------+-------+----------+--------+
> +#
> +# The number of extents we can get maybe more or less than 1024, this method
> +# just to get a btree inode format.
> +filesize=$((dbsize * 1024 * 4))
> +for i in `seq $filesize -$dbsize 0`; do
> +	if [ $((i % (3 * dbsize))) -eq 0 ]; then
> +		continue
> +	fi
> +	$XFS_IO_PROG -f -d -c "pwrite $i $dbsize" $localfile >> $seqres.full
> +done
> +
> +# Make a data fork owner change log
> +$XFS_FSR_PROG -v -d $localfile >> $seqres.full 2>&1

Hmm, so I tried this, and found that it fails (as expected) on
-mrmapbt=0 configs with my 5.18-merge branch.

Weirdly, when I tried it with my djwong-dev branch, fsr failed the
XFS_IOC_SWAPEXT with -22 (EINVAL) ... but this program doesn't detect
that, so it "passed".  I even popped all the patches off, but that
didn't change anything.... weird.  I'll keep looking.

--D

> +
> +# Truncate the file to 0, and change the inode format to extent, then shutdown
> +# the fs to keep the XFS_ILOG_DOWNER flag
> +$XFS_IO_PROG -t -x -c "pwrite 0 $dbsize" \
> +	     -c "fsync" \
> +	     -c "shutdown" $localfile >> $seqres.full
> +
> +# Cycle mount, to replay the log
> +_scratch_cycle_mount
> +
> +echo "Silence is golden"
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/999.out b/tests/xfs/999.out
> new file mode 100644
> index 00000000..3b276ca8
> --- /dev/null
> +++ b/tests/xfs/999.out
> @@ -0,0 +1,2 @@
> +QA output created by 999
> +Silence is golden
> -- 
> 2.31.1
>
diff mbox series

Patch

diff --git a/tests/xfs/999 b/tests/xfs/999
new file mode 100755
index 00000000..b1d58671
--- /dev/null
+++ b/tests/xfs/999
@@ -0,0 +1,58 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 Red Hat Inc.  All Rights Reserved.
+#
+# FS QA Test 999
+#
+# If an inode had been in btree format and had a data fork owner change
+# logged, after changing the format to non-btree, will hit an ASSERT or
+# fs corruption.
+# This case trys to cover: dc1baa715bbf ("xfs: do not log/recover swapext
+# extent owner changes for deleted inodes")
+#
+. ./common/preamble
+_begin_fstest auto quick fsr
+
+# Import common functions.
+. ./common/filter
+
+# real QA test starts here
+_supported_fs generic
+_require_scratch
+_scratch_mkfs_xfs | _filter_mkfs 2>$tmp.mkfs >/dev/null
+. $tmp.mkfs
+
+_scratch_mount
+localfile=$SCRATCH_MNT/fragfile
+
+# Try to create a file with 1024 * (3 blocks + 1 hole):
+# +----------+--------+-------+----------+--------+
+# | 3 blocks | 1 hole |  ...  | 3 blocks | 1 hole |
+# +----------+--------+-------+----------+--------+
+#
+# The number of extents we can get maybe more or less than 1024, this method
+# just to get a btree inode format.
+filesize=$((dbsize * 1024 * 4))
+for i in `seq $filesize -$dbsize 0`; do
+	if [ $((i % (3 * dbsize))) -eq 0 ]; then
+		continue
+	fi
+	$XFS_IO_PROG -f -d -c "pwrite $i $dbsize" $localfile >> $seqres.full
+done
+
+# Make a data fork owner change log
+$XFS_FSR_PROG -v -d $localfile >> $seqres.full 2>&1
+
+# Truncate the file to 0, and change the inode format to extent, then shutdown
+# the fs to keep the XFS_ILOG_DOWNER flag
+$XFS_IO_PROG -t -x -c "pwrite 0 $dbsize" \
+	     -c "fsync" \
+	     -c "shutdown" $localfile >> $seqres.full
+
+# Cycle mount, to replay the log
+_scratch_cycle_mount
+
+echo "Silence is golden"
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/999.out b/tests/xfs/999.out
new file mode 100644
index 00000000..3b276ca8
--- /dev/null
+++ b/tests/xfs/999.out
@@ -0,0 +1,2 @@ 
+QA output created by 999
+Silence is golden