[f2fs-dev] [PATCH] f2fs: fix to force buffered IO on inline_data inode

[f2fs-dev] [PATCH] f2fs: fix to force buffered IO on inline_data inode

[Chao Yu]

It will return all zero data when DIO reading from inline_data inode, it
is because f2fs_iomap_begin() assign iomap->type w/ IOMAP_HOLE incorrectly
for this case.

We can let iomap framework handle inline data via assigning iomap->type
and iomap->inline_data correctly, however, it will be a little bit
complicated when handling race case in between direct IO and buffered IO.

So, let's force to use buffered IO to fix this issue.

Cc: stable@vger.kernel.org
Reported-by: Barry Song <v-songbaohua@oppo.com>
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/file.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index db6236f27852..e038910ad1e5 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -851,6 +851,8 @@ static bool f2fs_force_buffered_io(struct inode *inode, int rw)
 		return true;
 	if (f2fs_compressed_file(inode))
 		return true;
+	if (f2fs_has_inline_data(inode))
+		return true;
 
 	/* disallow direct IO if any of devices has unaligned blksize */
 	if (f2fs_is_multi_device(sbi) && !sbi->aligned_blksize)
-- 
2.40.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[PATCH] f2fs: fix to force buffered IO on inline_data inode

[Chao Yu]

It will return all zero data when DIO reading from inline_data inode, it
is because f2fs_iomap_begin() assign iomap->type w/ IOMAP_HOLE incorrectly
for this case.

We can let iomap framework handle inline data via assigning iomap->type
and iomap->inline_data correctly, however, it will be a little bit
complicated when handling race case in between direct IO and buffered IO.

So, let's force to use buffered IO to fix this issue.

Cc: stable@vger.kernel.org
Reported-by: Barry Song <v-songbaohua@oppo.com>
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/file.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index db6236f27852..e038910ad1e5 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -851,6 +851,8 @@ static bool f2fs_force_buffered_io(struct inode *inode, int rw)
 		return true;
 	if (f2fs_compressed_file(inode))
 		return true;
+	if (f2fs_has_inline_data(inode))
+		return true;
 
 	/* disallow direct IO if any of devices has unaligned blksize */
 	if (f2fs_is_multi_device(sbi) && !sbi->aligned_blksize)
-- 
2.40.1

Re: [f2fs-dev] [PATCH] f2fs: fix to force buffered IO on inline_data inode

[patchwork-bot+f2fs]

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:

On Thu, 23 May 2024 21:29:48 +0800 you wrote:
> It will return all zero data when DIO reading from inline_data inode, it
> is because f2fs_iomap_begin() assign iomap->type w/ IOMAP_HOLE incorrectly
> for this case.
> 
> We can let iomap framework handle inline data via assigning iomap->type
> and iomap->inline_data correctly, however, it will be a little bit
> complicated when handling race case in between direct IO and buffered IO.
> 
> [...]

Here is the summary with links:
  - [f2fs-dev] f2fs: fix to force buffered IO on inline_data inode
    https://git.kernel.org/jaegeuk/f2fs/c/5c8764f8679e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html




_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: fix to force buffered IO on inline_data inode

[patchwork-bot+f2fs]

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:

On Thu, 23 May 2024 21:29:48 +0800 you wrote:
> It will return all zero data when DIO reading from inline_data inode, it
> is because f2fs_iomap_begin() assign iomap->type w/ IOMAP_HOLE incorrectly
> for this case.
> 
> We can let iomap framework handle inline data via assigning iomap->type
> and iomap->inline_data correctly, however, it will be a little bit
> complicated when handling race case in between direct IO and buffered IO.
> 
> [...]

Here is the summary with links:
  - [f2fs-dev] f2fs: fix to force buffered IO on inline_data inode
    https://git.kernel.org/jaegeuk/f2fs/c/5c8764f8679e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [f2fs-dev] [PATCH] f2fs: fix to force buffered IO on inline_data inode

[Chao Yu via Linux-f2fs-devel]

Hi jinsu,

+Cc f2fs mailing list

Thank you for the report.

Can you please try below patch?

 From 5aeb87cffa73f52b29c7e8dcc3729337f925bb60 Mon Sep 17 00:00:00 2001
From: Chao Yu <chao@kernel.org>
Date: Fri, 1 Nov 2024 11:18:57 +0800
Subject: [PATCH] fix

---
  fs/f2fs/file.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 0e7a0195eca8..f656ca3cc61c 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -883,7 +883,7 @@ static bool f2fs_force_buffered_io(struct inode *inode, int rw)
  		return true;
  	if (f2fs_compressed_file(inode))
  		return true;
-	if (f2fs_has_inline_data(inode))
+	if (f2fs_has_inline_data(inode) && rw == READ)
  		return true;

  	/* disallow direct IO if any of devices has unaligned blksize */
-- 
2.40.1



On 2024/10/31 13:09, 이진수 wrote:
> From: jinsu LEE <jinsu1.lee@samsung.com>
> 
> To: Chao Yu <chao@kernel.org>
> 
> cc: jaegeuk@kernel.org, v-songbaohua@oppo.com
> 
> 
> Hi Caho Yu
> 
> I'm Jinsu Lee and I work on memory(UFS) at Samsung Electronics.
> 
> 
> I'm sending you an e-mail because I have an inquiry about the f2fs commit below.
> 
> https://lore.kernel.org/linux-f2fs-devel/20240523132948.2250254-1-chao@kernel.org/#R <https://lore.kernel.org/linux-f2fs-devel/20240523132948.2250254-1-chao@kernel.org/#R>
> 
> 
>  From the commit message, it seems to be a modification to fix race case IOMAP_HOLE(all zero) using direct io.
> 
> However, in the v6.6-kernel that includes this commit, the fio sequential write performance is very low.
> 
> ​(v6.1-kernel) about 3900MB/s or more
> 
> (v6.6-kernel) about 800MB/s or less
> 
> 
> Due to the corresponding commit, "f2fs_buffered_write_iter" is being performed instead of "f2fs_dio_write_iter". (Red box part of perf call stack below)
> 
> I wonder if you know about this part and if there is any way to modify IOMAP_HOLE so as not to affect the performance of direct io.
> 
> 
>> perf data during fio test (v6.6 kernel with your commit)
> 
> fio DIO sequential write 800MB/s (very slow)
> 
> The part of the red box below is an action that did not exist in the previous kernel and is becoming a factor that reduces DIO seq write performance.
> 
> In this case, "f2fs_file_write_iter" takes much longer than the previous kernel.
> 
> 
>> perf data during fio test (v6.1 kernel)
> 
> fio DIO sequential write 3900MB/s
> 
> There is no call for the "f2fs_buffered_write_iter" or "generic_perform_write" function and "f2f2_file_write_iter" more fast and there is no problem with DIO seq write performance.
> 
> ​
> 
> Thank you
> 
> 
> 



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel