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

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

[Qu Wenruo]

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>
---
 .gitignore               |  1 +
 src/Makefile             |  2 +-
 src/dio-read-into-mmap.c | 81 ++++++++++++++++++++++++++++++++++++++++
 tests/generic/799        | 81 ++++++++++++++++++++++++++++++++++++++++
 tests/generic/799.out    |  2 +
 5 files changed, 166 insertions(+), 1 deletion(-)
 create mode 100644 src/dio-read-into-mmap.c
 create mode 100755 tests/generic/799
 create mode 100644 tests/generic/799.out

diff --git a/.gitignore b/.gitignore
index 0b6b9452..d52ba4ad 100644
--- a/.gitignore
+++ b/.gitignore
@@ -216,6 +216,7 @@ tags
 /src/truncate
 /src/t_btrfs_received_uuid_ioctl
 /src/btrfs_ioctl
+/src/dio-read-into-mmap
 
 # Symlinked files
 /tests/generic/035.out
diff --git a/src/Makefile b/src/Makefile
index 76cf50c3..f64e153a 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -21,7 +21,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
 	t_mmap_writev_overlap checkpoint_journal mmap-rw-fault allocstale \
 	t_mmap_cow_memory_failure fake-dump-rootino dio-buf-fault rewinddir-test \
 	readdir-while-renames dio-append-buf-fault dio-write-fsync-same-fd \
-	dio-writeback-race unlink-fsync truncate
+	dio-writeback-race unlink-fsync truncate dio-read-into-mmap
 
 LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
diff --git a/src/dio-read-into-mmap.c b/src/dio-read-into-mmap.c
new file mode 100644
index 00000000..07842c48
--- /dev/null
+++ b/src/dio-read-into-mmap.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) SUSE S.A.
+
+#include <fcntl.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <errno.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <err.h>
+
+static int read_source_fd = -1;
+static int mmap_dest_fd = -1;
+static void *buf = NULL;
+static int iosize = 4 * 1024 * 1024;
+
+static void usage()
+{
+	fprintf(stderr,
+	"Usage: dio-read-into-mmap <read_source> <mmap_dest>\n");
+}
+
+int main(int argc, char **argv)
+{
+	int ret = -EINVAL;
+	const int pagesize = sysconf(_SC_PAGESIZE);
+	unsigned int cur = 0;
+
+	if (argc != 3) {
+		usage();
+		goto error;
+	}
+	if (iosize < pagesize) {
+		ret = -EINVAL;
+		fprintf(stderr, "blocksize smaller than pagesize\n");
+		goto error;
+	}
+
+	read_source_fd = open(argv[1], O_RDONLY | O_DIRECT, 0600);
+	if (read_source_fd < 0) {
+		ret = -errno;
+		fprintf(stderr, "failed to open '%s': %m", argv[1]);
+		goto error;
+	}
+	mmap_dest_fd = open(argv[2], O_RDWR, 0600);
+	if (mmap_dest_fd < 0) {
+		ret = -errno;
+		fprintf(stderr, "failed to open '%s': %m", argv[2]);
+		goto error;
+	}
+	buf = mmap(NULL, iosize, PROT_WRITE, MAP_SHARED, mmap_dest_fd, 0);
+	if (buf == MAP_FAILED) {
+		buf = NULL;
+		fprintf(stderr, "failed to mmap: %m");
+		return -errno;
+	}
+	while (cur < iosize) {
+		ret = pread(read_source_fd, buf, iosize - cur, cur);
+		if (ret == 0) {
+			ret = -EINVAL;
+			fprintf(stderr, "reached EOF");
+			goto error;
+		}
+		if (ret < 0) {
+			ret = -errno;
+			fprintf(stderr, "failed to read: %m");
+			goto error;
+		}
+		cur += ret;
+	}
+error:
+	close(read_source_fd);
+	close(mmap_dest_fd);
+	if (buf)
+		munmap(buf, iosize);
+	if (ret < 0)
+		return EXIT_FAILURE;
+	return EXIT_SUCCESS;
+}
diff --git a/tests/generic/799 b/tests/generic/799
new file mode 100755
index 00000000..b6851f69
--- /dev/null
+++ b/tests/generic/799
@@ -0,0 +1,81 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2026 SUSE S.A.  All Rights Reserved.
+#
+# FS QA Test 799
+#
+# Test direct read into a mmaped range meanwhile doing data sync for the mmaped
+# range.
+# Such racy workload should cause direct endio function to mark the folio
+# dirty without going through buffered write nor page_mkwrite().
+#
+# Make sure the fs can handle such ->dirty_folio() callback correctly.
+#
+. ./common/preamble
+. ./common/filter
+_begin_fstest auto rw mmap
+
+_require_test_program dio-read-into-mmap
+_require_scratch
+_require_test
+
+_fixed_by_fs_commit btrfs xxxxxxxxxxxxxx \
+	"btrfs: trigger cow fixup via dirty_folio()"
+
+_cleanup()
+{
+	cd /
+	[ -n "$mount_pid" ] && kill $mount_pid &> /dev/null
+	[ -n "$remount_pid" ] && kill $remount_pid &> /dev/null
+	wait
+}
+
+trap "_cleanup; exit \$status" SIGINT SIGTERM
+_scratch_mkfs >> $seqres.full
+_scratch_mount
+
+# Create the 4MiB target file on TEST_MNT as the read source.
+$XFS_IO_PROG -f -c "pwrite -i /dev/urandom 0 4M" "$TEST_MNT/dio-read-source" >> $seqres.full
+
+# Another 4MiB target file on SCRATCH_MNT as the mmap dest
+$XFS_IO_PROG -f -c "pwrite 0 4M" "$SCRATCH_MNT/mmap-dest" >> $seqres.full
+
+read_workload()
+{
+	trap "wait; exit" SIGTERM
+	while true; do
+		$here/src/dio-read-into-mmap "$TEST_MNT/dio-read-source" \
+			"$SCRATCH_MNT/mmap-dest" &> /dev/null
+		if [ $? -ne 0 ]; then
+			echo "dio read failed"
+			break;
+		fi
+	done
+}
+
+sync_workload()
+{
+	trap "wait; exit" SIGTERM
+	while true; do
+		$XFS_IO_PROG -c "sync_range 0 4m" "$SCRATCH_MNT/mmap-dest"
+		if [ $? -ne 0 ]; then
+			echo "sync_range failed"
+			break;
+		fi
+	done
+}
+
+read_workload &
+read_pid=$!
+
+sync_workload &
+sync_pid=$!
+
+sleep $((15 * $TIME_FACTOR))
+
+kill "$read_pid" "$sync_pid" &> /dev/null
+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

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

