re-enable IOCB_NOWAIT writes to files

re-enable IOCB_NOWAIT writes to files

[Christoph Hellwig]

Hi all,

commit 66fa3cedf16a ("fs: Add async write file modification handling.")
effectively disabled IOCB_NOWAIT writes as timestamp updates currently
always require blocking, and the modern timestamp resolution means we
always update timestamps.  This leads to a lot of context switches from
applications using io_uring to submit file writes, making it often worse
than using the legacy aio code that is not using IOCB_NOWAIT.

This series allows non-blocking updates for lazytime if the file system
supports it, and adds that support for XFS.

It also fixes the layering bypass in btrfs when updating timestamps on
device files for devices removed from btrfs usage, and FMODE_NOCMTIME
handling in the VFS now that nfsd started using it.  Note that I'm still
not sure that nfsd usage is fully correct for all file systems, as only
XFS explicitly supports FMODE_NOCMTIME, but at least the generic code
does the right thing now.

Diffstat:
 Documentation/filesystems/locking.rst |    2 
 Documentation/filesystems/vfs.rst     |    6 ++
 fs/btrfs/inode.c                      |    3 +
 fs/btrfs/volumes.c                    |   11 +--
 fs/fat/misc.c                         |    3 +
 fs/fs-writeback.c                     |   53 ++++++++++++++----
 fs/gfs2/inode.c                       |    6 +-
 fs/inode.c                            |  100 +++++++++++-----------------------
 fs/internal.h                         |    3 -
 fs/orangefs/inode.c                   |    7 ++
 fs/overlayfs/inode.c                  |    3 +
 fs/sync.c                             |    4 -
 fs/ubifs/file.c                       |    9 +--
 fs/utimes.c                           |    1 
 fs/xfs/xfs_iops.c                     |   29 ++++++++-
 fs/xfs/xfs_super.c                    |   29 ---------
 include/linux/fs.h                    |   17 +++--
 include/trace/events/writeback.h      |    6 --
 18 files changed, 152 insertions(+), 140 deletions(-)

[PATCH 01/14] fs: refactor file timestamp update logic

[Christoph Hellwig]

Currently the two high-level APIs use two helper functions to implement
almost all of the logic.  Refactor the two helpers and the common logic
into a new file_update_time_flags routine that gets the iocb flags or
0 in case of file_update_time passed so that the entire logic is
contained in a single function and can be easily understood and modified.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/inode.c | 54 +++++++++++++++++-------------------------------------
 1 file changed, 17 insertions(+), 37 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index ec9339024ac3..4884ffa931e7 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2310,10 +2310,12 @@ struct timespec64 current_time(struct inode *inode)
 }
 EXPORT_SYMBOL(current_time);
 
-static int inode_needs_update_time(struct inode *inode)
+static int file_update_time_flags(struct file *file, unsigned int flags)
 {
+	struct inode *inode = file_inode(file);
 	struct timespec64 now, ts;
-	int sync_it = 0;
+	int sync_mode = 0;
+	int ret = 0;
 
 	/* First try to exhaust all avenues to not sync */
 	if (IS_NOCMTIME(inode))
@@ -2323,29 +2325,23 @@ static int inode_needs_update_time(struct inode *inode)
 
 	ts = inode_get_mtime(inode);
 	if (!timespec64_equal(&ts, &now))
-		sync_it |= S_MTIME;
-
+		sync_mode |= S_MTIME;
 	ts = inode_get_ctime(inode);
 	if (!timespec64_equal(&ts, &now))
-		sync_it |= S_CTIME;
-
+		sync_mode |= S_CTIME;
 	if (IS_I_VERSION(inode) && inode_iversion_need_inc(inode))
-		sync_it |= S_VERSION;
+		sync_mode |= S_VERSION;
 
-	return sync_it;
-}
-
-static int __file_update_time(struct file *file, int sync_mode)
-{
-	int ret = 0;
-	struct inode *inode = file_inode(file);
+	if (!sync_mode)
+		return 0;
 
-	/* try to update time settings */
-	if (!mnt_get_write_access_file(file)) {
-		ret = inode_update_time(inode, sync_mode);
-		mnt_put_write_access_file(file);
-	}
+	if (flags & IOCB_NOWAIT)
+		return -EAGAIN;
 
+	if (mnt_get_write_access_file(file))
+		return 0;
+	ret = inode_update_time(inode, sync_mode);
+	mnt_put_write_access_file(file);
 	return ret;
 }
 
@@ -2365,14 +2361,7 @@ static int __file_update_time(struct file *file, int sync_mode)
  */
 int file_update_time(struct file *file)
 {
-	int ret;
-	struct inode *inode = file_inode(file);
-
-	ret = inode_needs_update_time(inode);
-	if (ret <= 0)
-		return ret;
-
-	return __file_update_time(file, ret);
+	return file_update_time_flags(file, 0);
 }
 EXPORT_SYMBOL(file_update_time);
 
@@ -2394,7 +2383,6 @@ EXPORT_SYMBOL(file_update_time);
 static int file_modified_flags(struct file *file, int flags)
 {
 	int ret;
-	struct inode *inode = file_inode(file);
 
 	/*
 	 * Clear the security bits if the process is not being run by root.
@@ -2403,17 +2391,9 @@ static int file_modified_flags(struct file *file, int flags)
 	ret = file_remove_privs_flags(file, flags);
 	if (ret)
 		return ret;
-
 	if (unlikely(file->f_mode & FMODE_NOCMTIME))
 		return 0;
-
-	ret = inode_needs_update_time(inode);
-	if (ret <= 0)
-		return ret;
-	if (flags & IOCB_NOWAIT)
-		return -EAGAIN;
-
-	return __file_update_time(file, ret);
+	return file_update_time_flags(file, flags);
 }
 
 /**
-- 
2.47.3

[PATCH 02/14] fs: lift the FMODE_NOCMTIME check into file_update_time_flags

[Christoph Hellwig]

FMODE_NOCMTIME used to be just a hack for the legacy XFS handle-based
"invisible I/O", but commit e5e9b24ab8fa ("nfsd: freeze c/mtime updates
with outstanding WRITE_ATTRS delegation") started using it from
generic callers.

I'm not sure other file systems are actually read for this in general,
so the above commit should get a closer look, but for it to make any
sense, file_update_time needs to respect the flag.

Lift the check from file_modified_flags to file_update_time so that
users of file_update_time inherit the behavior and so that all the
checks are done in one place.

Fixes: e5e9b24ab8fa ("nfsd: freeze c/mtime updates with outstanding WRITE_ATTRS delegation")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/inode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 4884ffa931e7..24dab63844db 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2320,6 +2320,8 @@ static int file_update_time_flags(struct file *file, unsigned int flags)
 	/* First try to exhaust all avenues to not sync */
 	if (IS_NOCMTIME(inode))
 		return 0;
+	if (unlikely(file->f_mode & FMODE_NOCMTIME))
+		return 0;
 
 	now = current_time(inode);
 
@@ -2391,8 +2393,6 @@ static int file_modified_flags(struct file *file, int flags)
 	ret = file_remove_privs_flags(file, flags);
 	if (ret)
 		return ret;
-	if (unlikely(file->f_mode & FMODE_NOCMTIME))
-		return 0;
 	return file_update_time_flags(file, flags);
 }
 
-- 
2.47.3

[PATCH 03/14] fs: export vfs_utimes

[Christoph Hellwig]

This will be used to replace an incorrect direct call into
generic_update_time in btrfs.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/utimes.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/utimes.c b/fs/utimes.c
index c7c7958e57b2..3e7156396230 100644
--- a/fs/utimes.c
+++ b/fs/utimes.c
@@ -76,6 +76,7 @@ int vfs_utimes(const struct path *path, struct timespec64 *times)
 out:
 	return error;
 }
+EXPORT_SYMBOL_GPL(vfs_utimes);
 
 static int do_utimes_path(int dfd, const char __user *filename,
 		struct timespec64 *times, int flags)
-- 
2.47.3

[PATCH 04/14] btrfs: use vfs_utimes to update file timestamps

[Christoph Hellwig]

Btrfs updates the device node timestamps for block device special files
when it stop using the device.

