1. Patchew
  2. linux
  3. View series

[PATCH v2] f2fs: fix to avoid racing in between read and OPU dio write

Chao Yu posted 1 patch 1 year, 5 months ago
There is a newer version of this series
fs/f2fs/file.c | 4 ++++
1 file changed, 4 insertions(+)
[PATCH v2] f2fs: fix to avoid racing in between read and OPU dio write
Posted by Chao Yu 1 year, 5 months ago 
If lfs mode is on, buffered read may race w/ OPU dio write as below,
it may cause buffered read hits unwritten data unexpectly, and for
dio read, the race condition exists as well.

Thread A			Thread B
- f2fs_file_write_iter
 - f2fs_dio_write_iter
  - __iomap_dio_rw
   - f2fs_iomap_begin
    - f2fs_map_blocks
     - __allocate_data_block
      - allocated blkaddr #x
       - iomap_dio_submit_bio
				- f2fs_file_read_iter
				 - filemap_read
				  - f2fs_read_data_folio
				   - f2fs_mpage_readpages
				    - f2fs_map_blocks
				     : get blkaddr #x
				    - f2fs_submit_read_bio
				IRQ
				- f2fs_read_end_io
				 : read IO on blkaddr #x complete
IRQ
- iomap_dio_bio_end_io
 : direct write IO on blkaddr #x complete

In LFS mode, if there is inflight dio, let's force read to buffered
IO, this policy won't cover all race cases, however it is a tradeoff
which avoids abusing lock around IO paths.

Fixes: f847c699cff3 ("f2fs: allow out-place-update for direct IO in LFS mode")
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/file.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 278573974db4..866f1a34e92b 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -882,6 +882,10 @@ static bool f2fs_force_buffered_io(struct inode *inode, int rw)
 		return true;
 	if (is_sbi_flag_set(sbi, SBI_CP_DISABLED))
 		return true;
+	/* In LFS mode, if there is inflight dio, force read to buffered IO */
+	if (rw == READ && f2fs_lfs_mode(sbi) &&
+			atomic_read(&inode->i_dio_count))
+		return false;
 
 	return false;
 }
-- 
2.40.1
Re: [f2fs-dev] [PATCH v2] f2fs: fix to avoid racing in between read and OPU dio write
Posted by Zhiguo Niu 1 year, 5 months ago 
Chao Yu <chao@kernel.org> 于2024年6月25日周二 22:29写道：
>
> If lfs mode is on, buffered read may race w/ OPU dio write as below,
> it may cause buffered read hits unwritten data unexpectly, and for
> dio read, the race condition exists as well.
>
> Thread A                        Thread B
> - f2fs_file_write_iter
>  - f2fs_dio_write_iter
>   - __iomap_dio_rw
>    - f2fs_iomap_begin
>     - f2fs_map_blocks
>      - __allocate_data_block
>       - allocated blkaddr #x
>        - iomap_dio_submit_bio
>                                 - f2fs_file_read_iter
>                                  - filemap_read
>                                   - f2fs_read_data_folio
>                                    - f2fs_mpage_readpages
>                                     - f2fs_map_blocks
>                                      : get blkaddr #x
>                                     - f2fs_submit_read_bio
>                                 IRQ
>                                 - f2fs_read_end_io
>                                  : read IO on blkaddr #x complete
> IRQ
> - iomap_dio_bio_end_io
>  : direct write IO on blkaddr #x complete
>
> In LFS mode, if there is inflight dio, let's force read to buffered
> IO, this policy won't cover all race cases, however it is a tradeoff
> which avoids abusing lock around IO paths.
>
> Fixes: f847c699cff3 ("f2fs: allow out-place-update for direct IO in LFS mode")
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>  fs/f2fs/file.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 278573974db4..866f1a34e92b 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -882,6 +882,10 @@ static bool f2fs_force_buffered_io(struct inode *inode, int rw)
>                 return true;
>         if (is_sbi_flag_set(sbi, SBI_CP_DISABLED))
>                 return true;
> +       /* In LFS mode, if there is inflight dio, force read to buffered IO */
> +       if (rw == READ && f2fs_lfs_mode(sbi) &&
> +                       atomic_read(&inode->i_dio_count))
> +               return false;
Hi Chao,
A little doubt:)，force “buffered IO” should return "true"?
another want to confirm is, "thread B" in commit msg just doing buffer
read, so this modification just cover direct read case？
thanks！
>
>         return false;
>  }
> --
> 2.40.1
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs: fix to avoid racing in between read and OPU dio write
Posted by Chao Yu 1 year, 5 months ago 
On 2024/6/26 10:01, Zhiguo Niu wrote:
> Chao Yu <chao@kernel.org> 于2024年6月25日周二 22:29写道：
>>
>> If lfs mode is on, buffered read may race w/ OPU dio write as below,
>> it may cause buffered read hits unwritten data unexpectly, and for
>> dio read, the race condition exists as well.
>>
>> Thread A                        Thread B
>> - f2fs_file_write_iter
>>   - f2fs_dio_write_iter
>>    - __iomap_dio_rw
>>     - f2fs_iomap_begin
>>      - f2fs_map_blocks
>>       - __allocate_data_block
>>        - allocated blkaddr #x
>>         - iomap_dio_submit_bio
>>                                  - f2fs_file_read_iter
>>                                   - filemap_read
>>                                    - f2fs_read_data_folio
>>                                     - f2fs_mpage_readpages
>>                                      - f2fs_map_blocks
>>                                       : get blkaddr #x
>>                                      - f2fs_submit_read_bio
>>                                  IRQ
>>                                  - f2fs_read_end_io
>>                                   : read IO on blkaddr #x complete
>> IRQ
>> - iomap_dio_bio_end_io
>>   : direct write IO on blkaddr #x complete
>>
>> In LFS mode, if there is inflight dio, let's force read to buffered
>> IO, this policy won't cover all race cases, however it is a tradeoff
>> which avoids abusing lock around IO paths.
>>
>> Fixes: f847c699cff3 ("f2fs: allow out-place-update for direct IO in LFS mode")
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>   fs/f2fs/file.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 278573974db4..866f1a34e92b 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -882,6 +882,10 @@ static bool f2fs_force_buffered_io(struct inode *inode, int rw)
>>                  return true;
>>          if (is_sbi_flag_set(sbi, SBI_CP_DISABLED))
>>                  return true;
>> +       /* In LFS mode, if there is inflight dio, force read to buffered IO */
>> +       if (rw == READ && f2fs_lfs_mode(sbi) &&
>> +                       atomic_read(&inode->i_dio_count))
>> +               return false;
> Hi Chao,
> A little doubt:)，force “buffered IO” should return "true"?

Oops, too rush to send the patch...

> another want to confirm is, "thread B" in commit msg just doing buffer
> read, so this modification just cover direct read case？

Oh, the fix is incorrect, will look into it soon.

Thanks,

> thanks！
>>
>>          return false;
>>   }
>> --
>> 2.40.1
>>
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.