[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!

>  .gitignore               |  1 +
>  src/Makefile             |  2 +-
>  src/dio-read-into-mmap.c | 81 ++++++++++++++++++++++++++++++++++++++++
>  tests/generic/799        | 81 ++++++++++++++++++++++++++++++++++++++++
>  tests/generic/799.out    |  2 +
>  5 files changed, 166 insertions(+), 1 deletion(-)
>  create mode 100644 src/dio-read-into-mmap.c
>  create mode 100755 tests/generic/799
>  create mode 100644 tests/generic/799.out
> 
> diff --git a/.gitignore b/.gitignore
> index 0b6b9452..d52ba4ad 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -216,6 +216,7 @@ tags
>  /src/truncate
>  /src/t_btrfs_received_uuid_ioctl
>  /src/btrfs_ioctl
> +/src/dio-read-into-mmap
>  
>  # Symlinked files
>  /tests/generic/035.out
> diff --git a/src/Makefile b/src/Makefile
> index 76cf50c3..f64e153a 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -21,7 +21,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
>  	t_mmap_writev_overlap checkpoint_journal mmap-rw-fault allocstale \
>  	t_mmap_cow_memory_failure fake-dump-rootino dio-buf-fault rewinddir-test \
>  	readdir-while-renames dio-append-buf-fault dio-write-fsync-same-fd \
> -	dio-writeback-race unlink-fsync truncate
> +	dio-writeback-race unlink-fsync truncate dio-read-into-mmap
>  
>  LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
>  	preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
> diff --git a/src/dio-read-into-mmap.c b/src/dio-read-into-mmap.c
> new file mode 100644
> index 00000000..07842c48
> --- /dev/null
> +++ b/src/dio-read-into-mmap.c
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) SUSE S.A.
> +
> +#include <fcntl.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <sys/mman.h>
> +#include <sys/stat.h>
> +#include <err.h>
> +
> +static int read_source_fd = -1;
> +static int mmap_dest_fd = -1;
> +static void *buf = NULL;
> +static int iosize = 4 * 1024 * 1024;
> +
> +static void usage()
> +{
> +	fprintf(stderr,
> +	"Usage: dio-read-into-mmap <read_source> <mmap_dest>\n");
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	int ret = -EINVAL;
> +	const int pagesize = sysconf(_SC_PAGESIZE);
> +	unsigned int cur = 0;
> +
> +	if (argc != 3) {
> +		usage();
> +		goto error;
> +	}
> +	if (iosize < pagesize) {
> +		ret = -EINVAL;
> +		fprintf(stderr, "blocksize smaller than pagesize\n");
> +		goto error;
> +	}
> +
> +	read_source_fd = open(argv[1], O_RDONLY | O_DIRECT, 0600);
> +	if (read_source_fd < 0) {
> +		ret = -errno;
> +		fprintf(stderr, "failed to open '%s': %m", argv[1]);
> +		goto error;
> +	}
> +	mmap_dest_fd = open(argv[2], O_RDWR, 0600);
> +	if (mmap_dest_fd < 0) {
> +		ret = -errno;
> +		fprintf(stderr, "failed to open '%s': %m", argv[2]);
> +		goto error;
> +	}
> +	buf = mmap(NULL, iosize, PROT_WRITE, MAP_SHARED, mmap_dest_fd, 0);
> +	if (buf == MAP_FAILED) {
> +		buf = NULL;
> +		fprintf(stderr, "failed to mmap: %m");
> +		return -errno;
> +	}
> +	while (cur < iosize) {
> +		ret = pread(read_source_fd, buf, iosize - cur, cur);

Don't we need "(char *)buf + cur" at here?

> +		if (ret == 0) {
> +			ret = -EINVAL;
> +			fprintf(stderr, "reached EOF");
> +			goto error;
> +		}
> +		if (ret < 0) {
> +			ret = -errno;
> +			fprintf(stderr, "failed to read: %m");
> +			goto error;
> +		}
> +		cur += ret;
> +	}
> +error:
> +	close(read_source_fd);
> +	close(mmap_dest_fd);
> +	if (buf)
> +		munmap(buf, iosize);
> +	if (ret < 0)
> +		return EXIT_FAILURE;
> +	return EXIT_SUCCESS;
> +}
> diff --git a/tests/generic/799 b/tests/generic/799
> new file mode 100755
> index 00000000..b6851f69
> --- /dev/null
> +++ b/tests/generic/799
> @@ -0,0 +1,81 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2026 SUSE S.A.  All Rights Reserved.
> +#
> +# FS QA Test 799
> +#
> +# Test direct read into a mmaped range meanwhile doing data sync for the mmaped
> +# range.
> +# Such racy workload should cause direct endio function to mark the folio
> +# dirty without going through buffered write nor page_mkwrite().
> +#
> +# Make sure the fs can handle such ->dirty_folio() callback correctly.
> +#
> +. ./common/preamble
> +. ./common/filter
> +_begin_fstest auto rw mmap
> +
> +_require_test_program dio-read-into-mmap
> +_require_scratch
> +_require_test

