diff mbox series

[v3,15/17] generic: add race test between reflink and mmap read

Message ID 20211214081914.2478122-16-ruansy.fnst@fujitsu.com (mailing list archive)
State New, archived
Headers show
Series generic: add some mmap CoW tests | expand

Commit Message

Shiyang Ruan Dec. 14, 2021, 8:19 a.m. UTC 
Test for races or FS corruption between reflink and mmap reading the
target file. (MMAP version of generic/164,165)

Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com>
---
 common/reflink        | 11 +++++++
 tests/generic/913     | 72 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/913.out |  5 +++
 3 files changed, 88 insertions(+)
 create mode 100755 tests/generic/913
 create mode 100644 tests/generic/913.out

Comments

Darrick J. Wong Jan. 11, 2022, 6:55 p.m. UTC | #1 
On Tue, Dec 14, 2021 at 04:19:12PM +0800, Shiyang Ruan wrote:
> Test for races or FS corruption between reflink and mmap reading the
> target file. (MMAP version of generic/164,165)

Hi, now that this test has been running for a couple of weeks, I have
observed periodic false positives from this test:

QA output created by 670
Format and mount
Initialize files
Reflink and mmap reread the files!
00001000:  61 61 61 61 61 61 61 61 62 62 62 62 62 62 62 62 aaaabbbb
Finished reflinking

