Re: [f2fs-dev] [PATCH] f2fs: compress: don't force buffered io when in COMPR_MODE_USER mode

[Chao Yu <chao@kernel.org>]

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@vivo.com>
>>> Signed-off-by: Yangtao Li <frank.li@vivo.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 */


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