_require_odirect ?

> +
> +_fixed_by_fs_commit btrfs xxxxxxxxxxxxxx \
> +	"btrfs: trigger cow fixup via dirty_folio()"
> +
> +_cleanup()
> +{
> +	cd /
> +	[ -n "$mount_pid" ] && kill $mount_pid &> /dev/null
> +	[ -n "$remount_pid" ] && kill $remount_pid &> /dev/null
> +	wait

rm -r -f $tmp.*

> +}
> +
> +trap "_cleanup; exit \$status" SIGINT SIGTERM

If you need extra signal, you can run _register_cleanup, but as you
just need INT and TERM, so this line can be removed.

> +_scratch_mkfs >> $seqres.full
> +_scratch_mount
> +
> +# Create the 4MiB target file on TEST_MNT as the read source.
> +$XFS_IO_PROG -f -c "pwrite -i /dev/urandom 0 4M" "$TEST_MNT/dio-read-source" >> $seqres.full
> +
> +# Another 4MiB target file on SCRATCH_MNT as the mmap dest
> +$XFS_IO_PROG -f -c "pwrite 0 4M" "$SCRATCH_MNT/mmap-dest" >> $seqres.full
> +
> +read_workload()
> +{
> +	trap "wait; exit" SIGTERM
> +	while true; do
> +		$here/src/dio-read-into-mmap "$TEST_MNT/dio-read-source" \
> +			"$SCRATCH_MNT/mmap-dest" &> /dev/null
> +		if [ $? -ne 0 ]; then
> +			echo "dio read failed"
> +			break;
> +		fi
> +	done
> +}
> +
> +sync_workload()
> +{
> +	trap "wait; exit" SIGTERM
> +	while true; do
> +		$XFS_IO_PROG -c "sync_range 0 4m" "$SCRATCH_MNT/mmap-dest"

_require_xfs_io_command sync_range

> +		if [ $? -ne 0 ]; then
> +			echo "sync_range failed"
> +			break;
> +		fi
> +	done
> +}
> +
> +read_workload &
> +read_pid=$!
> +
> +sync_workload &
> +sync_pid=$!
> +
> +sleep $((15 * $TIME_FACTOR))
> +
> +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.

