[PATCH 1/1] ext4/046: Add test to verify unwritten extent conversion in buff-io

[PATCH 1/1] ext4/046: Add test to verify unwritten extent conversion in buff-io

[Ritesh Harjani]

There was an issue where with with filesize > 4G, map.m_lblk
was getting overflow in buff-IO path while converting unwritten to
written extent with dioread_nolock mount option with bs < ps.
Adding a testcase for the same.
This test doesn't force any explicit mount option within the test,
as it will be set anyway along with test config, although it does
force for blocksize < pagesize config (as this test is specific to
that). Patch at [1] fixes the issue.

[1]: https://patchwork.ozlabs.org/patch/1378632

Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 tests/ext4/046     | 101 +++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/046.out |   2 +
 tests/ext4/group   |   1 +
 3 files changed, 104 insertions(+)
 create mode 100755 tests/ext4/046
 create mode 100644 tests/ext4/046.out

diff --git a/tests/ext4/046 b/tests/ext4/046
new file mode 100755
index 000000000000..49de6bbb2c89
--- /dev/null
+++ b/tests/ext4/046
@@ -0,0 +1,101 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020 IBM Corporation. All Rights Reserved.
+#
+# FS QA Test No. generic/046
+#
+# Test writes to falloc file with filesize > 4GB and make sure to verify
+# the file checksum both before and after mount.
+# This test is to check whether unwritten extents gets properly converted
+# to written extent on a filesystem with bs < ps.
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# Modify as appropriate.
+_supported_fs ext4
+_supported_os Linux
+_require_scratch
+_require_xfs_io_command "falloc"
+
+# keep 4k as a blksz for 64k pagesz
+pagesz=$(getconf PAGE_SIZE)
+if [ $pagesz -eq 65536 ]; then
+	blksz=4096
+else
+	blksz=$(($pagesz/4))
+fi
+
+devsize=`blockdev --getsize64 $SCRATCH_DEV`
+if [ $devsize -lt 6442450944 ]; then
+	_notrun "Too small scratch device, need at least 6G"
+fi
+
+# Test for bs < ps
+export MKFS_OPTIONS="-F -b $blksz"
+_scratch_mkfs >> $seqres.full 2>&1
+_scratch_mount >> $seqres.full 2>&1
+
+# check blksz
+real_blksz=$(_get_file_block_size $SCRATCH_MNT)
+test $real_blksz != $blksz && _notrun "Failed to format with small blocksize."
+
+testfile=$SCRATCH_MNT/testfile-$seq
+
+# Fallocate testfile with size > 4G
+fsize=$((5 * 1024 * 1024 * 1024))
+$XFS_IO_PROG -f -c "falloc 0 $fsize" $testfile >> $seqres.full 2>&1
+
+# First write at offset < 4G (at few alternative blks)
+off=$((3 * 1024 * 1024 * 1024))
+for i in 1 2 3 4; do
+	$XFS_IO_PROG -f \
+		-c "pwrite $off $blksz" \
+		$testfile >> $seqres.full 2>&1
+	off=$(($off + (2*$blksz)))
+done
+
+# Then write at offset > 4G (at few alternative blks) to check
+# any 32bit overflow case in map.m_lblk
+off=$((4 * 1024 * 1024 * 1024))
+for i in 1 2 3 4; do
+	$XFS_IO_PROG -f \
+		-c "pwrite $off $blksz" \
+		$testfile >> $seqres.full 2>&1
+	off=$(($off + (2*$blksz)))
+done
+
+# ==== Pre-Remount ===
+md5_pre=`md5sum $testfile | cut -d' ' -f1`
+echo "Pre-Remount md5sum of $testfile = $md5_pre" >> $seqres.full
+
+_scratch_cycle_mount
+
+# ==== Post-Remount ===
+md5_post=`md5sum $testfile | cut -d' ' -f1`
+echo "Post-Remount md5sum of $testfile = $md5_post" >> $seqres.full
+test $md5_pre != $md5_post && echo "md5sum mismatch"
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/ext4/046.out b/tests/ext4/046.out
new file mode 100644
index 000000000000..52c445eb70bb
--- /dev/null
+++ b/tests/ext4/046.out
@@ -0,0 +1,2 @@
+QA output created by 046
+Silence is golden
diff --git a/tests/ext4/group b/tests/ext4/group
index 40351fd9ca0c..02a499a9d220 100644
--- a/tests/ext4/group
+++ b/tests/ext4/group
@@ -48,6 +48,7 @@
 043 auto quick
 044 auto quick
 045 auto dir
