Re: [PATCH] fstests: add a dio-read-into-mmap and sync race test case

=E5=9C=A8 2026/8/3 20:05, Zorro Lang =E5=86=99=E9=81=93:
> On Sat, Jul 25, 2026 at 08:37:24PM +0930, Qu Wenruo wrote:
>> There is a report that on btrfs, if the following workload are running,
>> btrfs can fail:
>>
>> - A dio read into a mmaped range
>>    Only the mmap range needs to be on btrfs.
>>    The dio read source makes no difference.
>>
>> - Sync_range on the mapped range
>>
>> The btrfs errors include:
>>
>> - Hang during data writeback
>> - Filesystem flips RO
>>
>> The mmap range is dirtied but written back by the sync_range process,
>> then dio read finished and found that the folios are no longer dirty,
>> so dio endio will mark those folios dirty again so that the fs can writ=
e
>> them back again.
>>
>> However for non-experimental btrfs with 4K block size and 4K page size,
>> there is a regression in v7.2 that such case is no longer handled
>> properly, due to the enablement of large folios and removal of cow
>> fixup.
>> And btrfs can never handle it for bs < ps from day 1.
>>
>> Add a regression test for it.
>>
>> Reported-by: Christian Borntraeger <[email protected]>
>> Link: https://lore.kernel.org/linux-btrfs/20260721191152.101118-1-bornt=
[email protected]/
>> Signed-off-by: Qu Wenruo <[email protected]>
>> ---
>=20
> Hi Wenruo,
>=20
> Thanks for this new test!

Thanks for the review. Will update the patch to address all the comments=
=20
and refresh the test number.

But one comment inlined below.

>> +
>> +kill "$read_pid" "$sync_pid" &> /dev/null
>=20
> How about:
>    kill -TERM -"$read_pid" -"$sync_pid" &> /dev/null
> ?
>=20
> As the (man 1 kill) says:
>=20
>    pid
>      Each pid can be expressed in one of the following ways:
>      ...
>      -n
>         where n is larger than 1. All processes in process group n are s=
ignaled.
>         When an argument of the form '-n' is given, and it is meant to d=
enote a
>         process group, either a signal must be specified first, or the a=
rgument
>         must be preceded by a '--' option, otherwise it will be taken as=
 the
>         signal to send.
>=20
> I didn't give it a try, but I think this might help to kill the
> dio-read-into-mmap and sync_range process too.

Unfortunately this doesn't seems to work.

If I removed the redirection, it shows the following error:

/home/adam/xfstests/tests/generic/801: line 76: kill: (-1271) - No such=20
process
/home/adam/xfstests/tests/generic/801: line 76: kill: (-1272) - No such=20
process

Thus it will not really kill the children processes.

Despite this change, all other comments will be addressed.

Thanks,
Qu

>=20
> (same in _cleanup)
>=20
>> +unset "$read_pid" "$sync_pid"
>=20
> unset read_pid sync_pid
>=20
>> +wait
>> +
>> +echo "Silence is golden"
>> +_exit 0
>> diff --git a/tests/generic/799.out b/tests/generic/799.out
>> new file mode 100644
>> index 00000000..f3fd9fa2
>> --- /dev/null
>> +++ b/tests/generic/799.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 799
>> +Silence is golden
>> --=20
>> 2.51.2
>>
>>
>=20