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

On 28/5/26 19:30, Filipe Manana wrote:
> On Thu, May 28, 2026 at 12:23 PM Qu Wenruo <[email protected]> wrote:
>>
>>
>>
>> 在 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?
> 

There will be many testcase and option combinations,
which makes this harder to scale and increases the
chances of missing a testcase + option combination
across federated teams.

I think it makes sense to maintain a standard testing config
(not sure what to call it, maybe a Test-Profile?).

V2 was sent here:

https://lore.kernel.org/fstests/cfb8c19533ac3c764edc1fe62b7fde75e76579a4.1743137470.git.anand.jain@oracle.com/

Any thoughts? I'm ok to revisit to send v3 if needed.

Thanks, Anand


> So in the past I attempted tests like that, making them btrfs specific
> and forcing a mount option.
> Some people (non-btrfs people, I don't recall exactly who, to be
> honest) disagreed with the claims that the test was actually generic,
> and that exercising the bug should be done by setting MKFS_OPTIONS in
> the command line.
> 
> I'm assuming people and our automations run tests with -O ^no-holes. I
> do it frequently in my test vms.
> 
>>
>> 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
>>