+046 auto quick
 271 auto rw quick
 301 aio auto ioctl rw stress defrag
 302 aio auto ioctl rw stress defrag
-- 
2.26.2

Re: [PATCH 1/1] ext4/046: Add test to verify unwritten extent conversion in buff-io

[Eryu Guan]

On Fri, Oct 09, 2020 at 05:12:24PM +0530, Ritesh Harjani wrote:
> There was an issue where with with filesize > 4G, map.m_lblk
> was getting overflow in buff-IO path while converting unwritten to
> written extent with dioread_nolock mount option with bs < ps.
> Adding a testcase for the same.
> This test doesn't force any explicit mount option within the test,
> as it will be set anyway along with test config, although it does
> force for blocksize < pagesize config (as this test is specific to

I don't think we need to force blksize < pagesize either, as testing
different blocksize configs is a common test setup and is part of
tester's resposibility.

> that). Patch at [1] fixes the issue.
> 
> [1]: https://patchwork.ozlabs.org/patch/1378632
> 
> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> ---
>  tests/ext4/046     | 101 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/ext4/046.out |   2 +
>  tests/ext4/group   |   1 +
>  3 files changed, 104 insertions(+)
>  create mode 100755 tests/ext4/046
>  create mode 100644 tests/ext4/046.out
> 
> diff --git a/tests/ext4/046 b/tests/ext4/046
> new file mode 100755
> index 000000000000..49de6bbb2c89
> --- /dev/null
> +++ b/tests/ext4/046
> @@ -0,0 +1,101 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2020 IBM Corporation. All Rights Reserved.
> +#
> +# FS QA Test No. generic/046
> +#
> +# Test writes to falloc file with filesize > 4GB and make sure to verify
> +# the file checksum both before and after mount.
> +# This test is to check whether unwritten extents gets properly converted
> +# to written extent on a filesystem with bs < ps.

Mention dioread_nolock mount option as well here?

> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# Modify as appropriate.
> +_supported_fs ext4

I don't see any ext4 specific setups in the test, so it could be a
generic test.

> +_supported_os Linux

"_suppored_os" is removed, just drop this line.

> +_require_scratch
> +_require_xfs_io_command "falloc"
> +
> +# keep 4k as a blksz for 64k pagesz
> +pagesz=$(getconf PAGE_SIZE)
> +if [ $pagesz -eq 65536 ]; then
> +	blksz=4096
> +else
> +	blksz=$(($pagesz/4))
> +fi
> +
> +devsize=`blockdev --getsize64 $SCRATCH_DEV`
> +if [ $devsize -lt 6442450944 ]; then
> +	_notrun "Too small scratch device, need at least 6G"
> +fi

_require_scratch_size is used to do this check.

> +
> +# Test for bs < ps
> +export MKFS_OPTIONS="-F -b $blksz"
> +_scratch_mkfs >> $seqres.full 2>&1
> +_scratch_mount >> $seqres.full 2>&1
> +
> +# check blksz
> +real_blksz=$(_get_file_block_size $SCRATCH_MNT)
> +test $real_blksz != $blksz && _notrun "Failed to format with small blocksize."
> +
> +testfile=$SCRATCH_MNT/testfile-$seq
> +
> +# Fallocate testfile with size > 4G
> +fsize=$((5 * 1024 * 1024 * 1024))
> +$XFS_IO_PROG -f -c "falloc 0 $fsize" $testfile >> $seqres.full 2>&1
> +
> +# First write at offset < 4G (at few alternative blks)
> +off=$((3 * 1024 * 1024 * 1024))
> +for i in 1 2 3 4; do
> +	$XFS_IO_PROG -f \
> +		-c "pwrite $off $blksz" \
> +		$testfile >> $seqres.full 2>&1
> +	off=$(($off + (2*$blksz)))
> +done
> +
> +# Then write at offset > 4G (at few alternative blks) to check
> +# any 32bit overflow case in map.m_lblk
> +off=$((4 * 1024 * 1024 * 1024))
> +for i in 1 2 3 4; do
> +	$XFS_IO_PROG -f \
> +		-c "pwrite $off $blksz" \
> +		$testfile >> $seqres.full 2>&1
> +	off=$(($off + (2*$blksz)))
> +done
> +
> +# ==== Pre-Remount ===
> +md5_pre=`md5sum $testfile | cut -d' ' -f1`
> +echo "Pre-Remount md5sum of $testfile = $md5_pre" >> $seqres.full
> +
> +_scratch_cycle_mount
> +
> +# ==== Post-Remount ===
> +md5_post=`md5sum $testfile | cut -d' ' -f1`
> +echo "Post-Remount md5sum of $testfile = $md5_post" >> $seqres.full
> +test $md5_pre != $md5_post && echo "md5sum mismatch"
> +
> +# success, all done
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/ext4/046.out b/tests/ext4/046.out
> new file mode 100644
> index 000000000000..52c445eb70bb
> --- /dev/null
> +++ b/tests/ext4/046.out
> @@ -0,0 +1,2 @@
> +QA output created by 046
> +Silence is golden
> diff --git a/tests/ext4/group b/tests/ext4/group
> index 40351fd9ca0c..02a499a9d220 100644
> --- a/tests/ext4/group
> +++ b/tests/ext4/group
> @@ -48,6 +48,7 @@
>  043 auto quick
>  044 auto quick
>  045 auto dir
> +046 auto quick

