On Mon, Apr 26, 2021 at 04:42:20PM +0800, changfeng...@vivo.com wrote:
> Thank you for the reminder, I hadn't thought about fallocate before.
> I have done some tests and the results are as expected.
> Here is my test method, create a compressed file, and use fallocate with keep 
> size, when write data to expand area, f2fs_prepare_compress_overwrite
> always return 0, the behavior is same as my patch , apply my patch can avoid 
> those check.
> Is there anything else I haven't thought of?

Nope, I didn't look into the implementation. Just a wild guess.

(I just wondered if the cluster size is somewhat large (e.g. 64k),
 but with a partial fallocate (e.g. 16k), and does it behave ok?
 or some other corner cases/conditions are needed.)

If that is fine, I have no problem about this, yet i_size here
is generally somewhat risky since after post-EOF behavior
changes (e.g. supporting FALLOC_FL_ZERO_RANGE with keep size
later), it may cause some potential regression.

> 
> -----邮件原件-----
> 发件人: Gao Xiang <hsiang...@redhat.com>
> 发送时间: 2021年4月26日 11:19
> 收件人: Fengnan Chang <changfeng...@vivo.com>
> 抄送: c...@kernel.org; jaeg...@kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> 主题: Re: [f2fs-dev] [PATCH] f2fs: compress: avoid unnecessary check in
> f2fs_prepare_compress_overwrite
> 
> On Mon, Apr 26, 2021 at 10:11:53AM +0800, Fengnan Chang wrote:
> > when write compressed file with O_TRUNC, there will be a lot of
> > unnecessary check valid blocks in f2fs_prepare_compress_overwrite,
> > especially when written in page size, remove it.
> >
> > Signed-off-by: Fengnan Chang <changfeng...@vivo.com>
> 
> Even though I didn't look into the whole thing, my reaction here is roughly
> how to handle fallocate with keep size? Does it work as expected?
> 
> > ---
> >  fs/f2fs/data.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index
> > cf935474ffba..9c3b0849f35e 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -3270,6 +3270,7 @@ static int f2fs_write_begin(struct file *file,
> > struct address_space *mapping,
> >     struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> >     struct page *page = NULL;
> >     pgoff_t index = ((unsigned long long) pos) >> PAGE_SHIFT;
> > +   pgoff_t end = (i_size_read(inode) + PAGE_SIZE - 1) >> PAGE_SHIFT;
> >     bool need_balance = false, drop_atomic = false;
> >     block_t blkaddr = NULL_ADDR;
> >     int err = 0;
> > @@ -3306,6 +3307,9 @@ static int f2fs_write_begin(struct file *file,
> > struct address_space *mapping,
> >
> >             *fsdata = NULL;
> >
> > +           if (index >= end)
> > +                   goto repeat;
> > +
> >             ret = f2fs_prepare_compress_overwrite(inode, pagep,
> >                                                     index, fsdata);
> >             if (ret < 0) {
> > --
> > 2.29.0
> 
> 
> 
> 
> 
> 
> 



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

Reply via email to