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 <[email protected]> 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 <[email protected]> > > --- > > 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; > > > _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