Commit 8f96a5bfa150 ("btrfs: update the bdev time directly when closing")
switch that update from the correct layering to directly call the
low-level helper on the bdev inode.  This is wrong and got fixed in
commit 54fde91f52f5 ("btrfs: update device path inode time instead of
bd_inode") by updating the file system inode instead of the bdev inode,
but this kept the incorrect bypassing of the VFS interfaces and file
system ->update_times method.  Fix this by using the propet vfs_utimes
interface.

Fixes: 8f96a5bfa150 ("btrfs: update the bdev time directly when closing")
Fixes: 54fde91f52f5 ("btrfs: update device path inode time instead of bd_inode")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/volumes.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 2bec544d8ba3..259e8b0496df 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2002,14 +2002,11 @@ static int btrfs_add_dev_item(struct btrfs_trans_handle *trans,
 static void update_dev_time(const char *device_path)
 {
 	struct path path;
-	int ret;
 
-	ret = kern_path(device_path, LOOKUP_FOLLOW, &path);
-	if (ret)
-		return;
-
-	inode_update_time(d_inode(path.dentry), S_MTIME | S_CTIME | S_VERSION);
-	path_put(&path);
+	if (!kern_path(device_path, LOOKUP_FOLLOW, &path)) {
+		vfs_utimes(&path, NULL);
+		path_put(&path);
+	}
 }
 
 static int btrfs_rm_dev_item(struct btrfs_trans_handle *trans,
-- 
2.47.3

[PATCH 05/14] fs: remove inode_update_time

[Christoph Hellwig]

The only external user is gone now, open code it in the two VFS
callers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/inode.c         | 23 ++++++++---------------
 include/linux/fs.h |  1 -
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 24dab63844db..d3edcc5baec9 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2107,19 +2107,6 @@ int generic_update_time(struct inode *inode, int flags)
 }
 EXPORT_SYMBOL(generic_update_time);
 
-/*
- * This does the actual work of updating an inodes time or version.  Must have
- * had called mnt_want_write() before calling this.
- */
-int inode_update_time(struct inode *inode, int flags)
-{
-	if (inode->i_op->update_time)
-		return inode->i_op->update_time(inode, flags);
-	generic_update_time(inode, flags);
-	return 0;
-}
-EXPORT_SYMBOL(inode_update_time);
-
 /**
  *	atime_needs_update	-	update the access time
  *	@path: the &struct path to update
@@ -2187,7 +2174,10 @@ void touch_atime(const struct path *path)
 	 * We may also fail on filesystems that have the ability to make parts
 	 * of the fs read only, e.g. subvolumes in Btrfs.
 	 */
-	inode_update_time(inode, S_ATIME);
+	if (inode->i_op->update_time)
+		inode->i_op->update_time(inode, S_ATIME);
+	else
+		generic_update_time(inode, S_ATIME);
 	mnt_put_write_access(mnt);
 skip_update:
 	sb_end_write(inode->i_sb);
@@ -2342,7 +2332,10 @@ static int file_update_time_flags(struct file *file, unsigned int flags)
 
 	if (mnt_get_write_access_file(file))
 		return 0;
-	ret = inode_update_time(inode, sync_mode);
+	if (inode->i_op->update_time)
+		ret = inode->i_op->update_time(inode, sync_mode);
+	else
+		generic_update_time(inode, sync_mode);
 	mnt_put_write_access_file(file);
 	return ret;
 }
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c895146c1444..a09cebdb4881 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2665,7 +2665,6 @@ enum file_time_flags {
 
 extern bool atime_needs_update(const struct path *, struct inode *);
 extern void touch_atime(const struct path *);
-int inode_update_time(struct inode *inode, int flags);
 
 static inline void file_accessed(struct file *file)
 {
-- 
2.47.3

[PATCH 06/14] organgefs: use inode_update_timestamps directly

[Christoph Hellwig]

Orangefs has no i_version handling and __orangefs_setattr already
explicitly marks the inode dirty.  So instead of the using
the flags return value from generic_update_time, just call the
lower level inode_update_timestamps helper directly.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/orangefs/inode.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
index a01400cd41fd..55f6c8026812 100644
--- a/fs/orangefs/inode.c
+++ b/fs/orangefs/inode.c
@@ -878,7 +878,9 @@ int orangefs_update_time(struct inode *inode, int flags)
 
 	gossip_debug(GOSSIP_INODE_DEBUG, "orangefs_update_time: %pU\n",
 	    get_khandle_from_ino(inode));
-	flags = generic_update_time(inode, flags);
+
+	flags = inode_update_timestamps(inode, flags);
+
 	memset(&iattr, 0, sizeof iattr);
         if (flags & S_ATIME)
 		iattr.ia_valid |= ATTR_ATIME;
-- 
2.47.3

[PATCH 07/14] fs: return a negative error from generic_update_time

[Christoph Hellwig]

Now that no caller looks at the updated flags, switch generic_update_time
to the same calling convention as the ->update_time method and return 0
or a negative errno.

This prepares for adding non-blocking timestamp updates that could return
-EAGAIN.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/gfs2/inode.c    | 3 +--
 fs/inode.c         | 6 +++---
 fs/ubifs/file.c    | 6 ++----
 fs/xfs/xfs_iops.c  | 6 ++----
 include/linux/fs.h | 2 +-
 5 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 8a7ed80d9f2d..601c14a3ac77 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2242,8 +2242,7 @@ static int gfs2_update_time(struct inode *inode, int flags)
 		if (error)
 			return error;
 	}
-	generic_update_time(inode, flags);
-	return 0;
+	return generic_update_time(inode, flags);
 }
 
 static const struct inode_operations gfs2_file_iops = {
diff --git a/fs/inode.c b/fs/inode.c
index d3edcc5baec9..74e672dd90aa 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2091,7 +2091,7 @@ EXPORT_SYMBOL(inode_update_timestamps);
  * or S_VERSION need to be updated we attempt to update all three of them. S_ATIME
  * updates can be handled done independently of the rest.
  *
- * Returns a S_* mask indicating which fields were updated.
+ * Returns a negative error value on error, else 0.
  */
 int generic_update_time(struct inode *inode, int flags)
 {
@@ -2103,7 +2103,7 @@ int generic_update_time(struct inode *inode, int flags)
 	if (updated & S_VERSION)
 		dirty_flags |= I_DIRTY_SYNC;
 	__mark_inode_dirty(inode, dirty_flags);
-	return updated;
+	return 0;
 }
 EXPORT_SYMBOL(generic_update_time);
 
@@ -2335,7 +2335,7 @@ static int file_update_time_flags(struct file *file, unsigned int flags)
 	if (inode->i_op->update_time)
 		ret = inode->i_op->update_time(inode, sync_mode);
 	else
-		generic_update_time(inode, sync_mode);
+		ret = generic_update_time(inode, sync_mode);
 	mnt_put_write_access_file(file);
 	return ret;
 }
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index ca41ce8208c4..3e119cb93ea9 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1379,10 +1379,8 @@ int ubifs_update_time(struct inode *inode, int flags)
 			.dirtied_ino_d = ALIGN(ui->data_len, 8) };
 	int err, release;
 
-	if (!IS_ENABLED(CONFIG_UBIFS_ATIME_SUPPORT)) {
-		generic_update_time(inode, flags);
-		return 0;
-	}
+	if (!IS_ENABLED(CONFIG_UBIFS_ATIME_SUPPORT))
+		return generic_update_time(inode, flags);
 
 	err = ubifs_budget_space(c, &req);
 	if (err)
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index caff0125faea..0ace5f790006 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1197,10 +1197,8 @@ xfs_vn_update_time(
 
 	if (inode->i_sb->s_flags & SB_LAZYTIME) {
 		if (!((flags & S_VERSION) &&
-		      inode_maybe_inc_iversion(inode, false))) {
-			generic_update_time(inode, flags);
-			return 0;
-		}
+		      inode_maybe_inc_iversion(inode, false)))
+			return generic_update_time(inode, flags);
 
 		/* Capture the iversion update that just occurred */
 		log_flags |= XFS_ILOG_CORE;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index a09cebdb4881..c1077ae7c6b2 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2823,7 +2823,7 @@ extern int current_umask(void);
 extern void ihold(struct inode * inode);
 extern void iput(struct inode *);
 int inode_update_timestamps(struct inode *inode, int flags);
-int generic_update_time(struct inode *, int);
+int generic_update_time(struct inode *inode, int flags);
 
 /* /sys/fs */
 extern struct kobject *fs_kobj;
-- 
2.47.3

[PATCH 08/14] fs: exit early in generic_update_time when there is no work

[Christoph Hellwig]

Exit early if not attributes are to be updated, to avoid a spurious call
to __mark_inode_dirty which can turn into a fairly expensive no-op due to
the extra checks and locking.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/inode.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/inode.c b/fs/inode.c
index 74e672dd90aa..57c458ee548d 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2098,6 +2098,9 @@ int generic_update_time(struct inode *inode, int flags)
 	int updated = inode_update_timestamps(inode, flags);
 	int dirty_flags = 0;
 
+	if (!updated)
+		return 0;
+
 	if (updated & (S_ATIME|S_MTIME|S_CTIME))
 		dirty_flags = inode->i_sb->s_flags & SB_LAZYTIME ? I_DIRTY_TIME : I_DIRTY_SYNC;
 	if (updated & S_VERSION)
-- 
2.47.3

[PATCH 09/14] fs: factor out a mark_inode_dirty_time helper

[Christoph Hellwig]

Factor out the inode dirtying vs lazytime logic from generic_update_time
into a new helper so that it can be reused in file system methods.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/fs-writeback.c  | 15 +++++++++++++++
 fs/inode.c         | 14 +++-----------
 include/linux/fs.h |  3 ++-
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 2b35e80037fe..930697f39153 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -2671,6 +2671,21 @@ void __mark_inode_dirty(struct inode *inode, int flags)
 }
 EXPORT_SYMBOL(__mark_inode_dirty);
 
+void mark_inode_dirty_time(struct inode *inode, unsigned int flags)
+{
+	if (inode->i_sb->s_flags & SB_LAZYTIME) {
+		int dirty_flags = 0;
+
+		if (flags & (S_ATIME | S_MTIME | S_CTIME))
+			dirty_flags = I_DIRTY_TIME;
+		if (flags & S_VERSION)
+			dirty_flags |= I_DIRTY_SYNC;
+		__mark_inode_dirty(inode, dirty_flags);
+	} else {
+		mark_inode_dirty_sync(inode);
+	}
+}
+
 /*
  * The @s_sync_lock is used to serialise concurrent sync operations
  * to avoid lock contention problems with concurrent wait_sb_inodes() calls.
diff --git a/fs/inode.c b/fs/inode.c
index 57c458ee548d..559ce5c07188 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2095,17 +2095,9 @@ EXPORT_SYMBOL(inode_update_timestamps);
  */
 int generic_update_time(struct inode *inode, int flags)
 {
-	int updated = inode_update_timestamps(inode, flags);
-	int dirty_flags = 0;
-
-	if (!updated)
-		return 0;
-
-	if (updated & (S_ATIME|S_MTIME|S_CTIME))
-		dirty_flags = inode->i_sb->s_flags & SB_LAZYTIME ? I_DIRTY_TIME : I_DIRTY_SYNC;
-	if (updated & S_VERSION)
-		dirty_flags |= I_DIRTY_SYNC;
-	__mark_inode_dirty(inode, dirty_flags);
+	flags = inode_update_timestamps(inode, flags);
+	if (flags)
+		mark_inode_dirty_time(inode, flags);
 	return 0;
 }
 EXPORT_SYMBOL(generic_update_time);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c1077ae7c6b2..5c762d80b8a8 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2608,7 +2608,8 @@ static inline void kiocb_clone(struct kiocb *kiocb, struct kiocb *kiocb_src,
 	};
 }
 
