linux-kernel - Re: [f2fs-dev] [PATCH] f2fs: fix to avoid forcing direct write to use buffered IO on inline

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20241104033115epcms2p2867a532b5cdb012b1612931597cedf4a@epcms2p2>
Date: Mon, 04 Nov 2024 12:31:15 +0900
From: Jinsu Lee <jinsu1.lee@...sung.com>
To: "linux-f2fs-devel@...ts.sourceforge.net"
	<linux-f2fs-devel@...ts.sourceforge.net>
CC: "chao@...nel.org" <chao@...nel.org>, "jaegeuk@...nel.org"
	<jaegeuk@...nel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [f2fs-dev] [PATCH] f2fs: fix to avoid forcing direct write to
 use buffered IO on inline_data ino

> Jinsu Lee reported aperformance regression issue, after commit
> 5c8764f8679e ("f2fs: fixto force buffered IO on inline_data
> inode"), we forced directwrite to use buffered IO on inline_data
> inode, it will causeperformace regression due to memory copy
> and data flush.
 
> It's fine to not force directwrite to use buffered IO, as it
> can convert inline inodebefore committing direct write IO.
 
> Fixes: 5c8764f8679e("f2fs: fix to force buffered IO on inline_data inode")
> Reported-by: Jinsu Lee<jinsu1.lee@...sung.com>
> Closes:https://lore.kernel.org/linux-f2fs-devel/af03dd2c-e361-4f80-b2fd-39440766cf6e@kernel.org
> Signed-off-by: Chao Yu<chao@...nel.org>
> ---
> fs/f2fs/file.c 6 +++++-
> 1 file changed, 5insertions(+), 1 deletion(-)
 
> diff --git a/fs/f2fs/file.cb/fs/f2fs/file.c
> index0e7a0195eca8..377a10b81bf3 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -883,7 +883,11 @@ staticbool 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 bufferedIO, for direct write,
> +     * it expects inline data conversion beforecommitting IO.
> +     */
> +     if (f2fs_has_inline_data(inode) &&rw == READ)
 
Hi, 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. 

    commit 2b6bb0cd3bdcb1108189301ec4ec76c89f939310
    Author: Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@...ts.sourceforge.net>
    Date:   Mon Nov 4 09:35:51 2024 +0800

        [f2fs-dev] [PATCH v2] f2fs: fix to map blocks correctly for direct write


the reason for the improvement to commit (2b6bb0cd3bdc) seems to be because of the "m_may_create" condition.
when performing "io_submit" directly, so performance regression issue may occur.