[PATCH 1/2] generic/251: don't snapshot $here during a test

[PATCH 1/2] generic/251: don't snapshot $here during a test

[Darrick J. Wong]

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

Zorro complained that the next patch caused him a regression:

generic/251 249s ... [failed, exit status 1]- output mismatch (see /root/git/xfstests/results//generic/251.out.bad)
    --- tests/generic/251.out   2022-04-29 23:07:23.263498297 +0800
    +++ /root/git/xfstests/results//generic/251.out.bad 2023-10-22 14:17:07.248059405 +0800
    @@ -1,2 +1,5 @@
     QA output created by 251
     Running the test: done.
    +5838a5839
    +> aa60581221897d3d7dd60458e1cca2fa  ./results/generic/251.full
    +!!!Checksums has changed - Filesystem possibly corrupted!!!\n
    ...
    (Run 'diff -u /root/git/xfstests/tests/generic/251.out /root/git/xfstests/results//generic/251.out.bad'  to see the entire diff)
Ran: generic/251
Failures: generic/251
Failed 1 of 1 tests

The next patch writes some debugging information into $seqres.full,
which is a file underneat $RESULT_BASE.  If the test operator does not
set RESULT_BASE, it will be set to a subdir of $here by default.  Since
this test also snapshots the contents of $here before starting its loop,
any logging to $seqres.full on such a system will cause the post-copy
checksum to fail due to a mismatch.

Fix all this by copying $here to $SCRATCH_DEV and checksumming the copy
before the FITRIM stress test begins to avoid problems with $seqres.full.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/generic/251 |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tests/generic/251 b/tests/generic/251
index 8ee74980cc..3b807df5fa 100755
--- a/tests/generic/251
+++ b/tests/generic/251
@@ -130,7 +130,13 @@ function run_process() {
 }
 
 nproc=20
-content=$here
+
+# Copy $here to the scratch fs and make coipes of the replica.  The fstests
+# output (and hence $seqres.full) could be in $here, so we need to snapshot
+# $here before computing file checksums.
+content=$SCRATCH_MNT/orig
+mkdir -p $content
+cp -axT $here/ $content/
 
 mkdir -p $tmp
 

[PATCH 2/2] generic/251: check min and max length and minlen for FSTRIM

[Darrick J. Wong]

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

Every now and then, this test fails with the following output when
running against my development tree when configured with an 8k fs block
size:

--- a/tests/generic/251.out	2023-07-11 12:18:21.624971186 -0700
+++ b/tests/generic/251.out.bad	2023-10-15 20:54:44.636000000 -0700
@@ -1,2 +1,4677 @@
 QA output created by 251
 Running the test: done.
+fstrim: /opt: FITRIM ioctl failed: Invalid argument
+fstrim: /opt: FITRIM ioctl failed: Invalid argument
...
+fstrim: /opt: FITRIM ioctl failed: Invalid argument

Dumping the exact fstrim command lines to seqres.full produces this at
the end:

/usr/sbin/fstrim -m 32544k -o 30247k -l 4k /opt
/usr/sbin/fstrim -m 32544k -o 30251k -l 4k /opt
...
/usr/sbin/fstrim -m 32544k -o 30255k -l 4k /opt

The count of failure messages is the same as the count as the "-l 4k"
fstrim invocations.  Since this is an 8k-block filesystem, the -l
parameter is clearly incorrect.  The test computes random -m and -l
options.

Therefore, create helper functions to guess at the minimum and maximum
length and minlen parameters that can be used with the fstrim program.
In the inner loop of the test, make sure that our choices for -m and -l
fall within those constraints.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/generic/251 |   59 ++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 51 insertions(+), 8 deletions(-)

diff --git a/tests/generic/251 b/tests/generic/251
index 3b807df5fa..b7a15f9189 100755
--- a/tests/generic/251
+++ b/tests/generic/251
@@ -53,14 +53,46 @@ _fail()
 	kill $mypid 2> /dev/null
 }
 
