| Message ID | 20250327055607.3829954-1-chao@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [f2fs-dev] f2fs: fix to set atomic write status more clear | expand |
On Wed, Mar 26, 2025 at 10:59 PM Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net> wrote: > > 1. After we start atomic write in a database file, before committing > all data, we'd better not set inode w/ vfs dirty status to avoid > redundant updates, instead, we only set inode w/ atomic dirty status. > > 2. After we commit all data, before committing metadata, we need to > clear atomic dirty status, and set vfs dirty status to allow vfs flush > dirty inode. > > Cc: Daeho Jeong <daehojeong@google.com> > Reported-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > Signed-off-by: Chao Yu <chao@kernel.org> > --- > fs/f2fs/inode.c | 4 +++- > fs/f2fs/segment.c | 6 ++++++ > fs/f2fs/super.c | 4 +++- > 3 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > index 5c8634eaef7b..f5991e8751b9 100644 > --- a/fs/f2fs/inode.c > +++ b/fs/f2fs/inode.c > @@ -34,7 +34,9 @@ void f2fs_mark_inode_dirty_sync(struct inode *inode, bool sync) > if (f2fs_inode_dirtied(inode, sync)) > return; > > - if (f2fs_is_atomic_file(inode)) > + /* only atomic file w/ FI_ATOMIC_COMMITTED can be set vfs dirty */ > + if (f2fs_is_atomic_file(inode) && > + !is_inode_flag_set(inode, FI_ATOMIC_COMMITTED)) > return; > > mark_inode_dirty_sync(inode); > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index dc360b4b0569..7c113b446f63 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -376,7 +376,13 @@ static int __f2fs_commit_atomic_write(struct inode *inode) > } else { > sbi->committed_atomic_block += fi->atomic_write_cnt; > set_inode_flag(inode, FI_ATOMIC_COMMITTED); > + > + /* > + * inode may has no FI_ATOMIC_DIRTIED flag due to no write > + * before commit. > + */ > if (is_inode_flag_set(inode, FI_ATOMIC_DIRTIED)) { > + /* clear atomic dirty status and set vfs dirty status */ > clear_inode_flag(inode, FI_ATOMIC_DIRTIED); > f2fs_mark_inode_dirty_sync(inode, true); > } > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 9a42a1323f42..a5cc9f6ee16a 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -1532,7 +1532,9 @@ int f2fs_inode_dirtied(struct inode *inode, bool sync) > } > spin_unlock(&sbi->inode_lock[DIRTY_META]); > > - if (!ret && f2fs_is_atomic_file(inode)) > + /* if atomic write is not committed, set inode w/ atomic dirty */ > + if (!ret && f2fs_is_atomic_file(inode) && > + !is_inode_flag_set(inode, FI_ATOMIC_COMMITTED)) > set_inode_flag(inode, FI_ATOMIC_DIRTIED); > > return ret; > -- > 2.49.0 > Reviewed-by: Daeho Jeong <daehojeong@google.com> Thanks. > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net> 于2025年3月27日周四 13:59写道： > > 1. After we start atomic write in a database file, before committing > all data, we'd better not set inode w/ vfs dirty status to avoid > redundant updates, instead, we only set inode w/ atomic dirty status. > > 2. After we commit all data, before committing metadata, we need to > clear atomic dirty status, and set vfs dirty status to allow vfs flush > dirty inode. > > Cc: Daeho Jeong <daehojeong@google.com> > Reported-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > Signed-off-by: Chao Yu <chao@kernel.org> Hi Chao, it is more clear and I understand, thanks a lot. Reviewed-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > --- > fs/f2fs/inode.c | 4 +++- > fs/f2fs/segment.c | 6 ++++++ > fs/f2fs/super.c | 4 +++- > 3 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > index 5c8634eaef7b..f5991e8751b9 100644 > --- a/fs/f2fs/inode.c > +++ b/fs/f2fs/inode.c > @@ -34,7 +34,9 @@ void f2fs_mark_inode_dirty_sync(struct inode *inode, bool sync) > if (f2fs_inode_dirtied(inode, sync)) > return; > > - if (f2fs_is_atomic_file(inode)) > + /* only atomic file w/ FI_ATOMIC_COMMITTED can be set vfs dirty */ > + if (f2fs_is_atomic_file(inode) && > + !is_inode_flag_set(inode, FI_ATOMIC_COMMITTED)) > return; > > mark_inode_dirty_sync(inode); > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index dc360b4b0569..7c113b446f63 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -376,7 +376,13 @@ static int __f2fs_commit_atomic_write(struct inode *inode) > } else { > sbi->committed_atomic_block += fi->atomic_write_cnt; > set_inode_flag(inode, FI_ATOMIC_COMMITTED); > + > + /* > + * inode may has no FI_ATOMIC_DIRTIED flag due to no write > + * before commit. > + */ > if (is_inode_flag_set(inode, FI_ATOMIC_DIRTIED)) { > + /* clear atomic dirty status and set vfs dirty status */ > clear_inode_flag(inode, FI_ATOMIC_DIRTIED); > f2fs_mark_inode_dirty_sync(inode, true); > } > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 9a42a1323f42..a5cc9f6ee16a 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -1532,7 +1532,9 @@ int f2fs_inode_dirtied(struct inode *inode, bool sync) > } > spin_unlock(&sbi->inode_lock[DIRTY_META]); > > - if (!ret && f2fs_is_atomic_file(inode)) > + /* if atomic write is not committed, set inode w/ atomic dirty */ > + if (!ret && f2fs_is_atomic_file(inode) && > + !is_inode_flag_set(inode, FI_ATOMIC_COMMITTED)) > set_inode_flag(inode, FI_ATOMIC_DIRTIED); > > return ret; > -- > 2.49.0 > > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c index 5c8634eaef7b..f5991e8751b9 100644 --- a/fs/f2fs/inode.c +++ b/fs/f2fs/inode.c @@ -34,7 +34,9 @@ void f2fs_mark_inode_dirty_sync(struct inode *inode, bool sync) if (f2fs_inode_dirtied(inode, sync)) return; - if (f2fs_is_atomic_file(inode)) + /* only atomic file w/ FI_ATOMIC_COMMITTED can be set vfs dirty */ + if (f2fs_is_atomic_file(inode) && + !is_inode_flag_set(inode, FI_ATOMIC_COMMITTED)) return; mark_inode_dirty_sync(inode); diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index dc360b4b0569..7c113b446f63 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -376,7 +376,13 @@ static int __f2fs_commit_atomic_write(struct inode *inode) } else { sbi->committed_atomic_block += fi->atomic_write_cnt; set_inode_flag(inode, FI_ATOMIC_COMMITTED); + + /* + * inode may has no FI_ATOMIC_DIRTIED flag due to no write + * before commit. + */ if (is_inode_flag_set(inode, FI_ATOMIC_DIRTIED)) { + /* clear atomic dirty status and set vfs dirty status */ clear_inode_flag(inode, FI_ATOMIC_DIRTIED); f2fs_mark_inode_dirty_sync(inode, true); } diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 9a42a1323f42..a5cc9f6ee16a 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -1532,7 +1532,9 @@ int f2fs_inode_dirtied(struct inode *inode, bool sync) } spin_unlock(&sbi->inode_lock[DIRTY_META]); - if (!ret && f2fs_is_atomic_file(inode)) + /* if atomic write is not committed, set inode w/ atomic dirty */ + if (!ret && f2fs_is_atomic_file(inode) && + !is_inode_flag_set(inode, FI_ATOMIC_COMMITTED)) set_inode_flag(inode, FI_ATOMIC_DIRTIED); return ret;
1. After we start atomic write in a database file, before committing all data, we'd better not set inode w/ vfs dirty status to avoid redundant updates, instead, we only set inode w/ atomic dirty status. 2. After we commit all data, before committing metadata, we need to clear atomic dirty status, and set vfs dirty status to allow vfs flush dirty inode. Cc: Daeho Jeong <daehojeong@google.com> Reported-by: Zhiguo Niu <zhiguo.niu@unisoc.com> Signed-off-by: Chao Yu <chao@kernel.org> --- fs/f2fs/inode.c | 4 +++- fs/f2fs/segment.c | 6 ++++++ fs/f2fs/super.c | 4 +++- 3 files changed, 12 insertions(+), 2 deletions(-)