From: Eric Biggers <ebiggers@kernel.org> To: Chao Yu <yuchao0@huawei.com> Cc: linux-fscrypt@vger.kernel.org, linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [PATCH 3/6] f2fs: skip truncate when verity in progress in ->write_begin() Date: Sun, 18 Aug 2019 13:24:26 -0700 [thread overview] Message-ID: <20190818202426.GB1824@zzz.localdomain> (raw) In-Reply-To: <13be698c-1a3d-9f6a-66d8-b9024b7963f3@huawei.com> On Tue, Aug 13, 2019 at 10:40:39AM +0800, Chao Yu wrote: > Hi Eric, > > On 2019/8/13 6:58, Eric Biggers wrote: > > Hi Chao, > > > > On Mon, Aug 12, 2019 at 08:25:33PM +0800, Chao Yu wrote: > >> Hi Eric, > >> > >> On 2019/8/12 5:35, Eric Biggers wrote: > >>> From: Eric Biggers <ebiggers@google.com> > >>> > >>> When an error (e.g. ENOSPC) occurs during f2fs_write_begin() when called > >>> from f2fs_write_merkle_tree_block(), skip truncating the file. i_size > >>> is not meaningful in this case, and the truncation is handled by > >>> f2fs_end_enable_verity() instead. > >>> > >>> Fixes: 60d7bf0f790f ("f2fs: add fs-verity support") > >>> Signed-off-by: Eric Biggers <ebiggers@google.com> > >>> --- > >>> fs/f2fs/data.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > >>> index 3f525f8a3a5fa..00b03fb87bd9b 100644 > >>> --- a/fs/f2fs/data.c > >>> +++ b/fs/f2fs/data.c > >>> @@ -2476,7 +2476,7 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to) > >>> struct inode *inode = mapping->host; > >>> loff_t i_size = i_size_read(inode); > >>> > >>> - if (to > i_size) { > >> > >> Maybe adding one single line comment to mention that it's redundant/unnecessary > >> truncation here is better, if I didn't misunderstand this. > >> > >> Thanks, > >> > >>> + if (to > i_size && !f2fs_verity_in_progress(inode)) { > >>> down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]); > >>> down_write(&F2FS_I(inode)->i_mmap_sem); > >>> > > > > Do you mean add a comment instead of the !f2fs_verity_in_progress() check, or in > > addition to it? ->write_begin(), ->writepages(), and ->write_end() are all > > Sorry, I didn't make this very clear, I meant adding the comment in addition on > above change. > > > supposed to ignore i_size when verity is in progress, so I don't think this > > particular part should be different, even if technically it's still correct to > > truncate twice. Also, ext4 needs this check in its ->write_begin() for locking > > reasons; we should keep f2fs similar. > > Agreed. > > > > > How about having both a comment and the check, like: > > > > /* In the fs-verity case, f2fs_end_enable_verity() does the truncate */ > > if (to > i_size && !f2fs_verity_in_progress(inode)) { > > The comment looks good to me. :) > > Thanks, > > > > > - Eric > > . > > > Okay, this is what I applied: diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index 3f525f8a3a5f..54cad80acb7d 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -2476,7 +2476,8 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to) struct inode *inode = mapping->host; loff_t i_size = i_size_read(inode); - if (to > i_size) { + /* In the fs-verity case, f2fs_end_enable_verity() does the truncate */ + if (to > i_size && !f2fs_verity_in_progress(inode)) { down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]); down_write(&F2FS_I(inode)->i_mmap_sem);
WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org> To: Chao Yu <yuchao0@huawei.com> Cc: linux-fscrypt@vger.kernel.org, linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [PATCH 3/6] f2fs: skip truncate when verity in progress in ->write_begin() Date: Sun, 18 Aug 2019 13:24:26 -0700 [thread overview] Message-ID: <20190818202426.GB1824@zzz.localdomain> (raw) In-Reply-To: <13be698c-1a3d-9f6a-66d8-b9024b7963f3@huawei.com> On Tue, Aug 13, 2019 at 10:40:39AM +0800, Chao Yu wrote: > Hi Eric, > > On 2019/8/13 6:58, Eric Biggers wrote: > > Hi Chao, > > > > On Mon, Aug 12, 2019 at 08:25:33PM +0800, Chao Yu wrote: > >> Hi Eric, > >> > >> On 2019/8/12 5:35, Eric Biggers wrote: > >>> From: Eric Biggers <ebiggers@google.com> > >>> > >>> When an error (e.g. ENOSPC) occurs during f2fs_write_begin() when called > >>> from f2fs_write_merkle_tree_block(), skip truncating the file. i_size > >>> is not meaningful in this case, and the truncation is handled by > >>> f2fs_end_enable_verity() instead. > >>> > >>> Fixes: 60d7bf0f790f ("f2fs: add fs-verity support") > >>> Signed-off-by: Eric Biggers <ebiggers@google.com> > >>> --- > >>> fs/f2fs/data.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > >>> index 3f525f8a3a5fa..00b03fb87bd9b 100644 > >>> --- a/fs/f2fs/data.c > >>> +++ b/fs/f2fs/data.c > >>> @@ -2476,7 +2476,7 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to) > >>> struct inode *inode = mapping->host; > >>> loff_t i_size = i_size_read(inode); > >>> > >>> - if (to > i_size) { > >> > >> Maybe adding one single line comment to mention that it's redundant/unnecessary > >> truncation here is better, if I didn't misunderstand this. > >> > >> Thanks, > >> > >>> + if (to > i_size && !f2fs_verity_in_progress(inode)) { > >>> down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]); > >>> down_write(&F2FS_I(inode)->i_mmap_sem); > >>> > > > > Do you mean add a comment instead of the !f2fs_verity_in_progress() check, or in > > addition to it? ->write_begin(), ->writepages(), and ->write_end() are all > > Sorry, I didn't make this very clear, I meant adding the comment in addition on > above change. > > > supposed to ignore i_size when verity is in progress, so I don't think this > > particular part should be different, even if technically it's still correct to > > truncate twice. Also, ext4 needs this check in its ->write_begin() for locking > > reasons; we should keep f2fs similar. > > Agreed. > > > > > How about having both a comment and the check, like: > > > > /* In the fs-verity case, f2fs_end_enable_verity() does the truncate */ > > if (to > i_size && !f2fs_verity_in_progress(inode)) { > > The comment looks good to me. :) > > Thanks, > > > > > - Eric > > . > > > Okay, this is what I applied: diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index 3f525f8a3a5f..54cad80acb7d 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -2476,7 +2476,8 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to) struct inode *inode = mapping->host; loff_t i_size = i_size_read(inode); - if (to > i_size) { + /* In the fs-verity case, f2fs_end_enable_verity() does the truncate */ + if (to > i_size && !f2fs_verity_in_progress(inode)) { down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]); down_write(&F2FS_I(inode)->i_mmap_sem); _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2019-08-18 20:24 UTC|newest] Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-11 21:35 [PATCH 0/6] fs-verity fixups Eric Biggers 2019-08-11 21:35 ` [f2fs-dev] " Eric Biggers 2019-08-11 21:35 ` [PATCH 1/6] fs-verity: fix crash on read error in build_merkle_tree_level() Eric Biggers 2019-08-11 21:35 ` [f2fs-dev] " Eric Biggers 2019-08-11 21:35 ` [f2fs-dev] [PATCH 2/6] ext4: skip truncate when verity in progress in ->write_begin() Eric Biggers 2019-08-11 21:35 ` Eric Biggers 2019-08-11 21:35 ` [PATCH 3/6] f2fs: " Eric Biggers 2019-08-11 21:35 ` [f2fs-dev] " Eric Biggers 2019-08-12 12:25 ` Chao Yu 2019-08-12 12:25 ` [f2fs-dev] " Chao Yu 2019-08-12 12:25 ` Chao Yu 2019-08-12 22:58 ` [f2fs-dev] " Eric Biggers 2019-08-12 22:58 ` Eric Biggers 2019-08-13 2:40 ` [f2fs-dev] " Chao Yu 2019-08-13 2:40 ` Chao Yu 2019-08-13 2:40 ` Chao Yu 2019-08-18 20:24 ` Eric Biggers [this message] 2019-08-18 20:24 ` [f2fs-dev] " Eric Biggers 2019-08-11 21:35 ` [PATCH 4/6] ext4: remove ext4_bio_encrypted() Eric Biggers 2019-08-11 21:35 ` [f2fs-dev] " Eric Biggers 2019-08-11 21:35 ` [PATCH 5/6] ext4: fix comment in ext4_end_enable_verity() Eric Biggers 2019-08-11 21:35 ` [f2fs-dev] " Eric Biggers 2019-08-11 21:35 ` [PATCH 6/6] f2fs: use EFSCORRUPTED in f2fs_get_verity_descriptor() Eric Biggers 2019-08-11 21:35 ` [f2fs-dev] " Eric Biggers 2019-08-12 12:26 ` Chao Yu 2019-08-12 12:26 ` [f2fs-dev] " Chao Yu 2019-08-12 12:26 ` Chao Yu 2019-08-18 20:23 ` [f2fs-dev] [PATCH 0/6] fs-verity fixups Eric Biggers 2019-08-18 20:23 ` Eric Biggers
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=20190818202426.GB1824@zzz.localdomain \ --to=ebiggers@kernel.org \ --cc=linux-ext4@vger.kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-fscrypt@vger.kernel.org \ --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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.