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

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2026/8/3 20:05, Zorro Lang 写道:
> 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 write
>> 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 <borntraeger@linux.ibm.com>
>> Link: https://lore.kernel.org/linux-btrfs/20260721191152.101118-1-borntraeger@linux.ibm.com/
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
> 
> Hi Wenruo,
> 
> Thanks for this new test!

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

But one comment inlined below.

>> +
>> +kill "$read_pid" "$sync_pid" &> /dev/null
> 
> How about:
>    kill -TERM -"$read_pid" -"$sync_pid" &> /dev/null
> ?
> 
> As the (man 1 kill) says:
> 
>    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 signaled.
>         When an argument of the form '-n' is given, and it is meant to denote a
>         process group, either a signal must be specified first, or the argument
>         must be preceded by a '--' option, otherwise it will be taken as the
>         signal to send.
> 
> 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 
process
/home/adam/xfstests/tests/generic/801: line 76: kill: (-1272) - No such 
process

Thus it will not really kill the children processes.

Despite this change, all other comments will be addressed.

Thanks,
Qu

> 
> (same in _cleanup)
> 
>> +unset "$read_pid" "$sync_pid"
> 
> unset read_pid sync_pid
> 
>> +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
>> -- 
>> 2.51.2
>>
>>
>