linux-kernel - Re: [PATCH] f2fs: compress: don't force buffered io when in COMPR_MODE

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7ab6b6f9-37fa-9bf2-69ce-7b1b1944d9f3@kernel.org>
Date:   Mon, 19 Jun 2023 08:54:33 +0800
From:   Chao Yu <chao@...nel.org>
To:     Yangtao Li <frank.li@...o.com>, Jaegeuk Kim <jaegeuk@...nel.org>
Cc:     Qi Han <hanqi@...o.com>, linux-f2fs-devel@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] f2fs: compress: don't force buffered io when in
 COMPR_MODE_USER mode

On 2023/6/13 12:14, Yangtao Li wrote:
> 
> On 2023/6/12 22:38, Chao Yu wrote:
>> On 2023/6/9 21:15, Yangtao Li wrote:
>>> It is observed that when in user compression mode
>>> (compress_extension=*),
>>> even though the file is not compressed, the file is still forced to use
>>> buffer io, which makes the AndroBench sequential read and write drop
>>> significantly. In fact, when the file is not compressed, we don't need
>>> to force it to buffer io.
>>>
>>>                    | w/o patch | w/ patch |
>>> seq read  (MB/s) | 1320.068  | 3696.154 |
>>> seq write (MB/s) | 617.996   | 2978.478 |
>>>
>>> Fixes: 4c8ff7095bef ("f2fs: support data compression")
>>> Signed-off-by: Qi Han <hanqi@...o.com>
>>> Signed-off-by: Yangtao Li <frank.li@...o.com>
>>> ---
>>>    fs/f2fs/f2fs.h | 14 ++++++++++++++
>>>    fs/f2fs/file.c |  2 +-
>>>    2 files changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 1efcfd9e5a99..7f5472525310 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -3168,6 +3168,20 @@ static inline int f2fs_compressed_file(struct
>>> inode *inode)
>>>            is_inode_flag_set(inode, FI_COMPRESSED_FILE);
>>>    }
>>>    +static inline bool f2fs_is_compressed_file(struct inode *inode)
>>> +{
>>> +    int compress_mode = F2FS_OPTION(F2FS_I_SB(inode)).compress_mode;
>>> +
>>> +    if (compress_mode == COMPR_MODE_FS)
>>> +        return f2fs_compressed_file(inode);
>>> +    else if (atomic_read(&F2FS_I(inode)->i_compr_blocks) ||
>>
>> Should check dirty page as well? i_compr_blocks may increase after
>> data writeback.
>>
> IIUC, in COMPR_MODE_USER mode, i_compr_blocks will only be updated when
> FI_ENABLE_COMPRESS is enabled.
> 
> If FI_ENABLE_COMPRESS is not enabled, i_compr_blocks will never be
> updated after data writeback.
> 
> So there is no need to additionally judge whether there is a dirty page?

Oh, user mode, that's correct.

If we allow dio/aio on compress file, it needs to consider race case in
between aio and ioc_compress_file.

Thanks,

> 
> 
> Thanks,
> 
>> Thanks,
>>
>>> +        is_inode_flag_set(inode, FI_COMPRESS_RELEASED) ||
>>> +        is_inode_flag_set(inode, FI_ENABLE_COMPRESS))
>>> +        return true;
>>> +
>>> +    return false;
>>> +}
>>> +
>>>    static inline bool f2fs_need_compress_data(struct inode *inode)
>>>    {
>>>        int compress_mode = F2FS_OPTION(F2FS_I_SB(inode)).compress_mode;
>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>> index 74ecc9e20619..0698129b2165 100644
>>> --- a/fs/f2fs/file.c
>>> +++ b/fs/f2fs/file.c
>>> @@ -821,7 +821,7 @@ static bool f2fs_force_buffered_io(struct inode
>>> *inode, int rw)
>>>            return true;
>>>        if (fsverity_active(inode))
>>>            return true;
>>> -    if (f2fs_compressed_file(inode))
>>> +    if (f2fs_is_compressed_file(inode))
>>>            return true;
>>>          /* disallow direct IO if any of devices has unaligned blksize */