linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Chao Yu <yuchao0@huawei.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>,
	changfengnan@vivo.com, linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] 答复: [PATCH v4] f2fs: compress: avoid unnecessary check in f2fs_prepare_compress_overwrite
Date: Mon, 17 May 2021 14:18:36 -0700	[thread overview]
Message-ID: <YKLdrGV9utVHDuE/@gmail.com> (raw)
In-Reply-To: <7f6b707c-dd45-21fe-e1f7-624fafcda3d3@huawei.com>

On Fri, May 14, 2021 at 09:15:54AM +0800, Chao Yu wrote:
> On 2021/5/14 5:17, Eric Biggers wrote:
> > On Wed, May 12, 2021 at 09:52:19AM +0800, Chao Yu wrote:
> > > On 2021/5/12 5:50, Jaegeuk Kim wrote:
> > > > On 05/11, changfengnan@vivo.com wrote:
> > > > > Hi Jaegeuk:
> > > > > 
> > > > > If there're existing clusters beyond i_size, may cause data corruption, but
> > > > > will this happen in normal? maybe some error can cause this, if i_size is
> > > > > error the data beyond size still can't handle properly.  Is there normal
> > > > > case can casue existing clusters beyond i_size?
> > > > 
> > > > We don't have a rule to sync between i_size and i_blocks.
> > > 
> > > I can't image a case that compressed cluster may cross filesize, it looks it's
> > > a bug if that happened, but I'm not sure I have considered all cases. So, I
> > > prefer to add a check condition as below, then testing w/ xfstest/por_fsstress
> > > for a while.
> > > 
> > > Subject: [PATCH] f2fs: compress: compressed cluster should not cross i_size
> > > 
> > > ---
> > >   fs/f2fs/data.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > index 06d1e58d3882..9acca358d578 100644
> > > --- a/fs/f2fs/data.c
> > > +++ b/fs/f2fs/data.c
> > > @@ -3325,6 +3325,8 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
> > >   			err = ret;
> > >   			goto fail;
> > >   		} else if (ret) {
> > > +			f2fs_bug_on(sbi, index >=
> > > +				DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE));
> > >   			return 0;
> > >   		}
> > >   	}
> > 
> > If a file has both fs-verity and compression enabled, it can have compressed
> > clusters past i_size.
> 
> Correct, any other case we missed for a writable file? let us know.
> 
> Thanks,
> 

Well, files become read-only once fs-verity is enabled on them, but that happens
after the blocks past i_size are written in the first place.  That part still
uses ->write_begin(), ->write_end(), ->writepages(), etc.

- Eric


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

  reply	other threads:[~2021-05-17 21:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-07  9:44 [f2fs-dev] [PATCH v4] f2fs: compress: avoid unnecessary check in f2fs_prepare_compress_overwrite Fengnan Chang
2021-05-10 15:43 ` Jaegeuk Kim
2021-05-11 11:10   ` [f2fs-dev] 答复: " changfengnan
2021-05-11 21:50     ` Jaegeuk Kim
2021-05-12  1:20       ` Gao Xiang
2021-05-12  1:52       ` Chao Yu
2021-05-13  1:41         ` [f2fs-dev] 答复: " changfengnan
2021-05-13 21:17         ` [f2fs-dev] " Eric Biggers
2021-05-14  1:15           ` Chao Yu
2021-05-17 21:18             ` Eric Biggers [this message]
2021-05-18  1:23               ` Chao Yu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YKLdrGV9utVHDuE/@gmail.com \
    --to=ebiggers@kernel.org \
    --cc=changfengnan@vivo.com \
    --cc=jaegeuk@kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=yuchao0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).