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

[이진수]

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