Re: [PATCH v2 1/3] f2fs/018: check data eof after partial truncation on compressed cluster

[Chao Yu <chao@kernel.org>]

On 8/15/25 04:53, Zorro Lang wrote:
> On Thu, Aug 14, 2025 at 05:07:10PM +0800, Chao Yu wrote:
>> This is a regression testcase, it is added to check below case
>> and its variants:
>> - write 16k data into compressed file (data will be compressed)
>> - truncate file to 12k (truncate partial data in compressed cluster)
>> - truncate file to 20k
>> - verify data in range of [12k, 16k] to see whether data is all zero
>> or not
>>
>> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>> v2:
>> - add missing _require_xfs_io_command "truncate"
>> - add missing _require_fio $fio_config
>> - use $XFS_IO_PROG instead of xfs_io
>> - avoid mkfs&mount .. umount for each round test
>>  tests/f2fs/018     | 77 ++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/f2fs/018.out |  2 ++
>>  2 files changed, 79 insertions(+)
>>  create mode 100755 tests/f2fs/018
>>  create mode 100644 tests/f2fs/018.out
>>
>> diff --git a/tests/f2fs/018 b/tests/f2fs/018
>> new file mode 100755
>> index 00000000..e1047f78
>> --- /dev/null
>> +++ b/tests/f2fs/018
>> @@ -0,0 +1,77 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2025 Chao Yu.  All Rights Reserved.
>> +#
>> +# FS QA Test No. f2fs/018
>> +#
>> +# This is a regression test to check whether page eof will be
>> +# zero or not after we truncate partial data in compressed
>> +# cluster.
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto quick rw compress
>> +
>> +_fixed_by_kernel_commit ba8dac350faf \
>> +	"f2fs: fix to zero post-eof page"
>> +_fixed_by_kernel_commit xxxxxxxxxxxx \
>> +	"f2fs: fix to zero data after EOF for compressed file correctly"
>> +
>> +_require_xfs_io_command "truncate"
>> +_require_scratch
>> +
>> +testfile=$SCRATCH_MNT/testfile
>> +fio_config=$tmp.fio
>> +
>> +cat >$fio_config <<EOF
>> +[global]

I updated task name from global to verify-data to avoid
"No job(s) defined" error from fio.

>> +filename=$testfile
>> +rw=read
>> +verify=pattern
>> +verify_pattern=0x00
>> +do_verify=1
>> +verify_only
>> +offset=0
>> +size=4k
>> +numjobs=1
>> +EOF
>> +_require_fio $fio_config
> 
> Hmm... that looks strange if you create fio config file but don't use it later :-D
> 
> How about:
> 
> build_fio_config()
> {
> 	local offset=$1
> 	local size=$2
> 
> 	cat >$fio_config <<EOF
> [global]
> filename=$testfile
> rw=read
> verify=pattern
> verify_pattern=0x00
> do_verify=1
> verify_only
> offset=$offset
> size=$size
> numjobs=1
> EOF

cat $fio_config >> $seqres.full

To record fio config in seqres.full

> }
> 
> build_fio_config 0 4k
> _require_fio $fio_config

Add _cleanup() to remove $testfile created in last testcase.

> 
>> +
>> +_scratch_mkfs "-O extra_attr,compression" >> $seqres.full || _fail "mkfs failed"
>> +_scratch_mount "-o compress_extension=*" >> $seqres.full
>> +
>> +check_data_eof()
>> +{
>> +	local eof_start=$1
>> +	local eof_size=$2
>> +	local filesize=$3
>> +	local offset1=$4
>> +	local offset2=$5
>> +	local offset3=$6
>> +
>> +
> 
> rm -f $testfile
> 
>> +	$XFS_IO_PROG -f -c "pwrite 0 $filesize" -c "fsync" $testfile >> $seqres.full
>> +	$XFS_IO_PROG -c "truncate $offset1" $testfile
>> +	$XFS_IO_PROG -c "truncate $offset2" $testfile
>> +
>> +	if [ "$offset3" ]; then
>> +		xfs_io -c "truncate $offset3" $testfile
>                 ^^^^^^
>        $XFS_IO_PROG
> 
>> +	fi
>> +
>> +	$FIO_PROG --name=verify-data --filename=$testfile --rw=read --verify=pattern \
>> +		--verify_pattern=0x00 --do_verify=1 --verify_only --offset=$eof_start \
>> +		--size=$eof_size >> $seqres.full 2>&1 || _fail "eof data is not zero"
> 
> build_fio_config $eof_start $eof_size
> $FIO_PROG $fio_config >> $seqres.full 2>&1
> 
>> +
>> +	rm -f $testfile
> 
> Move this line to above, then
> 
> return $?
> 
>> +}
>> +
>> +check_data_eof 12k 4k 16k 12k 20k
>> +check_data_eof 10k 6k 16k 10k 20k
>> +check_data_eof 12k 4k 16k 8k 12k 20k
>> +check_data_eof 10k 6k 16k 8k 10k 20k
> 
> run_check check_data_eof 12k 4k 16k 12k 20k
> run_check check_data_eof 10k 6k 16k 10k 20k
> run_check check_data_eof 12k 4k 16k 8k 12k 20k
> run_check check_data_eof 10k 6k 16k 8k 10k 20k
> 
> This can help you to know which line fails if one of them is failed. Or you
> always get "eof data is not zero", no matter which line fails :)

Looks nice!

> 
>> +
>> +_scratch_unmount
> 
> This unmount looks useless, due to SCRATCH_DEV is always unmounted after a test done.

Thanks for all your suggestion, will update v3.

Thanks,

> 
> Thanks,
> Zorro
> 
>> +
>> +echo "Silence is golden"
>> +
>> +status=0
>> +exit
>> diff --git a/tests/f2fs/018.out b/tests/f2fs/018.out
>> new file mode 100644
>> index 00000000..8849e303
>> --- /dev/null
>> +++ b/tests/f2fs/018.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 018
>> +Silence is golden
>> -- 
>> 2.49.0
>>
>