mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] f2fs: skip clean inode update during fsync
@ 2026-05-28 14:09 Wenjie Qi
  2026-06-15  8:03 ` Chao Yu
  0 siblings, 1 reply; 3+ messages in thread
From: Wenjie Qi @ 2026-05-28 14:09 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: linux-f2fs-devel, linux-kernel, qiwenjie, qwjhust

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@xiaomi.com>
---
 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);
+
 	if (!f2fs_is_time_consistent(inode))
 		return false;
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] f2fs: skip clean inode update during fsync
  2026-05-28 14:09 [PATCH] f2fs: skip clean inode update during fsync Wenjie Qi
@ 2026-06-15  8:03 ` Chao Yu
  2026-06-16 13:45   ` Wenjie Qi
  0 siblings, 1 reply; 3+ messages in thread
From: Chao Yu @ 2026-06-15  8:03 UTC (permalink / raw)
  To: Wenjie Qi, jaegeuk; +Cc: chao, linux-f2fs-devel, linux-kernel, qiwenjie

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@xiaomi.com>
> ---
>  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;
>  


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] f2fs: skip clean inode update during fsync
  2026-06-15  8:03 ` Chao Yu
@ 2026-06-16 13:45   ` Wenjie Qi
  0 siblings, 0 replies; 3+ messages in thread
From: Wenjie Qi @ 2026-06-16 13:45 UTC (permalink / raw)
  To: Chao Yu; +Cc: jaegeuk, linux-f2fs-devel, linux-kernel, qiwenjie

  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@kernel.org> 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@xiaomi.com>
> > ---
> >  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;
> >
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-06-16 13:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-28 14:09 [PATCH] f2fs: skip clean inode update during fsync Wenjie Qi
2026-06-15  8:03 ` Chao Yu
2026-06-16 13:45   ` Wenjie Qi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®