diff mbox series

f2fs: test for race condition in between atomic_write and dio

Message ID 20240625140233.3907012-1-chao@kernel.org (mailing list archive)
State New, archived
Headers show
Series f2fs: test for race condition in between atomic_write and dio | expand

Commit Message

Chao Yu June 25, 2024, 2:02 p.m. UTC
In this patch, we simulate race case in between sqlite atomic write
and direct IO w/ below steps:
1. create a regular file, and initialize it w/ 0xff data
2. start transaction (via F2FS_IOC_START_ATOMIC_WRITE) on it
3. write transaction data
4. trigger direct read/write IO on it to check whether it fails or not
5. commit and end the transaction (via F2FS_IOC_COMMIT_ATOMIC_WRITE)
This is a regression test to check handling of race condition in
between atomic_write and direct IO.

Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: Daeho Jeong <daehojeong@google.com>
Signed-off-by: Chao Yu <chao@kernel.org>
---
 tests/f2fs/004     | 45 +++++++++++++++++++++++++++++++++++++++++++++
 tests/f2fs/004.out |  3 +++
 2 files changed, 48 insertions(+)
 create mode 100755 tests/f2fs/004
 create mode 100644 tests/f2fs/004.out

Comments

Zorro Lang July 16, 2024, 11:03 a.m. UTC | #1
On Tue, Jun 25, 2024 at 10:02:33PM +0800, Chao Yu wrote:
> In this patch, we simulate race case in between sqlite atomic write
> and direct IO w/ below steps:
> 1. create a regular file, and initialize it w/ 0xff data
> 2. start transaction (via F2FS_IOC_START_ATOMIC_WRITE) on it
> 3. write transaction data
> 4. trigger direct read/write IO on it to check whether it fails or not
> 5. commit and end the transaction (via F2FS_IOC_COMMIT_ATOMIC_WRITE)
> This is a regression test to check handling of race condition in
> between atomic_write and direct IO.
> 
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> Cc: Daeho Jeong <daehojeong@google.com>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>  tests/f2fs/004     | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/f2fs/004.out |  3 +++
>  2 files changed, 48 insertions(+)
>  create mode 100755 tests/f2fs/004
>  create mode 100644 tests/f2fs/004.out
> 
> diff --git a/tests/f2fs/004 b/tests/f2fs/004
> new file mode 100755
> index 00000000..26862cb5
> --- /dev/null
> +++ b/tests/f2fs/004
> @@ -0,0 +1,45 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Oppo.  All Rights Reserved.
> +#
> +# FS QA Test No. f2fs/004
> +#
> +# Test that we will simulate race case in between sqlite atomic write
> +# and direct IO w/ below steps:
> +# 1. create a regular file, and initialize it w/ 0xff data
> +# 2. start transaction (via F2FS_IOC_START_ATOMIC_WRITE) on it
> +# 3. write transaction data
> +# 4. trigger direct read/write IO to check whether it fails or not
> +# 5. commit and end the transaction (via F2FS_IOC_COMMIT_ATOMIC_WRITE)
> +# This is a regression test to check handling of race condition in
> +# between atomic_write and direct IO.

Same as last patch, please add _fixed_by_kernel_commit.

> +#
> +. ./common/preamble
> +_begin_fstest auto quick
> +
> +. ./common/filter
> +
> +_supported_fs f2fs
   ^^^^^
It's not needed

> +_require_scratch
> +_require_xfs_io_command "pwrite"
   ^^^^
It can be removed too I think.

> +
> +_scratch_mkfs >> $seqres.full
> +_scratch_mount >> $seqres.full
> +
> +dbfile=$SCRATCH_MNT/dbfile
> +
> +# start atomic_write on dbfile & write data to dbfile
> +touch $dbfile
> +$F2FS_IO_PROG write 1 0 32 zero atomic_commit $dbfile 3000 >> $seqres.full &

There's a backgroup process too. Better to make sure it's done "kill && wait"
before the end of the test.

> +
> +# simulate concurrent direct read/write IO
> +$XFS_IO_PROG -d -c "pread 0 128k" $dbfile
> +$XFS_IO_PROG -d -c "pwrite 0 128k" $dbfile

Do you need _require_odirect (not sure about that:).

> +
> +# wait for atomic_write commit completion
> +sleep 5
> +
> +_scratch_unmount
   ^^^^
It's not necessary.

Thanks,
Zorro

