On Wed, Feb 28, 2024 at 01:16:12PM -0800, Suraj Jitindar Singh wrote:
> Regression test for upstream commit added in v6.8-rc1:
> 7c784d624819a ext4: allow for the last group to be marked as trimmed
>
> Which fixes bug introduced by upstream commit in v6.6-rc2:
> 45e4ab320c9b5 ext4: move setting of trimmed bit into ext4_try_to_trim_range()
>
> Applicable to kernels 4.19..6.7:
> Kernel Bug Introduced Bug Fixed
> kver - commit sha kver - commit sha
> 4.19 v4.19.296 - d61445f6a5c57 v4.19.307 - 5b6a7f323b533
> 5.4 v5.4.258 - 4db34feaf2977 v5.4.269 - a7edaf40fccae
> 5.10 v5.10.198 - c502b09d9befc v5.10.210 - fa94912241835
> 5.15 v5.15.134 - a9d3bb58da959 v5.15.149 - cb904f5c71629
> 6.1 v6.1.56 - b4d5db1c77fac v6.1.76 - 852b6b2a2f7b7
> 6.6 v6.6-rc2 - 45e4ab320c9b5 v6.6.15 - da9008da96404
> 6.7 v6.6-rc2 - 45e4ab320c9b5 v6.7.3 - 73986e8d2808c
>
> Signed-off-by: Suraj Jitindar Singh <[email protected]>
> ---
> tests/ext4/060 | 62 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/ext4/060.out | 2 ++
Great, a new ext4 case :) CC ext4 list to get more review :)
> 2 files changed, 64 insertions(+)
> create mode 100755 tests/ext4/060
> create mode 100644 tests/ext4/060.out
>
> diff --git a/tests/ext4/060 b/tests/ext4/060
> new file mode 100755
> index 00000000..cc5f3819
> --- /dev/null
> +++ b/tests/ext4/060
> @@ -0,0 +1,62 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# FS QA Test No. 309
> +#
> +# Regression test for upstream commit added in v6.8-rc1:
> +# 7c784d624819a ext4: allow for the last group to be marked as trimmed
> +#
> +# Which fixes bug introduced by upstream commit in v6.6-rc2:
> +# 45e4ab320c9b5 ext4: move setting of trimmed bit into ext4_try_to_trim_range()
> +#
> +# Applicable to kernels 4.19..6.7:
> +# Kernel Bug Introduced Bug Fixed
> +# kver - commit sha kver - commit sha
> +# 4.19 v4.19.296 - d61445f6a5c57 v4.19.307 - 5b6a7f323b533
> +# 5.4 v5.4.258 - 4db34feaf2977 v5.4.269 - a7edaf40fccae
> +# 5.10 v5.10.198 - c502b09d9befc v5.10.210 - fa94912241835
> +# 5.15 v5.15.134 - a9d3bb58da959 v5.15.149 - cb904f5c71629
> +# 6.1 v6.1.56 - b4d5db1c77fac v6.1.76 - 852b6b2a2f7b7
> +# 6.6 v6.6-rc2 - 45e4ab320c9b5 v6.6.15 - da9008da96404
> +# 6.7 v6.6-rc2 - 45e4ab320c9b5 v6.7.3 - 73986e8d2808c
> +
> +. ./common/preamble
> +_begin_fstest auto
> +
> +# Override the default cleanup function.
> +_cleanup()
> +{
> + _scratch_unmount
This cleanup is useless, the SCRATCH_DEV will be unmounted and checked by default,
so you can remove this specific _cleanup if you don't need other cleanup steps.
> +}
> +
> +# Import common functions.
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs ext4
fstrim isn't an ext4 specific operation. Can this case be a generic case?
And you can have:
[[ "$FSTYP" =~ ext[0-9]+ ]] && _fixed_by_kernel_commit 7c784d624819a \
ext4: allow for the last group to be marked as trimmed
> +
> +_require_scratch
> +_require_fstrim
> +
> +# Make an ext4 fs where the last group has fewer blocks than blocks per group
> +blksz=$(_get_page_size)
> +blocks_per_group=8192
> +
> +$MKFS_EXT4_PROG -F -b $blksz -g $blocks_per_group $SCRATCH_DEV $(( blocks_per_group - 1 )) >>$seqres.full 2>&1
Can it be replaced by _scratch_mkfs_sized? e.g.
MKFS_OPTIONS="-b $blksz -g $blocks_per_group" _scratch_mkfs_sized $(( blocks_per_group - 1 )) >> $seqres.full 2>&1
If you don't mind the effection of $MKFS_OPTIONS outside, you can keep it:
MKFS_OPTIONS="-b $blksz -g $blocks_per_group $MKFS_OPTIONS" ....
And of course better to check if the mkfs fails or not:
... _scratch_mkfs_sized .... || _fail "......"
It also helps to make this case be generic.
> +_scratch_mount
> +
> +$FSTRIM_PROG -v $SCRATCH_MNT >>$seqres.full 2>&1
Due to _require_fstrim only checks [ -z "$FSTRIM_PROG" ], the fstrim might still not
be supported by the SCRATCH_DEV, so we can check at here:
$FSTRIM_PROG -v $SCRATCH_MNT >>$seqres.full 2>&1 || _notrun "FSTRIM not supported"
> +# If we have the fix commit then the above trim command should have marked the
> +# group as trimmed and subsequent trim operations shouldn't discard anything.
> +# If we don't have the fix commit then the group won't have been marked as
> +# trimmed and the below trim operation will discard more than 0.
> +bytes=$($FSTRIM_PROG -v $SCRATCH_MNT | tee -a $seqres.full | _filter_fstrim)
> +if [ $bytes -gt 0 ]; then
> + status=1
> + echo "Final group in filesystem not marked as trimmed after trimming entire fs."
> +else
> + status=0
> + echo "Final group in filesystem correctly marked as trimmed after trimming entire fs."
> +fi
How about simplify this part as:
$FSTRIM_PROG -v $SCRATCH_MNT | _filter_scratch
It will get "SCRATCH_MNT: 0 B (0 bytes) trimmed" as golden image (in .out file).
So if it's "0", then the test will fail by the golden image broken.
> +
> +exit
> diff --git a/tests/ext4/060.out b/tests/ext4/060.out
> new file mode 100644
> index 00000000..f3457134
> --- /dev/null
> +++ b/tests/ext4/060.out
> @@ -0,0 +1,2 @@
> +QA output created by 060
> +Final group in filesystem correctly marked as trimmed after trimming entire fs.
> --
> 2.34.1
>
>