[RFC PATCH 0/3] io_uring: add IORING_OP_UTIMENSAT

[RFC PATCH 0/3] io_uring: add IORING_OP_UTIMENSAT

[Méven Car]

A file manager or cp -p that copies many files sets the times of every
copy after its last write. KDE's KIO does not use io_uring yet, but
could in the future. In a KIO prototype that copies batches of small
files through io_uring, open, create, read and write run in the ring,
but the metadata of each copy is set outside the ring, with futimens()
and, when needed, fchmod() and fchown(). On NFS and SMB each of these
calls is a round trip to the server.

In that prototype, setting the metadata from a pool of 16 threads
instead of one file at a time cut the copy of 200 files of 4 KiB by 22%
on NFS and 45% on SMB. The test copied from ext4 to an NFS 4.2 and an
SMB mount of the same machine, over loopback with a netem delay of 2 ms
each way. An opcode would give the same parallelism from the ring, and
would let a copy be one linked chain per file: WRITE, UTIMENSAT, CLOSE.

Patch 1 splits filename_utimensat() out of do_utimes_path(), so that
io_uring can pass a name imported at submission time, as it does for
mkdirat and the other path ops.

Patch 2 adds IORING_OP_UTIMENSAT with the semantics of utimensat(2).
Like the other path ops it runs from io-wq.

Patch 3 is the RFC part. On a local filesystem the handoff to io-wq
costs more than the setattr: in a qemu guest on v7.2.3, setting the
times of 2000 files on tmpfs took 6-8 ms with utimensat(2) and 19-29 ms
through the ring with patch 2 alone, 32 requests per submit. The patch
tries the request inline with LOOKUP_CACHED and trylocks, on filesystems
that declare FS_SETATTR_TIMES_NOWAIT, which only tmpfs sets. The ring
then matches the syscall (6.4-6.8 ms). ext4, xfs and btrfs cannot set
the flag, because their ->setattr() starts a journal transaction. Is a
per-filesystem flag acceptable here, or would you rather keep patch 2
alone?

IORING_OP_FCHMOD was proposed in 2024 [1] and not merged. The same
questions apply to fchmod and fchown, which a copy also needs, and a
single setattr opcode could set the mode, owner and times in one
->setattr() call. I started with the times because a copy always sets
them.

Testing: a userspace test of 28 cases (paths, AT_EMPTY_PATH on O_PATH
fds, NULL path, UTIME_OMIT, permission and read-only errors, links, copy
at submission, inline on tmpfs and punt on ramfs) passes in a qemu
guest, on v7.2.3 and on for-7.4/io_uring. A liburing helper, test and
man page will follow if the interface is acceptable.

[1] https://lore.kernel.org/io-uring/e291085644e14b3eb4d1c3995098bf4e@huawei.com/

Méven Car (3):
  fs: add filename_utimensat()
  io_uring: add support for utimensat
  io_uring: set times inline when the filesystem cannot sleep

 fs/internal.h                 |   3 +
 fs/namespace.c                |  20 +++++++
 fs/utimes.c                   |  56 ++++++++++++++----
 include/linux/fs.h            |   1 +
 include/uapi/linux/io_uring.h |   2 +
 io_uring/fs.c                 | 105 ++++++++++++++++++++++++++++++++++
 io_uring/fs.h                 |   4 ++
 io_uring/opdef.c              |   8 +++
 mm/shmem.c                    |   3 +-
 9 files changed, 191 insertions(+), 11 deletions(-)


base-commit: ecb6817b769fd7d71af2c9f446188649020b2e53
-- 
2.56.0

[RFC PATCH 1/3] fs: add filename_utimensat()

[Méven Car]

Copy utilities that preserve timestamps, like cp -p or a file manager,
set the times of every copy after its last write. To do that through
io_uring, the request needs the utimensat() lookup on a name that was
imported at submission time.

io_uring takes the path of a request when it is submitted and looks it
up later, from a worker thread. The path ops it already has call the
filename_*() helpers of fs/namei.c with a struct filename. utimensat(2)
had no such helper: do_utimes_path() took the user pointer and imported
the name itself.

Split the lookup and the vfs_utimes() call out of do_utimes_path() into
filename_utimensat(), declared in fs/internal.h next to the other
filename_*() helpers. do_utimes_path() now only imports the name.

Signed-off-by: Méven Car <meven@kde.org>
---
 fs/internal.h |  2 ++
 fs/utimes.c   | 12 +++++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index c658c8a5ebd5..985b2bc5895e 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -63,6 +63,8 @@ int filename_mknodat(int dfd, struct filename *name, umode_t mode, unsigned int
 int filename_symlinkat(struct filename *from, int newdfd, struct filename *to);
 int filename_linkat(int olddfd, struct filename *old, int newdfd,
 			struct filename *new, int flags);
+int filename_utimensat(int dfd, struct filename *name,
+		       struct timespec64 *times, int flags);
 int vfs_tmpfile(struct mnt_idmap *idmap,
 		const struct path *parentpath,
 		struct file *file, umode_t mode);
diff --git a/fs/utimes.c b/fs/utimes.c
index e22664e4115f..8072f05ac8dc 100644
--- a/fs/utimes.c
+++ b/fs/utimes.c
@@ -79,8 +79,8 @@ int vfs_utimes(const struct path *path, struct timespec64 *times)
 }
 EXPORT_SYMBOL_GPL(vfs_utimes);
 