-_guess_max_minlen()
+# Set FSTRIM_{MIN,MAX}_MINLEN to the lower and upper bounds of the -m(inlen)
+# parameter to fstrim on the scratch filesystem.
+set_minlen_constraints()
 {
-	mmlen=100000
-	while [ $mmlen -gt 1 ]; do
+	local mmlen
+
+	for ((mmlen = 100000; mmlen > 0; mmlen /= 2)); do
 		$FSTRIM_PROG -l $(($mmlen*2))k -m ${mmlen}k $SCRATCH_MNT &> /dev/null && break
-		mmlen=$(($mmlen/2))
 	done
-	echo $mmlen
+	test $mmlen -gt 0 || \
+		_notrun "could not determine maximum FSTRIM minlen param"
+	FSTRIM_MAX_MINLEN=$mmlen
+
+	for ((mmlen = 1; mmlen < FSTRIM_MAX_MINLEN; mmlen *= 2)); do
+		$FSTRIM_PROG -l $(($mmlen*2))k -m ${mmlen}k $SCRATCH_MNT &> /dev/null && break
+	done
+	test $mmlen -le $FSTRIM_MAX_MINLEN || \
+		_notrun "could not determine minimum FSTRIM minlen param"
+	FSTRIM_MIN_MINLEN=$mmlen
+}
+
+# Set FSTRIM_{MIN,MAX}_LEN to the lower and upper bounds of the -l(ength)
+# parameter to fstrim on the scratch filesystem.
+set_length_constraints()
+{
+	local mmlen
+
+	for ((mmlen = 100000; mmlen > 0; mmlen /= 2)); do
+		$FSTRIM_PROG -l ${mmlen}k $SCRATCH_MNT &> /dev/null && break
+	done
+	test $mmlen -gt 0 || \
+		_notrun "could not determine maximum FSTRIM length param"
+	FSTRIM_MAX_LEN=$mmlen
+
+	for ((mmlen = 1; mmlen < FSTRIM_MAX_LEN; mmlen *= 2)); do
+		$FSTRIM_PROG -l ${mmlen}k $SCRATCH_MNT &> /dev/null && break
+	done
+	test $mmlen -le $FSTRIM_MAX_LEN || \
+		_notrun "could not determine minimum FSTRIM length param"
+	FSTRIM_MIN_LEN=$mmlen
 }
 
 ##
