Re: [PATCH] f2fs: skip clean inode update during fsync

[Wenjie Qi]

  Hi Chao,

  Yes, f2fs_write_inode() already skips updating the inode page in
this
  case.

  The difference is in the caller.  Without this patch,
  f2fs_skip_inode_update() returns false for a full fsync when
  FI_AUTO_RECOVER is not set, so f2fs_do_sync_file() calls
  f2fs_write_inode() and then unconditionally jumps to go_write.

  For a clean inode, f2fs_write_inode() returns without updating the inode
  page, but f2fs_do_sync_file() still continues through the fsync node path
  and can finally reach flush_out.  In the clean-file test this issued one
  flush per fsync.

  With this patch, the same clean-inode condition is checked before that
  goto go_write decision.  Then f2fs_do_sync_file() can take the no-written-
  data path and return without issuing a flush.

  So the improvement mainly comes from avoiding the redundant flush, not
  from avoiding the inode page update itself.

  The important part of the measurement is
  that the baseline issued one flush per clean fsync, while the patched
  kernel did not increase the flush count.  So the benefit comes from
  avoiding that redundant flush; the exact latency improvement will depend
  on the device.

  Thanks,

On Mon, Jun 15, 2026 at 4:04 PM Chao Yu <chao@xxxxxxxxxx> wrote:
>
> On 5/28/26 22:09, Wenjie Qi wrote:
> > f2fs_do_sync_file() calls f2fs_skip_inode_update() before deciding
> > whether it has to write an inode block and continue into the recovery
> > info/flush path.
> >
> > For a full fsync, f2fs_skip_inode_update() currently returns false when
> > FI_AUTO_RECOVER is not set.  That makes fsync on an already clean file
> > call f2fs_write_inode().  f2fs_write_inode() then returns immediately if
> > the in-memory timestamps match the inode block and FI_DIRTY_INODE is not
> > set, but f2fs_do_sync_file() still continues through go_write and may end
> > at f2fs_issue_flush().
> >
> > Avoid that unnecessary path for clean, time-consistent inodes without
> > FI_AUTO_RECOVER.  Keep the existing conservative checks for keep-size
> > files and non-block-aligned i_size before allowing the skip, and leave the
> > FI_AUTO_RECOVER path unchanged.
> >
> > On a QEMU/KASAN test VM, repeated fsync() on an existing clean F2FS file
> > improved from about 35.7 us/fsync to about 1.13 us/fsync.  The baseline
> > issued one flush per fsync, while the patched kernel kept the F2FS flush
> > count unchanged over 140000 clean fsync calls.
> >
> > Signed-off-by: Wenjie Qi <qiwenjie@xxxxxxxxxx>
> > ---
> >  fs/f2fs/f2fs.h | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 9f24287de4c3..ebd485abecb4 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -3668,11 +3668,13 @@ static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync)
> >               spin_unlock(&sbi->inode_lock[DIRTY_META]);
> >               return ret;
> >       }
> > -     if (!is_inode_flag_set(inode, FI_AUTO_RECOVER) ||
> > -                     file_keep_isize(inode) ||
> > -                     i_size_read(inode) & ~PAGE_MASK)
> > +     if (file_keep_isize(inode) || i_size_read(inode) & ~PAGE_MASK)
> >               return false;
> >
> > +     if (!is_inode_flag_set(inode, FI_AUTO_RECOVER))
> > +             return f2fs_is_time_consistent(inode) &&
> > +                     !is_inode_flag_set(inode, FI_DIRTY_INODE);
>
> IIUC, without this additional check condition, f2fs_write_inode() will skip to update
> inode page as well according to the same check condition, so, do you know why will we
> have so many benefits on fsync?
>
> int f2fs_write_inode(struct inode *inode, struct writeback_control *wbc)
> {
> ...
>         /*
>          * atime could be updated without dirtying f2fs inode in lazytime mode
>          */
>         if (f2fs_is_time_consistent(inode) &&
>                 !is_inode_flag_set(inode, FI_DIRTY_INODE))
>                 return 0;
>
> Thanks,
>
> > +
> >       if (!f2fs_is_time_consistent(inode))
> >               return false;
> >
>