All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] generic/392: support to test fallocate
@ 2017-11-05 12:39 Chao Yu
  2017-11-07  4:48 ` Eryu Guan
  0 siblings, 1 reply; 3+ messages in thread
From: Chao Yu @ 2017-11-05 12:39 UTC (permalink / raw)
  To: fstests, eguan; +Cc: jaegeuk, chao, Chao Yu

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>
---
 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"\
+			$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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] generic/392: support to test fallocate
  2017-11-05 12:39 [PATCH] generic/392: support to test fallocate Chao Yu
@ 2017-11-07  4:48 ` Eryu Guan
  2017-11-07 15:29   ` Chao Yu
  0 siblings, 1 reply; 3+ messages in thread
From: Eryu Guan @ 2017-11-07  4:48 UTC (permalink / raw)
  To: Chao Yu; +Cc: fstests, jaegeuk, Chao Yu

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
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] generic/392: support to test fallocate
  2017-11-07  4:48 ` Eryu Guan
@ 2017-11-07 15:29   ` Chao Yu
  0 siblings, 0 replies; 3+ messages in thread
From: Chao Yu @ 2017-11-07 15:29 UTC (permalink / raw)
  To: Eryu Guan; +Cc: fstests, jaegeuk, Chao Yu

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-11-07 15:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-05 12:39 [PATCH] generic/392: support to test fallocate Chao Yu
2017-11-07  4:48 ` Eryu Guan
2017-11-07 15:29   ` Chao Yu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.