I suspect that if the _mread_range of file3 races with the page cache
invalidation that FICLONERANGE performs, it is possible that the mread
dump will contain a mix of 0x61 and 0x62.  Looking at mread_f, it looks
like it does a byte-at-a-time copy of the mmap...

	if (rflag) {
		for (tmp = length - 1, c = 0; tmp >= 0; tmp--, c = 1) {
			*bp = *(((char *)mapping->addr) + dumpoffset + tmp);
			cnt++;

...which is a sufficient window for the page cache mapping to get
invalidated such that the mread will block on the page fault until the
reflink operation finishes.

I think the solution here is to adjust the egrep regexp above to find
any line that does /not/ contain a's or b's, since (in principle) the
reflink could run fast enough that every byte read hits a pgae fault.
What do you think?

--D

> Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com>
> ---
>  common/reflink        | 11 +++++++
>  tests/generic/913     | 72 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/913.out |  5 +++
>  3 files changed, 88 insertions(+)
>  create mode 100755 tests/generic/913
>  create mode 100644 tests/generic/913.out
> 
> diff --git a/common/reflink b/common/reflink
> index 68dbdedd..455260c6 100644
> --- a/common/reflink
> +++ b/common/reflink
> @@ -186,6 +186,17 @@ _read_range() {
>  	$XFS_IO_PROG $xfs_io_args -f -c "pread -q -v $offset $len" "$file" | cut -d ' ' -f '3-18'
>  }
>  
> +# Prints a range of a file as a hex dump
> +_mread_range() {
> +	file="$1"
> +	offset="$2"
> +	len="$3"
> +	xfs_io_args="$4"
> +
> +	$XFS_IO_PROG $xfs_io_args -f -c "mmap -rw 0 $((offset + len))" \
> +		-c "mread -v $offset $len" "$file" | cut -d ' ' -f '3-18'
> +}
> +
>  # Compare ranges of two files
>  _compare_range() {
>  	file1="$1"
> diff --git a/tests/generic/913 b/tests/generic/913
> new file mode 100755
> index 00000000..f709c36c
> --- /dev/null
> +++ b/tests/generic/913
> @@ -0,0 +1,72 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# FS QA Test No. 913
> +#
> +# Test for races or FS corruption between reflink and mmap reading the
> +# target file. (MMAP version of generic/164,165)
> +#
> +. ./common/preamble
> +_begin_fstest auto clone
> +
> +_register_cleanup "_cleanup" BUS
> +
> +# Import common functions.
> +. ./common/filter
> +. ./common/reflink
> +
> +# real QA test starts here
> +_require_scratch_reflink
> +_require_cp_reflink
> +
> +echo "Format and mount"
> +_scratch_mkfs > $seqres.full 2>&1
> +_scratch_mount >> $seqres.full 2>&1
> +
> +testdir=$SCRATCH_MNT/test-$seq
> +finished_file=/tmp/finished
> +rm -rf $finished_file
> +mkdir $testdir
> +
> +loops=512
> +nr_loops=$((loops - 1))
> +blksz=65536
> +
> +echo "Initialize files"
> +echo >> $seqres.full
> +_pwrite_byte 0x61 0 $((loops * blksz)) $testdir/file1 >> $seqres.full
> +_pwrite_byte 0x62 0 $((loops * blksz)) $testdir/file2 >> $seqres.full
> +_cp_reflink $testdir/file1 $testdir/file3
> +_scratch_cycle_mount
> +
> +fbytes() {
> +	egrep -v '(61 61 61 61 61 61 61 61 61 61 61 61 61 61 61 61|62 62 62 62 62 62 62 62 62 62 62 62 62 62 62 62)'
> +}
> +
> +reader() {
> +	while [ ! -e $finished_file ]; do
> +		_mread_range $testdir/file3 0 $((loops * blksz)) | fbytes
> +	done
> +}
> +
> +echo "Reflink and mmap reread the files!"
> +reader &
> +for i in `seq 1 2`; do
> +	seq $nr_loops -1 0 | while read i; do
> +		_reflink_range  $testdir/file1 $((i * blksz)) \
> +				$testdir/file3 $((i * blksz)) $blksz >> $seqres.full
> +		[ $? -ne 0 ] && break
> +	done
> +	seq $nr_loops -1 0 | while read i; do
> +		_reflink_range  $testdir/file2 $((i * blksz)) \
> +				$testdir/file3 $((i * blksz)) $blksz >> $seqres.full
> +		[ $? -ne 0 ] && break
> +	done
> +done
> +echo "Finished reflinking"
> +touch $finished_file
> +wait
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/913.out b/tests/generic/913.out
> new file mode 100644
> index 00000000..a34df6ce
> --- /dev/null
> +++ b/tests/generic/913.out
> @@ -0,0 +1,5 @@
> +QA output created by 913
> +Format and mount
> +Initialize files
> +Reflink and mmap reread the files!
> +Finished reflinking
> -- 
> 2.34.1
> 
> 
>
Shiyang Ruan Jan. 12, 2022, 2:38 a.m. UTC | #2 
在 2022/1/12 2:55, Darrick J. Wong 写道:
> On Tue, Dec 14, 2021 at 04:19:12PM +0800, Shiyang Ruan wrote:
>> Test for races or FS corruption between reflink and mmap reading the
>> target file. (MMAP version of generic/164,165)
> 
> Hi, now that this test has been running for a couple of weeks, I have
> observed periodic false positives from this test:
> 
> QA output created by 670
> Format and mount
> Initialize files
> Reflink and mmap reread the files!
> 00001000:  61 61 61 61 61 61 61 61 62 62 62 62 62 62 62 62 aaaabbbb
> Finished reflinking
> 
> I suspect that if the _mread_range of file3 races with the page cache
> invalidation that FICLONERANGE performs, it is possible that the mread
> dump will contain a mix of 0x61 and 0x62.  Looking at mread_f, it looks
> like it does a byte-at-a-time copy of the mmap...
> 
> 	if (rflag) {
> 		for (tmp = length - 1, c = 0; tmp >= 0; tmp--, c = 1) {
> 			*bp = *(((char *)mapping->addr) + dumpoffset + tmp);
> 			cnt++;
> 
> ...which is a sufficient window for the page cache mapping to get
> invalidated such that the mread will block on the page fault until the
> reflink operation finishes.
> 
> I think the solution here is to adjust the egrep regexp above to find
> any line that does /not/ contain a's or b's, since (in principle) the
> reflink could run fast enough that every byte read hits a pgae fault.
> What do you think?

Reasonable.  I couldn't reproduce it in my test environment, so I didn't 
take that situation into consideration.

I'll change the regexp as below:
fbytes() {
-	egrep -v '(61 61 61 61 61 61 61 61 61 61 61 61 61 61 61 61|62 62 62 62 
62 62 62 62 62 62 62 62 62 62 62 62)'
+	egrep -v '((61|62) ){15}(61|62)'
}


--
Thanks,
Ruan.

> 
> --D
> 
>> Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com>
>> ---
>>   common/reflink        | 11 +++++++
>>   tests/generic/913     | 72 +++++++++++++++++++++++++++++++++++++++++++
>>   tests/generic/913.out |  5 +++
>>   3 files changed, 88 insertions(+)
>>   create mode 100755 tests/generic/913
>>   create mode 100644 tests/generic/913.out
>>
>> diff --git a/common/reflink b/common/reflink
>> index 68dbdedd..455260c6 100644
>> --- a/common/reflink
>> +++ b/common/reflink
>> @@ -186,6 +186,17 @@ _read_range() {
>>   	$XFS_IO_PROG $xfs_io_args -f -c "pread -q -v $offset $len" "$file" | cut -d ' ' -f '3-18'
>>   }
>>   
>> +# Prints a range of a file as a hex dump
>> +_mread_range() {
>> +	file="$1"
>> +	offset="$2"
>> +	len="$3"
>> +	xfs_io_args="$4"
>> +
>> +	$XFS_IO_PROG $xfs_io_args -f -c "mmap -rw 0 $((offset + len))" \
>> +		-c "mread -v $offset $len" "$file" | cut -d ' ' -f '3-18'
>> +}
>> +
>>   # Compare ranges of two files
>>   _compare_range() {
>>   	file1="$1"
>> diff --git a/tests/generic/913 b/tests/generic/913
>> new file mode 100755
>> index 00000000..f709c36c
>> --- /dev/null
>> +++ b/tests/generic/913
>> @@ -0,0 +1,72 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +#
>> +# FS QA Test No. 913
>> +#
>> +# Test for races or FS corruption between reflink and mmap reading the
>> +# target file. (MMAP version of generic/164,165)
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto clone
>> +
>> +_register_cleanup "_cleanup" BUS
>> +
>> +# Import common functions.
>> +. ./common/filter
>> +. ./common/reflink
>> +
>> +# real QA test starts here
>> +_require_scratch_reflink
>> +_require_cp_reflink
>> +
>> +echo "Format and mount"
>> +_scratch_mkfs > $seqres.full 2>&1
>> +_scratch_mount >> $seqres.full 2>&1
>> +
>> +testdir=$SCRATCH_MNT/test-$seq
>> +finished_file=/tmp/finished
>> +rm -rf $finished_file
>> +mkdir $testdir
>> +
>> +loops=512
>> +nr_loops=$((loops - 1))
>> +blksz=65536
>> +
>> +echo "Initialize files"
>> +echo >> $seqres.full
>> +_pwrite_byte 0x61 0 $((loops * blksz)) $testdir/file1 >> $seqres.full
>> +_pwrite_byte 0x62 0 $((loops * blksz)) $testdir/file2 >> $seqres.full
>> +_cp_reflink $testdir/file1 $testdir/file3
>> +_scratch_cycle_mount
>> +
>> +fbytes() {
>> +	egrep -v '(61 61 61 61 61 61 61 61 61 61 61 61 61 61 61 61|62 62 62 62 62 62 62 62 62 62 62 62 62 62 62 62)'
>> +}
>> +
>> +reader() {
>> +	while [ ! -e $finished_file ]; do
>> +		_mread_range $testdir/file3 0 $((loops * blksz)) | fbytes
>> +	done
>> +}
>> +
>> +echo "Reflink and mmap reread the files!"
>> +reader &
>> +for i in `seq 1 2`; do
>> +	seq $nr_loops -1 0 | while read i; do
>> +		_reflink_range  $testdir/file1 $((i * blksz)) \
>> +				$testdir/file3 $((i * blksz)) $blksz >> $seqres.full
>> +		[ $? -ne 0 ] && break
>> +	done
>> +	seq $nr_loops -1 0 | while read i; do
>> +		_reflink_range  $testdir/file2 $((i * blksz)) \
>> +				$testdir/file3 $((i * blksz)) $blksz >> $seqres.full
>> +		[ $? -ne 0 ] && break
>> +	done
>> +done
>> +echo "Finished reflinking"
>> +touch $finished_file
>> +wait
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/generic/913.out b/tests/generic/913.out
>> new file mode 100644
>> index 00000000..a34df6ce
>> --- /dev/null
>> +++ b/tests/generic/913.out
>> @@ -0,0 +1,5 @@
>> +QA output created by 913
>> +Format and mount
>> +Initialize files
>> +Reflink and mmap reread the files!
>> +Finished reflinking
>> -- 
>> 2.34.1
>>
>>
>>
diff mbox series

Patch

diff --git a/common/reflink b/common/reflink
index 68dbdedd..455260c6 100644
--- a/common/reflink
+++ b/common/reflink
@@ -186,6 +186,17 @@  _read_range() {
 	$XFS_IO_PROG $xfs_io_args -f -c "pread -q -v $offset $len" "$file" | cut -d ' ' -f '3-18'
 }
 
+# Prints a range of a file as a hex dump
+_mread_range() {
+	file="$1"
+	offset="$2"
+	len="$3"
+	xfs_io_args="$4"
+
+	$XFS_IO_PROG $xfs_io_args -f -c "mmap -rw 0 $((offset + len))" \
+		-c "mread -v $offset $len" "$file" | cut -d ' ' -f '3-18'
+}
+
 # Compare ranges of two files
 _compare_range() {
 	file1="$1"
diff --git a/tests/generic/913 b/tests/generic/913
new file mode 100755
index 00000000..f709c36c
--- /dev/null
+++ b/tests/generic/913
@@ -0,0 +1,72 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# FS QA Test No. 913
+#
+# Test for races or FS corruption between reflink and mmap reading the
+# target file. (MMAP version of generic/164,165)
+#
+. ./common/preamble
+_begin_fstest auto clone
+
+_register_cleanup "_cleanup" BUS
+
+# Import common functions.
+. ./common/filter
+. ./common/reflink
+
+# real QA test starts here
+_require_scratch_reflink
+_require_cp_reflink
+
+echo "Format and mount"
+_scratch_mkfs > $seqres.full 2>&1
+_scratch_mount >> $seqres.full 2>&1
+
+testdir=$SCRATCH_MNT/test-$seq
+finished_file=/tmp/finished
+rm -rf $finished_file
+mkdir $testdir
+
+loops=512
+nr_loops=$((loops - 1))
+blksz=65536
+
+echo "Initialize files"
+echo >> $seqres.full
+_pwrite_byte 0x61 0 $((loops * blksz)) $testdir/file1 >> $seqres.full
+_pwrite_byte 0x62 0 $((loops * blksz)) $testdir/file2 >> $seqres.full
+_cp_reflink $testdir/file1 $testdir/file3
+_scratch_cycle_mount
+
+fbytes() {
+	egrep -v '(61 61 61 61 61 61 61 61 61 61 61 61 61 61 61 61|62 62 62 62 62 62 62 62 62 62 62 62 62 62 62 62)'
+}
+
+reader() {
+	while [ ! -e $finished_file ]; do
+		_mread_range $testdir/file3 0 $((loops * blksz)) | fbytes
+	done
+}
+
+echo "Reflink and mmap reread the files!"
+reader &
+for i in `seq 1 2`; do
+	seq $nr_loops -1 0 | while read i; do
+		_reflink_range  $testdir/file1 $((i * blksz)) \
+				$testdir/file3 $((i * blksz)) $blksz >> $seqres.full
+		[ $? -ne 0 ] && break
+	done
+	seq $nr_loops -1 0 | while read i; do
+		_reflink_range  $testdir/file2 $((i * blksz)) \
+				$testdir/file3 $((i * blksz)) $blksz >> $seqres.full
+		[ $? -ne 0 ] && break
+	done
+done
+echo "Finished reflinking"
+touch $finished_file
+wait
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/913.out b/tests/generic/913.out
new file mode 100644
index 00000000..a34df6ce
--- /dev/null
+++ b/tests/generic/913.out
@@ -0,0 +1,5 @@ 
+QA output created by 913
+Format and mount
+Initialize files
+Reflink and mmap reread the files!
+Finished reflinking