[PATCHSET 0/2] fstests: random fixes

[PATCHSET 0/2] fstests: random fixes

[Darrick J. Wong]

Hi all,

This week I have two random fixes:

The first update removes xfs_check from the default test configuration,
as I have finally completed my quest to make sure that xfs_repair (5.13)
catches all the corruption errors that xfs_check does.  Since xfs_check
has a tendency to OOM, let's remove the redundant code to save everyone
some run time.

The other is an update to the swapfile corruption test to add the commit
id for the kernel fix and fix some problems on arm64.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=random-fixes

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=random-fixes

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=random-fixes
---
 README                |    4 ++++
 common/xfs            |   12 ++++++++----
 tests/generic/643     |   33 ++++++++++++++++++++++-----------
 tests/generic/643.out |    2 +-
 4 files changed, 35 insertions(+), 16 deletions(-)

[PATCH 1/2] common/xfs: skip xfs_check unless the test runner forces us to

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

At long last I've completed my quest to ensure that every corruption
found by xfs_check can also be found by xfs_repair.  Since xfs_check
uses more memory than repair and has long been obsolete, let's stop
running it automatically from _check_xfs_filesystem unless the test
runner makes us do it.

Tests that explicitly want xfs_check can call it via _scratch_xfs_check
or _xfs_check; that part doesn't go away.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 README     |    4 ++++
 common/xfs |   12 ++++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)


diff --git a/README b/README
index 84c217ce..63f0641a 100644
--- a/README
+++ b/README
@@ -125,6 +125,10 @@ Preparing system for tests:
 	       time we should try a patient module remove. The default is 50
 	       seconds. Set this to "forever" and we'll wait forever until the
 	       module is gone.
+             - Set FORCE_XFS_CHECK_PROG=yes to have _check_xfs_filesystem run
+               xfs_check to check the filesystem.  As of August 2021,
+               xfs_repair finds all filesystem corruptions found by xfs_check,
+               and more, which means that xfs_check is no longer run by default.
 
         - or add a case to the switch in common/config assigning
           these variables based on the hostname of your test
diff --git a/common/xfs b/common/xfs
index c5e39427..bfb1bf1e 100644
--- a/common/xfs
+++ b/common/xfs
@@ -595,10 +595,14 @@ _check_xfs_filesystem()
 		ok=0
 	fi
 
-	# xfs_check runs out of memory on large files, so even providing the test
-	# option (-t) to avoid indexing the free space trees doesn't make it pass on
-	# large filesystems. Avoid it.
-	if [ "$LARGE_SCRATCH_DEV" != yes ]; then
+	# xfs_check runs out of memory on large files, so even providing the
+	# test option (-t) to avoid indexing the free space trees doesn't make
+	# it pass on large filesystems. Avoid it.
+	#
+	# As of August 2021, xfs_repair completely supersedes xfs_check's
+	# ability to find corruptions, so we no longer run xfs_check unless
+	# forced to run it.
+	if [ "$LARGE_SCRATCH_DEV" != yes ] && [ "$FORCE_XFS_CHECK_PROG" = "yes" ]; then
 		_xfs_check $extra_log_options $device 2>&1 > $tmp.fs_check
 	fi
 	if [ -s $tmp.fs_check ]; then

[PATCH 2/2] generic/643: fix weird problems on 64k-page arm systems

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

I noticed the following regression on an arm64 system with 64k pages:

--- generic/643.out
+++ generic/643.out.bad
@@ -1,2 +1,3 @@
 QA output created by 643
+swapon added 960 pages, expected 896
 Silence is golden

Evidently mkswap writes the swapfile header advertising one memory page
less than the size of the file, and on some architectures the kernel
can sometimes grab one page less than what's advertised.  This variance
is weird but tolerable; we simply don't want to see the page count
doubling when the file size doubles.

While we're at it, include the commit id of the fix in the commit
message.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/generic/643     |   33 ++++++++++++++++++++++-----------
 tests/generic/643.out |    2 +-
 2 files changed, 23 insertions(+), 12 deletions(-)


diff --git a/tests/generic/643 b/tests/generic/643
index 2bb9d220..7a1d3ec7 100755
--- a/tests/generic/643
+++ b/tests/generic/643
@@ -4,8 +4,10 @@
 #
 # FS QA Test No. 643
 #
-# Regression test for "mm/swap: consider max pages in iomap_swapfile_add_extent"
-
+# Regression test for commit:
+#
+# 36ca7943ac18 ("mm/swap: consider max pages in iomap_swapfile_add_extent")
+#
 # Xu Yu found that the iomap swapfile activation code failed to constrain
 # itself to activating however many swap pages that the mm asked us for.  This
 # is an deviation in behavior from the classic swapfile code.  It also leads to
@@ -22,6 +24,8 @@ _cleanup()
 	test -n "$swapfile" && swapoff $swapfile &> /dev/null
 }
 
+. ./common/filter
+
 # real QA test starts here
 _supported_fs generic
 _require_scratch_swapfile
@@ -34,29 +38,36 @@ _scratch_mount >> $seqres.full
 swapfile=$SCRATCH_MNT/386spart.par
 _format_swapfile $swapfile 1m >> $seqres.full
 