-static int do_utimes_path(int dfd, const char __user *filename,
-		struct timespec64 *times, int flags)
+int filename_utimensat(int dfd, struct filename *name,
+		       struct timespec64 *times, int flags)
 {
 	struct path path;
 	int lookup_flags = 0, error;
@@ -91,7 +91,6 @@ static int do_utimes_path(int dfd, const char __user *filename,
 	if (!(flags & AT_SYMLINK_NOFOLLOW))
 		lookup_flags |= LOOKUP_FOLLOW;
 
-	CLASS(filename_uflags, name)(filename, flags);
 retry:
 	error = filename_lookup(dfd, name, lookup_flags, &path, NULL);
 	if (error)
@@ -105,6 +104,13 @@ static int do_utimes_path(int dfd, const char __user *filename,
 	return error;
 }
 
+static int do_utimes_path(int dfd, const char __user *filename,
+		struct timespec64 *times, int flags)
+{
+	CLASS(filename_uflags, name)(filename, flags);
+	return filename_utimensat(dfd, name, times, flags);
+}
+
 static int do_utimes_fd(int fd, struct timespec64 *times, int flags)
 {
 	if (flags)
-- 
2.56.0

[RFC PATCH 2/3] io_uring: add support for utimensat

[Méven Car]

A program that copies many files sets the times of every copy after
its last write. On NFS and SMB each of those calls is a round trip to
the server, and with io_uring it is one of the steps of the copy that
still have to leave the ring and run one file at a time.

IORING_OP_UTIMENSAT behaves like utimensat(2). sqe->fd is the directory
fd, sqe->addr the path, sqe->addr2 a pointer to two struct
__kernel_timespec, and sqe->utimes_flags takes AT_SYMLINK_NOFOLLOW and
AT_EMPTY_PATH. An addr2 of zero sets both times to the current time, as
a NULL times argument does. The times and the path are copied at
submission.

As with utimensat(2), a NULL path sets the times of sqe->fd itself, and
two UTIME_OMIT times complete with 0 without a lookup. Unlike the
syscall, a NULL path or AT_EMPTY_PATH also works on an O_PATH
descriptor.

Like the other path ops, the request is issued from io-wq, apart from
the no-op, because the lookup and notify_change() can sleep. A fixed
file is not accepted as the directory fd. A failed request fails the
rest of its link.

Signed-off-by: Méven Car <meven@kde.org>
---
 include/uapi/linux/io_uring.h |  2 +
 io_uring/fs.c                 | 99 +++++++++++++++++++++++++++++++++++
 io_uring/fs.h                 |  4 ++
 io_uring/opdef.c              |  8 +++
 4 files changed, 113 insertions(+)

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index b42b66b424a7..6a98c9282e0b 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -76,6 +76,7 @@ struct io_uring_sqe {
 		__u32		install_fd_flags;
 		__u32		nop_flags;
 		__u32		pipe_flags;
+		__u32		utimes_flags;
 	};
 	__u64	user_data;	/* data to be passed back at completion time */
 	/* pack this to avoid bogus arm OABI complaints */
@@ -319,6 +320,7 @@ enum io_uring_op {
 	IORING_OP_PIPE,
 	IORING_OP_NOP128,
 	IORING_OP_URING_CMD128,
+	IORING_OP_UTIMENSAT,
 
 	/* this goes last, obviously */
 	IORING_OP_LAST,
diff --git a/io_uring/fs.c b/io_uring/fs.c
index 26ea841a22e7..85e0a1cb6b47 100644
--- a/io_uring/fs.c
+++ b/io_uring/fs.c
@@ -304,3 +304,102 @@ void io_link_cleanup(struct io_kiocb *req)
 	dismiss_delayed_filename(&sl->oldpath);
 	dismiss_delayed_filename(&sl->newpath);
 }
+
+/* Internal flags, above the AT_ flags of the SQE */
+#define IO_UTIMENSAT_NOOP	BIT(30)	/* both times UTIME_OMIT */
+#define IO_UTIMENSAT_NO_PATH	BIT(31)	/* NULL path: the dfd itself */
+
+struct io_utimensat {
+	struct file			*file;
+	int				dfd;
+	unsigned int			flags;
+	struct delayed_filename		filename;
+	/* NULL sets both times to the current time */
+	struct timespec64		*times;
+	struct timespec64		ts[2];
+};
+
+int io_utimensat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+	struct io_utimensat *ut = io_kiocb_to_cmd(req, struct io_utimensat);
+	struct __kernel_timespec __user *uts;
+	const char __user *fname;
+	int err;
+
+	if (sqe->len || sqe->buf_index || sqe->splice_fd_in || sqe->addr3 ||
+	    sqe->__pad2[0])
+		return -EINVAL;
+	if (unlikely(req->flags & REQ_F_FIXED_FILE))
+		return -EBADF;
+
+	ut->dfd = READ_ONCE(sqe->fd);
+	ut->flags = READ_ONCE(sqe->utimes_flags);
+	if (ut->flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH))
+		return -EINVAL;
+
+	uts = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+	ut->times = NULL;
+	if (uts) {
+		if (get_timespec64(&ut->ts[0], &uts[0]) ||
+		    get_timespec64(&ut->ts[1], &uts[1]))
+			return -EFAULT;
+		ut->times = ut->ts;
+		if (ut->ts[0].tv_nsec == UTIME_OMIT &&
+		    ut->ts[1].tv_nsec == UTIME_OMIT) {
+			ut->flags |= IO_UTIMENSAT_NOOP;
+			return 0;
+		}
+	}
+
+	fname = u64_to_user_ptr(READ_ONCE(sqe->addr));
+	if (!fname) {
+		if (ut->dfd == AT_FDCWD)
+			return -EFAULT;
+		ut->flags |= IO_UTIMENSAT_NO_PATH;
+		goto out;
+	}
+	err = delayed_getname_uflags(&ut->filename, fname, ut->flags);
+	if (unlikely(err))
+		return err;
+	req->flags |= REQ_F_NEED_CLEANUP;
+out:
+	req->flags |= REQ_F_FORCE_ASYNC;
+	return 0;
+}
+
+static int __io_utimensat(struct io_utimensat *ut, struct filename *name)
+{
+	int flags = ut->flags & (AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH);
+
+	return filename_utimensat(ut->dfd, name, ut->times, flags);
+}
+
+int io_utimensat(struct io_kiocb *req, unsigned int issue_flags)
+{
+	struct io_utimensat *ut = io_kiocb_to_cmd(req, struct io_utimensat);
+	int ret = 0;
+
+	WARN_ON_ONCE((issue_flags & IO_URING_F_NONBLOCK) &&
+		     !(ut->flags & IO_UTIMENSAT_NOOP));
+
+	if (ut->flags & IO_UTIMENSAT_NO_PATH) {
+		CLASS(filename_kernel, name)("");
+		ret = __io_utimensat(ut, name);
+	} else if (!(ut->flags & IO_UTIMENSAT_NOOP)) {
+		CLASS(filename_complete_delayed, name)(&ut->filename);
+		ret = __io_utimensat(ut, name);
+	}
+
+	req->flags &= ~REQ_F_NEED_CLEANUP;
+	if (ret < 0)
+		req_set_fail(req);
+	io_req_set_res(req, ret, 0);
+	return IOU_COMPLETE;
+}
+
+void io_utimensat_cleanup(struct io_kiocb *req)
+{
+	struct io_utimensat *ut = io_kiocb_to_cmd(req, struct io_utimensat);
+
+	dismiss_delayed_filename(&ut->filename);
+}
diff --git a/io_uring/fs.h b/io_uring/fs.h
index 0bb5efe3d6bb..2812dc3b4b2f 100644
--- a/io_uring/fs.h
+++ b/io_uring/fs.h
@@ -18,3 +18,7 @@ int io_symlinkat(struct io_kiocb *req, unsigned int issue_flags);
 int io_linkat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
 int io_linkat(struct io_kiocb *req, unsigned int issue_flags);
 void io_link_cleanup(struct io_kiocb *req);
+
+int io_utimensat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_utimensat(struct io_kiocb *req, unsigned int issue_flags);
+void io_utimensat_cleanup(struct io_kiocb *req);
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index cf3aa2242cd7..383a9b9d1df4 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -591,6 +591,10 @@ const struct io_issue_def io_issue_defs[] = {
 		.prep			= io_uring_cmd_prep,
 		.issue			= io_uring_cmd,
 	},
+	[IORING_OP_UTIMENSAT] = {
+		.prep			= io_utimensat_prep,
+		.issue			= io_utimensat,
+	},
 };
 
 const struct io_cold_def io_cold_defs[] = {
@@ -849,6 +853,10 @@ const struct io_cold_def io_cold_defs[] = {
 		.sqe_copy		= io_uring_cmd_sqe_copy,
 		.cleanup		= io_uring_cmd_cleanup,
 	},
+	[IORING_OP_UTIMENSAT] = {
+		.name			= "UTIMENSAT",
+		.cleanup		= io_utimensat_cleanup,
+	},
 };
 
 const char *io_uring_get_opcode(u8 opcode)