-extern void __mark_inode_dirty(struct inode *, int);
+void mark_inode_dirty_time(struct inode *inode, unsigned int flags);
+void __mark_inode_dirty(struct inode *inode, int flags);
 static inline void mark_inode_dirty(struct inode *inode)
 {
 	__mark_inode_dirty(inode, I_DIRTY);
-- 
2.47.3

[PATCH 10/14] fs: factor out a sync_lazytime helper

[Christoph Hellwig]

Centralize how we synchronize a lazytime update into the actual on-disk
timestamp into a single helper.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/fs-writeback.c                | 27 +++++++++++++++++----------
 fs/inode.c                       |  5 +----
 fs/internal.h                    |  3 ++-
 fs/sync.c                        |  4 ++--
 include/trace/events/writeback.h |  6 ------
 5 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 930697f39153..ae6d1f1ccc71 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -1693,6 +1693,16 @@ static void requeue_inode(struct inode *inode, struct bdi_writeback *wb,
 	}
 }
 
+bool sync_lazytime(struct inode *inode)
+{
+	if (!(inode->i_state & I_DIRTY_TIME))
+		return false;
+
+	trace_writeback_lazytime(inode);
+	mark_inode_dirty_sync(inode);
+	return false;
+}
+
 /*
  * Write out an inode and its dirty pages (or some of its dirty pages, depending
  * on @wbc->nr_to_write), and clear the relevant dirty flags from i_state.
@@ -1732,17 +1742,14 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
 	}
 
 	/*
-	 * If the inode has dirty timestamps and we need to write them, call
-	 * mark_inode_dirty_sync() to notify the filesystem about it and to
-	 * change I_DIRTY_TIME into I_DIRTY_SYNC.
+	 * For data integrity writeback, or when the dirty interval expired,
+	 * ask the file system to propagata lazy timestamp updates into real
+	 * dirty state.
 	 */
-	if ((inode->i_state & I_DIRTY_TIME) &&
-	    (wbc->sync_mode == WB_SYNC_ALL ||
-	     time_after(jiffies, inode->dirtied_time_when +
-			dirtytime_expire_interval * HZ))) {
-		trace_writeback_lazytime(inode);
-		mark_inode_dirty_sync(inode);
-	}
+	if (wbc->sync_mode == WB_SYNC_ALL ||
+	    time_after(jiffies, inode->dirtied_time_when +
+			dirtytime_expire_interval * HZ))
+		sync_lazytime(inode);
 
 	/*
 	 * Get and clear the dirty flags from i_state.  This needs to be done
diff --git a/fs/inode.c b/fs/inode.c
index 559ce5c07188..34d572c99313 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1942,11 +1942,8 @@ void iput(struct inode *inode)
 	if (atomic_add_unless(&inode->i_count, -1, 1))
 		return;
 
-	if ((inode->i_state & I_DIRTY_TIME) && inode->i_nlink) {
-		trace_writeback_lazytime_iput(inode);
-		mark_inode_dirty_sync(inode);
+	if (inode->i_nlink && sync_lazytime(inode))
 		goto retry;
-	}
 
 	spin_lock(&inode->i_lock);
 	if (unlikely((inode->i_state & I_DIRTY_TIME) && inode->i_nlink)) {
diff --git a/fs/internal.h b/fs/internal.h
index 9b2b4d116880..da6e62f1183f 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -211,7 +211,8 @@ bool in_group_or_capable(struct mnt_idmap *idmap,
 /*
  * fs-writeback.c
  */
-extern long get_nr_dirty_inodes(void);
+long get_nr_dirty_inodes(void);
+bool sync_lazytime(struct inode *inode);
 
 /*
  * dcache.c
diff --git a/fs/sync.c b/fs/sync.c
index 2955cd4c77a3..a86395e266b1 100644
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -182,8 +182,8 @@ int vfs_fsync_range(struct file *file, loff_t start, loff_t end, int datasync)
 
 	if (!file->f_op->fsync)
 		return -EINVAL;
-	if (!datasync && (inode->i_state & I_DIRTY_TIME))
-		mark_inode_dirty_sync(inode);
+	if (!datasync)
+		sync_lazytime(inode);
 	return file->f_op->fsync(file, start, end, datasync);
 }
 EXPORT_SYMBOL(vfs_fsync_range);
diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index c08aff044e80..75eae86798ba 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -856,12 +856,6 @@ DEFINE_EVENT(writeback_inode_template, writeback_lazytime,
 	TP_ARGS(inode)
 );
 
-DEFINE_EVENT(writeback_inode_template, writeback_lazytime_iput,
-	TP_PROTO(struct inode *inode),
-
-	TP_ARGS(inode)
-);
-
 DEFINE_EVENT(writeback_inode_template, writeback_dirty_inode_enqueue,
 
 	TP_PROTO(struct inode *inode),
-- 
2.47.3

[PATCH 11/14] fs: add a ->sync_lazytime method

[Christoph Hellwig]

Allow the file system to explicitly implement lazytime syncing instead
of pigging back on generic inode dirtying.  This allows to simplify
the XFS implementation and prepares for non-blocking lazytime timestamp
updates.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 Documentation/filesystems/locking.rst |  2 ++
 Documentation/filesystems/vfs.rst     |  6 ++++++
 fs/fs-writeback.c                     | 13 +++++++++++--
 include/linux/fs.h                    |  1 +
 4 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst
index 77704fde9845..9b2f14ada8cd 100644
--- a/Documentation/filesystems/locking.rst
+++ b/Documentation/filesystems/locking.rst
@@ -81,6 +81,7 @@ prototypes::
 	ssize_t (*listxattr) (struct dentry *, char *, size_t);
 	int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start, u64 len);
 	void (*update_time)(struct inode *, struct timespec *, int);
+	void (*sync_lazytime)(struct inode *inode);
 	int (*atomic_open)(struct inode *, struct dentry *,
 				struct file *, unsigned open_flag,
 				umode_t create_mode);
@@ -117,6 +118,7 @@ getattr:	no
 listxattr:	no
 fiemap:		no
 update_time:	no
+sync_lazytime:	no
 atomic_open:	shared (exclusive if O_CREAT is set in open flags)
 tmpfile:	no
 fileattr_get:	no or exclusive
diff --git a/Documentation/filesystems/vfs.rst b/Documentation/filesystems/vfs.rst
index 4f13b01e42eb..ff59760daae2 100644
--- a/Documentation/filesystems/vfs.rst
+++ b/Documentation/filesystems/vfs.rst
@@ -486,6 +486,7 @@ As of kernel 2.6.22, the following members are defined:
 		int (*getattr) (struct mnt_idmap *, const struct path *, struct kstat *, u32, unsigned int);
 		ssize_t (*listxattr) (struct dentry *, char *, size_t);
 		void (*update_time)(struct inode *, struct timespec *, int);
+		void (*sync_lazytime)(struct inode *inode);
 		int (*atomic_open)(struct inode *, struct dentry *, struct file *,
 				   unsigned open_flag, umode_t create_mode);
 		int (*tmpfile) (struct mnt_idmap *, struct inode *, struct file *, umode_t);
@@ -642,6 +643,11 @@ otherwise noted.
 	an inode.  If this is not defined the VFS will update the inode
 	itself and call mark_inode_dirty_sync.
 
+``sync_lazytime``:
+	called by the writeback code to update the lazy time stamps to
+	regular time stamp updates that get syncing into the on-disk
+	inode.
+
 ``atomic_open``
 	called on the last component of an open.  Using this optional
 	method the filesystem can look up, possibly create and open the
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index ae6d1f1ccc71..7245f547416f 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -1699,7 +1699,10 @@ bool sync_lazytime(struct inode *inode)
 		return false;
 
 	trace_writeback_lazytime(inode);
-	mark_inode_dirty_sync(inode);
+	if (inode->i_op->sync_lazytime)
+		inode->i_op->sync_lazytime(inode);
+	else
+		mark_inode_dirty_sync(inode);
 	return false;
 }
 
@@ -2547,6 +2550,8 @@ void __mark_inode_dirty(struct inode *inode, int flags)
 	trace_writeback_mark_inode_dirty(inode, flags);
 
 	if (flags & I_DIRTY_INODE) {
+		bool was_dirty_time = true;
+
 		/*
 		 * Inode timestamp update will piggback on this dirtying.
 		 * We tell ->dirty_inode callback that timestamps need to
@@ -2557,6 +2562,7 @@ void __mark_inode_dirty(struct inode *inode, int flags)
 			if (inode->i_state & I_DIRTY_TIME) {
 				inode->i_state &= ~I_DIRTY_TIME;
 				flags |= I_DIRTY_TIME;
+				was_dirty_time = true;
 			}
 			spin_unlock(&inode->i_lock);
 		}
@@ -2569,9 +2575,12 @@ void __mark_inode_dirty(struct inode *inode, int flags)
 		 * for just I_DIRTY_PAGES or I_DIRTY_TIME.
 		 */
 		trace_writeback_dirty_inode_start(inode, flags);
