The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] fs: fix forced iversion increment on lazytime timestamp updates
@ 2026-05-11 11:19 Pankaj Raghav
  2026-05-11 11:24 ` Jeff Layton
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Pankaj Raghav @ 2026-05-11 11:19 UTC (permalink / raw)
  To: Alexander Viro, Christian Brauner
  Cc: linux-fsdevel, Andres Freund, Jeff Layton, Carlos Maiolino,
	linux-xfs, hch, open list, pankaj.raghav, Jan Kara, Pankaj Raghav

When updating timestamps with lazytime enabled, if only I_DIRTY_TIME is
set (pure lazytime update), inode_maybe_inc_iversion() should not be
forced to increment i_version. The force parameter should only be true
when actual data or metadata changes require an iversion bump.

The current code uses "!!dirty" which evaluates to true whenever dirty
has any bits set, including the I_DIRTY_TIME bit alone. This forces an
iversion increment on every lazytime timestamp update, which then sets
I_DIRTY_SYNC, triggering expensive log flushes on subsequent fdatasync
calls. Andres reported this issue when he noticed a perf regression[1].

Fix this by using "dirty != I_DIRTY_TIME" as the force parameter. This
passes false for pure lazytime updates (allowing the I_VERSION_QUERIED
optimization to work), while still forcing the increment when dirty
contains other flags indicating real changes that require iversion
updates.

[1] https://lore.kernel.org/linux-xfs/7ys6erh3nnyeerv2nybyfvp7dmaknuxrlxv74wx56ocdothkc6@ekfiadtkfn2r/

Fixes: 85c871a02b03 ("fs: add support for non-blocking timestamp updates")
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 fs/inode.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/inode.c b/fs/inode.c
index 6a3cbc7dcd28..62c579a0cf7d 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2124,7 +2124,13 @@ static int inode_update_cmtime(struct inode *inode, unsigned int flags)
 			    inode_iversion_need_inc(inode))
 				return -EAGAIN;
 		} else {
-			if (inode_maybe_inc_iversion(inode, !!dirty))
+			/*
+			 * Don't force iversion increment for pure lazytime
+			 * updates (I_DIRTY_TIME only), let I_VERSION_QUERIED
+			 * dictate whether the increment is needed.
+			 */
+			if (inode_maybe_inc_iversion(inode,
+						     dirty != I_DIRTY_TIME))
 				dirty |= I_DIRTY_SYNC;
 		}
 	}

base-commit: 4cd074ae20bbcc293bbbce9163abe99d68ae6ae0
-- 
2.51.2


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

* Re: [PATCH] fs: fix forced iversion increment on lazytime timestamp updates
  2026-05-11 11:19 [PATCH] fs: fix forced iversion increment on lazytime timestamp updates Pankaj Raghav
@ 2026-05-11 11:24 ` Jeff Layton
  2026-05-11 12:10 ` Christoph Hellwig
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Jeff Layton @ 2026-05-11 11:24 UTC (permalink / raw)
  To: Pankaj Raghav, Alexander Viro, Christian Brauner
  Cc: linux-fsdevel, Andres Freund, Carlos Maiolino, linux-xfs, hch,
	open list, pankaj.raghav, Jan Kara

On Mon, 2026-05-11 at 13:19 +0200, Pankaj Raghav wrote:
> When updating timestamps with lazytime enabled, if only I_DIRTY_TIME is
> set (pure lazytime update), inode_maybe_inc_iversion() should not be
> forced to increment i_version. The force parameter should only be true
> when actual data or metadata changes require an iversion bump.
> 
> The current code uses "!!dirty" which evaluates to true whenever dirty
> has any bits set, including the I_DIRTY_TIME bit alone. This forces an
> iversion increment on every lazytime timestamp update, which then sets
> I_DIRTY_SYNC, triggering expensive log flushes on subsequent fdatasync
> calls. Andres reported this issue when he noticed a perf regression[1].
> 
> Fix this by using "dirty != I_DIRTY_TIME" as the force parameter. This
> passes false for pure lazytime updates (allowing the I_VERSION_QUERIED
> optimization to work), while still forcing the increment when dirty
> contains other flags indicating real changes that require iversion
> updates.
> 
> [1] https://lore.kernel.org/linux-xfs/7ys6erh3nnyeerv2nybyfvp7dmaknuxrlxv74wx56ocdothkc6@ekfiadtkfn2r/
> 
> Fixes: 85c871a02b03 ("fs: add support for non-blocking timestamp updates")
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  fs/inode.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index 6a3cbc7dcd28..62c579a0cf7d 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -2124,7 +2124,13 @@ static int inode_update_cmtime(struct inode *inode, unsigned int flags)
>  			    inode_iversion_need_inc(inode))
>  				return -EAGAIN;
>  		} else {
> -			if (inode_maybe_inc_iversion(inode, !!dirty))
> +			/*
> +			 * Don't force iversion increment for pure lazytime
> +			 * updates (I_DIRTY_TIME only), let I_VERSION_QUERIED
> +			 * dictate whether the increment is needed.
> +			 */
> +			if (inode_maybe_inc_iversion(inode,
> +						     dirty != I_DIRTY_TIME))
>  				dirty |= I_DIRTY_SYNC;
>  		}
>  	}
> 
> base-commit: 4cd074ae20bbcc293bbbce9163abe99d68ae6ae0

Reviewed-by: Jeff Layton <jlayton@kernel.org>

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

* Re: [PATCH] fs: fix forced iversion increment on lazytime timestamp updates
  2026-05-11 11:19 [PATCH] fs: fix forced iversion increment on lazytime timestamp updates Pankaj Raghav
  2026-05-11 11:24 ` Jeff Layton
