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

[Yangtao Li]

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@xxxxxxxx>
> > Signed-off-by: Yangtao Li <frank.li@xxxxxxxx>
> > ---
> >   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?


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 */