Also add 'prealloc' group

Thanks,
Eryu

>  271 auto rw quick
>  301 aio auto ioctl rw stress defrag
>  302 aio auto ioctl rw stress defrag
> -- 
> 2.26.2

Re: [PATCH 1/1] ext4/046: Add test to verify unwritten extent conversion in buff-io

[Ritesh Harjani]

Thanks Eryu for detailed review.

On 10/11/20 11:41 AM, Eryu Guan wrote:
> On Fri, Oct 09, 2020 at 05:12:24PM +0530, Ritesh Harjani wrote:
>> There was an issue where with with filesize > 4G, map.m_lblk
>> was getting overflow in buff-IO path while converting unwritten to
>> written extent with dioread_nolock mount option with bs < ps.
>> Adding a testcase for the same.
>> This test doesn't force any explicit mount option within the test,
>> as it will be set anyway along with test config, although it does
>> force for blocksize < pagesize config (as this test is specific to
> 
> I don't think we need to force blksize < pagesize either, as testing
> different blocksize configs is a common test setup and is part of
> tester's resposibility.

Right, but generally bs < ps configuration is not tested for every mount 
option. The issue mentioned here happens with bs < ps for dioread_nolock 
mount opt.
So I need to force at least bs < ps or the mount option of dioread_nolock.
I think since it is mainly ext4 specific, maybe I will change the patch 
to enforce this for dioread_nolock mount opt. Then since bs < ps by 
default everyone tests, so this test will get exercised automatically.


> 
>> that). Patch at [1] fixes the issue.
>>
>> [1]: https://patchwork.ozlabs.org/patch/1378632
>>
>> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
>> ---
>>   tests/ext4/046     | 101 +++++++++++++++++++++++++++++++++++++++++++++
>>   tests/ext4/046.out |   2 +
>>   tests/ext4/group   |   1 +
>>   3 files changed, 104 insertions(+)
>>   create mode 100755 tests/ext4/046
>>   create mode 100644 tests/ext4/046.out
>>
>> diff --git a/tests/ext4/046 b/tests/ext4/046
>> new file mode 100755
>> index 000000000000..49de6bbb2c89
>> --- /dev/null
>> +++ b/tests/ext4/046
>> @@ -0,0 +1,101 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2020 IBM Corporation. All Rights Reserved.
>> +#
>> +# FS QA Test No. generic/046
>> +#
>> +# Test writes to falloc file with filesize > 4GB and make sure to verify
>> +# the file checksum both before and after mount.
>> +# This test is to check whether unwritten extents gets properly converted
>> +# to written extent on a filesystem with bs < ps.
> 
> Mention dioread_nolock mount option as well here?

yup, will mention that.