> +
> +status=0
> +exit
> diff --git a/tests/f2fs/004.out b/tests/f2fs/004.out
> new file mode 100644
> index 00000000..3af79541
> --- /dev/null
> +++ b/tests/f2fs/004.out
> @@ -0,0 +1,3 @@
> +QA output created by 004
> +pread: Operation not supported
> +pwrite: Operation not supported
> -- 
> 2.40.1
> 
>
Chao Yu July 17, 2024, 2:15 a.m. UTC | #2
On 2024/7/16 19:03, Zorro Lang wrote:
> On Tue, Jun 25, 2024 at 10:02:33PM +0800, Chao Yu wrote:
>> In this patch, we simulate race case in between sqlite atomic write
>> and direct IO w/ below steps:
>> 1. create a regular file, and initialize it w/ 0xff data
>> 2. start transaction (via F2FS_IOC_START_ATOMIC_WRITE) on it
>> 3. write transaction data
>> 4. trigger direct read/write IO on it to check whether it fails or not
>> 5. commit and end the transaction (via F2FS_IOC_COMMIT_ATOMIC_WRITE)
>> This is a regression test to check handling of race condition in
>> between atomic_write and direct IO.
>>
>> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
>> Cc: Daeho Jeong <daehojeong@google.com>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>   tests/f2fs/004     | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>   tests/f2fs/004.out |  3 +++
>>   2 files changed, 48 insertions(+)
>>   create mode 100755 tests/f2fs/004
>>   create mode 100644 tests/f2fs/004.out
>>
>> diff --git a/tests/f2fs/004 b/tests/f2fs/004
>> new file mode 100755
>> index 00000000..26862cb5
>> --- /dev/null
>> +++ b/tests/f2fs/004
>> @@ -0,0 +1,45 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2024 Oppo.  All Rights Reserved.
>> +#
>> +# FS QA Test No. f2fs/004
>> +#
>> +# Test that we will simulate race case in between sqlite atomic write
>> +# and direct IO w/ below steps:
>> +# 1. create a regular file, and initialize it w/ 0xff data
>> +# 2. start transaction (via F2FS_IOC_START_ATOMIC_WRITE) on it
>> +# 3. write transaction data
>> +# 4. trigger direct read/write IO to check whether it fails or not
>> +# 5. commit and end the transaction (via F2FS_IOC_COMMIT_ATOMIC_WRITE)
>> +# This is a regression test to check handling of race condition in
>> +# between atomic_write and direct IO.
> 
> Same as last patch, please add _fixed_by_kernel_commit.
> 
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto quick
>> +
>> +. ./common/filter
>> +
>> +_supported_fs f2fs
>     ^^^^^
> It's not needed
> 
>> +_require_scratch
>> +_require_xfs_io_command "pwrite"
>     ^^^^
> It can be removed too I think.
> 
>> +
>> +_scratch_mkfs >> $seqres.full
>> +_scratch_mount >> $seqres.full
>> +
>> +dbfile=$SCRATCH_MNT/dbfile
>> +
>> +# start atomic_write on dbfile & write data to dbfile
>> +touch $dbfile
>> +$F2FS_IO_PROG write 1 0 32 zero atomic_commit $dbfile 3000 >> $seqres.full &
> 
> There's a backgroup process too. Better to make sure it's done "kill && wait"
> before the end of the test.
> 
>> +
>> +# simulate concurrent direct read/write IO
>> +$XFS_IO_PROG -d -c "pread 0 128k" $dbfile
>> +$XFS_IO_PROG -d -c "pwrite 0 128k" $dbfile
> 
> Do you need _require_odirect (not sure about that:).
> 
>> +
>> +# wait for atomic_write commit completion
>> +sleep 5
>> +
>> +_scratch_unmount
>     ^^^^
> It's not necessary.

Zorro, thanks for the comments, will update.

Thanks,

> 
> Thanks,
> Zorro
> 
>> +
>> +status=0
>> +exit
>> diff --git a/tests/f2fs/004.out b/tests/f2fs/004.out
>> new file mode 100644
>> index 00000000..3af79541
>> --- /dev/null
>> +++ b/tests/f2fs/004.out
>> @@ -0,0 +1,3 @@
>> +QA output created by 004
>> +pread: Operation not supported
>> +pwrite: Operation not supported
>> -- 
>> 2.40.1
>>
>>
>
diff mbox series

Patch

diff --git a/tests/f2fs/004 b/tests/f2fs/004
new file mode 100755
index 00000000..26862cb5
--- /dev/null
+++ b/tests/f2fs/004
@@ -0,0 +1,45 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Oppo.  All Rights Reserved.
+#
+# FS QA Test No. f2fs/004
+#
+# Test that we will simulate race case in between sqlite atomic write
+# and direct IO w/ below steps:
+# 1. create a regular file, and initialize it w/ 0xff data
+# 2. start transaction (via F2FS_IOC_START_ATOMIC_WRITE) on it
+# 3. write transaction data
+# 4. trigger direct read/write IO to check whether it fails or not
+# 5. commit and end the transaction (via F2FS_IOC_COMMIT_ATOMIC_WRITE)
+# This is a regression test to check handling of race condition in
+# between atomic_write and direct IO.
+#
+. ./common/preamble
+_begin_fstest auto quick
+
+. ./common/filter
+
+_supported_fs f2fs
+_require_scratch
+_require_xfs_io_command "pwrite"
+
+_scratch_mkfs >> $seqres.full
+_scratch_mount >> $seqres.full
+
+dbfile=$SCRATCH_MNT/dbfile
+
+# start atomic_write on dbfile & write data to dbfile
+touch $dbfile
+$F2FS_IO_PROG write 1 0 32 zero atomic_commit $dbfile 3000 >> $seqres.full &
+
+# simulate concurrent direct read/write IO
+$XFS_IO_PROG -d -c "pread 0 128k" $dbfile
+$XFS_IO_PROG -d -c "pwrite 0 128k" $dbfile
+
+# wait for atomic_write commit completion
+sleep 5
+
+_scratch_unmount
+
+status=0
+exit
diff --git a/tests/f2fs/004.out b/tests/f2fs/004.out
new file mode 100644
index 00000000..3af79541
--- /dev/null
+++ b/tests/f2fs/004.out
@@ -0,0 +1,3 @@ 
+QA output created by 004
+pread: Operation not supported
+pwrite: Operation not supported