@@ -70,13 +102,24 @@ _guess_max_minlen()
 ##
 fstrim_loop()
 {
+	set_minlen_constraints
+	set_length_constraints
+	echo "MINLEN max=$FSTRIM_MAX_MINLEN min=$FSTRIM_MIN_MINLEN" >> $seqres.full
+	echo "LENGTH max=$FSTRIM_MAX_LEN min=$FSTRIM_MIN_LEN" >> $seqres.full
+
 	trap "_destroy_fstrim; exit \$status" 2 15
 	fsize=$(_discard_max_offset_kb "$SCRATCH_MNT" "$SCRATCH_DEV")
-	mmlen=$(_guess_max_minlen)
 
 	while true ; do
-		step=$((RANDOM*$RANDOM+4))
-		minlen=$(((RANDOM*($RANDOM%2+1))%$mmlen))
+		while true; do
+			step=$((RANDOM*$RANDOM+4))
+			test "$step" -ge "$FSTRIM_MIN_LEN" && break
+		done
+		while true; do
+			minlen=$(( (RANDOM * (RANDOM % 2 + 1)) % FSTRIM_MAX_MINLEN ))
+			test "$minlen" -ge "$FSTRIM_MIN_MINLEN" && break
+		done
+
 		start=$RANDOM
 		if [ $((RANDOM%10)) -gt 7 ]; then
 			$FSTRIM_PROG $SCRATCH_MNT &

Re: [PATCH 1/2] generic/251: don't snapshot $here during a test

[Zorro Lang]

On Wed, Oct 25, 2023 at 08:12:02PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Zorro complained that the next patch caused him a regression:
> 
> generic/251 249s ... [failed, exit status 1]- output mismatch (see /root/git/xfstests/results//generic/251.out.bad)
>     --- tests/generic/251.out   2022-04-29 23:07:23.263498297 +0800
>     +++ /root/git/xfstests/results//generic/251.out.bad 2023-10-22 14:17:07.248059405 +0800
>     @@ -1,2 +1,5 @@
>      QA output created by 251
>      Running the test: done.
>     +5838a5839
>     +> aa60581221897d3d7dd60458e1cca2fa  ./results/generic/251.full
>     +!!!Checksums has changed - Filesystem possibly corrupted!!!\n
>     ...
>     (Run 'diff -u /root/git/xfstests/tests/generic/251.out /root/git/xfstests/results//generic/251.out.bad'  to see the entire diff)
> Ran: generic/251
> Failures: generic/251
> Failed 1 of 1 tests
> 
> The next patch writes some debugging information into $seqres.full,
> which is a file underneat $RESULT_BASE.  If the test operator does not
> set RESULT_BASE, it will be set to a subdir of $here by default.  Since
> this test also snapshots the contents of $here before starting its loop,
> any logging to $seqres.full on such a system will cause the post-copy
> checksum to fail due to a mismatch.
> 
> Fix all this by copying $here to $SCRATCH_DEV and checksumming the copy
> before the FITRIM stress test begins to avoid problems with $seqres.full.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Thanks, it works for me.

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

>  tests/generic/251 |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/generic/251 b/tests/generic/251
> index 8ee74980cc..3b807df5fa 100755
> --- a/tests/generic/251
> +++ b/tests/generic/251
> @@ -130,7 +130,13 @@ function run_process() {
>  }
>  
>  nproc=20
> -content=$here
> +
> +# Copy $here to the scratch fs and make coipes of the replica.  The fstests
> +# output (and hence $seqres.full) could be in $here, so we need to snapshot
> +# $here before computing file checksums.
> +content=$SCRATCH_MNT/orig
> +mkdir -p $content
> +cp -axT $here/ $content/
>  
>  mkdir -p $tmp
>  
> 

Re: [PATCH 2/2] generic/251: check min and max length and minlen for FSTRIM

[Zorro Lang]

On Wed, Oct 25, 2023 at 08:21:51PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Every now and then, this test fails with the following output when
> running against my development tree when configured with an 8k fs block
> size:
> 
> --- a/tests/generic/251.out	2023-07-11 12:18:21.624971186 -0700
> +++ b/tests/generic/251.out.bad	2023-10-15 20:54:44.636000000 -0700
> @@ -1,2 +1,4677 @@
>  QA output created by 251
>  Running the test: done.
> +fstrim: /opt: FITRIM ioctl failed: Invalid argument
> +fstrim: /opt: FITRIM ioctl failed: Invalid argument
> ...
> +fstrim: /opt: FITRIM ioctl failed: Invalid argument
> 
> Dumping the exact fstrim command lines to seqres.full produces this at
> the end:
> 
> /usr/sbin/fstrim -m 32544k -o 30247k -l 4k /opt
> /usr/sbin/fstrim -m 32544k -o 30251k -l 4k /opt
> ...
> /usr/sbin/fstrim -m 32544k -o 30255k -l 4k /opt
> 
> The count of failure messages is the same as the count as the "-l 4k"
> fstrim invocations.  Since this is an 8k-block filesystem, the -l
> parameter is clearly incorrect.  The test computes random -m and -l
> options.
> 
> Therefore, create helper functions to guess at the minimum and maximum
> length and minlen parameters that can be used with the fstrim program.
> In the inner loop of the test, make sure that our choices for -m and -l
> fall within those constraints.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Good to me, although I need to add some spaces to those lines with "+", to
merge it successfully :)

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

>  tests/generic/251 |   59 ++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 51 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/generic/251 b/tests/generic/251
> index 3b807df5fa..b7a15f9189 100755
> --- a/tests/generic/251
> +++ b/tests/generic/251
> @@ -53,14 +53,46 @@ _fail()
>  	kill $mypid 2> /dev/null
>  }
>  
> -_guess_max_minlen()
> +# Set FSTRIM_{MIN,MAX}_MINLEN to the lower and upper bounds of the -m(inlen)
> +# parameter to fstrim on the scratch filesystem.
> +set_minlen_constraints()
>  {
> -	mmlen=100000
> -	while [ $mmlen -gt 1 ]; do
> +	local mmlen
> +
> +	for ((mmlen = 100000; mmlen > 0; mmlen /= 2)); do
>  		$FSTRIM_PROG -l $(($mmlen*2))k -m ${mmlen}k $SCRATCH_MNT &> /dev/null && break
> -		mmlen=$(($mmlen/2))
>  	done
> -	echo $mmlen
> +	test $mmlen -gt 0 || \
> +		_notrun "could not determine maximum FSTRIM minlen param"
> +	FSTRIM_MAX_MINLEN=$mmlen
> +
> +	for ((mmlen = 1; mmlen < FSTRIM_MAX_MINLEN; mmlen *= 2)); do
> +		$FSTRIM_PROG -l $(($mmlen*2))k -m ${mmlen}k $SCRATCH_MNT &> /dev/null && break
> +	done
> +	test $mmlen -le $FSTRIM_MAX_MINLEN || \
> +		_notrun "could not determine minimum FSTRIM minlen param"
> +	FSTRIM_MIN_MINLEN=$mmlen
> +}
> +
> +# Set FSTRIM_{MIN,MAX}_LEN to the lower and upper bounds of the -l(ength)
> +# parameter to fstrim on the scratch filesystem.
> +set_length_constraints()
> +{
> +	local mmlen
> +
> +	for ((mmlen = 100000; mmlen > 0; mmlen /= 2)); do
> +		$FSTRIM_PROG -l ${mmlen}k $SCRATCH_MNT &> /dev/null && break
> +	done
> +	test $mmlen -gt 0 || \
> +		_notrun "could not determine maximum FSTRIM length param"
> +	FSTRIM_MAX_LEN=$mmlen
> +
> +	for ((mmlen = 1; mmlen < FSTRIM_MAX_LEN; mmlen *= 2)); do
> +		$FSTRIM_PROG -l ${mmlen}k $SCRATCH_MNT &> /dev/null && break
> +	done
> +	test $mmlen -le $FSTRIM_MAX_LEN || \
> +		_notrun "could not determine minimum FSTRIM length param"
> +	FSTRIM_MIN_LEN=$mmlen
>  }
>  
>  ##
> @@ -70,13 +102,24 @@ _guess_max_minlen()
>  ##
>  fstrim_loop()
>  {
> +	set_minlen_constraints
> +	set_length_constraints
> +	echo "MINLEN max=$FSTRIM_MAX_MINLEN min=$FSTRIM_MIN_MINLEN" >> $seqres.full
> +	echo "LENGTH max=$FSTRIM_MAX_LEN min=$FSTRIM_MIN_LEN" >> $seqres.full
> +
>  	trap "_destroy_fstrim; exit \$status" 2 15
>  	fsize=$(_discard_max_offset_kb "$SCRATCH_MNT" "$SCRATCH_DEV")
> -	mmlen=$(_guess_max_minlen)
>  
>  	while true ; do
> -		step=$((RANDOM*$RANDOM+4))
> -		minlen=$(((RANDOM*($RANDOM%2+1))%$mmlen))
> +		while true; do
> +			step=$((RANDOM*$RANDOM+4))
> +			test "$step" -ge "$FSTRIM_MIN_LEN" && break
> +		done
> +		while true; do
> +			minlen=$(( (RANDOM * (RANDOM % 2 + 1)) % FSTRIM_MAX_MINLEN ))
> +			test "$minlen" -ge "$FSTRIM_MIN_MINLEN" && break
> +		done
> +
>  		start=$RANDOM
>  		if [ $((RANDOM%10)) -gt 7 ]; then
>  			$FSTRIM_PROG $SCRATCH_MNT &
>