Re: [f2fs-dev] [PATCH] f2fs: fix to avoid forcing direct write to use buffered IO on inline_data inode

From: 이진수
Date: Mon Nov 04 2024 - 04:44:31 EST


>>> Jinsu Lee reported a performance regression issue, after commit
>>
>>> 5c8764f8679e ("f2fs: fix to force buffered IO on inline_data
>>
>>> inode"), we forced direct write to use buffered IO on inline_data
>>
>>> inode, it will cause performace regression due to memory copy
>>
>>> and data flush.
>>
>>
>>> It's fine to not force direct write to use buffered IO, as it
>>
>>> can convert inline inode before committing direct write IO.
>>
>>
>>>> Fixes: 5c8764f8679e ("f2fs: fix to force buffered IO on inline_data inode")
>>
>>> Reported-by: Jinsu Lee <jinsu1.lee@xxxxxxxxxxx>
>>
>>> Closes: https://lore.kernel.org/linux-f2fs-devel/af03dd2c-e361-4f80-b2fd-39440766cf6e@xxxxxxxxxx
>>
>>> Signed-off-by: Chao Yu <chao@xxxxxxxxxx>
>>
>>> ---
>>
>>> fs/f2fs/file.c | 6 +++++-
>>
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>>
>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>
>>> index 0e7a0195eca8..377a10b81bf3 100644
>>
>>> --- a/fs/f2fs/file.c
>>
>>> +++ b/fs/f2fs/file.c
>>
>>> @@ -883,7 +883,11 @@ 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))
>>
>>> + /*
>>
>>> + * only force direct read to use buffered IO, for direct write,
>>
>>> + * it expects inline data conversion before committing IO.
>>
>>> + */
>>
>>> + if (f2fs_has_inline_data(inode) && rw == READ)
>>
>>
>> Chao Yu,
>>
>> The fio direct performance problem I reported did not improve when reflecting this commit.
>>
>> Rather, it has been improved when reflecting your commit below.
>>
>>
>> The previous commit seems to be mistitled as read and the following commit appears to be the final version.
>>
>> The reason for the improvement with the commit below is that there is no "m_may_create" condition
>>
>> when performing "io_submit" directly, so performance regression issue may occur.
>>
>>
>> I wonder if "rw == READ" should be additionally reflected.
>
> Alright, thanks for your feedback.
>
> I thought you have bisected this performance issue to commit
> 5c8764f8679e ("f2fs: fix to force buffered IO on inline_data inode"),
> so I sent this patch for comments.
>
> Can you please apply both below dio fixes, and help to check final
> performance?
>
> https://lore.kernel.org/linux-f2fs-devel/20241104015016.228963-1-chao@xxxxxxxxxx
> https://lore.kernel.org/linux-f2fs-devel/20241104013551.218037-1-chao@xxxxxxxxxx
>
> Thanks,

Chao Yu,
After reflecting the following two commits, the fio DIO seq write operates with normal performance.

https://lore.kernel.org/linux-f2fs-devel/20241104015016.228963-1-chao@xxxxxxxxxx
https://lore.kernel.org/linux-f2fs-devel/20241104013551.218037-1-chao@xxxxxxxxxx

However, Antutu Apk's "AI READ" performance has more than tripled compared to before patch reflection, so it seems necessary to check if there is any problem with DIO performance.