[PATCH 0/4] several long time unmerged patches from zlang

[PATCH 0/4] several long time unmerged patches from zlang

[Zorro Lang]

Recently I try to clean up all my old local branches of xfstests,  and found
some patches aren't merged for long time. I abandoned most of them, but some
of them might be still worth reviewing, so I rebased them to latest xfstests
and send out to get review.

[PATCH 1/4] is tiny patch for glusterfs testing failure.
[PATCH 2/4] is for large fs testing
[PATCH 3/4] and [4/4] are two different new cases to cover regression issues.

So feel free to give them your review point, especially the [4/4], it still
trigger a unknown xfs failure on latest upstream kernel.

Thanks,
Zorro

[PATCH 1/4] src/t_rename_overwrite: fsync to flush the rename operation result

[Zorro Lang]

The generic/035 fails on glusterfs due to glusterfs (or others like
it), the file state can't be updated in time in client side, there's
time delay. So call fsync to get the right file state after rename.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---
 src/t_rename_overwrite.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/t_rename_overwrite.c b/src/t_rename_overwrite.c
index c5cdd1db..8dcf8d46 100644
--- a/src/t_rename_overwrite.c
+++ b/src/t_rename_overwrite.c
@@ -2,6 +2,7 @@
 #include <fcntl.h>
 #include <err.h>
 #include <sys/stat.h>
+#include <unistd.h>
 
 int main(int argc, char *argv[])
 {
@@ -25,6 +26,7 @@ int main(int argc, char *argv[])
 	res = rename(path1, path2);
 	if (res == -1)
 		err(1, "rename(\"%s\", \"%s\")", path1, path2);
+	fsync(fd);
 
 	res = fstat(fd, &stbuf);
 	if (res == -1)
-- 
2.31.1

[PATCH 2/4] fstests: avoid dedupe testing blocked on large fs long time

[Zorro Lang]

When test on large fs (--large-fs), xfstests allocates a large
file in SCRATCH_MNT/ at first. g/559~561 try to dedupe of extents
within the same file, so they'll waste lots of time to deal with
that large file.

So bring in a common function named _duperemove(), which decide if
allow dedupe of extents with in the same file. If specify "same"
or "nosame" in the 1st argument, follow the specified option. Else
use "same" by default except testing on large filesystem.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---
 common/reflink    | 39 +++++++++++++++++++++++++++++++++++++++
 tests/generic/559 | 10 +++++-----
 tests/generic/560 |  3 +--
 tests/generic/561 |  2 +-
 4 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/common/reflink b/common/reflink
index 76e9cb7d..48136650 100644
--- a/common/reflink
+++ b/common/reflink
@@ -488,3 +488,42 @@ _sweave_reflink_rainbow_delalloc() {
 		_pwrite_byte 0x62 $((blksz * i)) $blksz $dfile.chk
 	done
 }
+
+# Do De-dupe on a directory (or a file). $DUPEREMOVE_PROG is required if you
+# call this function, and -d -r options are recommended.
+#
+# The 1st argument can be used to forcibly decide if dedupe extents with in
+# the same file (better to use "same" if you run on a single file). If it's
+# not set to "same" or "nosame", the function will use "same" by default
+# except testing on large filesystem.
+_duperemove()
+{
+	local dedupe_opt=""
+
+	# Decide if allow dedupe of extents with in the same file. If specify
+	# "same" or "nosame", follow the specified option, else if test on
+	# large filesystem, "nosame" by default.
+	if [ "$1" = "same" ]; then
+		dedupe_opt="same"
+		shift
+	elif [ "$1" = "nosame" ]; then
+		dedupe_opt="nosame"
+		shift
+	elif [ "$LARGE_SCRATCH_DEV" = "yes" ]; then
+		# Don't allow dedupe of extents with in the same file if test
+		# on large filesystem. Due to xfstests pre-alloc a huge size
+		# file to take most fs free space at every test beginning if
+		# --large-fs option is used.
+		dedupe_opt="nosame"
+	fi
+
+	# If above judgements set $dedupe_opt, then use this option, or "same"
+	# by default.
+	if [ -n "$dedupe_opt" ]; then
+		dedupe_opt="--dedupe-options=${dedupe_opt}"
+	else
+		dedupe_opt="--dedupe-options=same"
+	fi
+
+	$DUPEREMOVE_PROG $dedupe_opt "$@"
+}
diff --git a/tests/generic/559 b/tests/generic/559
index 98ab4474..9817013f 100755
--- a/tests/generic/559
+++ b/tests/generic/559
@@ -21,18 +21,18 @@ fssize=$((2 * 1024 * 1024 * 1024))
 _scratch_mkfs_sized $fssize > $seqres.full 2>&1
 _scratch_mount >> $seqres.full 2>&1
 
+testfile="$SCRATCH_MNT/${seq}.file"
 # fill the fs with a big file has same contents
-$XFS_IO_PROG -f -c "pwrite -S 0x55 0 $fssize" $SCRATCH_MNT/${seq}.file \
-	>> $seqres.full 2>&1
-md5sum $SCRATCH_MNT/${seq}.file > ${tmp}.md5sum
+$XFS_IO_PROG -f -c "pwrite -S 0x55 0 $fssize" $testfile >> $seqres.full 2>&1
+md5sum $testfile > ${tmp}.md5sum
 
 echo "= before cycle mount ="
 # Dedupe with 1M blocksize
-$DUPEREMOVE_PROG -dr --dedupe-options=same -b 1048576 $SCRATCH_MNT/ >>$seqres.full 2>&1
+_duperemove -dr -b 1048576 $SCRATCH_MNT/ >>$seqres.full 2>&1
 # Verify integrity
 md5sum -c --quiet ${tmp}.md5sum
 # Dedupe with 64k blocksize
-$DUPEREMOVE_PROG -dr --dedupe-options=same -b 65536 $SCRATCH_MNT/ >>$seqres.full 2>&1
+_duperemove -dr -b 65536 $SCRATCH_MNT/ >>$seqres.full 2>&1
 # Verify integrity again
 md5sum -c --quiet ${tmp}.md5sum
 
diff --git a/tests/generic/560 b/tests/generic/560
index e3f28667..43b32293 100755
--- a/tests/generic/560
+++ b/tests/generic/560
@@ -35,8 +35,7 @@ function iterate_dedup_verify()
 		$FSSTRESS_PROG $fsstress_opts -d $noisedir \
 			       -n 200 -p $((5 * LOAD_FACTOR)) >/dev/null 2>&1
 		# Too many output, so only save error output
-		$DUPEREMOVE_PROG -dr --dedupe-options=same $dupdir \
-			>/dev/null 2>$seqres.full
+		_duperemove same -dr $dupdir >/dev/null 2>$seqres.full
 		md5sum -c --quiet $md5file$index
 		src=$dest
 		dest=$dupdir/$((index + 1))
diff --git a/tests/generic/561 b/tests/generic/561
index 44f07802..1d8109cf 100755
--- a/tests/generic/561
+++ b/tests/generic/561
@@ -67,7 +67,7 @@ for ((i = 0; i < $((2 * LOAD_FACTOR)); i++)); do
 		# dupremove processes in an arbitrary order, which leaves the
 		# memory in an inconsistent state long enough for the assert
 		# to trip.
-		cmd="$DUPEREMOVE_PROG -dr --dedupe-options=same $testdir"
+		cmd="_duperemove -dr $testdir"
 		bash -c "$cmd" >> $seqres.full 2>&1
 	done 2>&1 | sed -e '/Terminated/d' &
 	dedup_pids="$! $dedup_pids"
-- 
2.31.1

[PATCH 3/4] generic: test data corruption when blocksize < pagesize for mmaped data

[Zorro Lang]

page_mkwrite() is used by filesystems to allocate blocks under a page
which is becoming writeably mmapped in some process' address space.
This allows a filesystem to return a page fault if there is not enough
space available, user exceeds quota or similar problem happens, rather
than silently discarding data later when writepage is called. However
VFS fails to call ->page_mkwrite() in all the cases where filesystems
need it when blocksize < pagesize.

At the moment page_mkwrite() is called, filesystem can allocate only
one block for the page if i_size < page size. Otherwise it would
create blocks beyond i_size which is generally undesirable. But later
at writepage() time, we also need to store data at offset 4095 but we
don't have block allocated for it.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---
 tests/generic/999     | 75 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  5 +++
 2 files changed, 80 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out

diff --git a/tests/generic/999 b/tests/generic/999
new file mode 100755
index 00000000..f1b65982
--- /dev/null
+++ b/tests/generic/999
@@ -0,0 +1,75 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 Red Hat Inc.  All Rights Reserved.
+#
+# FS QA Test 999
+#
+# Regression test for linux commit 90a80202 ("data corruption when
+# blocksize < pagesize for mmaped data")
+#
+. ./common/preamble
+_begin_fstest auto quick
+
+# Import common functions.
+. ./common/filter
+
+# real QA test starts here
+_supported_fs generic
+_require_scratch
+_require_block_device $SCRATCH_DEV
+_require_xfs_io_command mmap "-s <size>"
+_require_xfs_io_command mremap
+_require_xfs_io_command truncate
+_require_xfs_io_command mwrite
+
+sector_size=`_min_dio_alignment $SCRATCH_DEV`
+page_size=$(get_page_size)
+block_size=$((page_size/2))
+if [ $sector_size -gt $block_size ];then
+	_notrun "need sector size < page size"
+fi
+
+unset MKFS_OPTIONS
+# For save time, 512MiB is enough to reproduce bug
+_scratch_mkfs_sized 536870912 $block_size >$seqres.full 2>&1 || _fail "mkfs failed"
+_scratch_mount
+
+# take one block size space
+$XFS_IO_PROG -f -t -c "pwrite 0 $block_size" $SCRATCH_MNT/testfile >>$seqres.full 2>&1
+
+# Fill all free space, dd can keep writing until no space. Note: if the fs
+# isn't be full, this test will fail.
+for ((i=0; i<2; i++));do
+	dd if=/dev/zero of=$SCRATCH_MNT/full bs=$block_size >>$seqres.full 2>&1
+	_scratch_cycle_mount
+done
+
+# truncate 0
+# pwrite -S 0x61 0 $block_size
+# mmap -rw -s $((page_size * 2)) 0 $block_size
+# mwrite -S 0x61 0 1  --> page_mkwrite() for index 0 is called
+# truncate $((page_size * 2))
+# mremap $((page_size * 2))
+# mwrite -S 0x61 $((page_size - 1)) 1  --> [bug] no page_mkwrite() called
+#
+# If there's a bug, the last step will be killed by SIGBUS, and it won't
+# write a "0x61" on the disk.
+#
+# Note: mremap maybe failed when memory load is heavy.
+$XFS_IO_PROG -f \
+             -c "truncate 0" \
+             -c "pwrite -S 0x61 0 $block_size" \
+             -c "mmap -rw -s $((page_size * 2)) 0 $block_size" \
+             -c "mwrite -S 0x61 0 1" \
+             -c "truncate $((page_size * 2))" \
+             -c "mremap $((page_size * 2))" \
+             -c "mwrite -S 0x61 $((page_size - 1)) 1" \
+             $SCRATCH_MNT/testfile | _filter_xfs_io
+
+# we will see 0x61 written by "mwrite -S 0x61 0 1", but we shouldn't see one
+# more 0x61 by the last mwrite (except this fs isn't be full, or a bug)
+od -An -c $SCRATCH_MNT/testfile
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/999.out b/tests/generic/999.out
new file mode 100644
index 00000000..f1c59e83
--- /dev/null
+++ b/tests/generic/999.out
@@ -0,0 +1,5 @@
+QA output created by 999
+   a   a   a   a   a   a   a   a   a   a   a   a   a   a   a   a
+*
+  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0
+*
-- 
2.31.1

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

[Zorro Lang]

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
+
+# 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

Re: [PATCH 1/4] src/t_rename_overwrite: fsync to flush the rename operation result

[Darrick J. Wong]

On Wed, Apr 20, 2022 at 04:36:50PM +0800, Zorro Lang wrote:
> The generic/035 fails on glusterfs due to glusterfs (or others like
> it), the file state can't be updated in time in client side, there's
> time delay. So call fsync to get the right file state after rename.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
>  src/t_rename_overwrite.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/t_rename_overwrite.c b/src/t_rename_overwrite.c
> index c5cdd1db..8dcf8d46 100644
> --- a/src/t_rename_overwrite.c
> +++ b/src/t_rename_overwrite.c
> @@ -2,6 +2,7 @@
>  #include <fcntl.h>
>  #include <err.h>
>  #include <sys/stat.h>
> +#include <unistd.h>
>  
>  int main(int argc, char *argv[])
>  {
> @@ -25,6 +26,7 @@ int main(int argc, char *argv[])
>  	res = rename(path1, path2);
>  	if (res == -1)
>  		err(1, "rename(\"%s\", \"%s\")", path1, path2);
> +	fsync(fd);

The callsite needs error checking, right?

Also, what's going on with glusterfs?  We unlinked path2 from its parent
directory, which means that it's now an unlinked open file.  The next
code chunk in this file checks st_nlink != 0.  Does the fsync force the
operation to the server so that the fstat call pulls data from the
server?  Or put another way, does the fstat call return stale stat data
after a rename?

--D

>  
>  	res = fstat(fd, &stbuf);
>  	if (res == -1)
> -- 
> 2.31.1
> 

Re: [PATCH 3/4] generic: test data corruption when blocksize < pagesize for mmaped data

[Darrick J. Wong]

On Wed, Apr 20, 2022 at 04:36:52PM +0800, Zorro Lang wrote:
> page_mkwrite() is used by filesystems to allocate blocks under a page
> which is becoming writeably mmapped in some process' address space.
> This allows a filesystem to return a page fault if there is not enough
> space available, user exceeds quota or similar problem happens, rather
> than silently discarding data later when writepage is called. However
> VFS fails to call ->page_mkwrite() in all the cases where filesystems
> need it when blocksize < pagesize.
> 
> At the moment page_mkwrite() is called, filesystem can allocate only
> one block for the page if i_size < page size. Otherwise it would
> create blocks beyond i_size which is generally undesirable. But later
> at writepage() time, we also need to store data at offset 4095 but we
> don't have block allocated for it.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
>  tests/generic/999     | 75 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/999.out |  5 +++
>  2 files changed, 80 insertions(+)
>  create mode 100755 tests/generic/999
>  create mode 100644 tests/generic/999.out
> 
> diff --git a/tests/generic/999 b/tests/generic/999
> new file mode 100755
> index 00000000..f1b65982
> --- /dev/null
> +++ b/tests/generic/999
> @@ -0,0 +1,75 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Red Hat Inc.  All Rights Reserved.
> +#
> +# FS QA Test 999
> +#
> +# Regression test for linux commit 90a80202 ("data corruption when
> +# blocksize < pagesize for mmaped data")
> +#
> +. ./common/preamble
> +_begin_fstest auto quick
> +
> +# Import common functions.
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs generic
> +_require_scratch
> +_require_block_device $SCRATCH_DEV
> +_require_xfs_io_command mmap "-s <size>"
> +_require_xfs_io_command mremap
> +_require_xfs_io_command truncate
> +_require_xfs_io_command mwrite
> +
> +sector_size=`_min_dio_alignment $SCRATCH_DEV`
> +page_size=$(get_page_size)
> +block_size=$((page_size/2))
> +if [ $sector_size -gt $block_size ];then
> +	_notrun "need sector size < page size"
> +fi
> +
> +unset MKFS_OPTIONS
> +# For save time, 512MiB is enough to reproduce bug
> +_scratch_mkfs_sized 536870912 $block_size >$seqres.full 2>&1 || _fail "mkfs failed"
> +_scratch_mount
> +
> +# take one block size space
> +$XFS_IO_PROG -f -t -c "pwrite 0 $block_size" $SCRATCH_MNT/testfile >>$seqres.full 2>&1
> +
> +# Fill all free space, dd can keep writing until no space. Note: if the fs
> +# isn't be full, this test will fail.
> +for ((i=0; i<2; i++));do
> +	dd if=/dev/zero of=$SCRATCH_MNT/full bs=$block_size >>$seqres.full 2>&1
> +	_scratch_cycle_mount
> +done

_fill_fs ?

Also, this test ought to check that we actually consumed all the
freespace and note that in the golden output so that a test runner can
tell the difference between "test preconditions not satisfied" vs.
"kernel needs patching".

--D

> +
> +# truncate 0
> +# pwrite -S 0x61 0 $block_size
> +# mmap -rw -s $((page_size * 2)) 0 $block_size
> +# mwrite -S 0x61 0 1  --> page_mkwrite() for index 0 is called
> +# truncate $((page_size * 2))
> +# mremap $((page_size * 2))
> +# mwrite -S 0x61 $((page_size - 1)) 1  --> [bug] no page_mkwrite() called
> +#
> +# If there's a bug, the last step will be killed by SIGBUS, and it won't
> +# write a "0x61" on the disk.
> +#
> +# Note: mremap maybe failed when memory load is heavy.
> +$XFS_IO_PROG -f \
> +             -c "truncate 0" \
> +             -c "pwrite -S 0x61 0 $block_size" \
> +             -c "mmap -rw -s $((page_size * 2)) 0 $block_size" \
> +             -c "mwrite -S 0x61 0 1" \
> +             -c "truncate $((page_size * 2))" \
> +             -c "mremap $((page_size * 2))" \
> +             -c "mwrite -S 0x61 $((page_size - 1)) 1" \
> +             $SCRATCH_MNT/testfile | _filter_xfs_io
> +
> +# we will see 0x61 written by "mwrite -S 0x61 0 1", but we shouldn't see one
> +# more 0x61 by the last mwrite (except this fs isn't be full, or a bug)
> +od -An -c $SCRATCH_MNT/testfile
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/999.out b/tests/generic/999.out
> new file mode 100644
> index 00000000..f1c59e83
> --- /dev/null
> +++ b/tests/generic/999.out
> @@ -0,0 +1,5 @@
> +QA output created by 999
> +   a   a   a   a   a   a   a   a   a   a   a   a   a   a   a   a
> +*
> +  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0
> +*
> -- 
> 2.31.1
> 

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

[Darrick J. Wong]

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
> 

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

[Darrick J. Wong]

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
>