@ 2026-05-11 12:10 ` Christoph Hellwig
  2026-05-11 12:12 ` Carlos Maiolino
  2026-05-11 13:32 ` Christian Brauner
  3 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2026-05-11 12:10 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: Alexander Viro, Christian Brauner, linux-fsdevel, Andres Freund,
	Jeff Layton, Carlos Maiolino, linux-xfs, hch, open list,
	pankaj.raghav, Jan Kara

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>


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

* Re: [PATCH] fs: fix forced iversion increment on lazytime timestamp updates
  2026-05-11 11:19 [PATCH] fs: fix forced iversion increment on lazytime timestamp updates Pankaj Raghav
  2026-05-11 11:24 ` Jeff Layton
  2026-05-11 12:10 ` Christoph Hellwig
@ 2026-05-11 12:12 ` Carlos Maiolino
  2026-05-11 13:32 ` Christian Brauner
  3 siblings, 0 replies; 5+ messages in thread
From: Carlos Maiolino @ 2026-05-11 12:12 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: Alexander Viro, Christian Brauner, linux-fsdevel, Andres Freund,
	Jeff Layton, linux-xfs, hch, open list, pankaj.raghav, Jan Kara

On Mon, May 11, 2026 at 01:19:18PM +0200, Pankaj Raghav wrote:
> When updating timestamps with lazytime enabled, if only I_DIRTY_TIME is
> set (pure lazytime update), inode_maybe_inc_iversion() should not be
> forced to increment i_version. The force parameter should only be true
> when actual data or metadata changes require an iversion bump.
> 
> The current code uses "!!dirty" which evaluates to true whenever dirty
> has any bits set, including the I_DIRTY_TIME bit alone. This forces an
> iversion increment on every lazytime timestamp update, which then sets
> I_DIRTY_SYNC, triggering expensive log flushes on subsequent fdatasync
> calls. Andres reported this issue when he noticed a perf regression[1].
> 
> Fix this by using "dirty != I_DIRTY_TIME" as the force parameter. This
> passes false for pure lazytime updates (allowing the I_VERSION_QUERIED
> optimization to work), while still forcing the increment when dirty
> contains other flags indicating real changes that require iversion
> updates.
> 
> [1] https://lore.kernel.org/linux-xfs/7ys6erh3nnyeerv2nybyfvp7dmaknuxrlxv74wx56ocdothkc6@ekfiadtkfn2r/
> 
> Fixes: 85c871a02b03 ("fs: add support for non-blocking timestamp updates")
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  fs/inode.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index 6a3cbc7dcd28..62c579a0cf7d 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -2124,7 +2124,13 @@ static int inode_update_cmtime(struct inode *inode, unsigned int flags)
>  			    inode_iversion_need_inc(inode))
>  				return -EAGAIN;
>  		} else {
> -			if (inode_maybe_inc_iversion(inode, !!dirty))
> +			/*
> +			 * Don't force iversion increment for pure lazytime
> +			 * updates (I_DIRTY_TIME only), let I_VERSION_QUERIED
> +			 * dictate whether the increment is needed.
> +			 */
> +			if (inode_maybe_inc_iversion(inode,
> +						     dirty != I_DIRTY_TIME))
>  				dirty |= I_DIRTY_SYNC;
>  		}
>  	}
> 

Looks good.
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>


> base-commit: 4cd074ae20bbcc293bbbce9163abe99d68ae6ae0
> -- 
> 2.51.2
> 
> 

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

* Re: [PATCH] fs: fix forced iversion increment on lazytime timestamp updates
  2026-05-11 11:19 [PATCH] fs: fix forced iversion increment on lazytime timestamp updates Pankaj Raghav
                   ` (2 preceding siblings ...)
  2026-05-11 12:12 ` Carlos Maiolino
@ 2026-05-11 13:32 ` Christian Brauner
  3 siblings, 0 replies; 5+ messages in thread
From: Christian Brauner @ 2026-05-11 13:32 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: Christian Brauner, Alexander Viro, linux-fsdevel, Andres Freund,
	Jeff Layton, Carlos Maiolino, linux-xfs, hch, linux-kernel,
	pankaj.raghav, Jan Kara

On Mon, 11 May 2026 13:19:18 +0200, Pankaj Raghav wrote:
> When updating timestamps with lazytime enabled, if only I_DIRTY_TIME is
> set (pure lazytime update), inode_maybe_inc_iversion() should not be
> forced to increment i_version. The force parameter should only be true
> when actual data or metadata changes require an iversion bump.
> 
> The current code uses "!!dirty" which evaluates to true whenever dirty
> has any bits set, including the I_DIRTY_TIME bit alone. This forces an
> iversion increment on every lazytime timestamp update, which then sets
> I_DIRTY_SYNC, triggering expensive log flushes on subsequent fdatasync
> calls. Andres reported this issue when he noticed a perf regression[1].
> 
> [...]

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] fs: fix forced iversion increment on lazytime timestamp updates
      https://git.kernel.org/vfs/vfs/c/834e98acb748

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

end of thread, other threads:[~2026-05-11 13:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-11 11:19 [PATCH] fs: fix forced iversion increment on lazytime timestamp updates Pankaj Raghav
2026-05-11 11:24 ` Jeff Layton
2026-05-11 12:10 ` Christoph Hellwig
2026-05-11 12:12 ` Carlos Maiolino
2026-05-11 13:32 ` Christian Brauner

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