-swapfile_pages() {
+page_size=$(getconf PAGE_SIZE)
+
+swapfile_blocks() {
 	local swapfile="$1"
 
 	grep "$swapfile" /proc/swaps | awk '{print $3}'
 }
 
 _swapon_file $swapfile
-before_pages=$(swapfile_pages "$swapfile")
+before_blocks=$(swapfile_blocks "$swapfile")
 swapoff $swapfile
 
 # Extend the length of the swapfile but do not rewrite the header.
-# The subsequent swapon should set up 1MB worth of pages, not 2MB.
+# The subsequent swapon should set up 1MB worth of blocks, not 2MB.
 $XFS_IO_PROG -f -c 'pwrite 1m 1m' $swapfile >> $seqres.full
 
 _swapon_file $swapfile
-after_pages=$(swapfile_pages "$swapfile")
+after_blocks=$(swapfile_blocks "$swapfile")
 swapoff $swapfile
 
-# Both swapon attempts should have found the same number of pages.
-test "$before_pages" -eq "$after_pages" || \
-	echo "swapon added $after_pages pages, expected $before_pages"
+# Both swapon attempts should have found approximately the same number of
+# blocks.  Unfortunately, mkswap and the kernel are a little odd -- the number
+# of pages that mkswap writes into the swapfile header is one page less than
+# the file size, and then the kernel itself doesn't always grab all the pages
+# advertised in the header.  Hence we let the number of swap pages increase by
+# two pages.  I'm looking at you, Mr. 64k pages on arm64...
+page_variance=$(( page_size / 512 ))
+_within_tolerance "swap blocks" $after_blocks $before_blocks 0 $page_variance -v
+
+echo "pagesize: $page_size; before: $before_blocks; after: $after_blocks" >> $seqres.full
 
-# success, all done
-echo Silence is golden
 status=0
 exit
diff --git a/tests/generic/643.out b/tests/generic/643.out
index 32f1c629..1b14d66e 100644
--- a/tests/generic/643.out
+++ b/tests/generic/643.out
@@ -1,2 +1,2 @@
 QA output created by 643
-Silence is golden
+swap blocks is in range

Re: [PATCH 1/2] common/xfs: skip xfs_check unless the test runner forces us to

[Christoph Hellwig]

On Tue, Aug 31, 2021 at 05:11:16PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> At long last I've completed my quest to ensure that every corruption
> found by xfs_check can also be found by xfs_repair.  Since xfs_check
> uses more memory than repair and has long been obsolete, let's stop
> running it automatically from _check_xfs_filesystem unless the test
> runner makes us do it.
> 
> Tests that explicitly want xfs_check can call it via _scratch_xfs_check
> or _xfs_check; that part doesn't go away.

Awesome!

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/2] generic/643: fix weird problems on 64k-page arm systems

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/2] common/xfs: skip xfs_check unless the test runner forces us to

[Zorro Lang]

On Tue, Aug 31, 2021 at 05:11:16PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> At long last I've completed my quest to ensure that every corruption
> found by xfs_check can also be found by xfs_repair.  Since xfs_check
> uses more memory than repair and has long been obsolete, let's stop
> running it automatically from _check_xfs_filesystem unless the test
> runner makes us do it.
> 
> Tests that explicitly want xfs_check can call it via _scratch_xfs_check
> or _xfs_check; that part doesn't go away.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  README     |    4 ++++
>  common/xfs |   12 ++++++++----
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/README b/README
> index 84c217ce..63f0641a 100644
> --- a/README
> +++ b/README
> @@ -125,6 +125,10 @@ Preparing system for tests:
>  	       time we should try a patient module remove. The default is 50
>  	       seconds. Set this to "forever" and we'll wait forever until the
>  	       module is gone.
> +             - Set FORCE_XFS_CHECK_PROG=yes to have _check_xfs_filesystem run
> +               xfs_check to check the filesystem.  As of August 2021,
> +               xfs_repair finds all filesystem corruptions found by xfs_check,
> +               and more, which means that xfs_check is no longer run by default.
>  
>          - or add a case to the switch in common/config assigning
>            these variables based on the hostname of your test
> diff --git a/common/xfs b/common/xfs
> index c5e39427..bfb1bf1e 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -595,10 +595,14 @@ _check_xfs_filesystem()
>  		ok=0
>  	fi
>  
> -	# xfs_check runs out of memory on large files, so even providing the test
> -	# option (-t) to avoid indexing the free space trees doesn't make it pass on
> -	# large filesystems. Avoid it.
> -	if [ "$LARGE_SCRATCH_DEV" != yes ]; then
> +	# xfs_check runs out of memory on large files, so even providing the
> +	# test option (-t) to avoid indexing the free space trees doesn't make
> +	# it pass on large filesystems. Avoid it.
> +	#
> +	# As of August 2021, xfs_repair completely supersedes xfs_check's
> +	# ability to find corruptions, so we no longer run xfs_check unless
> +	# forced to run it.

I have to say I've waited for this change long time :-D

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

> +	if [ "$LARGE_SCRATCH_DEV" != yes ] && [ "$FORCE_XFS_CHECK_PROG" = "yes" ]; then
>  		_xfs_check $extra_log_options $device 2>&1 > $tmp.fs_check
>  	fi
>  	if [ -s $tmp.fs_check ]; then
>