-- 
2.56.0

[RFC PATCH 3/3] io_uring: set times inline when the filesystem cannot sleep

[Méven Car]

IORING_OP_UTIMENSAT always went to io-wq. On tmpfs the setattr only
changes the inode in memory, so the handoff to a worker cost more than
the work: 2000 files took 7 ms with utimensat(2) and 20 ms through the
ring.

Try the request inline first. The lookup uses LOOKUP_CACHED, and the
freeze protection and the inode lock are only tried, the former with
the new mnt_want_write_trylock(). A filesystem whose ->setattr() never
sleeps for a change of times alone says so with
FS_SETATTR_TIMES_NOWAIT, which tmpfs sets. Any other filesystem, an
uncached path, a contended lock or a delegation returns -EAGAIN, and
io_uring sends the request to io-wq as before.

Signed-off-by: Méven Car <meven@kde.org>
---
 fs/internal.h      |  3 ++-
 fs/namespace.c     | 20 +++++++++++++++++++
 fs/utimes.c        | 48 +++++++++++++++++++++++++++++++++++++---------
 include/linux/fs.h |  1 +
 io_uring/fs.c      | 26 +++++++++++++++----------
 mm/shmem.c         |  3 ++-
 6 files changed, 80 insertions(+), 21 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index 985b2bc5895e..e991b63bb714 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -64,7 +64,7 @@ int filename_symlinkat(struct filename *from, int newdfd, struct filename *to);
 int filename_linkat(int olddfd, struct filename *old, int newdfd,
 			struct filename *new, int flags);
 int filename_utimensat(int dfd, struct filename *name,
-		       struct timespec64 *times, int flags);
+		       struct timespec64 *times, int flags, bool nowait);
 int vfs_tmpfile(struct mnt_idmap *idmap,
 		const struct path *parentpath,
 		struct file *file, umode_t mode);