-		if (sb->s_op->dirty_inode)
+		if (sb->s_op->dirty_inode) {
 			sb->s_op->dirty_inode(inode,
 				flags & (I_DIRTY_INODE | I_DIRTY_TIME));
+		} else if (was_dirty_time && inode->i_op->sync_lazytime) {
+			inode->i_op->sync_lazytime(inode);
+		}
 		trace_writeback_dirty_inode(inode, flags);
 
 		/* I_DIRTY_INODE supersedes I_DIRTY_TIME. */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5c762d80b8a8..61051c86dbd2 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2363,6 +2363,7 @@ struct inode_operations {
 	int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start,
 		      u64 len);
 	int (*update_time)(struct inode *, int);
+	void (*sync_lazytime)(struct inode *inode);
 	int (*atomic_open)(struct inode *, struct dentry *,
 			   struct file *, unsigned open_flag,
 			   umode_t create_mode);
-- 
2.47.3

[PATCH 12/14] fs: add support for non-blocking timestamp updates

[Christoph Hellwig]

Currently file_update_time_flags unconditionally returns -EAGAIN if any
timestamp needs to be updated and IOCB_NOWAIT is passed.  This makes
non-blocking direct writes impossible on file systems with granular
enough timestamps.

Add a S_NOWAIT to ask for timestamps to not block, and return -EAGAIN in
all methods for now.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/inode.c     |  3 +++
 fs/fat/misc.c        |  3 +++
 fs/gfs2/inode.c      |  3 +++
 fs/inode.c           |  5 ++++-
 fs/orangefs/inode.c  |  3 +++
 fs/overlayfs/inode.c |  3 +++
 fs/ubifs/file.c      |  3 +++
 fs/xfs/xfs_iops.c    |  3 +++
 include/linux/fs.h   | 10 ++++++----
 9 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 6282911e536f..dea798d325d1 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6302,6 +6302,9 @@ static int btrfs_update_time(struct inode *inode, int flags)
 	if (btrfs_root_readonly(root))
 		return -EROFS;
 
+	if (flags & S_NOWAIT)
+		return -EAGAIN;
+
 	dirty = inode_update_timestamps(inode, flags);
 	return dirty ? btrfs_dirty_inode(BTRFS_I(inode)) : 0;
 }
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index 950da09f0961..5df3193c35f9 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -346,6 +346,9 @@ int fat_update_time(struct inode *inode, int flags)
 	if (inode->i_ino == MSDOS_ROOT_INO)
 		return 0;
 
