On 03/14, Jaegeuk Kim wrote:
> On 03/14, Yeongjin Gil wrote:
> > In the case of the following call stack for an atomic file,
> > FI_DIRTY_INODE is set, but FI_ATOMIC_DIRTIED is not subsequently set.
> >
> > f2fs_file_write_iter
> > f2fs_map_blocks
> > f2fs_reserve_new_blocks
> > inc_valid_block_count
> > __mark_inode_dirty(dquot)
> > f2fs_dirty_inode
> >
> > If FI_ATOMIC_DIRTIED is not set, atomic file can encounter corruption
> > due to a mismatch between old file size and new data.
> >
> > To resolve this issue, I changed to set FI_ATOMIC_DIRTIED when
> > FI_DIRTY_INODE is set. This ensures that FI_DIRTY_INODE, which was
> > previously cleared by the Writeback thread during the commit atomic, is
> > set and i_size is updated.
> >
> > Fixes: fccaa81de87e ("f2fs: prevent atomic file from being dirtied before
> > commit")
> > Reviewed-by: Sungjong Seo <sj1557....@samsung.com>
> > Reviewed-by: Sunmin Jeong <s_min.je...@samsung.com>
> > Signed-off-by: Yeongjin Gil <youngjin....@samsung.com>
> > ---
> > fs/f2fs/inode.c | 4 +---
> > fs/f2fs/super.c | 4 ++++
> > 2 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> > index aa2f41696a88..83f862578fc8 100644
> > --- a/fs/f2fs/inode.c
> > +++ b/fs/f2fs/inode.c
> > @@ -34,10 +34,8 @@ void f2fs_mark_inode_dirty_sync(struct inode *inode,
> > bool sync)
> > if (f2fs_inode_dirtied(inode, sync))
> > return;
> >
> > - if (f2fs_is_atomic_file(inode)) {
> > - set_inode_flag(inode, FI_ATOMIC_DIRTIED);
> > + if (f2fs_is_atomic_file(inode))
> > return;
> > - }
> >
> > mark_inode_dirty_sync(inode);
> > }
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index 397df271885c..c08d52c6467a 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -1534,6 +1534,10 @@ int f2fs_inode_dirtied(struct inode *inode, bool
> > sync)
> > inc_page_count(sbi, F2FS_DIRTY_IMETA);
> > }
> > spin_unlock(&sbi->inode_lock[DIRTY_META]);
> > +
> > + if (!ret && f2fs_is_atomic_file(inode))
> > + set_inode_flag(inode, FI_ATOMIC_DIRTIED);
> > +
>
> I'm not sure what's different here.
>
> Before and after this patch, set_inode_flag(inode, FI_ATOMIC_DIRTIED) is
> called
> only if f2fs_inode_dirtied() returns zero and f2fs_is_atomic_file(inode).
>
> Note, f2fs_mark_inode_dirty_sync() looks the single caller of
> f2fs_inode_dirtied.
Ok, I missed another caller, f2fs_dirty_inode, per discussion with Daeho.
Let me apply this.
>
>
> > return ret;
> > }
> >
> > --
> > 2.34.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
