Re: [PATCH] generic: add a test case for writes with prealloc extents beyond i_size

在 2026/5/28 19:57, [email protected] 写道:
> From: Filipe Manana <[email protected]>
> 
> Test writing into a file range containing prealloc extents beyond the current
> i_size, with an unmount and mount after fallocate and the write, to verify
> that the file data, size and extent layout were not lost.
> 
> This used to fail on btrfs when not using the no-holes feature (which is
> a default since btrfs-progs 5.15) before this recent kernel fix:
> 
>     080ecbd05432 ("btrfs: mark file extent range dirty after converting prealloc extents")
> 
> So in order to reproduce the failure when using an unpatched kernel and
> a btrfs-progs >= 5.15, one must run the test with:
> 
>     MKFS_OPTIONS="-O ^no-holes"
> 
> Signed-off-by: Filipe Manana <[email protected]>

Reviewed-by: Qu Wenruo <[email protected]>


Just one question related to the specified mkfs option.

As you mentioned, this bug is only affecting ^no-holes, thus default 
mount option runs won't trigger it.

This makes me wondering, should we have a dedicated btrfs test case just 
exercising the ^no-holes path.

I know this will cause duplication, thus will not be a good idea to 
maintain, but we also have dedicated test cases utilizing specified 
mount option/mkfs options already.

So what is the prefer method here?

Thanks,
Qu
> ---
>   tests/generic/796     | 54 +++++++++++++++++++++++++++++++++++++++++++
>   tests/generic/796.out | 10 ++++++++
>   2 files changed, 64 insertions(+)
>   create mode 100755 tests/generic/796
>   create mode 100644 tests/generic/796.out
> 
> diff --git a/tests/generic/796 b/tests/generic/796
> new file mode 100755
> index 00000000..c42a4722
> --- /dev/null
> +++ b/tests/generic/796
> @@ -0,0 +1,54 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2026 SUSE S.A.  All Rights Reserved.
> +#
> +# FS QA Test 796
> +#
> +# Test writing into a file range containing prealloc extents beyond the current
> +# i_size, with an unmount and mount after fallocate and the write, to verify
> +# that the file data, size and extent layout were not lost.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick prealloc preallocrw fiemap
> +
> +. ./common/filter
> +. ./common/punch # for _filter_fiemap
> +
> +_require_scratch
> +_require_xfs_io_command "falloc" "-k"
> +_require_xfs_io_command "fiemap"
> +
> +_fixed_by_fs_commit btrfs 080ecbd05432 \
> +	"btrfs: mark file extent range dirty after converting prealloc extents"
> +
> +_scratch_mkfs >>$seqres.full 2>&1
> +_scratch_mount
> +
> +# The fiemap results in the golden output requires file allocations to align to
> +# 1M boundaries.
> +_require_congruent_file_oplen $SCRATCH_MNT 1048576
> +
> +# Create our file with a size of 0 and a prealloc extent in the range [0, 2M].
> +$XFS_IO_PROG -f -c "falloc -k 0 2M" $SCRATCH_MNT/foo
> +
> +# Unmount and mount again to remove any in memory state of the inode. We will
> +# verify later that neither metadata nor extents were lost during unmount.
> +_scratch_cycle_mount
> +
> +# Write into the [0, 1M] range, which increases the inode's i_size.
> +$XFS_IO_PROG -c "pwrite -S 0xab -b 1M 0 1M" $SCRATCH_MNT/foo | _filter_xfs_io
> +
> +# Unmount and mount again to remove any in memory state of the inode. We will
> +# verify later that neither metadata nor extents were lost during unmount.
> +_scratch_cycle_mount
> +
> +# Check file data (and size).
> +echo "File data:"
> +_hexdump $SCRATCH_MNT/foo
> +
> +# Check we have unwritten extents in range [1M, 2M].
> +echo "Fiemap output:"
> +$XFS_IO_PROG -c "fiemap -v" $SCRATCH_MNT/foo | _filter_fiemap
> +
> +# Success, all done.
> +_exit 0
> diff --git a/tests/generic/796.out b/tests/generic/796.out
> new file mode 100644
> index 00000000..c6c6e6a8
> --- /dev/null
> +++ b/tests/generic/796.out
> @@ -0,0 +1,10 @@
> +QA output created by 796
> +wrote 1048576/1048576 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +File data:
> +000000 ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab  >................<
> +*
> +100000
> +Fiemap output:
> +0: [0..2047]: data
> +1: [2048..4095]: unwritten