+	if (flags & S_NOWAIT)
+		return -EAGAIN;
+
 	if (flags & (S_ATIME | S_CTIME | S_MTIME)) {
 		fat_truncate_time(inode, NULL, flags);
 		if (inode->i_sb->s_flags & SB_LAZYTIME)
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 601c14a3ac77..0184cb64fe9f 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2234,6 +2234,9 @@ static int gfs2_update_time(struct inode *inode, int flags)
 	struct gfs2_holder *gh;
 	int error;
 
+	if (flags & S_NOWAIT)
+		return -EAGAIN;
+
 	gh = gfs2_glock_is_locked_by_me(gl);
 	if (gh && gl->gl_state != LM_ST_EXCLUSIVE) {
 		gfs2_glock_dq(gh);
diff --git a/fs/inode.c b/fs/inode.c
index 34d572c99313..dc6bb6236361 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2092,6 +2092,9 @@ EXPORT_SYMBOL(inode_update_timestamps);
  */
 int generic_update_time(struct inode *inode, int flags)
 {
+	if (flags & S_NOWAIT)
+		return -EAGAIN;
+
 	flags = inode_update_timestamps(inode, flags);
 	if (flags)
 		mark_inode_dirty_time(inode, flags);
@@ -2320,7 +2323,7 @@ static int file_update_time_flags(struct file *file, unsigned int flags)
 		return 0;
 
 	if (flags & IOCB_NOWAIT)
-		return -EAGAIN;
+		sync_mode |= S_NOWAIT;
 
 	if (mnt_get_write_access_file(file))
 		return 0;
diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
index 55f6c8026812..9abd4153c85d 100644
--- a/fs/orangefs/inode.c
+++ b/fs/orangefs/inode.c
@@ -879,6 +879,9 @@ int orangefs_update_time(struct inode *inode, int flags)
 	gossip_debug(GOSSIP_INODE_DEBUG, "orangefs_update_time: %pU\n",
 	    get_khandle_from_ino(inode));
 
+	if (flags & S_NOWAIT)
+		return -EAGAIN;
+
 	flags = inode_update_timestamps(inode, flags);
 
 	memset(&iattr, 0, sizeof iattr);
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index e11f310ce092..c132d1f5502b 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -576,6 +576,9 @@ int ovl_update_time(struct inode *inode, int flags)
 			.dentry = ovl_upperdentry_dereference(OVL_I(inode)),
 		};
 
+		if (flags & S_NOWAIT)
+			return -EAGAIN;
+
 		if (upperpath.dentry) {
 			touch_atime(&upperpath);
 			inode_set_atime_to_ts(inode,
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 3e119cb93ea9..1caa2a93eaf5 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1382,6 +1382,9 @@ int ubifs_update_time(struct inode *inode, int flags)
 	if (!IS_ENABLED(CONFIG_UBIFS_ATIME_SUPPORT))
 		return generic_update_time(inode, flags);
 
+	if (flags & S_NOWAIT)
+		return -EAGAIN;
+
 	err = ubifs_budget_space(c, &req);
 	if (err)
 		return err;
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 0ace5f790006..da055dade25f 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1195,6 +1195,9 @@ xfs_vn_update_time(
 
 	trace_xfs_update_time(ip);
 
+	if (flags & S_NOWAIT)
+		return -EAGAIN;
+
 	if (inode->i_sb->s_flags & SB_LAZYTIME) {
 		if (!((flags & S_VERSION) &&
 		      inode_maybe_inc_iversion(inode, false)))
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 61051c86dbd2..36592fac1887 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2659,10 +2659,12 @@ static inline void inode_dec_link_count(struct inode *inode)
 }
 
 enum file_time_flags {
-	S_ATIME = 1,
-	S_MTIME = 2,
-	S_CTIME = 4,
-	S_VERSION = 8,
+	S_ATIME		= 1U << 0,
+	S_MTIME		= 1U << 1,
+	S_CTIME		= 1U << 2,
+	S_VERSION	= 1U << 4,
+
+	S_NOWAIT	= 1U << 15,
 };
 
 extern bool atime_needs_update(const struct path *, struct inode *);
-- 
2.47.3

[PATCH 13/14] xfs: implement ->sync_lazytime

[Christoph Hellwig]

Switch to the new explicit lazytime syncing method instead of trying
to second guess what could be a lazytime update in ->dirty_inode.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_iops.c  | 20 ++++++++++++++++++++
 fs/xfs/xfs_super.c | 29 -----------------------------
 2 files changed, 20 insertions(+), 29 deletions(-)

diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index da055dade25f..bd0b7e81f6ab 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1227,6 +1227,22 @@ xfs_vn_update_time(
 	return xfs_trans_commit(tp);
 }
 
+static void
+xfs_vn_sync_lazytime(
+	struct inode		*inode)
+{
+	struct xfs_inode	*ip = XFS_I(inode);
+	struct xfs_mount	*mp = ip->i_mount;
+	struct xfs_trans	*tp;
+
+	if (xfs_trans_alloc(mp, &M_RES(mp)->tr_fsyncts, 0, 0, 0, &tp))
+		return;
+	xfs_ilock(ip, XFS_ILOCK_EXCL);
+	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
+	xfs_trans_log_inode(tp, ip, XFS_ILOG_TIMESTAMP);
+	xfs_trans_commit(tp);
+}
+
 STATIC int
 xfs_vn_fiemap(
 	struct inode		*inode,
@@ -1270,6 +1286,7 @@ static const struct inode_operations xfs_inode_operations = {
 	.listxattr		= xfs_vn_listxattr,
 	.fiemap			= xfs_vn_fiemap,
 	.update_time		= xfs_vn_update_time,
+	.sync_lazytime		= xfs_vn_sync_lazytime,
 	.fileattr_get		= xfs_fileattr_get,
 	.fileattr_set		= xfs_fileattr_set,
 };
@@ -1296,6 +1313,7 @@ static const struct inode_operations xfs_dir_inode_operations = {
 	.setattr		= xfs_vn_setattr,
 	.listxattr		= xfs_vn_listxattr,
 	.update_time		= xfs_vn_update_time,
+	.sync_lazytime		= xfs_vn_sync_lazytime,
 	.tmpfile		= xfs_vn_tmpfile,
 	.fileattr_get		= xfs_fileattr_get,
 	.fileattr_set		= xfs_fileattr_set,
@@ -1323,6 +1341,7 @@ static const struct inode_operations xfs_dir_ci_inode_operations = {
 	.setattr		= xfs_vn_setattr,
 	.listxattr		= xfs_vn_listxattr,
 	.update_time		= xfs_vn_update_time,
+	.sync_lazytime		= xfs_vn_sync_lazytime,
 	.tmpfile		= xfs_vn_tmpfile,
 	.fileattr_get		= xfs_fileattr_get,
 	.fileattr_set		= xfs_fileattr_set,
@@ -1334,6 +1353,7 @@ static const struct inode_operations xfs_symlink_inode_operations = {
 	.setattr		= xfs_vn_setattr,
 	.listxattr		= xfs_vn_listxattr,
 	.update_time		= xfs_vn_update_time,
+	.sync_lazytime		= xfs_vn_sync_lazytime,
 	.fileattr_get		= xfs_fileattr_get,
 	.fileattr_set		= xfs_fileattr_set,
 };
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 1067ebb3b001..230153d6815a 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -712,34 +712,6 @@ xfs_fs_destroy_inode(
 	xfs_inode_mark_reclaimable(ip);
 }
 
-static void
-xfs_fs_dirty_inode(
-	struct inode			*inode,
-	int				flags)
-{
-	struct xfs_inode		*ip = XFS_I(inode);
-	struct xfs_mount		*mp = ip->i_mount;
-	struct xfs_trans		*tp;
-
-	if (!(inode->i_sb->s_flags & SB_LAZYTIME))
-		return;
-
-	/*
-	 * Only do the timestamp update if the inode is dirty (I_DIRTY_SYNC)
-	 * and has dirty timestamp (I_DIRTY_TIME). I_DIRTY_TIME can be passed
-	 * in flags possibly together with I_DIRTY_SYNC.
-	 */
-	if ((flags & ~I_DIRTY_TIME) != I_DIRTY_SYNC || !(flags & I_DIRTY_TIME))
-		return;
-
-	if (xfs_trans_alloc(mp, &M_RES(mp)->tr_fsyncts, 0, 0, 0, &tp))
-		return;
-	xfs_ilock(ip, XFS_ILOCK_EXCL);
-	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
-	xfs_trans_log_inode(tp, ip, XFS_ILOG_TIMESTAMP);
-	xfs_trans_commit(tp);
-}
-
 /*
  * Slab object creation initialisation for the XFS inode.
  * This covers only the idempotent fields in the XFS inode;
@@ -1304,7 +1276,6 @@ xfs_fs_show_stats(
 static const struct super_operations xfs_super_operations = {
 	.alloc_inode		= xfs_fs_alloc_inode,
 	.destroy_inode		= xfs_fs_destroy_inode,
-	.dirty_inode		= xfs_fs_dirty_inode,
 	.drop_inode		= xfs_fs_drop_inode,
 	.evict_inode		= xfs_fs_evict_inode,
 	.put_super		= xfs_fs_put_super,
-- 
2.47.3

[PATCH 14/14] xfs: enable non-blocking timestamp updates

[Christoph Hellwig]

The lazytime path using generic_update_time can never block in XFS
because there is no ->dirty_inode method that could block.  Allow
non-blocking timestamp updates for this case.

Fixes: 66fa3cedf16a ("fs: Add async write file modification handling.")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_iops.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index bd0b7e81f6ab..3d7b89ffacde 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1195,9 +1195,6 @@ xfs_vn_update_time(
 
 	trace_xfs_update_time(ip);
 
-	if (flags & S_NOWAIT)
-		return -EAGAIN;
-
 	if (inode->i_sb->s_flags & SB_LAZYTIME) {
 		if (!((flags & S_VERSION) &&
 		      inode_maybe_inc_iversion(inode, false)))
@@ -1207,6 +1204,9 @@ xfs_vn_update_time(
 		log_flags |= XFS_ILOG_CORE;
 	}
 
+	if (flags & S_NOWAIT)
+		return -EAGAIN;
+
 	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_fsyncts, 0, 0, 0, &tp);
 	if (error)
 		return error;
-- 
2.47.3

Re: re-enable IOCB_NOWAIT writes to files

[Jeff Layton]

On Fri, 2025-11-14 at 07:26 +0100, Christoph Hellwig wrote:
> Hi all,
> 
> commit 66fa3cedf16a ("fs: Add async write file modification handling.")
> effectively disabled IOCB_NOWAIT writes as timestamp updates currently
> always require blocking, and the modern timestamp resolution means we
> always update timestamps.  This leads to a lot of context switches from
> applications using io_uring to submit file writes, making it often worse
> than using the legacy aio code that is not using IOCB_NOWAIT.
> 
> This series allows non-blocking updates for lazytime if the file system
> supports it, and adds that support for XFS.
> 
> It also fixes the layering bypass in btrfs when updating timestamps on
> device files for devices removed from btrfs usage, and FMODE_NOCMTIME
> handling in the VFS now that nfsd started using it.  Note that I'm still
> not sure that nfsd usage is fully correct for all file systems, as only
> XFS explicitly supports FMODE_NOCMTIME, but at least the generic code
> does the right thing now.
> 
> Diffstat:
>  Documentation/filesystems/locking.rst |    2 
>  Documentation/filesystems/vfs.rst     |    6 ++
>  fs/btrfs/inode.c                      |    3 +
>  fs/btrfs/volumes.c                    |   11 +--
>  fs/fat/misc.c                         |    3 +
>  fs/fs-writeback.c                     |   53 ++++++++++++++----
>  fs/gfs2/inode.c                       |    6 +-
>  fs/inode.c                            |  100 +++++++++++-----------------------
>  fs/internal.h                         |    3 -
>  fs/orangefs/inode.c                   |    7 ++
>  fs/overlayfs/inode.c                  |    3 +
>  fs/sync.c                             |    4 -
>  fs/ubifs/file.c                       |    9 +--
>  fs/utimes.c                           |    1 
>  fs/xfs/xfs_iops.c                     |   29 ++++++++-
>  fs/xfs/xfs_super.c                    |   29 ---------
>  include/linux/fs.h                    |   17 +++--
>  include/trace/events/writeback.h      |    6 --
>  18 files changed, 152 insertions(+), 140 deletions(-)

This all looks pretty reasonable to me. There are a few changelog and
subject line typos, but the code changes look fine. You can add:

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

As far as nfsd's usage of FMODE_NOCMTIME, it looks OK to me. That's
implemented today by the check in file_modified_flags(), which is
generic and should work across filesystems.

The main exception is xfs_exchange_range() which has some special
handling for it, but nfsd doesn't use that functionality so that
shouldn't be an issue.

Am I missing some subtlety?

Re: [PATCH 06/14] organgefs: use inode_update_timestamps directly

[Jeff Layton]

Please do fix the spelling of orangefs in the subject line. It'll be
hard to grep for otherwise...

On Fri, 2025-11-14 at 07:26 +0100, Christoph Hellwig wrote:
> Orangefs has no i_version handling and __orangefs_setattr already
> explicitly marks the inode dirty.  So instead of the using
> the flags return value from generic_update_time, just call the
> lower level inode_update_timestamps helper directly.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/orangefs/inode.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
> index a01400cd41fd..55f6c8026812 100644
> --- a/fs/orangefs/inode.c
> +++ b/fs/orangefs/inode.c
> @@ -878,7 +878,9 @@ int orangefs_update_time(struct inode *inode, int flags)
>  
>  	gossip_debug(GOSSIP_INODE_DEBUG, "orangefs_update_time: %pU\n",
>  	    get_khandle_from_ino(inode));
> -	flags = generic_update_time(inode, flags);
> +
> +	flags = inode_update_timestamps(inode, flags);
> +
>  	memset(&iattr, 0, sizeof iattr);
>          if (flags & S_ATIME)
>  		iattr.ia_valid |= ATTR_ATIME;

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 06/14] organgefs: use inode_update_timestamps directly

[Christoph Hellwig]

On Fri, Nov 14, 2025 at 09:06:33AM -0500, Jeff Layton wrote:
> Please do fix the spelling of orangefs in the subject line. It'll be
> hard to grep for otherwise...

Ooops, sorry.

Re: re-enable IOCB_NOWAIT writes to files

[Christoph Hellwig]

On Fri, Nov 14, 2025 at 09:04:58AM -0500, Jeff Layton wrote:
> This all looks pretty reasonable to me. There are a few changelog and
> subject line typos, but the code changes look fine. You can add:

Please tell me about them so I can fix them. 

> As far as nfsd's usage of FMODE_NOCMTIME, it looks OK to me. That's
> implemented today by the check in file_modified_flags(), which is
> generic and should work across filesystems.

Nothing requires file_update_time / file_modified_flags are helpers
that a file system may or may not call.  I've not done an audit
if everyone actually uses them.

Re: [PATCH 14/14] xfs: enable non-blocking timestamp updates

[Christoph Hellwig]

On Fri, Nov 14, 2025 at 07:26:17AM +0100, Christoph Hellwig wrote:
> The lazytime path using generic_update_time can never block in XFS
> because there is no ->dirty_inode method that could block.  Allow
> non-blocking timestamp updates for this case.

As the report noted, it turns out my rebase lost the most important
thing here, which is to not reject S_NOWAIT for the lazytime path.
The incremental patch is below.  I'll resend on Monday, and officially
declare that Friday the 14th is the new Friday the 13th.

diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 3d7b89ffacde..35dbabf1e111 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1196,9 +1196,13 @@ xfs_vn_update_time(
 	trace_xfs_update_time(ip);
 
 	if (inode->i_sb->s_flags & SB_LAZYTIME) {
-		if (!((flags & S_VERSION) &&
-		      inode_maybe_inc_iversion(inode, false)))
-			return generic_update_time(inode, flags);
+		int updated = inode_update_timestamps(inode, flags);
+
+		if (!(updated & S_VERSION)) {
+			if (updated)
+				mark_inode_dirty_time(inode, updated);
+			return 0;
+		}
 
 		/* Capture the iversion update that just occurred */
 		log_flags |= XFS_ILOG_CORE;

Re: re-enable IOCB_NOWAIT writes to files

[Darrick J. Wong]

On Fri, Nov 14, 2025 at 09:04:58AM -0500, Jeff Layton wrote:
> On Fri, 2025-11-14 at 07:26 +0100, Christoph Hellwig wrote:
> > Hi all,
> > 
> > commit 66fa3cedf16a ("fs: Add async write file modification handling.")
> > effectively disabled IOCB_NOWAIT writes as timestamp updates currently
> > always require blocking, and the modern timestamp resolution means we
> > always update timestamps.  This leads to a lot of context switches from
> > applications using io_uring to submit file writes, making it often worse
> > than using the legacy aio code that is not using IOCB_NOWAIT.
> > 
> > This series allows non-blocking updates for lazytime if the file system
> > supports it, and adds that support for XFS.
> > 
> > It also fixes the layering bypass in btrfs when updating timestamps on
> > device files for devices removed from btrfs usage, and FMODE_NOCMTIME
> > handling in the VFS now that nfsd started using it.  Note that I'm still
> > not sure that nfsd usage is fully correct for all file systems, as only
> > XFS explicitly supports FMODE_NOCMTIME, but at least the generic code
> > does the right thing now.
> > 
> > Diffstat:
> >  Documentation/filesystems/locking.rst |    2 
> >  Documentation/filesystems/vfs.rst     |    6 ++
> >  fs/btrfs/inode.c                      |    3 +
> >  fs/btrfs/volumes.c                    |   11 +--
> >  fs/fat/misc.c                         |    3 +
> >  fs/fs-writeback.c                     |   53 ++++++++++++++----
> >  fs/gfs2/inode.c                       |    6 +-
> >  fs/inode.c                            |  100 +++++++++++-----------------------
> >  fs/internal.h                         |    3 -
> >  fs/orangefs/inode.c                   |    7 ++
> >  fs/overlayfs/inode.c                  |    3 +
> >  fs/sync.c                             |    4 -
> >  fs/ubifs/file.c                       |    9 +--
> >  fs/utimes.c                           |    1 
> >  fs/xfs/xfs_iops.c                     |   29 ++++++++-
> >  fs/xfs/xfs_super.c                    |   29 ---------
> >  include/linux/fs.h                    |   17 +++--
> >  include/trace/events/writeback.h      |    6 --
> >  18 files changed, 152 insertions(+), 140 deletions(-)
> 
> This all looks pretty reasonable to me. There are a few changelog and
> subject line typos, but the code changes look fine. You can add:
> 
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> 
> As far as nfsd's usage of FMODE_NOCMTIME, it looks OK to me. That's
> implemented today by the check in file_modified_flags(), which is
> generic and should work across filesystems.
> 
> The main exception is xfs_exchange_range() which has some special
> handling for it, but nfsd doesn't use that functionality so that
> shouldn't be an issue.
> 
> Am I missing some subtlety?

In exchangerange specifically?

The FMODE_NOCMTIME checks in xfs_exchange_range exist to tell the
exchange-range code to update cmtime, but only if it decides to actually
go through with the mapping exchange.  Since the mapping exchange
requires a transaction anyway, it's cheap to bundle in timestamp
updates.

Also there's no way that we can do nonblocking exchangerange so a NOWAIT
flag wouldn't be much help here anyway.

(I hope that answers your question)

--D

Re: re-enable IOCB_NOWAIT writes to files

[Jeff Layton]

On Fri, 2025-11-14 at 09:01 -0800, Darrick J. Wong wrote:
> On Fri, Nov 14, 2025 at 09:04:58AM -0500, Jeff Layton wrote:
> > On Fri, 2025-11-14 at 07:26 +0100, Christoph Hellwig wrote:
> > > Hi all,
> > > 
> > > commit 66fa3cedf16a ("fs: Add async write file modification handling.")
> > > effectively disabled IOCB_NOWAIT writes as timestamp updates currently
> > > always require blocking, and the modern timestamp resolution means we
> > > always update timestamps.  This leads to a lot of context switches from
> > > applications using io_uring to submit file writes, making it often worse
> > > than using the legacy aio code that is not using IOCB_NOWAIT.
> > > 
> > > This series allows non-blocking updates for lazytime if the file system
> > > supports it, and adds that support for XFS.
> > > 
> > > It also fixes the layering bypass in btrfs when updating timestamps on
> > > device files for devices removed from btrfs usage, and FMODE_NOCMTIME
> > > handling in the VFS now that nfsd started using it.  Note that I'm still
> > > not sure that nfsd usage is fully correct for all file systems, as only
> > > XFS explicitly supports FMODE_NOCMTIME, but at least the generic code
> > > does the right thing now.
> > > 
> > > Diffstat:
> > >  Documentation/filesystems/locking.rst |    2 
> > >  Documentation/filesystems/vfs.rst     |    6 ++
> > >  fs/btrfs/inode.c                      |    3 +
> > >  fs/btrfs/volumes.c                    |   11 +--
> > >  fs/fat/misc.c                         |    3 +
> > >  fs/fs-writeback.c                     |   53 ++++++++++++++----
> > >  fs/gfs2/inode.c                       |    6 +-
> > >  fs/inode.c                            |  100 +++++++++++-----------------------
> > >  fs/internal.h                         |    3 -
> > >  fs/orangefs/inode.c                   |    7 ++
> > >  fs/overlayfs/inode.c                  |    3 +
> > >  fs/sync.c                             |    4 -
> > >  fs/ubifs/file.c                       |    9 +--
> > >  fs/utimes.c                           |    1 
> > >  fs/xfs/xfs_iops.c                     |   29 ++++++++-
> > >  fs/xfs/xfs_super.c                    |   29 ---------
> > >  include/linux/fs.h                    |   17 +++--
> > >  include/trace/events/writeback.h      |    6 --
> > >  18 files changed, 152 insertions(+), 140 deletions(-)
> > 
> > This all looks pretty reasonable to me. There are a few changelog and
> > subject line typos, but the code changes look fine. You can add:
> > 
> > Reviewed-by: Jeff Layton <jlayton@kernel.org>
> > 
> > As far as nfsd's usage of FMODE_NOCMTIME, it looks OK to me. That's
> > implemented today by the check in file_modified_flags(), which is
> > generic and should work across filesystems.
> > 
> > The main exception is xfs_exchange_range() which has some special
> > handling for it, but nfsd doesn't use that functionality so that
> > shouldn't be an issue.
> > 
> > Am I missing some subtlety?
> 
> In exchangerange specifically?
> 
> The FMODE_NOCMTIME checks in xfs_exchange_range exist to tell the
> exchange-range code to update cmtime, but only if it decides to actually
> go through with the mapping exchange.  Since the mapping exchange
> requires a transaction anyway, it's cheap to bundle in timestamp
> updates.
> 
> Also there's no way that we can do nonblocking exchangerange so a NOWAIT
> flag wouldn't be much help here anyway.
> 
> (I hope that answers your question)
> 
> 

Christoph mentioned nfsd might be doing something wrong, which is my
main interest here. nfsd doesn't have a way to expose exchangerange
functionality right now, but if it did then it seems like that would
just work too.

HCH says:

> Nothing requires file_update_time / file_modified_flags are helpers
> that a file system may or may not call.  I've not done an audit
> if everyone actually uses them.

I'll have to think about how to efficiently audit that. The good news
is that nfsd really only cares about the write() and page_mkwrite()
codepaths. For other activity, the delegation will be broken and
recalled.
-- 
Jeff Layton <jlayton@kernel.org>

Re: re-enable IOCB_NOWAIT writes to files

[Jeff Layton]

On Fri, 2025-11-14 at 16:28 +0100, Christoph Hellwig wrote:
> On Fri, Nov 14, 2025 at 09:04:58AM -0500, Jeff Layton wrote:
> > This all looks pretty reasonable to me. There are a few changelog and
> > subject line typos, but the code changes look fine. You can add:
> 
> Please tell me about them so I can fix them. 
> 
> > As far as nfsd's usage of FMODE_NOCMTIME, it looks OK to me. That's
> > implemented today by the check in file_modified_flags(), which is
> > generic and should work across filesystems.
> 
> Nothing requires file_update_time / file_modified_flags are helpers
> that a file system may or may not call.  I've not done an audit
> if everyone actually uses them.

FWIW, I turned claude loose on this, and it produced:

-------------------------------8<----------------------------------
  Findings:                                                           
                                                                                                         
  1. coda_file_write_iter (fs/coda/file.c:66-94)                      
                                                                                                                                                                                                                   
  Location: fs/coda/file.c:86Issue: Manually updates ctime in
write_iter operation                                                  
  inode_set_mtime_to_ts(coda_inode,
inode_set_ctime_current(coda_inode));
  Context: This is a stacking filesystem that delegates writes to a
container file via vfs_iter_write(), then manually copies attributes
back. However, it bypasses file_update_time() or file_modified(),     
  which means it doesn't handle:                    
  - Read-only filesystem checks                                       
  - Immutable inode checks                                            
  - S_NOCMTIME flag                                                   
  - i_version updates                                                 
  - Proper dirty marking                                              
                                                                                                         
  2. efivarfs_file_write (fs/efivarfs/file.c:15-77)                   

  Location: fs/efivarfs/file.c:66Issue: Manually updates ctime in write
operation                        
  inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
  Context: Uses the legacy .write operation (not .write_iter), but
still a write path that should use proper timestamp helpers.

  3. ocfs2_write_end_nolock (fs/ocfs2/aops.c:1926-2050)

  Location: fs/ocfs2/aops.c:2024-2026Issue: Manually updates ctime in
write_end callback
  inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
  di->i_mtime = di->i_ctime = cpu_to_le64(inode_get_mtime_sec(inode));
  di->i_mtime_nsec = di->i_ctime_nsec =
cpu_to_le32(inode_get_mtime_nsec(inode));
  Context: This is called from OCFS2's write path. While
ocfs2_file_write_iter calls __generic_file_write_iter, the write_end
callback manually manages timestamps. This is used both by regular
writes and
  page_mkwrite via __ocfs2_page_mkwrite.

  4. ubifs_vm_page_mkwrite (fs/ubifs/file.c:1493-1580)

  Location: fs/ubifs/file.c:1570Issue: Manually updates ctime in
page_mkwrite operation
  inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
  Context: UBIFS implements its own timestamp update logic with
mctime_update_needed() and manual budgeting for space, bypassing
file_update_time().
-------------------------------8<----------------------------------

Only ocfs2 is exportable, so I think we want to convert that one. The
others I'm not sure of yet.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 14/14] xfs: enable non-blocking timestamp updates

[Dave Chinner]

On Fri, Nov 14, 2025 at 07:26:17AM +0100, Christoph Hellwig wrote:
> The lazytime path using generic_update_time can never block in XFS
> because there is no ->dirty_inode method that could block.  Allow
> non-blocking timestamp updates for this case.
> 
> Fixes: 66fa3cedf16a ("fs: Add async write file modification handling.")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_iops.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index bd0b7e81f6ab..3d7b89ffacde 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -1195,9 +1195,6 @@ xfs_vn_update_time(
>  
>  	trace_xfs_update_time(ip);
>  
> -	if (flags & S_NOWAIT)
> -		return -EAGAIN;
> -
>  	if (inode->i_sb->s_flags & SB_LAZYTIME) {
>  		if (!((flags & S_VERSION) &&
>  		      inode_maybe_inc_iversion(inode, false)))
> @@ -1207,6 +1204,9 @@ xfs_vn_update_time(
>  		log_flags |= XFS_ILOG_CORE;
>  	}
>  
> +	if (flags & S_NOWAIT)
> +		return -EAGAIN;
> +
>  	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_fsyncts, 0, 0, 0, &tp);
>  	if (error)
>  		return error;

Not sure this is correct - this can now bump iversion and then
return -EAGAIN. That means S_VERSION likely won't be set on the
retry, and we'll go straight through the non-blocking path to
generic_update_time() and skip logging the iversion update....

-Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 01/14] fs: refactor file timestamp update logic

[Chaitanya Kulkarni]

On 11/13/25 22:26, Christoph Hellwig wrote:
> Currently the two high-level APIs use two helper functions to implement
> almost all of the logic.  Refactor the two helpers and the common logic
> into a new file_update_time_flags routine that gets the iocb flags or
> 0 in case of file_update_time passed so that the entire logic is
> contained in a single function and can be easily understood and modified.
>
> Signed-off-by: Christoph Hellwig<hch@lst.de>

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH 02/14] fs: lift the FMODE_NOCMTIME check into file_update_time_flags

[Chaitanya Kulkarni]

On 11/13/25 22:26, Christoph Hellwig wrote:
> FMODE_NOCMTIME used to be just a hack for the legacy XFS handle-based
> "invisible I/O", but commit e5e9b24ab8fa ("nfsd: freeze c/mtime updates
> with outstanding WRITE_ATTRS delegation") started using it from
> generic callers.
>
> I'm not sure other file systems are actually read for this in general,
> so the above commit should get a closer look, but for it to make any
> sense, file_update_time needs to respect the flag.
>
> Lift the check from file_modified_flags to file_update_time so that
> users of file_update_time inherit the behavior and so that all the
> checks are done in one place.
>
> Fixes: e5e9b24ab8fa ("nfsd: freeze c/mtime updates with outstanding WRITE_ATTRS delegation")
> Signed-off-by: Christoph Hellwig<hch@lst.de>

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH 03/14] fs: export vfs_utimes

[Chaitanya Kulkarni]

On 11/13/25 22:26, Christoph Hellwig wrote:
> This will be used to replace an incorrect direct call into
> generic_update_time in btrfs.
>
> Signed-off-by: Christoph Hellwig<hch@lst.de>


Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH 05/14] fs: remove inode_update_time

[Chaitanya Kulkarni]

On 11/13/25 22:26, Christoph Hellwig wrote:
> The only external user is gone now, open code it in the two VFS
> callers.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   fs/inode.c         | 23 ++++++++---------------
>   include/linux/fs.h |  1 -
>   2 files changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 24dab63844db..d3edcc5baec9 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -2107,19 +2107,6 @@ int generic_update_time(struct inode *inode, int flags)
>   }
>   EXPORT_SYMBOL(generic_update_time);
>   
> -/*
> - * This does the actual work of updating an inodes time or version.  Must have
> - * had called mnt_want_write() before calling this.
> - */
> -int inode_update_time(struct inode *inode, int flags)
> -{
> -	if (inode->i_op->update_time)
> -		return inode->i_op->update_time(inode, flags);
> -	generic_update_time(inode, flags);
> -	return 0;
> -}
> -EXPORT_SYMBOL(inode_update_time);
> -
>   /**
>    *	atime_needs_update	-	update the access time
>    *	@path: the &struct path to update
> @@ -2187,7 +2174,10 @@ void touch_atime(const struct path *path)
>   	 * We may also fail on filesystems that have the ability to make parts
>   	 * of the fs read only, e.g. subvolumes in Btrfs.
>   	 */
> -	inode_update_time(inode, S_ATIME);
> +	if (inode->i_op->update_time)
> +		inode->i_op->update_time(inode, S_ATIME);
> +	else
> +		generic_update_time(inode, S_ATIME);
>   	mnt_put_write_access(mnt);
>   skip_update:
>   	sb_end_write(inode->i_sb);
> @@ -2342,7 +2332,10 @@ static int file_update_time_flags(struct file *file, unsigned int flags)
>   
>   	if (mnt_get_write_access_file(file))
>   		return 0;
> -	ret = inode_update_time(inode, sync_mode);
> +	if (inode->i_op->update_time)
> +		ret = inode->i_op->update_time(inode, sync_mode);
> +	else
> +		generic_update_time(inode, sync_mode);
>   	mnt_put_write_access_file(file);
>   	return ret;
>   }

do you need to catch the value from generic_update_time() to match
if case ? although original code was returning 0 for generic_update_time()
case :

	if (inode->i_op->update_time)
    		ret = inode->i_op->update_time(inode, sync_mode);
    	else
   -		generic_update_time(inode, sync_mode);
   +		ret = generic_update_time(inode, sync_mode);
    	mnt_put_write_access_file(file);
    	return ret;


if not ignore this comment, looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH 07/14] fs: return a negative error from generic_update_time

[Chaitanya Kulkarni]

On 11/13/25 22:26, Christoph Hellwig wrote:
> Now that no caller looks at the updated flags, switch generic_update_time
> to the same calling convention as the ->update_time method and return 0
> or a negative errno.
>
> This prepares for adding non-blocking timestamp updates that could return
> -EAGAIN.
>
> Signed-off-by: Christoph Hellwig<hch@lst.de>
> ---
>   fs/gfs2/inode.c    | 3 +--
>   fs/inode.c         | 6 +++---
>   fs/ubifs/file.c    | 6 ++----
>   fs/xfs/xfs_iops.c  | 6 ++----
>   include/linux/fs.h | 2 +-
>   5 files changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index 8a7ed80d9f2d..601c14a3ac77 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -2242,8 +2242,7 @@ static int gfs2_update_time(struct inode *inode, int flags)
>   		if (error)
>   			return error;
>   	}
> -	generic_update_time(inode, flags);
> -	return 0;
> +	return generic_update_time(inode, flags);
>   }
>   
>   static const struct inode_operations gfs2_file_iops = {
> diff --git a/fs/inode.c b/fs/inode.c
> index d3edcc5baec9..74e672dd90aa 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -2091,7 +2091,7 @@ EXPORT_SYMBOL(inode_update_timestamps);
>    * or S_VERSION need to be updated we attempt to update all three of them. S_ATIME
>    * updates can be handled done independently of the rest.
>    *
> - * Returns a S_* mask indicating which fields were updated.
> + * Returns a negative error value on error, else 0.
>    */
>   int generic_update_time(struct inode *inode, int flags)
>   {
> @@ -2103,7 +2103,7 @@ int generic_update_time(struct inode *inode, int flags)
>   	if (updated & S_VERSION)
>   		dirty_flags |= I_DIRTY_SYNC;
>   	__mark_inode_dirty(inode, dirty_flags);
> -	return updated;
> +	return 0;
>   }
>   EXPORT_SYMBOL(generic_update_time);
>   
> @@ -2335,7 +2335,7 @@ static int file_update_time_flags(struct file *file, unsigned int flags)
>   	if (inode->i_op->update_time)
>   		ret = inode->i_op->update_time(inode, sync_mode);
>   	else
> -		generic_update_time(inode, sync_mode);
> +		ret = generic_update_time(inode, sync_mode);
>   	mnt_put_write_access_file(file);
>   	return ret;
>   }

ret is assigned here ...

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH 09/14] fs: factor out a mark_inode_dirty_time helper