> 
>> +#
>> +seq=`basename $0`
>> +seqres=$RESULT_DIR/$seq
>> +echo "QA output created by $seq"
>> +
>> +here=`pwd`
>> +tmp=/tmp/$$
>> +status=1	# failure is the default!
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +_cleanup()
>> +{
>> +	cd /
>> +	rm -f $tmp.*
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/filter
>> +
>> +# remove previous $seqres.full before test
>> +rm -f $seqres.full
>> +
>> +# Modify as appropriate.
>> +_supported_fs ext4
> 
> I don't see any ext4 specific setups in the test, so it could be a
> generic test.

as mentioned in above comments. Let me enforce the dioread_nolock
ext4 mount opt for this test.

> 
>> +_supported_os Linux
> 
> "_suppored_os" is removed, just drop this line.

ok.

> 
>> +_require_scratch
>> +_require_xfs_io_command "falloc"
>> +
>> +# keep 4k as a blksz for 64k pagesz
>> +pagesz=$(getconf PAGE_SIZE)
>> +if [ $pagesz -eq 65536 ]; then
>> +	blksz=4096
>> +else
>> +	blksz=$(($pagesz/4))
>> +fi
>> +
>> +devsize=`blockdev --getsize64 $SCRATCH_DEV`
>> +if [ $devsize -lt 6442450944 ]; then
>> +	_notrun "Too small scratch device, need at least 6G"
>> +fi
> 
> _require_scratch_size is used to do this check.

ok.

> 
>> +
>> +# Test for bs < ps
>> +export MKFS_OPTIONS="-F -b $blksz"
>> +_scratch_mkfs >> $seqres.full 2>&1
>> +_scratch_mount >> $seqres.full 2>&1
>> +
>> +# check blksz
>> +real_blksz=$(_get_file_block_size $SCRATCH_MNT)
>> +test $real_blksz != $blksz && _notrun "Failed to format with small blocksize."
>> +
>> +testfile=$SCRATCH_MNT/testfile-$seq
>> +
>> +# Fallocate testfile with size > 4G
>> +fsize=$((5 * 1024 * 1024 * 1024))
>> +$XFS_IO_PROG -f -c "falloc 0 $fsize" $testfile >> $seqres.full 2>&1
>> +
>> +# First write at offset < 4G (at few alternative blks)
>> +off=$((3 * 1024 * 1024 * 1024))
>> +for i in 1 2 3 4; do
>> +	$XFS_IO_PROG -f \
>> +		-c "pwrite $off $blksz" \
>> +		$testfile >> $seqres.full 2>&1
>> +	off=$(($off + (2*$blksz)))
>> +done
>> +
>> +# Then write at offset > 4G (at few alternative blks) to check
>> +# any 32bit overflow case in map.m_lblk
>> +off=$((4 * 1024 * 1024 * 1024))
>> +for i in 1 2 3 4; do
>> +	$XFS_IO_PROG -f \
>> +		-c "pwrite $off $blksz" \
>> +		$testfile >> $seqres.full 2>&1
>> +	off=$(($off + (2*$blksz)))
>> +done
>> +
>> +# ==== Pre-Remount ===
>> +md5_pre=`md5sum $testfile | cut -d' ' -f1`
>> +echo "Pre-Remount md5sum of $testfile = $md5_pre" >> $seqres.full
>> +
>> +_scratch_cycle_mount
>> +
>> +# ==== Post-Remount ===
>> +md5_post=`md5sum $testfile | cut -d' ' -f1`
>> +echo "Post-Remount md5sum of $testfile = $md5_post" >> $seqres.full
>> +test $md5_pre != $md5_post && echo "md5sum mismatch"
>> +
>> +# success, all done
>> +echo "Silence is golden"
>> +status=0
>> +exit
>> diff --git a/tests/ext4/046.out b/tests/ext4/046.out
>> new file mode 100644
>> index 000000000000..52c445eb70bb
>> --- /dev/null
>> +++ b/tests/ext4/046.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 046
>> +Silence is golden
>> diff --git a/tests/ext4/group b/tests/ext4/group
>> index 40351fd9ca0c..02a499a9d220 100644
>> --- a/tests/ext4/group
>> +++ b/tests/ext4/group
>> @@ -48,6 +48,7 @@
>>   043 auto quick
>>   044 auto quick
>>   045 auto dir
>> +046 auto quick
> 
> Also add 'prealloc' group

ok.

> 
> Thanks,
> Eryu
> 
>>   271 auto rw quick
>>   301 aio auto ioctl rw stress defrag
>>   302 aio auto ioctl rw stress defrag
>> -- 
>> 2.26.2