| Message ID | 20171105123935.14702-1-chao@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Nov 05, 2017 at 08:39:35PM +0800, Chao Yu wrote: > From: Chao Yu <yuchao0@huawei.com> > > f2fs can skip isize updating in fsync(), since during mount, f2fs tries > to recovery isize according to valid block address or preallocated flag > in last fsynced dnode block. > > However, fallocate() breaks our rule with setting FALLOC_FL_KEEP_SIZE > flag, since it can preallocated block cross EOF, once the file is fsynced, > in POR, we will recover isize incorrectly based on these fallocated > blocks. > > This patch enables generic/392 to test fallocate case, in order to verify > whether filesystem will do incorrect recovery on isize. > > Signed-off-by: Chao Yu <yuchao0@huawei.com> Usually we don't add new sub-tests to existing tests, so new failures introduced by the new sub-tests won't be treated as false regressions. Please add a new test instead. (I notice the new falloc test fails on ext4 too, the allocated block counts changed after power cycle). > --- > tests/generic/392 | 14 ++++++++++++++ > tests/generic/392.out | 6 ++++++ > 2 files changed, 20 insertions(+) > > diff --git a/tests/generic/392 b/tests/generic/392 > index 6922f7d2..f4ebeb2f 100755 > --- a/tests/generic/392 > +++ b/tests/generic/392 > @@ -125,12 +125,26 @@ test_punch() > check_inode_metadata $1 > } > > +test_falloc() > +{ > + echo "==== falloc $2 test with $1 ====" | tee -a $seqres.full > + $XFS_IO_PROG -f -c "truncate 4202496" \ > + -c "pwrite 0 4202496" \ > + -c "fsync" \ > + -c "falloc -k 4202496 $2"\ Need to require falloc -k support by _require_xfs_io_command "falloc" "-k" And maybe test falloc without keep_size too? Thanks, Eryu > + $testfile >/dev/null > + check_inode_metadata $1 > +} > + > for i in fsync fdatasync; do > test_i_size $i 1024 > test_i_size $i 4096 > test_i_time $i > test_punch $i 1024 > test_punch $i 4096 > + test_falloc $i 1024 > + test_falloc $i 4096 > + test_falloc $i 104857600 > done > > exit > diff --git a/tests/generic/392.out b/tests/generic/392.out > index 5d3330a6..d278bdf9 100644 > --- a/tests/generic/392.out > +++ b/tests/generic/392.out > @@ -4,8 +4,14 @@ QA output created by 392 > ==== i_time test with fsync ==== > ==== fpunch 1024 test with fsync ==== > ==== fpunch 4096 test with fsync ==== > +==== falloc 1024 test with fsync ==== > +==== falloc 4096 test with fsync ==== > +==== falloc 104857600 test with fsync ==== > ==== i_size 1024 test with fdatasync ==== > ==== i_size 4096 test with fdatasync ==== > ==== i_time test with fdatasync ==== > ==== fpunch 1024 test with fdatasync ==== > ==== fpunch 4096 test with fdatasync ==== > +==== falloc 1024 test with fdatasync ==== > +==== falloc 4096 test with fdatasync ==== > +==== falloc 104857600 test with fdatasync ==== > -- > 2.14.1.145.gb3622a4ee > -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2017/11/7 12:48, Eryu Guan wrote: > On Sun, Nov 05, 2017 at 08:39:35PM +0800, Chao Yu wrote: >> From: Chao Yu <yuchao0@huawei.com> >> >> f2fs can skip isize updating in fsync(), since during mount, f2fs tries >> to recovery isize according to valid block address or preallocated flag >> in last fsynced dnode block. >> >> However, fallocate() breaks our rule with setting FALLOC_FL_KEEP_SIZE >> flag, since it can preallocated block cross EOF, once the file is fsynced, >> in POR, we will recover isize incorrectly based on these fallocated >> blocks. >> >> This patch enables generic/392 to test fallocate case, in order to verify >> whether filesystem will do incorrect recovery on isize. >> >> Signed-off-by: Chao Yu <yuchao0@huawei.com> > > Usually we don't add new sub-tests to existing tests, so new failures > introduced by the new sub-tests won't be treated as false regressions. > Please add a new test instead. (I notice the new falloc test fails on > ext4 too, the allocated block counts changed after power cycle). Ah, make sense to me, I will prepare patch to add a new test. > >> --- >> tests/generic/392 | 14 ++++++++++++++ >> tests/generic/392.out | 6 ++++++ >> 2 files changed, 20 insertions(+) >> >> diff --git a/tests/generic/392 b/tests/generic/392 >> index 6922f7d2..f4ebeb2f 100755 >> --- a/tests/generic/392 >> +++ b/tests/generic/392 >> @@ -125,12 +125,26 @@ test_punch() >> check_inode_metadata $1 >> } >> >> +test_falloc() >> +{ >> + echo "==== falloc $2 test with $1 ====" | tee -a $seqres.full >> + $XFS_IO_PROG -f -c "truncate 4202496" \ >> + -c "pwrite 0 4202496" \ >> + -c "fsync" \ >> + -c "falloc -k 4202496 $2"\ > > Need to require falloc -k support by > > _require_xfs_io_command "falloc" "-k" Got it, thanks for the reminder. > > And maybe test falloc without keep_size too? Agreed, will add. :) Thanks, > > Thanks, > Eryu > >> + $testfile >/dev/null >> + check_inode_metadata $1 >> +} >> + >> for i in fsync fdatasync; do >> test_i_size $i 1024 >> test_i_size $i 4096 >> test_i_time $i >> test_punch $i 1024 >> test_punch $i 4096 >> + test_falloc $i 1024 >> + test_falloc $i 4096 >> + test_falloc $i 104857600 >> done >> >> exit >> diff --git a/tests/generic/392.out b/tests/generic/392.out >> index 5d3330a6..d278bdf9 100644 >> --- a/tests/generic/392.out >> +++ b/tests/generic/392.out >> @@ -4,8 +4,14 @@ QA output created by 392 >> ==== i_time test with fsync ==== >> ==== fpunch 1024 test with fsync ==== >> ==== fpunch 4096 test with fsync ==== >> +==== falloc 1024 test with fsync ==== >> +==== falloc 4096 test with fsync ==== >> +==== falloc 104857600 test with fsync ==== >> ==== i_size 1024 test with fdatasync ==== >> ==== i_size 4096 test with fdatasync ==== >> ==== i_time test with fdatasync ==== >> ==== fpunch 1024 test with fdatasync ==== >> ==== fpunch 4096 test with fdatasync ==== >> +==== falloc 1024 test with fdatasync ==== >> +==== falloc 4096 test with fdatasync ==== >> +==== falloc 104857600 test with fdatasync ==== >> -- >> 2.14.1.145.gb3622a4ee >> -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tests/generic/392 b/tests/generic/392 index 6922f7d2..f4ebeb2f 100755 --- a/tests/generic/392 +++ b/tests/generic/392 @@ -125,12 +125,26 @@ test_punch() check_inode_metadata $1 } +test_falloc() +{ + echo "==== falloc $2 test with $1 ====" | tee -a $seqres.full + $XFS_IO_PROG -f -c "truncate 4202496" \ + -c "pwrite 0 4202496" \ + -c "fsync" \ + -c "falloc -k 4202496 $2"\ + $testfile >/dev/null + check_inode_metadata $1 +} + for i in fsync fdatasync; do test_i_size $i 1024 test_i_size $i 4096 test_i_time $i test_punch $i 1024 test_punch $i 4096 + test_falloc $i 1024 + test_falloc $i 4096 + test_falloc $i 104857600 done exit diff --git a/tests/generic/392.out b/tests/generic/392.out index 5d3330a6..d278bdf9 100644 --- a/tests/generic/392.out +++ b/tests/generic/392.out @@ -4,8 +4,14 @@ QA output created by 392 ==== i_time test with fsync ==== ==== fpunch 1024 test with fsync ==== ==== fpunch 4096 test with fsync ==== +==== falloc 1024 test with fsync ==== +==== falloc 4096 test with fsync ==== +==== falloc 104857600 test with fsync ==== ==== i_size 1024 test with fdatasync ==== ==== i_size 4096 test with fdatasync ==== ==== i_time test with fdatasync ==== ==== fpunch 1024 test with fdatasync ==== ==== fpunch 4096 test with fdatasync ==== +==== falloc 1024 test with fdatasync ==== +==== falloc 4096 test with fdatasync ==== +==== falloc 104857600 test with fdatasync ====