[Chaitanya Kulkarni]

On 11/13/25 22:26, Christoph Hellwig wrote:
> Factor out the inode dirtying vs lazytime logic from generic_update_time
> into a new helper so that it can be reused in file system methods.
>
> Signed-off-by: Christoph Hellwig<hch@lst.de>

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH 10/14] fs: factor out a sync_lazytime helper

[Chaitanya Kulkarni]

On 11/13/25 22:26, Christoph Hellwig wrote:
> Centralize how we synchronize a lazytime update into the actual on-disk
> timestamp into a single helper.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   fs/fs-writeback.c                | 27 +++++++++++++++++----------
>   fs/inode.c                       |  5 +----
>   fs/internal.h                    |  3 ++-
>   fs/sync.c                        |  4 ++--
>   include/trace/events/writeback.h |  6 ------
>   5 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 930697f39153..ae6d1f1ccc71 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1693,6 +1693,16 @@ static void requeue_inode(struct inode *inode, struct bdi_writeback *wb,
>   	}
>   }
>   
> +bool sync_lazytime(struct inode *inode)
> +{
> +	if (!(inode->i_state & I_DIRTY_TIME))
> +		return false;
> +
> +	trace_writeback_lazytime(inode);
> +	mark_inode_dirty_sync(inode);
> +	return false;
> +}
> +