(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
> 
> 

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

[Qu Wenruo]

在 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
>>
>>
> 

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

[Zorro Lang]

On Tue, Aug 04, 2026 at 09:11:24AM +0930, Qu Wenruo wrote:
> 
> 
> 在 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.

Hi Wenruo,

Sorry for the confusion! I assumed that approach would work, but from the
error message, it seems running "read_workload &" doesn't create a proper
Process Group (PGID).

How about this approach instead?

1) kill -TERM "$read_pid" 2>/dev/null

Based on the trap in read_workload(), it should enter "wait" upon receiving
SIGTERM, preventing any new iterations of the loop. Then we can run:

2) pkill -P "$read_pid" 2>/dev/null

to clean up any remaining child processes, followed by a wait for the main
process to exit.

3) wait "$read_pid"
   unset read_pid

(We can wrap this logic in a function and invoke it inside _cleanup)

I hope this can prevent the script from waiting long time, and no orphaned
processes are left behind. Feel free to tell me if anyone has better idea :)

Thanks,
Zorro

> 
> 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
> > > 
> > > 
> > 
> 
> 

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

[Qu Wenruo]

在 2026/8/6 00:25, Zorro Lang 写道:
> On Tue, Aug 04, 2026 at 09:11:24AM +0930, Qu Wenruo wrote:
>>
>>
>> 在 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.
> 
> Hi Wenruo,
> 
> Sorry for the confusion! I assumed that approach would work, but from the
> error message, it seems running "read_workload &" doesn't create a proper
> Process Group (PGID).
> 
> How about this approach instead?
> 
> 1) kill -TERM "$read_pid" 2>/dev/null
> 
> Based on the trap in read_workload(), it should enter "wait" upon receiving
> SIGTERM, preventing any new iterations of the loop. Then we can run:
> 
> 2) pkill -P "$read_pid" 2>/dev/null
> 
> to clean up any remaining child processes, followed by a wait for the main
> process to exit.

Not sure if pkill is really needed, as the last wait will handle it well.


So in short, the existing

kill "$pid"
wait "$pid"
unset pid

is good enough, that's what I did in the v2.

Although the child processes still need to trap the corresponding 
signals, which is also done in v2.

Finally I have already tested the signal interruption for v2 several 
times, it all properly catch the signal and terminate the children 
processes and the main script without any delay.

So I think the one used in v2 is working properly.

Thanks,
Qu
> 
> 3) wait "$read_pid"
>     unset read_pid
> 
> (We can wrap this logic in a function and invoke it inside _cleanup)
> 
> I hope this can prevent the script from waiting long time, and no orphaned
> processes are left behind. Feel free to tell me if anyone has better idea :)
> 
> Thanks,
> Zorro
> 
>>
>> 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
>>>>
>>>>
>>>
>>
>>
>