@@ -86,6 +86,7 @@ extern int sb_prepare_remount_readonly(struct super_block *);
 extern void __init mnt_init(void);
 
 int mnt_get_write_access_file(struct file *file);
+int mnt_want_write_trylock(struct vfsmount *m);
 void mnt_put_write_access_file(struct file *file);
 
 extern void dissolve_on_fput(struct vfsmount *);
diff --git a/fs/namespace.c b/fs/namespace.c
index ae5dc64f8b45..f4aadd4e1958 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -499,6 +499,26 @@ int mnt_want_write(struct vfsmount *m)
 }
 EXPORT_SYMBOL_GPL(mnt_want_write);
 
+/**
+ * mnt_want_write_trylock - get write access to a mount without waiting
+ * @m: the mount on which to take a write
+ *
+ * Like mnt_want_write(), but returns -EAGAIN instead of waiting when the
+ * filesystem is frozen.  When the write operation is finished,
+ * mnt_drop_write() must be called.
+ */
+int mnt_want_write_trylock(struct vfsmount *m)
+{
+	int ret;
+
+	if (!sb_start_write_trylock(m->mnt_sb))
+		return -EAGAIN;
+	ret = mnt_get_write_access(m);
+	if (ret)
+		sb_end_write(m->mnt_sb);
+	return ret;
+}
+
 /**
  * mnt_get_write_access_file - get write access to a file's mount
  * @file: the file who's mount on which to take a write
diff --git a/fs/utimes.c b/fs/utimes.c
index 8072f05ac8dc..f70ad9003fa9 100644
--- a/fs/utimes.c
+++ b/fs/utimes.c
@@ -18,7 +18,9 @@ static bool nsec_valid(long nsec)
 	return nsec >= 0 && nsec <= 999999999;
 }
 
-int vfs_utimes(const struct path *path, struct timespec64 *times)
+/* With @nowait, returns -EAGAIN instead of sleeping */
+static int __vfs_utimes(const struct path *path, struct timespec64 *times,
+			bool nowait)
 {
 	int error;
 	struct iattr newattrs;
@@ -34,9 +36,17 @@ int vfs_utimes(const struct path *path, struct timespec64 *times)
 			times = NULL;
 	}
 
-	error = mnt_want_write(path->mnt);
-	if (error)
-		goto out;
+	if (nowait) {
+		if (!(inode->i_sb->s_type->fs_flags & FS_SETATTR_TIMES_NOWAIT))
+			return -EAGAIN;
+		error = mnt_want_write_trylock(path->mnt);
+		if (error)
+			goto out;
+	} else {
+		error = mnt_want_write(path->mnt);
+		if (error)
+			goto out;
+	}
 
 	newattrs.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_ATIME;
 	if (times) {
@@ -63,24 +73,40 @@ int vfs_utimes(const struct path *path, struct timespec64 *times)
 		newattrs.ia_valid |= ATTR_TOUCH;
 	}
 retry_deleg:
-	inode_lock(inode);
+	if (!nowait) {
+		inode_lock(inode);
+	} else if (!inode_trylock(inode)) {
+		error = -EAGAIN;
+		goto drop;
+	}
 	error = notify_change(mnt_idmap(path->mnt), path->dentry, &newattrs,
 			      &delegated_inode);
 	inode_unlock(inode);
 	if (is_delegated(&delegated_inode)) {
+		if (nowait) {
+			iput(delegated_inode.di_inode);
+			delegated_inode.di_inode = NULL;
+			error = -EAGAIN;
+			goto drop;
+		}
 		error = break_deleg_wait(&delegated_inode);
 		if (!error)
 			goto retry_deleg;
 	}
-
+drop:
 	mnt_drop_write(path->mnt);
 out:
 	return error;
 }
