generic: Add some tests around journal replay/recoveryloop
v2 cover generic: Add some tests around journal replay/recoveryloop
Hello,
Sending v2 with tests/ext4/ converted to tests/generic/
(although I had not received any review comments on v1).
It seems all of the tests which I had sent in v1 are not ext4 specific anyways. So in v2, I have made those into tests/generic/.
These are some of the tests which when tested with ext4 fast_commit feature w/o kernel fixes, could cause tests failures and/or KASAN bug (generic/486).
I gave these tests a run with default xfs, btrfs and f2fs configs (along with ext4). No surprises observed.
[v1]: https://lore.kernel.org/all/[email protected]/
Ritesh Harjani (4):
generic/468: Add another falloc test entry common/punch: Add block_size argument to _filter_fiemap_** generic/676: Add a new shutdown recovery test generic/677: Add a test to check unwritten extents tracking
common/punch | 12 +++++--- tests/generic/468 | 4 +++ tests/generic/468.out | 2 ++ tests/generic/676 | 72 +++++++++++++++++++++++++++++++++++++++++++ tests/generic/676.out | 7 +++++ tests/generic/677 | 64 ++++++++++++++++++++++++++++++++++++++ tests/generic/677.out | 6 ++++ 7 files changed, 162 insertions(+), 5 deletions(-) create mode 100755 tests/generic/676 create mode 100644 tests/generic/676.out create mode 100755 tests/generic/677 create mode 100644 tests/generic/677.out
--
2.31.1
PATCH v2 1/4 generic/468: Add another falloc test entry
Add another falloc test entry which could hit a kernel bug with ext4 fast_commit feature w/o below kernel commit [1].
<log>
[ 410.888496][ T2743] BUG: KASAN: use-after-free in ext4_mb_mark_bb+0x26a/0x6c0 [ 410.890432][ T2743] Read of size 8 at addr ffff888171886000 by task mount/2743
This happens when falloc -k size is huge which spans across more than 1 flex block group in ext4. This causes a bug in fast_commit replay code which is fixed by kernel commit at [1].
--- tests/generic/468 | 4 ++++ tests/generic/468.out | 2 ++ 2 files changed, 6 insertions(+)
tests/generic/468 +4 -0
@@ -34,6 +34,9 @@ _scratch_mkfs >/dev/null 2>&1 _require_metadata_journaling $SCRATCH_DEV _scratch_mount +blocksize=4096 +fact=18 + testfile=$SCRATCH_MNT/testfile # check inode metadata after shutdown
@@ -85,6 +88,7 @@ for i in fsync fdatasync; do test_falloc $i "-k " 1024 test_falloc $i "-k " 4096 test_falloc $i "-k " 104857600 + test_falloc $i "-k " $((32768*$blocksize*$fact)) done status=0
tests/generic/468.out +2 -0
@@ -5,9 +5,11 @@ QA output created by 468 ==== falloc -k 1024 test with fsync ==== ==== falloc -k 4096 test with fsync ==== ==== falloc -k 104857600 test with fsync ==== +==== falloc -k 2415919104 test with fsync ==== ==== falloc 1024 test with fdatasync ==== ==== falloc 4096 test with fdatasync ==== ==== falloc 104857600 test with fdatasync ==== ==== falloc -k 1024 test with fdatasync ==== ==== falloc -k 4096 test with fdatasync ==== ==== falloc -k 104857600 test with fdatasync ==== +==== falloc -k 2415919104 test with fdatasync ====
--
2.31.1
PATCH v2 2/4 common/punch: Add block_size argument to _filter_fiemap_**
In reply to Ritesh HarjaniAdd block_size paramter to _filter_fiemap_flags() and _filter_hole_fiemap(). This is used in next patches
Also this fixes some of the end of line whitespace issues while we are at it.
--- common/punch | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
common/punch +7 -5
@@ -109,6 +109,7 @@ _filter_fiemap() _filter_fiemap_flags() { + block_size=$1 $AWK_PROG ' $3 ~ /hole/ { print $1, $2, $3;
@@ -135,23 +136,24 @@ _filter_fiemap_flags() } print $1, $2, flag_str }' | - _coalesce_extents + _coalesce_extents $block_size } -# Filters fiemap output to only print the +# Filters fiemap output to only print the # file offset column and whether or not # it is an extent or a hole _filter_hole_fiemap() { + block_size=$1 $AWK_PROG ' $3 ~ /hole/ { - print $1, $2, $3; + print $1, $2, $3; next; - } + } $5 ~ /0x[[:xdigit:]]+/ { print $1, $2, "extent"; }' | - _coalesce_extents + _coalesce_extents $block_size } # 10000 Unwritten preallocated extent
--
2.31.1
PATCH v2 3/4 generic/676: Add a new shutdown recovery test
In reply to Ritesh HarjaniIn certain cases (it is noted with ext4 fast_commit feature) that, replay phase may not delete the right range of blocks (after sudden FS shutdown) due to some operations which depends on inode->i_size (which during replay of an inode with fast_commit could be 0 for sometime).
This fstest is added to test for such scenarios for all generic fs.
This test case is based on the test case shared via Xin Yin.
--- tests/generic/676 | 72 +++++++++++++++++++++++++++++++++++++++++++ tests/generic/676.out | 7 +++++ 2 files changed, 79 insertions(+) create mode 100755 tests/generic/676 create mode 100644 tests/generic/676.out
tests/generic/676 new file +72 -0
@@ -0,0 +1,72 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2022 IBM Corporation. All Rights Reserved. +# +# FS QA Test 676 +# +# This test with ext4 fast_commit feature w/o below patch missed to delete the right +# range during replay phase, since it depends upon inode->i_size (which might not be +# stable during replay phase, at least for ext4). +# 0b5b5a62b945a141: ext4: use ext4_ext_remove_space() for fast commit replay delete range +# (Based on test case shared by Xin Yin <[email protected]>) +# + +. ./common/preamble +_begin_fstest auto shutdown quick log recoveryloop + +# Override the default cleanup function. +_cleanup() +{ + cd / + rm -r -f $tmp.* + _scratch_unmount > /dev/null 2>&1 +} + +# Import common functions. +. ./common/filter +. ./common/punch + +# real QA test starts here + +# Modify as appropriate. +_supported_fs generic +_require_scratch +_require_xfs_io_command "fpunch" +_require_xfs_io_command "fzero" +_require_xfs_io_command "fiemap" + +t1=$SCRATCH_MNT/foo +t2=$SCRATCH_MNT/bar + +_scratch_mkfs > $seqres.full 2>&1 + +_scratch_mount >> $seqres.full 2>&1 + +bs=$(_get_block_size $SCRATCH_MNT) + +# create and write data to t1 +$XFS_IO_PROG -f -c "pwrite 0 $((100*$bs))" $t1 | _filter_xfs_io_numbers + +# fzero certain range in between with -k +$XFS_IO_PROG -c "fzero -k $((40*$bs)) $((20*$bs))" $t1 + +# create and fsync a new file t2 +$XFS_IO_PROG -f -c "fsync" $t2 + +# fpunch within the i_size of a file +$XFS_IO_PROG -c "fpunch $((30*$bs)) $((20*$bs))" $t1 + +# fsync t1 to trigger journal operation +$XFS_IO_PROG -c "fsync" $t1 + +# shutdown FS now for replay journal to kick in next mount +_scratch_shutdown -v >> $seqres.full 2>&1 + +_scratch_cycle_mount + +# check fiemap reported is valid or not +$XFS_IO_PROG -c "fiemap -v" $t1 | _filter_fiemap_flags $bs + +# success, all done +status=0 +exit
tests/generic/676.out new file +7 -0
@@ -0,0 +1,7 @@ +QA output created by 676 +wrote XXXX/XXXX bytes at offset XXXX +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +0: [0..29]: none +1: [30..49]: hole +2: [50..59]: unwritten +3: [60..99]: nonelast
--
2.31.1
PATCH v2 4/4 generic/677: Add a test to check unwritten extents tracking
In reply to Ritesh HarjaniWith these sequence of operation (in certain cases like with ext4 fast_commit) could miss to track unwritten extents during replay phase
(after sudden FS shutdown).
This fstest adds a test case to test this.
5e4d0eba1ccaf19f
ext4: fix fast commit may miss tracking range for FALLOC_FL_ZERO_RANGE
--- tests/generic/677 | 64 +++++++++++++++++++++++++++++++++++++++++++ tests/generic/677.out | 6 ++++ 2 files changed, 70 insertions(+) create mode 100755 tests/generic/677 create mode 100644 tests/generic/677.out
tests/generic/677 new file +64 -0
@@ -0,0 +1,64 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2022 IBM Corporation. All Rights Reserved. +# +# FS QA Test 677 +# +# Test below sequence of operation which (w/o below kernel patch) in case of +# ext4 with fast_commit may misss to track unwritten extents. +# commit 5e4d0eba1ccaf19f +# ext4: fix fast commit may miss tracking range for FALLOC_FL_ZERO_RANGE +# +. ./common/preamble +_begin_fstest auto quick log shutdown recoveryloop + +# Override the default cleanup function. +_cleanup() +{ + cd / + rm -r -f $tmp.* +} + +# Import common functions. +. ./common/filter +. ./common/punch + +# real QA test starts here + +# Modify as appropriate. +_supported_fs generic +_require_scratch +_require_xfs_io_command "fzero" +_require_xfs_io_command "fiemap" + +t1=$SCRATCH_MNT/t1 + +_scratch_mkfs > $seqres.full 2>&1 + +_scratch_mount >> $seqres.full 2>&1 + +bs=$(_get_block_size $SCRATCH_MNT) + +# create and write data to t1 +$XFS_IO_PROG -f -c "pwrite 0 $((100*$bs))" $t1 | _filter_xfs_io_numbers + +# fsync t1 +$XFS_IO_PROG -c "fsync" $t1 + +# fzero certain range in between +$XFS_IO_PROG -c "fzero -k $((40*$bs)) $((20*$bs))" $t1 + +# fsync t1 +$XFS_IO_PROG -c "fsync" $t1 + +# shutdown FS now for replay of journal to kick during next mount +_scratch_shutdown -v >> $seqres.full 2>&1 + +_scratch_cycle_mount + +# check fiemap reported is valid or not +$XFS_IO_PROG -c "fiemap -v" $t1 | _filter_fiemap_flags $bs + +# success, all done +status=0 +exit
tests/generic/677.out new file +6 -0
@@ -0,0 +1,6 @@ +QA output created by 677 +wrote XXXX/XXXX bytes at offset XXXX +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +0: [0..39]: none +1: [40..59]: unwritten +2: [60..99]: nonelast
--
2.31.1
Re: [PATCHv2 1/4] generic/468: Add another falloc test entry
In reply to Ritesh HarjaniQuoted 24 lines · Ritesh Harjani
Add another falloc test entry which could hit a kernel bug with ext4
fast_commitfeature w/o below kernel commit [1].<log>
[ 410.888496][ T2743] BUG: KASAN: use-after-free in ext4_mb_mark_bb+0x26a/0x6c0 [ 410.890432][ T2743] Read of size 8 at addr ffff888171886000 by task mount/2743This happens when falloc -k size is huge which spans across more than 1 flex block group in ext4. This causes a bug in
fast_commitreplay code which is fixed by kernel commit at [1].Signed-off-by Ritesh Harjani <[email protected]>--- tests/generic/468 | 4 ++++ tests/generic/468.out | 2 ++ 2 files changed, 6 insertions(+)tests/generic/468 +0
-0
tests/generic/468 _scratch_mkfs >/dev/null 2>&1 · line 37
Quoted 4 lines
tests/generic/468 +1
-0@@ -34,6 +34,9 @@ _scratch_mkfs >/dev/null 2>&1 _require_metadata_journaling $SCRATCH_DEV _scratch_mount +blocksize=4096
What happens if the file blocksize isn't 4k? Does fastcommit only support one block size? I didn't think it has any such restriction?
tests/generic/468 _scratch_mkfs >/dev/null 2>&1 · line 38
Quoted 1 line
tests/generic/468 +1
-0+fact=18
This needs a bit more explanation -- why 18? I think the reason is that you need the fallocate to cross into another flexbg, and flexbgs (by default) are 16bg long, right?
If that's the case, then don't you need to detect the flexbg size so that this is still an effective test if someone runs fstests with MKFS_OPTIONS='-G 32' or something?
--D
Quoted 31 lines
tests/generic/468 +2
-0+ testfile=$SCRATCH_MNT/testfile # check inode metadata after shutdown@@ -85,6 +88,7 @@ for i in fsync fdatasync; do test_falloc $i "-k " 1024 test_falloc $i "-k " 4096 test_falloc $i "-k " 104857600 + test_falloc $i "-k " $((32768*$blocksize*$fact)) done status=0tests/generic/468.out +2
-1@@ -5,9 +5,11 @@ QA output created by 468 ==== falloc -k 1024 test with fsync ==== ==== falloc -k 4096 test with fsync ==== ==== falloc -k 104857600 test with fsync ==== +==== falloc -k 2415919104 test with fsync ==== ==== falloc 1024 test with fdatasync ==== ==== falloc 4096 test with fdatasync ==== ==== falloc 104857600 test with fdatasync ==== ==== falloc -k 1024 test with fdatasync ==== ==== falloc -k 4096 test with fdatasync ==== ==== falloc -k 104857600 test with fdatasync ==== +==== falloc -k 2415919104 test with fdatasync ==== --2.31.1
Re: [PATCHv2 3/4] generic/676: Add a new shutdown recovery test
In reply to Ritesh HarjaniQuoted 27 lines · Ritesh Harjani
In certain cases (it is noted with ext4
fast_commitfeature) that, replay phase may not delete the right range of blocks (after sudden FS shutdown) due to some operations which depends oninode->i_size(which during replay of an inode withfast_commitcould be 0 for sometime).
This fstest is added to test for such scenarios for all generic fs.This test case is based on the test case shared via Xin Yin.
Signed-off-by Ritesh Harjani <[email protected]>--- tests/generic/676 | 72 +++++++++++++++++++++++++++++++++++++++++++ tests/generic/676.out | 7 +++++ 2 files changed, 79 insertions(+) create mode 100755 tests/generic/676 create mode 100644 tests/generic/676.outtests/generic/676 new file +5
-0@@ -0,0 +1,72 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2022 IBM Corporation. All Rights Reserved. +# +# FS QA Test 676
tests/generic/676 line 15
Quoted 10 lines
tests/generic/676 new file +10
-0+# +# This test with ext4 fast_commit feature w/o below patch missed to delete the right +# range during replay phase, since it depends upon inode->i_size (which might not be +# stable during replay phase, at least for ext4). +# 0b5b5a62b945a141: ext4: use ext4_ext_remove_space() for fast commit replay delete range +# (Based on test case shared by Xin Yin <[email protected]>) +# + +. ./common/preamble +_begin_fstest auto shutdown quick log recoveryloop
This isn't a looping recovery test. Maybe we should create a 'recovery' group for tests that only run once? I think we already have a few fstests like that.
tests/generic/676 line 22
Quoted 7 lines
tests/generic/676 new file +7
-0+ +# Override the default cleanup function. +_cleanup() +{ + cd / + rm -r -f $tmp.* + _scratch_unmount > /dev/null 2>&1
I think the test harness does this for you already, right?
Quoted 4 lines
tests/generic/676 new file +4
-0+} + +# Import common functions. +. ./common/filter
tests/generic/676 line 36
Quoted 10 lines
tests/generic/676 new file +10
-0+. ./common/punch + +# real QA test starts here + +# Modify as appropriate. +_supported_fs generic +_require_scratch +_require_xfs_io_command "fpunch" +_require_xfs_io_command "fzero" +_require_xfs_io_command "fiemap"
_require_scratch_shutdown
tests/generic/676 line 45
Quoted 9 lines
tests/generic/676 new file +9
-0+ +t1=$SCRATCH_MNT/foo +t2=$SCRATCH_MNT/bar + +_scratch_mkfs > $seqres.full 2>&1 + +_scratch_mount >> $seqres.full 2>&1 + +bs=$(_get_block_size $SCRATCH_MNT)
_get_file_block_size, in case the file allocation unit isn't the same as the fs blocksize? (e.g. bigalloc, xfs realtime, etc.)
--D
Quoted 43 lines
tests/generic/676 new file +27
-0+ +# create and write data to t1 +$XFS_IO_PROG -f -c "pwrite 0 $((100*$bs))" $t1 | _filter_xfs_io_numbers + +# fzero certain range in between with -k +$XFS_IO_PROG -c "fzero -k $((40*$bs)) $((20*$bs))" $t1 + +# create and fsync a new file t2 +$XFS_IO_PROG -f -c "fsync" $t2 + +# fpunch within the i_size of a file +$XFS_IO_PROG -c "fpunch $((30*$bs)) $((20*$bs))" $t1 + +# fsync t1 to trigger journal operation +$XFS_IO_PROG -c "fsync" $t1 + +# shutdown FS now for replay journal to kick in next mount +_scratch_shutdown -v >> $seqres.full 2>&1 + +_scratch_cycle_mount + +# check fiemap reported is valid or not +$XFS_IO_PROG -c "fiemap -v" $t1 | _filter_fiemap_flags $bs + +# success, all done +status=0 +exittests/generic/676.out new file +7
-1@@ -0,0 +1,7 @@ +QA output created by 676 +wrote XXXX/XXXX bytes at offset XXXX +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +0: [0..29]: none +1: [30..49]: hole +2: [50..59]: unwritten +3: [60..99]: nonelast --2.31.1
Re: [PATCHv2 4/4] generic/677: Add a test to check unwritten extents tracking
In reply to Ritesh HarjaniQuoted 53 lines · Ritesh Harjani
With these sequence of operation (in certain cases like with ext4
fast_commit) could miss to track unwritten extents during replay phase
(after sudden FS shutdown).This fstest adds a test case to test this.
5e4d0eba1ccaf19f
ext4: fix fast commit may miss tracking range forFALLOC_FL_ZERO_RANGESigned-off-by Ritesh Harjani <[email protected]>--- tests/generic/677 | 64 +++++++++++++++++++++++++++++++++++++++++++ tests/generic/677.out | 6 ++++ 2 files changed, 70 insertions(+) create mode 100755 tests/generic/677 create mode 100644 tests/generic/677.outtests/generic/677 new file +30
-0@@ -0,0 +1,64 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2022 IBM Corporation. All Rights Reserved. +# +# FS QA Test 677 +# +# Test below sequence of operation which (w/o below kernel patch) in case of +# ext4 with fast_commit may misss to track unwritten extents. +# commit 5e4d0eba1ccaf19f +# ext4: fix fast commit may miss tracking range for FALLOC_FL_ZERO_RANGE +# +. ./common/preamble +_begin_fstest auto quick log shutdown recoveryloop + +# Override the default cleanup function. +_cleanup() +{ + cd / + rm -r -f $tmp.* +} + +# Import common functions. +. ./common/filter +. ./common/punch + +# real QA test starts here + +# Modify as appropriate. +_supported_fs generic +_require_scratch
tests/generic/677 line 40
Quoted 10 lines
tests/generic/677 new file +10
-0+_require_xfs_io_command "fzero" +_require_xfs_io_command "fiemap" + +t1=$SCRATCH_MNT/t1 + +_scratch_mkfs > $seqres.full 2>&1 + +_scratch_mount >> $seqres.full 2>&1 + +bs=$(_get_block_size $SCRATCH_MNT)
Same comments about blocksize, group names, and_require_scratch_shutdown as the last patch.
--D
Quoted 39 lines
tests/generic/677 new file +24
-0+ +# create and write data to t1 +$XFS_IO_PROG -f -c "pwrite 0 $((100*$bs))" $t1 | _filter_xfs_io_numbers + +# fsync t1 +$XFS_IO_PROG -c "fsync" $t1 + +# fzero certain range in between +$XFS_IO_PROG -c "fzero -k $((40*$bs)) $((20*$bs))" $t1 + +# fsync t1 +$XFS_IO_PROG -c "fsync" $t1 + +# shutdown FS now for replay of journal to kick during next mount +_scratch_shutdown -v >> $seqres.full 2>&1 + +_scratch_cycle_mount + +# check fiemap reported is valid or not +$XFS_IO_PROG -c "fiemap -v" $t1 | _filter_fiemap_flags $bs + +# success, all done +status=0 +exittests/generic/677.out new file +6
-1@@ -0,0 +1,6 @@ +QA output created by 677 +wrote XXXX/XXXX bytes at offset XXXX +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +0: [0..39]: none +1: [40..59]: unwritten +2: [60..99]: nonelast --2.31.1
Re: [PATCHv2 1/4] generic/468: Add another falloc test entry
In reply to Darrick J. WongFirst of all thanks Darrick for the review.
And sorry about such a long delay in getting back to this.
Quoted 28 lines · Darrick J. Wong
Quoted 27 lines · Ritesh Harjani
Add another falloc test entry which could hit a kernel bug with ext4
fast_commitfeature w/o below kernel commit [1].<log>
[ 410.888496][ T2743] BUG: KASAN: use-after-free in ext4_mb_mark_bb+0x26a/0x6c0 [ 410.890432][ T2743] Read of size 8 at addr ffff888171886000 by task mount/2743This happens when falloc -k size is huge which spans across more than 1 flex block group in ext4. This causes a bug in
fast_commitreplay code which is fixed by kernel commit at [1].Signed-off-by Ritesh Harjani <[email protected]>--- tests/generic/468 | 4 ++++ tests/generic/468.out | 2 ++ 2 files changed, 6 insertions(+)tests/generic/468 +0
-0@@ -34,6 +34,9 @@ _scratch_mkfs >/dev/null 2>&1 _require_metadata_journaling $SCRATCH_DEV _scratch_mount
Quoted 4 lines
tests/generic/468 _scratch_mkfs >/dev/null 2>&1 · line 37
Quoted 1 line
tests/generic/468 +1
-0+blocksize=4096What happens if the file blocksize isn't 4k? Does fastcommit only support one block size? I didn't think it has any such restriction?
It does support other block size too.
Now, for bs < 4096 it is still fine. Yes, it won't trigger for 64K bs. But that is ok since anyway to trigger this issue with 64K, we will need a much larger disk size that anyway folks doesn't run fstests with $((32768*65536*18))
Quoted 6 lines
tests/generic/468 _scratch_mkfs >/dev/null 2>&1 · line 38
Quoted 2 lines
tests/generic/468 +1
-0+fact=18This needs a bit more explanation -- why 18? I think the reason is that you need the fallocate to cross into another flexbg, and flexbgs (by default) are 16bg long, right?
That is right. Sure, I will add a comment explaining this, in the next revision.
Quoted 4 lines
If that's the case, then don't you need to detect the flexbg size so that this is still an effective test if someone runs fstests with MKFS_OPTIONS='-G 32' or something?
We can do that. But it is rare for anyone to test only with '-G 32' and not test with a default option. 32 might still be ok, but anything bigger then that might still require the test to not run due to requirement of disk size.
And plus I wanted to keep the test generic enough such that it covers the regression test with default fast_commit mkfs option.
-ritesh
Quoted 34 lines
--D
Quoted 31 lines
tests/generic/468 +2
-0+ testfile=$SCRATCH_MNT/testfile # check inode metadata after shutdown@@ -85,6 +88,7 @@ for i in fsync fdatasync; do test_falloc $i "-k " 1024 test_falloc $i "-k " 4096 test_falloc $i "-k " 104857600 + test_falloc $i "-k " $((32768*$blocksize*$fact)) done status=0tests/generic/468.out +2
-1@@ -5,9 +5,11 @@ QA output created by 468 ==== falloc -k 1024 test with fsync ==== ==== falloc -k 4096 test with fsync ==== ==== falloc -k 104857600 test with fsync ==== +==== falloc -k 2415919104 test with fsync ==== ==== falloc 1024 test with fdatasync ==== ==== falloc 4096 test with fdatasync ==== ==== falloc 104857600 test with fdatasync ==== ==== falloc -k 1024 test with fdatasync ==== ==== falloc -k 4096 test with fdatasync ==== ==== falloc -k 104857600 test with fdatasync ==== +==== falloc -k 2415919104 test with fdatasync ==== --2.31.1
Re: [PATCHv2 3/4] generic/676: Add a new shutdown recovery test
In reply to Darrick J. WongQuoted 34 lines · Darrick J. Wong
Quoted 33 lines · Ritesh Harjani
In certain cases (it is noted with ext4
fast_commitfeature) that, replay phase may not delete the right range of blocks (after sudden FS shutdown) due to some operations which depends oninode->i_size(which during replay of an inode withfast_commitcould be 0 for sometime).
This fstest is added to test for such scenarios for all generic fs.This test case is based on the test case shared via Xin Yin.
Signed-off-by Ritesh Harjani <[email protected]>--- tests/generic/676 | 72 +++++++++++++++++++++++++++++++++++++++++++ tests/generic/676.out | 7 +++++ 2 files changed, 79 insertions(+) create mode 100755 tests/generic/676 create mode 100644 tests/generic/676.outtests/generic/676 new file +11
-0@@ -0,0 +1,72 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2022 IBM Corporation. All Rights Reserved. +# +# FS QA Test 676 +# +# This test with ext4 fast_commit feature w/o below patch missed to delete the right +# range during replay phase, since it depends upon inode->i_size (which might not be +# stable during replay phase, at least for ext4). +# 0b5b5a62b945a141: ext4: use ext4_ext_remove_space() for fast commit replay delete range +# (Based on test case shared by Xin Yin <[email protected]>)
Quoted 8 lines
tests/generic/676 line 15
Quoted 4 lines
tests/generic/676 new file +4
-0+# + +. ./common/preamble +_begin_fstest auto shutdown quick log recoveryloopThis isn't a looping recovery test. Maybe we should create a 'recovery' group for tests that only run once? I think we already have a few fstests like that.
I gave it a thought, but I feel it might be unncessary.
From a developer/tester perspective who wanted to test anything related to recovery would then have to use both recovery and recoveryloop. Thoughts?
Quoted 10 lines
tests/generic/676 line 22
Quoted 8 lines
tests/generic/676 new file +7
-0+ +# Override the default cleanup function. +_cleanup() +{ + cd / + rm -r -f $tmp.* + _scratch_unmount > /dev/null 2>&1I think the test harness does this for you already, right?
Although, it looks like after running the test by default the run_section() in check script, will do _test_unmount and _scratch_unmount.
But I still feel it's better if the individual test cleans up whatever it did while running the test in it's cleanup routine, before exiting.
Quoted 18 lines
Quoted 5 lines
tests/generic/676 new file +4
-0+} + +# Import common functions. +. ./common/filtertests/generic/676 line 36
Quoted 10 lines
tests/generic/676 new file +10
-0+. ./common/punch + +# real QA test starts here + +# Modify as appropriate. +_supported_fs generic +_require_scratch +_require_xfs_io_command "fpunch" +_require_xfs_io_command "fzero" +_require_xfs_io_command "fiemap"
_require_scratch_shutdown
Quoted 12 lines
tests/generic/676 line 45
Quoted 9 lines
tests/generic/676 new file +9
-0+ +t1=$SCRATCH_MNT/foo +t2=$SCRATCH_MNT/bar + +_scratch_mkfs > $seqres.full 2>&1 + +_scratch_mount >> $seqres.full 2>&1 + +bs=$(_get_block_size $SCRATCH_MNT)
_get_file_block_size, in case the file allocation unit isn't the same as the fs blocksize? (e.g. bigalloc, xfs realtime, etc.)
Sure. Agreed. Will make the change.
Thanks
-ritesh
Re: [PATCHv2 4/4] generic/677: Add a test to check unwritten extents tracking
In reply to Darrick J. WongQuoted 59 lines · Darrick J. Wong
Quoted 58 lines · Ritesh Harjani
With these sequence of operation (in certain cases like with ext4
fast_commit) could miss to track unwritten extents during replay phase
(after sudden FS shutdown).This fstest adds a test case to test this.
5e4d0eba1ccaf19f
ext4: fix fast commit may miss tracking range forFALLOC_FL_ZERO_RANGESigned-off-by Ritesh Harjani <[email protected]>--- tests/generic/677 | 64 +++++++++++++++++++++++++++++++++++++++++++ tests/generic/677.out | 6 ++++ 2 files changed, 70 insertions(+) create mode 100755 tests/generic/677 create mode 100644 tests/generic/677.outtests/generic/677 new file +35
-0@@ -0,0 +1,64 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2022 IBM Corporation. All Rights Reserved. +# +# FS QA Test 677 +# +# Test below sequence of operation which (w/o below kernel patch) in case of +# ext4 with fast_commit may misss to track unwritten extents. +# commit 5e4d0eba1ccaf19f +# ext4: fix fast commit may miss tracking range for FALLOC_FL_ZERO_RANGE +# +. ./common/preamble +_begin_fstest auto quick log shutdown recoveryloop + +# Override the default cleanup function. +_cleanup() +{ + cd / + rm -r -f $tmp.* +} + +# Import common functions. +. ./common/filter +. ./common/punch + +# real QA test starts here + +# Modify as appropriate. +_supported_fs generic +_require_scratch +_require_xfs_io_command "fzero" +_require_xfs_io_command "fiemap" + +t1=$SCRATCH_MNT/t1 +
Quoted 8 lines
tests/generic/677 line 40
Quoted 5 lines
tests/generic/677 new file +5
-0+_scratch_mkfs > $seqres.full 2>&1 + +_scratch_mount >> $seqres.full 2>&1 + +bs=$(_get_block_size $SCRATCH_MNT)Same comments about blocksize, group names, and
_require_scratch_shutdownas the last patch.
Sure, I will fix blocksize and _require_scratch_shutdown part. I have added my thoughts above group names in previous patch.
Thanks for reviewing this.
-ritesh
Re: [PATCHv2 3/4] generic/676: Add a new shutdown recovery test
In reply to Ritesh HarjaniQuoted 44 lines · Ritesh Harjani
Quoted 34 lines · Darrick J. Wong
Quoted 33 lines · Ritesh Harjani
In certain cases (it is noted with ext4
fast_commitfeature) that, replay phase may not delete the right range of blocks (after sudden FS shutdown) due to some operations which depends oninode->i_size(which during replay of an inode withfast_commitcould be 0 for sometime).
This fstest is added to test for such scenarios for all generic fs.This test case is based on the test case shared via Xin Yin.
Signed-off-by Ritesh Harjani <[email protected]>--- tests/generic/676 | 72 +++++++++++++++++++++++++++++++++++++++++++ tests/generic/676.out | 7 +++++ 2 files changed, 79 insertions(+) create mode 100755 tests/generic/676 create mode 100644 tests/generic/676.outtests/generic/676 new file +11
-0@@ -0,0 +1,72 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2022 IBM Corporation. All Rights Reserved. +# +# FS QA Test 676 +# +# This test with ext4 fast_commit feature w/o below patch missed to delete the right +# range during replay phase, since it depends upon inode->i_size (which might not be +# stable during replay phase, at least for ext4). +# 0b5b5a62b945a141: ext4: use ext4_ext_remove_space() for fast commit replay delete range +# (Based on test case shared by Xin Yin <[email protected]>)Quoted 8 lines
tests/generic/676 line 15
Quoted 4 lines
tests/generic/676 new file +4
-0+# + +. ./common/preamble +_begin_fstest auto shutdown quick log recoveryloopThis isn't a looping recovery test. Maybe we should create a 'recovery' group for tests that only run once? I think we already have a few fstests like that.
Quoted 15 lines
I gave it a thought, but I feel it might be unncessary.
From a developer/tester perspective who wanted to test anything related to recovery would then have to use both recovery and recoveryloop. Thoughts?Quoted 10 lines
tests/generic/676 line 22
Quoted 8 lines
tests/generic/676 new file +7
-0+ +# Override the default cleanup function. +_cleanup() +{ + cd / + rm -r -f $tmp.* + _scratch_unmount > /dev/null 2>&1I think the test harness does this for you already, right?
Ok, I agree with this. I will remove _scratch_unmount operation from these two new tests in v3.
-ritesh