This sync_lazytime() will always return false ?
shouldn't this be returning true at sometime if not then why not
change return type to void ?

returning same value doesn't add any value here ..

>   /*
>    * Write out an inode and its dirty pages (or some of its dirty pages, depending
>    * on @wbc->nr_to_write), and clear the relevant dirty flags from i_state.
> @@ -1732,17 +1742,14 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
>   	}
>   
>   	/*
> -	 * If the inode has dirty timestamps and we need to write them, call
> -	 * mark_inode_dirty_sync() to notify the filesystem about it and to
> -	 * change I_DIRTY_TIME into I_DIRTY_SYNC.
> +	 * For data integrity writeback, or when the dirty interval expired,
> +	 * ask the file system to propagata lazy timestamp updates into real
> +	 * dirty state.
>   	 */
> -	if ((inode->i_state & I_DIRTY_TIME) &&
> -	    (wbc->sync_mode == WB_SYNC_ALL ||
> -	     time_after(jiffies, inode->dirtied_time_when +
> -			dirtytime_expire_interval * HZ))) {
> -		trace_writeback_lazytime(inode);
> -		mark_inode_dirty_sync(inode);
> -	}
> +	if (wbc->sync_mode == WB_SYNC_ALL ||
> +	    time_after(jiffies, inode->dirtied_time_when +
> +			dirtytime_expire_interval * HZ))
> +		sync_lazytime(inode);
>   
>   	/*
>   	 * Get and clear the dirty flags from i_state.  This needs to be done
> diff --git a/fs/inode.c b/fs/inode.c
> index 559ce5c07188..34d572c99313 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1942,11 +1942,8 @@ void iput(struct inode *inode)
>   	if (atomic_add_unless(&inode->i_count, -1, 1))
>   		return;
>   
> -	if ((inode->i_state & I_DIRTY_TIME) && inode->i_nlink) {
> -		trace_writeback_lazytime_iput(inode);
> -		mark_inode_dirty_sync(inode);
> +	if (inode->i_nlink && sync_lazytime(inode))

since sync_lazytime() is always returning false goto below will
never execute ? which makes following goto dead code in this patch ?


otherwise, looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH 11/14] fs: add a ->sync_lazytime method

[Chaitanya Kulkarni]

On 11/13/25 22:26, Christoph Hellwig wrote:
> Allow the file system to explicitly implement lazytime syncing instead
> of pigging back on generic inode dirtying.  This allows to simplify
> the XFS implementation and prepares for non-blocking lazytime timestamp
> updates.
>
> Signed-off-by: Christoph Hellwig<hch@lst.de>
> ---
>   Documentation/filesystems/locking.rst |  2 ++
>   Documentation/filesystems/vfs.rst     |  6 ++++++
>   fs/fs-writeback.c                     | 13 +++++++++++--
>   include/linux/fs.h                    |  1 +
>   4 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst
> index 77704fde9845..9b2f14ada8cd 100644
> --- a/Documentation/filesystems/locking.rst
> +++ b/Documentation/filesystems/locking.rst
> @@ -81,6 +81,7 @@ prototypes::
>   	ssize_t (*listxattr) (struct dentry *, char *, size_t);
>   	int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start, u64 len);
>   	void (*update_time)(struct inode *, struct timespec *, int);
> +	void (*sync_lazytime)(struct inode *inode);
>   	int (*atomic_open)(struct inode *, struct dentry *,
>   				struct file *, unsigned open_flag,
>   				umode_t create_mode);
> @@ -117,6 +118,7 @@ getattr:	no
>   listxattr:	no
>   fiemap:		no
>   update_time:	no
> +sync_lazytime:	no
>   atomic_open:	shared (exclusive if O_CREAT is set in open flags)
>   tmpfile:	no
>   fileattr_get:	no or exclusive
> diff --git a/Documentation/filesystems/vfs.rst b/Documentation/filesystems/vfs.rst
> index 4f13b01e42eb..ff59760daae2 100644
> --- a/Documentation/filesystems/vfs.rst
> +++ b/Documentation/filesystems/vfs.rst
> @@ -486,6 +486,7 @@ As of kernel 2.6.22, the following members are defined:
>   		int (*getattr) (struct mnt_idmap *, const struct path *, struct kstat *, u32, unsigned int);
>   		ssize_t (*listxattr) (struct dentry *, char *, size_t);
>   		void (*update_time)(struct inode *, struct timespec *, int);
> +		void (*sync_lazytime)(struct inode *inode);
>   		int (*atomic_open)(struct inode *, struct dentry *, struct file *,
>   				   unsigned open_flag, umode_t create_mode);
>   		int (*tmpfile) (struct mnt_idmap *, struct inode *, struct file *, umode_t);
> @@ -642,6 +643,11 @@ otherwise noted.
>   	an inode.  If this is not defined the VFS will update the inode
>   	itself and call mark_inode_dirty_sync.
>   
> +``sync_lazytime``:
> +	called by the writeback code to update the lazy time stamps to
> +	regular time stamp updates that get syncing into the on-disk
> +	inode.
> +
>   ``atomic_open``
>   	called on the last component of an open.  Using this optional
>   	method the filesystem can look up, possibly create and open the
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index ae6d1f1ccc71..7245f547416f 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1699,7 +1699,10 @@ bool sync_lazytime(struct inode *inode)
>   		return false;
>   
>   	trace_writeback_lazytime(inode);
> -	mark_inode_dirty_sync(inode);
> +	if (inode->i_op->sync_lazytime)
> +		inode->i_op->sync_lazytime(inode);
> +	else
> +		mark_inode_dirty_sync(inode);
>   	return false;
>   }
>   
> @@ -2547,6 +2550,8 @@ void __mark_inode_dirty(struct inode *inode, int flags)
>   	trace_writeback_mark_inode_dirty(inode, flags);
>   
>   	if (flags & I_DIRTY_INODE) {
> +		bool was_dirty_time = true;
> +
>   		/*
>   		 * Inode timestamp update will piggback on this dirtying.
>   		 * We tell ->dirty_inode callback that timestamps need to
> @@ -2557,6 +2562,7 @@ void __mark_inode_dirty(struct inode *inode, int flags)
>   			if (inode->i_state & I_DIRTY_TIME) {
>   				inode->i_state &= ~I_DIRTY_TIME;
>   				flags |= I_DIRTY_TIME;
> +				was_dirty_time = true;
>   			}
>   			spin_unlock(&inode->i_lock);
>   		}

was_dirty_time is initialized it to true and above true again,
perhaps it should be initialized it to false, so it will be only set to
true when inode->i_state &= I_DIRTY_TIME; ?

if was_dirty_time always set to true at the time of initialization then
below check will always call as long as callback is set
inode->i_op->sync_lazytime(), irrespective of
inode->i_state &= I_DIRTY_TIME; assignment.


-ck

Re: [PATCH 05/14] fs: remove inode_update_time

[Christoph Hellwig]

On Mon, Nov 17, 2025 at 06:59:25AM +0000, Chaitanya Kulkarni wrote:
> > -	ret = inode_update_time(inode, sync_mode);
> > +	if (inode->i_op->update_time)
> > +		ret = inode->i_op->update_time(inode, sync_mode);
> > +	else
> > +		generic_update_time(inode, sync_mode);
> >   	mnt_put_write_access_file(file);
> >   	return ret;
> >   }
> 
> do you need to catch the value from generic_update_time() to match
> if case ? although original code was returning 0 for generic_update_time()
> case :

Yes.  It doesn't matter for this series, but it's good future-proofing.

Re: [PATCH 10/14] fs: factor out a sync_lazytime helper

[Christoph Hellwig]

On Mon, Nov 17, 2025 at 07:38:29AM +0000, Chaitanya Kulkarni wrote:
> 
> This sync_lazytime() will always return false ?
> shouldn't this be returning true at sometime if not then why not
> change return type to void ?
> 
> returning same value doesn't add any value here ..

Yes, it should return true when actually doing work.  So currently the
syncing on final inode eviction is broken, but it looks like non of the
tests actually hits it.

Re: [PATCH 14/14] xfs: enable non-blocking timestamp updates

[Christoph Hellwig]

On Sun, Nov 16, 2025 at 07:23:11PM +1100, Dave Chinner wrote:
> On Fri, Nov 14, 2025 at 07:26:17AM +0100, Christoph Hellwig wrote:
> > The lazytime path using generic_update_time can never block in XFS
> > because there is no ->dirty_inode method that could block.  Allow
> > non-blocking timestamp updates for this case.
> > 
> > Fixes: 66fa3cedf16a ("fs: Add async write file modification handling.")
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  fs/xfs/xfs_iops.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> > index bd0b7e81f6ab..3d7b89ffacde 100644
> > --- a/fs/xfs/xfs_iops.c
> > +++ b/fs/xfs/xfs_iops.c
> > @@ -1195,9 +1195,6 @@ xfs_vn_update_time(
> >  
> >  	trace_xfs_update_time(ip);
> >  
> > -	if (flags & S_NOWAIT)
> > -		return -EAGAIN;
> > -
> >  	if (inode->i_sb->s_flags & SB_LAZYTIME) {
> >  		if (!((flags & S_VERSION) &&
> >  		      inode_maybe_inc_iversion(inode, false)))
> > @@ -1207,6 +1204,9 @@ xfs_vn_update_time(
> >  		log_flags |= XFS_ILOG_CORE;
> >  	}
> >  
> > +	if (flags & S_NOWAIT)
> > +		return -EAGAIN;
> > +
> >  	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_fsyncts, 0, 0, 0, &tp);
> >  	if (error)
> >  		return error;
> 
> Not sure this is correct - this can now bump iversion and then
> return -EAGAIN. That means S_VERSION likely won't be set on the
> retry, and we'll go straight through the non-blocking path to
> generic_update_time() and skip logging the iversion update....

Thanks.

I'll fix this by propagating S_NOWAIT to inode_update_timestamps.