+
+int vfs_utimes(const struct path *path, struct timespec64 *times)
+{
+	return __vfs_utimes(path, times, false);
+}
 EXPORT_SYMBOL_GPL(vfs_utimes);
 
 int filename_utimensat(int dfd, struct filename *name,
-		       struct timespec64 *times, int flags)
+		       struct timespec64 *times, int flags, bool nowait)
 {
 	struct path path;
 	int lookup_flags = 0, error;
@@ -90,13 +116,17 @@ int filename_utimensat(int dfd, struct filename *name,
 
 	if (!(flags & AT_SYMLINK_NOFOLLOW))
 		lookup_flags |= LOOKUP_FOLLOW;
+	if (nowait)
+		lookup_flags |= LOOKUP_CACHED;
 
 retry:
 	error = filename_lookup(dfd, name, lookup_flags, &path, NULL);
 	if (error)
 		return error;
-	error = vfs_utimes(&path, times);
+	error = __vfs_utimes(&path, times, nowait);
 	path_put(&path);
+	if (nowait && error == -ESTALE)
+		return -EAGAIN;
 	if (retry_estale(error, lookup_flags)) {
 		lookup_flags |= LOOKUP_REVAL;
 		goto retry;
@@ -108,7 +138,7 @@ static int do_utimes_path(int dfd, const char __user *filename,
 		struct timespec64 *times, int flags)
 {
 	CLASS(filename_uflags, name)(filename, flags);
-	return filename_utimensat(dfd, name, times, flags);
+	return filename_utimensat(dfd, name, times, flags, false);
 }
 
 static int do_utimes_fd(int fd, struct timespec64 *times, int flags)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f9d1e05e8ae6..d1800fba15d8 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2296,6 +2296,7 @@ struct file_system_type {
 #define FS_POWER_FREEZE		256	/* Always freeze on suspend/hibernate */
 #define FS_USERNS_MOUNT_RESTRICTED 512	/* Restrict mount in userns if not already visible */
 #define FS_USERNS_DELEGATABLE	1024	/* Can be mounted inside userns from outside */
+#define FS_SETATTR_TIMES_NOWAIT	2048	/* ->setattr() of times alone never sleeps */
 #define FS_RENAME_DOES_D_MOVE	32768	/* FS will handle d_move() during rename() internally. */
 	int (*init_fs_context)(struct fs_context *);
 	const struct fs_parameter_spec *parameters;
diff --git a/io_uring/fs.c b/io_uring/fs.c
index 85e0a1cb6b47..79f6f39ce267 100644
--- a/io_uring/fs.c
+++ b/io_uring/fs.c
@@ -356,38 +356,44 @@ int io_utimensat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		if (ut->dfd == AT_FDCWD)
 			return -EFAULT;
 		ut->flags |= IO_UTIMENSAT_NO_PATH;
-		goto out;
+		return 0;
 	}
 	err = delayed_getname_uflags(&ut->filename, fname, ut->flags);
 	if (unlikely(err))
 		return err;
 	req->flags |= REQ_F_NEED_CLEANUP;
-out:
-	req->flags |= REQ_F_FORCE_ASYNC;
 	return 0;
 }
 
-static int __io_utimensat(struct io_utimensat *ut, struct filename *name)
+static int __io_utimensat(struct io_utimensat *ut, struct filename *name,
+			  bool force_nonblock)
 {
 	int flags = ut->flags & (AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH);
 
-	return filename_utimensat(ut->dfd, name, ut->times, flags);
+	return filename_utimensat(ut->dfd, name, ut->times, flags,
+				  force_nonblock);
 }
 
 int io_utimensat(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_utimensat *ut = io_kiocb_to_cmd(req, struct io_utimensat);
+	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
 	int ret = 0;
 
-	WARN_ON_ONCE((issue_flags & IO_URING_F_NONBLOCK) &&
-		     !(ut->flags & IO_UTIMENSAT_NOOP));
-
 	if (ut->flags & IO_UTIMENSAT_NO_PATH) {
 		CLASS(filename_kernel, name)("");
-		ret = __io_utimensat(ut, name);
+		ret = __io_utimensat(ut, name, force_nonblock);
+		if (ret == -EAGAIN && force_nonblock)
+			return -EAGAIN;
 	} else if (!(ut->flags & IO_UTIMENSAT_NOOP)) {
 		CLASS(filename_complete_delayed, name)(&ut->filename);
-		ret = __io_utimensat(ut, name);
+		ret = __io_utimensat(ut, name, force_nonblock);
+		if (ret == -EAGAIN && force_nonblock) {
+			ret = putname_to_delayed(&ut->filename,
+						 no_free_ptr(name));
+			if (likely(!ret))
+				return -EAGAIN;
+		}
 	}
 
 	req->flags &= ~REQ_F_NEED_CLEANUP;
diff --git a/mm/shmem.c b/mm/shmem.c
index 897fa2b61346..405b39fc5d0e 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -5310,7 +5310,8 @@ static struct file_system_type shmem_fs_type = {
 	.parameters	= shmem_fs_parameters,
 #endif
 	.kill_sb	= kill_anon_super,
-	.fs_flags	= FS_USERNS_MOUNT | FS_ALLOW_IDMAP | FS_MGTIME,
+	.fs_flags	= FS_USERNS_MOUNT | FS_ALLOW_IDMAP | FS_MGTIME |
+			  FS_SETATTR_TIMES_NOWAIT,
 };
 
 #if defined(CONFIG_SYSFS) && defined(CONFIG_TMPFS)
-- 
2.56.0

Re: [RFC PATCH 2/3] io_uring: add support for utimensat

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] io_uring: missing flags check and wrong NULL path handling in io_utimensat()
--

commit 8d82d4a2e30073e0345719074be6f5140b1c8829
Author: Méven Car <meven29@gmail.com>

io_uring: add support for utimensat

This commit adds the IORING_OP_UTIMENSAT opcode to io_uring to support
updating file timestamps asynchronously via io-wq. It copies the path and
timespec arguments during submission prep and invokes filename_utimensat()
when issued.

> diff --git a/io_uring/fs.c b/io_uring/fs.c
> index 26ea841a22e70..85e0a1cb6b479 100644
> --- a/io_uring/fs.c
> +++ b/io_uring/fs.c
> @@ -304,3 +304,102 @@ void io_link_cleanup(struct io_kiocb *req)
>  	dismiss_delayed_filename(&sl->oldpath);
>  	dismiss_delayed_filename(&sl->newpath);
>  }
> +
> +/* Internal flags, above the AT_ flags of the SQE */
> +#define IO_UTIMENSAT_NOOP	BIT(30)	/* both times UTIME_OMIT */
> +#define IO_UTIMENSAT_NO_PATH	BIT(31)	/* NULL path: the dfd itself */
> +
> +struct io_utimensat {
> +	struct file			*file;
> +	int				dfd;
> +	unsigned int			flags;
> +	struct delayed_filename		filename;
> +	/* NULL sets both times to the current time */
> +	struct timespec64		*times;
> +	struct timespec64		ts[2];
> +};
> +
> +int io_utimensat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> +{
> +	struct io_utimensat *ut = io_kiocb_to_cmd(req, struct io_utimensat);
> +	struct __kernel_timespec __user *uts;
> +	const char __user *fname;
> +	int err;
> +
> +	if (sqe->len || sqe->buf_index || sqe->splice_fd_in || sqe->addr3 ||
> +	    sqe->__pad2[0])
> +		return -EINVAL;
> +	if (unlikely(req->flags & REQ_F_FIXED_FILE))
> +		return -EBADF;
> +
> +	ut->dfd = READ_ONCE(sqe->fd);
> +	ut->flags = READ_ONCE(sqe->utimes_flags);
> +	if (ut->flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH))
> +		return -EINVAL;
> +
> +	uts = u64_to_user_ptr(READ_ONCE(sqe->addr2));
> +	ut->times = NULL;
> +	if (uts) {
> +		if (get_timespec64(&ut->ts[0], &uts[0]) ||
> +		    get_timespec64(&ut->ts[1], &uts[1]))
> +			return -EFAULT;
> +		ut->times = ut->ts;
> +		if (ut->ts[0].tv_nsec == UTIME_OMIT &&
> +		    ut->ts[1].tv_nsec == UTIME_OMIT) {
> +			ut->flags |= IO_UTIMENSAT_NOOP;
> +			return 0;
> +		}
> +	}
> +
> +	fname = u64_to_user_ptr(READ_ONCE(sqe->addr));
> +	if (!fname) {
> +		if (ut->dfd == AT_FDCWD)
> +			return -EFAULT;
> +		ut->flags |= IO_UTIMENSAT_NO_PATH;
> +		goto out;
> +	}
> +	err = delayed_getname_uflags(&ut->filename, fname, ut->flags);
> +	if (unlikely(err))
> +		return err;
> +	req->flags |= REQ_F_NEED_CLEANUP;
> +out:
> +	req->flags |= REQ_F_FORCE_ASYNC;
> +	return 0;
> +}
> +
> +static int __io_utimensat(struct io_utimensat *ut, struct filename *name)
> +{
> +	int flags = ut->flags & (AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH);
> +
> +	return filename_utimensat(ut->dfd, name, ut->times, flags);
> +}
> +
> +int io_utimensat(struct io_kiocb *req, unsigned int issue_flags)
> +{
> +	struct io_utimensat *ut = io_kiocb_to_cmd(req, struct io_utimensat);
> +	int ret = 0;
> +
> +	WARN_ON_ONCE((issue_flags & IO_URING_F_NONBLOCK) &&
> +		     !(ut->flags & IO_UTIMENSAT_NOOP));
> +
> +	if (ut->flags & IO_UTIMENSAT_NO_PATH) {
> +		CLASS(filename_kernel, name)("");
> +		ret = __io_utimensat(ut, name);

[Severity: Medium]
When sqe->addr is NULL (the IO_UTIMENSAT_NO_PATH case), should
io_utimensat_prep() and io_utimensat() follow do_utimes_fd() instead of
allocating an empty filename and routing through filename_utimensat()?

In fs/utimes.c, do_utimes() dispatches a NULL filename (with dfd != AT_FDCWD)
to do_utimes_fd():

fs/utimes.c:do_utimes_fd() {
	if (flags)
		return -EINVAL;

	CLASS(fd, f)(fd);
	if (fd_empty(f))
		return -EBADF;
	return vfs_utimes(&fd_file(f)->f_path, times);
}

Routing IO_UTIMENSAT_NO_PATH through getname_kernel("") -> __io_utimensat() ->
filename_utimensat() -> filename_lookup() differs from utimensat(2) in several
ways:

1. In io_utimensat_prep(), non-zero sqe->utimes_flags (AT_SYMLINK_NOFOLLOW or
AT_EMPTY_PATH) are accepted when fname is NULL, whereas do_utimes_fd() rejects
any non-zero flags with -EINVAL.

2. In filename_lookup() -> path_lookupat(), an empty string returns from
link_path_walk() with nd->last_type == LAST_ROOT without calling step_into(),
so LOOKUP_FOLLOW is not applied to dfd itself. If sqe->fd is an O_PATH |
O_NOFOLLOW descriptor referring to a symlink and sqe->addr is NULL, this
modifies the symlink itself even when AT_SYMLINK_NOFOLLOW is not set.

3. In path_lookupat(), path_init() sets ND_JUMPED and complete_walk() invokes
d_op->d_weak_revalidate() on filesystems such as NFS and 9p. If sqe->fd refers
to an open file that has been unlinked, d_weak_revalidate() fails and
filename_utimensat() returns -ESTALE (after an extra server round-trip),
whereas do_utimes_fd() calls vfs_utimes() directly on fd_file(f)->f_path and
succeeds.

4. Every IO_UTIMENSAT_NO_PATH call allocates a 4KB struct filename from
names_cachep and records a synthetic empty filename in audit_getname().

Also, the commit message states:

"Unlike the syscall, a NULL path or AT_EMPTY_PATH also works on an O_PATH
descriptor."

Doesn't the utimensat(2) syscall already support AT_EMPTY_PATH with a non-NULL
empty path on an O_PATH descriptor via do_utimes_path() ->
filename_utimensat() -> filename_lookup(), while only rejecting O_PATH on a
NULL path in do_utimes_fd()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030123945.1704118-1-meven@kde.org?part=2

Re: [RFC PATCH 3/3] io_uring: set times inline when the filesystem cannot sleep

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] fs: utimes: direct di_inode access and iput() in __vfs_utimes() under nowait
- [Medium] fs: utimes: blocking lookup and audit before FS_SETATTR_TIMES_NOWAIT check
- [Medium] io_uring: fs: missing flags==0 check for NULL path in io_utimensat_prep()
--

commit 1030cd9e18d185bfa4d7791c093821be89bb0205
Author: Méven Car <meven29@gmail.com>

io_uring: set times inline when the filesystem cannot sleep

This commit adds non-blocking inline execution support for
IORING_OP_UTIMENSAT when the target filesystem sets FS_SETATTR_TIMES_NOWAIT
(enabled for tmpfs), falling back to io-wq on -EAGAIN.

> diff --git a/fs/utimes.c b/fs/utimes.c
> index 8072f05ac8dcc..f70ad9003fa9e 100644
> --- a/fs/utimes.c
> +++ b/fs/utimes.c
> @@ -18,7 +18,9 @@ static bool nsec_valid(long nsec)
>  	return nsec >= 0 && nsec <= 999999999;
>  }
>  
> -int vfs_utimes(const struct path *path, struct timespec64 *times)
> +/* With @nowait, returns -EAGAIN instead of sleeping */
> +static int __vfs_utimes(const struct path *path, struct timespec64 *times,
> +			bool nowait)
>  {

[ ... ]

> @@ -63,24 +73,40 @@ int vfs_utimes(const struct path *path, struct timespec64 *times)
>  		newattrs.ia_valid |= ATTR_TOUCH;
>  	}
>  retry_deleg:
> -	inode_lock(inode);
> +	if (!nowait) {
> +		inode_lock(inode);
> +	} else if (!inode_trylock(inode)) {
> +		error = -EAGAIN;
> +		goto drop;
> +	}
>  	error = notify_change(mnt_idmap(path->mnt), path->dentry, &newattrs,
>  			      &delegated_inode);
>  	inode_unlock(inode);
>  	if (is_delegated(&delegated_inode)) {
> +		if (nowait) {
> +			iput(delegated_inode.di_inode);
> +			delegated_inode.di_inode = NULL;
> +			error = -EAGAIN;
> +			goto drop;
> +		}

[Severity: Medium]
Does this break the build when CONFIG_FILE_LOCKING is disabled? In
include/linux/filelock.h, struct delegated_inode only defines di_inode under
CONFIG_FILE_LOCKING, and is an empty struct otherwise:

include/linux/filelock.h:
    #else /* !CONFIG_FILE_LOCKING */
    struct delegated_inode { };

    static inline bool is_delegated(struct delegated_inode *di)
    {
            return false;
    }

Because fs/utimes.c is built unconditionally, referencing
delegated_inode.di_inode in __vfs_utimes() will fail to compile when
CONFIG_FILE_LOCKING=n.

Also, when CONFIG_FILE_LOCKING=y, calling iput(delegated_inode.di_inode) on
the nowait path invokes might_sleep() and can sleep if the file was
concurrently unlinked after inode_unlock(inode).

If __vfs_utimes() passes nowait ? NULL : &delegated_inode to notify_change()
instead, try_break_deleg() will initiate the non-blocking break and return
-EWOULDBLOCK (-EAGAIN) without taking an extra inode reference or populating
delegated_inode:

include/linux/filelock.h:try_break_deleg()
    ret = break_deleg(inode, flags | LEASE_BREAK_NONBLOCK);
    if (ret == -EWOULDBLOCK && di) {
            di->di_inode = inode;
            ihold(inode);
    }
    return ret;

Would passing NULL for delegated_inode when nowait is true avoid both the
di_inode access and the iput() call here?

>  		error = break_deleg_wait(&delegated_inode);
>  		if (!error)
>  			goto retry_deleg;
>  	}
> -
> +drop:
>  	mnt_drop_write(path->mnt);
>  out:
>  	return error;
>  }
> +
> +int vfs_utimes(const struct path *path, struct timespec64 *times)
> +{
> +	return __vfs_utimes(path, times, false);
> +}
>  EXPORT_SYMBOL_GPL(vfs_utimes);
>  
>  int filename_utimensat(int dfd, struct filename *name,
> -		       struct timespec64 *times, int flags)
> +		       struct timespec64 *times, int flags, bool nowait)
>  {
>  	struct path path;
>  	int lookup_flags = 0, error;
> @@ -90,13 +116,17 @@ int filename_utimensat(int dfd, struct filename *name,
>  
>  	if (!(flags & AT_SYMLINK_NOFOLLOW))
>  		lookup_flags |= LOOKUP_FOLLOW;
> +	if (nowait)
> +		lookup_flags |= LOOKUP_CACHED;
>  
>  retry:
>  	error = filename_lookup(dfd, name, lookup_flags, &path, NULL);
>  	if (error)
>  		return error;
> -	error = vfs_utimes(&path, times);
> +	error = __vfs_utimes(&path, times, nowait);
>  	path_put(&path);

[Severity: Medium]
Can filename_lookup() sleep or perform duplicate work on the nowait path
before __vfs_utimes() checks FS_SETATTR_TIMES_NOWAIT?

Inside filename_lookup() -> path_lookupat() -> complete_walk(), LOOKUP_CACHED
is cleared before try_to_unlazy(), and if ND_JUMPED is set (such as for empty
paths, '..', '/', mountpoint crossings, or trailing symlinks) on a filesystem
with DCACHE_OP_WEAK_REVALIDATE (e.g. NFS, 9p, or overlayfs),
d_op->d_weak_revalidate() is called in blocking mode without LOOKUP_RCU or
LOOKUP_CACHED:

fs/namei.c:complete_walk()
    nd->flags &= ~LOOKUP_CACHED;
    if (!try_to_unlazy(nd))
            return -ECHILD;
    ...
    if (likely(!(nd->state & ND_JUMPED)))
            return 0;

    if (likely(!(dentry->d_flags & DCACHE_OP_WEAK_REVALIDATE)))
            return 0;

    status = dentry->d_op->d_weak_revalidate(dentry, nd->flags);

In addition, when path_lookupat() succeeds on a cached dentry of any
filesystem lacking FS_SETATTR_TIMES_NOWAIT (such as ext4, xfs, or btrfs),
filename_lookup() calls audit_inode() before __vfs_utimes() rejects the
filesystem with -EAGAIN. When auditing is active, audit_inode() ->
audit_copy_fcaps() -> get_vfs_caps_from_disk() can sleep reading xattrs from
disk, records a duplicate audit entry, and bumps name->refcnt (forcing
putname_to_delayed() to allocate a second struct filename), while path_put()
also runs on the inline submission path.

> diff --git a/io_uring/fs.c b/io_uring/fs.c
> index 85e0a1cb6b479..79f6f39ce267a 100644
> --- a/io_uring/fs.c
> +++ b/io_uring/fs.c
> @@ -356,38 +356,44 @@ int io_utimensat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>  		if (ut->dfd == AT_FDCWD)
>  			return -EFAULT;
>  		ut->flags |= IO_UTIMENSAT_NO_PATH;
> -		goto out;
> +		return 0;
>  	}

[Severity: Medium]
When fname is NULL and ut->dfd != AT_FDCWD, should io_utimensat_prep() reject
non-zero ut->flags with -EINVAL to match do_utimes_fd() in fs/utimes.c?

In fs/utimes.c, a NULL filename with a valid fd is routed to do_utimes_fd(),
which enforces:

fs/utimes.c:do_utimes_fd()
    if (flags)
            return -EINVAL;

whereas io_utimensat_prep() allows AT_SYMLINK_NOFOLLOW or AT_EMPTY_PATH when
sqe->addr is NULL and passes them through __io_utimensat() to
filename_utimensat(). Also, since IO_UTIMENSAT_NO_PATH operates directly on
ut->dfd, could io_utimensat() use fdget()/CLASS(fd) and __vfs_utimes() like
do_utimes_fd() instead of allocating a struct filename via
CLASS(filename_kernel, name)("") and doing a path lookup on Both the inline
and io-wq attempts?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030123945.1704118-1-meven@kde.org?part=3