[PATCHSET RFC 0/6] add support for name_to, open_by_handle_at(2) to io_uring

[PATCHSET RFC 0/6] add support for name_to, open_by_handle_at(2) to io_uring

[Thomas Bertschinger]

This series adds support for name_to_handle_at() and open_by_handle_at()
to io_uring. The idea is for these opcodes to be useful for userspace
NFS servers that want to use io_uring.

name_to_handle_at()
===================

Support for name_to_handle_at() is added in patches 1 and 2.

In order to do a non-blocking name_to_handle_at(), a new helper
do_name_to_handle_at() is created that takes a lookup_flags argument.

This is to support non-blocking lookup when called with
IO_URING_F_NONBLOCK--user_path_at() will be called with LOOKUP_CACHED
in that case.

Aside from the lookup, I don't think there is anything else that
do_name_to_handle_at() does that would be a problem in the non-blocking
case. There is a GFP_KERNEL allocation:

do_name_to_handle_at()
  -> do_path_to_handle()
    -> kzalloc(..., GFP_KERNEL)

But I think that's OK? Let me know if there's anything else I'm
missing...

open_by_handle_at()
===================

Patch 3 is a fixup to fhandle.c:do_handle_open() that (I believe) fixes
a bug and can exist independently of this series, but it fits in with
these changes so I'm including it here.

Support for open_by_handle_at() is added in patches 4 - 6.

A helper __do_handle_open() is created that does the file open without
installing a file descriptor for it. This is needed because io_uring
needs to decide between using a file descriptor or a fixed file.

No attempt is made to support a non-blocking open_by_handle_at()--the
attempt is always immediately returned with -EAGAIN if
IO_URING_F_NONBLOCK is set.

This isn't ideal and it would be nice to add support for non-blocking
open by handle in the future. This would presumably require updates to
the ->encode_fh() implementation for filesystems that want to
support this.

I see that lack of support for non-blocking operation was a dealbreaker
for adding getdents to io_uring previously:

https://lore.kernel.org/io-uring/20230428050640.GA1969623@dread.disaster.area/

On the other hand, AFAICT, support for openat() was originally added in
15b71abe7b52 (io_uring: add support for IORING_OP_OPENAT) without a non-
blocking lookup, and the possibility of non-blocking lookup later added
in 3a81fd02045c (io_uring: enable LOOKUP_CACHED path resolution for
filename lookups).

(To be honest I'm a little confused by the history here. The commit
message of 15b71abe7b52 says

> For the normal case of a non-blocking path lookup this will complete
> inline. If we have to do IO to perform the open, it'll be done from
> async context.

but from the commit contents this would NOT appear to be the case: 

> +       if (force_nonblock) {
> +               req->work.flags |= IO_WQ_WORK_NEEDS_FILES;
> +               return -EAGAIN;
> +       }

until the support is really added in the later commit. Am I confused or
is the commit message wrong?)

In any event, based on my reading of the history, it would appear to be
OK to add open_by_handle_at() initially without support for inline
completion, and then later add that when the filesystem implementations
can be updated to support this.

Please let me know if I am wrong on my interpretation of the history or
if anyone disagrees with the conclusion.

Testing
=======

A liburing branch that includes support for the new opcodes, as well as
a test, is available at:

https://github.com/bertschingert/liburing/tree/open_by_handle_at

To run the test:

$ ./test/open_by_handle_at.t

Thomas Bertschinger (6):
  fhandle: create helper for name_to_handle_at(2)
  io_uring: add support for IORING_OP_NAME_TO_HANDLE_AT
  fhandle: do_handle_open() should get FD with user flags
  fhandle: create __do_handle_open() helper
  io_uring: add __io_open_prep() helper
  io_uring: add support for IORING_OP_OPEN_BY_HANDLE_AT

 fs/fhandle.c                  |  85 ++++++++++++---------
 fs/internal.h                 |   9 +++
 include/uapi/linux/io_uring.h |   2 +
 io_uring/opdef.c              |  14 ++++
 io_uring/openclose.c          | 137 +++++++++++++++++++++++++++++++---
 io_uring/openclose.h          |   5 ++
 6 files changed, 209 insertions(+), 43 deletions(-)

-- 
2.50.1

[PATCH 1/6] fhandle: create helper for name_to_handle_at(2)

[Thomas Bertschinger]

Create a helper do_name_to_handle_at() that takes an additional argument,
lookup_flags, beyond the syscall arguments.

Because name_to_handle_at(2) doesn't take any lookup flags, it always
passes 0 for this argument.

Future callers like io_uring may pass LOOKUP_CACHED in order to request
a non-blocking lookup.

This helper's name is confusingly similar to do_sys_name_to_handle()
which takes care of returning the file handle, once the filename has
been turned into a struct path. To distinguish the names more clearly,
rename the latter to do_path_to_handle().

Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
---
 fs/fhandle.c  | 61 ++++++++++++++++++++++++++++-----------------------
 fs/internal.h |  7 ++++++
 2 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/fs/fhandle.c b/fs/fhandle.c
index 7c236f64cdea..57da648ca866 100644
--- a/fs/fhandle.c
+++ b/fs/fhandle.c
@@ -14,10 +14,10 @@
 #include "internal.h"
 #include "mount.h"
 
-static long do_sys_name_to_handle(const struct path *path,
-				  struct file_handle __user *ufh,
-				  void __user *mnt_id, bool unique_mntid,
-				  int fh_flags)
+static long do_path_to_handle(const struct path *path,
+			      struct file_handle __user *ufh,
+			      void __user *mnt_id, bool unique_mntid,
+			      int fh_flags)
 {
 	long retval;
 	struct file_handle f_handle;
@@ -111,27 +111,11 @@ static long do_sys_name_to_handle(const struct path *path,
 	return retval;
 }
 
-/**
- * sys_name_to_handle_at: convert name to handle
- * @dfd: directory relative to which name is interpreted if not absolute
- * @name: name that should be converted to handle.
- * @handle: resulting file handle
- * @mnt_id: mount id of the file system containing the file
- *          (u64 if AT_HANDLE_MNT_ID_UNIQUE, otherwise int)
- * @flag: flag value to indicate whether to follow symlink or not
- *        and whether a decodable file handle is required.
- *
- * @handle->handle_size indicate the space available to store the
- * variable part of the file handle in bytes. If there is not
- * enough space, the field is updated to return the minimum
- * value required.
- */
-SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
-		struct file_handle __user *, handle, void __user *, mnt_id,
-		int, flag)
+long do_name_to_handle_at(int dfd, const char __user *name,
+			  struct file_handle __user *handle, void __user *mnt_id,
+			  int flag, int lookup_flags)
 {
 	struct path path;
-	int lookup_flags;
 	int fh_flags = 0;
 	int err;
 
@@ -155,19 +139,42 @@ SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
 	else if (flag & AT_HANDLE_CONNECTABLE)
 		fh_flags |= EXPORT_FH_CONNECTABLE;
 
-	lookup_flags = (flag & AT_SYMLINK_FOLLOW) ? LOOKUP_FOLLOW : 0;
+	if (flag & AT_SYMLINK_FOLLOW)
+		lookup_flags |= LOOKUP_FOLLOW;
 	if (flag & AT_EMPTY_PATH)
 		lookup_flags |= LOOKUP_EMPTY;
 	err = user_path_at(dfd, name, lookup_flags, &path);
 	if (!err) {
-		err = do_sys_name_to_handle(&path, handle, mnt_id,
-					    flag & AT_HANDLE_MNT_ID_UNIQUE,
-					    fh_flags);
+		err = do_path_to_handle(&path, handle, mnt_id,
+					flag & AT_HANDLE_MNT_ID_UNIQUE,
+					fh_flags);
 		path_put(&path);
 	}
 	return err;
 }
 
+/**
+ * sys_name_to_handle_at: convert name to handle
+ * @dfd: directory relative to which name is interpreted if not absolute
+ * @name: name that should be converted to handle.
+ * @handle: resulting file handle
+ * @mnt_id: mount id of the file system containing the file
+ *          (u64 if AT_HANDLE_MNT_ID_UNIQUE, otherwise int)
+ * @flag: flag value to indicate whether to follow symlink or not
+ *        and whether a decodable file handle is required.
+ *
+ * @handle->handle_size indicate the space available to store the
+ * variable part of the file handle in bytes. If there is not
+ * enough space, the field is updated to return the minimum
+ * value required.
+ */
+SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
+		struct file_handle __user *, handle, void __user *, mnt_id,
+		int, flag)
+{
+	return do_name_to_handle_at(dfd, name, handle, mnt_id, flag, 0);
+}
+
 static int get_path_anchor(int fd, struct path *root)
 {
 	if (fd >= 0) {
diff --git a/fs/internal.h b/fs/internal.h
index 38e8aab27bbd..af7e0810a90d 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -355,3 +355,10 @@ int anon_inode_getattr(struct mnt_idmap *idmap, const struct path *path,
 int anon_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 		       struct iattr *attr);
 void pidfs_get_root(struct path *path);
+
+/*
+ * fs/fhandle.c
+ */
+long do_name_to_handle_at(int dfd, const char __user *name,
+			  struct file_handle __user *handle,
+			  void __user *mnt_id, int flag, int lookup_flags);
-- 
2.50.1

[PATCH 2/6] io_uring: add support for IORING_OP_NAME_TO_HANDLE_AT

[Thomas Bertschinger]

Add support for name_to_handle_at(2) to io_uring.

Like openat*(), this tries to do a non-blocking lookup first and resorts
to async lookup when that fails.

This uses sqe->addr for the path, ->addr2 for the file handle which is
filled in by the kernel, and ->addr3 for the mouint_id which is filled
in by the kernel.

Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
---
 include/uapi/linux/io_uring.h |  1 +
 io_uring/opdef.c              |  7 ++++++
 io_uring/openclose.c          | 43 +++++++++++++++++++++++++++++++++++
 io_uring/openclose.h          |  3 +++
 4 files changed, 54 insertions(+)

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 6957dc539d83..596bae788b48 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -289,6 +289,7 @@ enum io_uring_op {
 	IORING_OP_READV_FIXED,
 	IORING_OP_WRITEV_FIXED,
 	IORING_OP_PIPE,
+	IORING_OP_NAME_TO_HANDLE_AT,
 
 	/* this goes last, obviously */
 	IORING_OP_LAST,
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index 9568785810d9..ff2672bbd583 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -574,6 +574,10 @@ const struct io_issue_def io_issue_defs[] = {
 		.prep			= io_pipe_prep,
 		.issue			= io_pipe,
 	},
+	[IORING_OP_NAME_TO_HANDLE_AT] = {
+		.prep			= io_name_to_handle_at_prep,
+		.issue			= io_name_to_handle_at,
+	},
 };
 
 const struct io_cold_def io_cold_defs[] = {
@@ -824,6 +828,9 @@ const struct io_cold_def io_cold_defs[] = {
 	[IORING_OP_PIPE] = {
 		.name			= "PIPE",
 	},
+	[IORING_OP_NAME_TO_HANDLE_AT] = {
+		.name			= "NAME_TO_HANDLE_AT",
+	},
 };
 
 const char *io_uring_get_opcode(u8 opcode)
diff --git a/io_uring/openclose.c b/io_uring/openclose.c
index d70700e5cef8..f15a9307f811 100644
--- a/io_uring/openclose.c
+++ b/io_uring/openclose.c
@@ -27,6 +27,15 @@ struct io_open {
 	unsigned long			nofile;
 };
 
+struct io_name_to_handle {
+	struct file			*file;
+	int				dfd;
+	int				open_flag;
+	struct file_handle __user	*ufh;
+	char __user			*path;
+	void __user			*mount_id;
+};
+
 struct io_close {
 	struct file			*file;
 	int				fd;
@@ -187,6 +196,40 @@ void io_open_cleanup(struct io_kiocb *req)
 		putname(open->filename);
 }
 
+int io_name_to_handle_at_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+	struct io_name_to_handle *nh = io_kiocb_to_cmd(req, struct io_name_to_handle);
+
+	nh->dfd = READ_ONCE(sqe->fd);
+	nh->open_flag = READ_ONCE(sqe->open_flags);
+	nh->path = u64_to_user_ptr(READ_ONCE(sqe->addr));
+	nh->ufh = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+	nh->mount_id = u64_to_user_ptr(READ_ONCE(sqe->addr3));
+
+	return 0;
+}
+
+int io_name_to_handle_at(struct io_kiocb *req, unsigned int issue_flags)
+{
+	struct io_name_to_handle *nh = io_kiocb_to_cmd(req, struct io_name_to_handle);
+	int lookup_flags = 0;
+	long ret;
+
+	if (issue_flags & IO_URING_F_NONBLOCK)
+		lookup_flags = LOOKUP_CACHED;
+
+	ret = do_name_to_handle_at(nh->dfd, nh->path, nh->ufh, nh->mount_id,
+				   nh->open_flag, lookup_flags);
+
+	if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK))
+		return -EAGAIN;
+
+	if (ret < 0)
+		req_set_fail(req);
+	io_req_set_res(req, ret, 0);
+	return IOU_COMPLETE;
+}
+
 int __io_close_fixed(struct io_ring_ctx *ctx, unsigned int issue_flags,
 		     unsigned int offset)
 {
diff --git a/io_uring/openclose.h b/io_uring/openclose.h
index 4ca2a9935abc..3d1096abffac 100644
--- a/io_uring/openclose.h
+++ b/io_uring/openclose.h
@@ -10,6 +10,9 @@ void io_open_cleanup(struct io_kiocb *req);
 int io_openat2_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
 int io_openat2(struct io_kiocb *req, unsigned int issue_flags);
 
+int io_name_to_handle_at_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_name_to_handle_at(struct io_kiocb *req, unsigned int issue_flags);
+
 int io_close_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
 int io_close(struct io_kiocb *req, unsigned int issue_flags);
 
-- 
2.50.1

[PATCH 3/6] fhandle: do_handle_open() should get FD with user flags

[Thomas Bertschinger]

In f07c7cc4684a, do_handle_open() was switched to use the automatic
cleanup method for getting a FD. In that change it was also switched
to pass O_CLOEXEC unconditionally to get_unused_fd_flags() instead
of passing the user-specified flags.

I don't see anything in that commit description that indicates this was
intentional, so I am assuming it was an oversight.

With this fix, the FD will again be opened with, or without, O_CLOEXEC
according to what the user requested.

Fixes: f07c7cc4684a ("fhandle: simplify error handling")
Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
---
 fs/fhandle.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fhandle.c b/fs/fhandle.c
index 57da648ca866..dbb273a26214 100644
--- a/fs/fhandle.c
+++ b/fs/fhandle.c
@@ -409,7 +409,7 @@ static long do_handle_open(int mountdirfd, struct file_handle __user *ufh,
 	if (retval)
 		return retval;
 
-	CLASS(get_unused_fd, fd)(O_CLOEXEC);
+	CLASS(get_unused_fd, fd)(open_flag);
 	if (fd < 0)
 		return fd;
 
-- 
2.50.1

[PATCH 4/6] fhandle: create __do_handle_open() helper

[Thomas Bertschinger]

do_handle_open() takes care of both opening a file via its file handle,
and associating it with a file descriptor.

For io_uring, it is useful to do just the opening part separately,
because io_uring might not want to install it into the main open files
table when using fixed descriptors.

This creates a helper which will enable io_uring to do that.

Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
---
 fs/fhandle.c  | 24 +++++++++++++++++-------
 fs/internal.h |  2 ++
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/fs/fhandle.c b/fs/fhandle.c
index dbb273a26214..b14884a93867 100644
--- a/fs/fhandle.c
+++ b/fs/fhandle.c
@@ -397,8 +397,8 @@ static int handle_to_path(int mountdirfd, struct file_handle __user *ufh,
 	return retval;
 }
 
-static long do_handle_open(int mountdirfd, struct file_handle __user *ufh,
-			   int open_flag)
+struct file *__do_handle_open(int mountdirfd, struct file_handle __user *ufh,
+			      int open_flag)
 {
 	long retval = 0;
 	struct path path __free(path_put) = {};
@@ -407,17 +407,27 @@ static long do_handle_open(int mountdirfd, struct file_handle __user *ufh,
 
 	retval = handle_to_path(mountdirfd, ufh, &path, open_flag);
 	if (retval)
-		return retval;
-
-	CLASS(get_unused_fd, fd)(open_flag);
-	if (fd < 0)
-		return fd;
+		return ERR_PTR(retval);
 
 	eops = path.mnt->mnt_sb->s_export_op;
 	if (eops->open)
 		file = eops->open(&path, open_flag);
 	else
 		file = file_open_root(&path, "", open_flag, 0);
+
+	return file;
+}
+
+static long do_handle_open(int mountdirfd, struct file_handle __user *ufh,
+			   int open_flag)
+{
+	struct file *file;
+
+	CLASS(get_unused_fd, fd)(open_flag);
+	if (fd < 0)
+		return fd;
+
+	file = __do_handle_open(mountdirfd, ufh, open_flag);
 	if (IS_ERR(file))
 		return PTR_ERR(file);
 
diff --git a/fs/internal.h b/fs/internal.h
index af7e0810a90d..26ac6f356313 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -362,3 +362,5 @@ void pidfs_get_root(struct path *path);
 long do_name_to_handle_at(int dfd, const char __user *name,
 			  struct file_handle __user *handle,
 			  void __user *mnt_id, int flag, int lookup_flags);
+struct file *__do_handle_open(int mountdirfd, struct file_handle __user *ufh,
+			      int open_flag);
-- 
2.50.1

[PATCH 5/6] io_uring: add __io_open_prep() helper

[Thomas Bertschinger]

This adds a helper, __io_open_prep(), which does the part of preparing
for an open that is shared between openat*() and open_by_handle_at().

It excludes reading in the user path or file handle--this will be done
by functions specific to the kind of open().

Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
---
 io_uring/openclose.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/io_uring/openclose.c b/io_uring/openclose.c
index f15a9307f811..8be061783207 100644
--- a/io_uring/openclose.c
+++ b/io_uring/openclose.c
@@ -58,11 +58,10 @@ static bool io_openat_force_async(struct io_open *open)
 	return open->how.flags & (O_TRUNC | O_CREAT | __O_TMPFILE);
 }
 
-static int __io_openat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+/* Prep for open that is common to both openat*() and open_by_handle_at() */
+static int __io_open_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	struct io_open *open = io_kiocb_to_cmd(req, struct io_open);
-	const char __user *fname;
-	int ret;
 
 	if (unlikely(sqe->buf_index))
 		return -EINVAL;
@@ -74,6 +73,26 @@ static int __io_openat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe
 		open->how.flags |= O_LARGEFILE;
 
 	open->dfd = READ_ONCE(sqe->fd);
+
+	open->file_slot = READ_ONCE(sqe->file_index);
+	if (open->file_slot && (open->how.flags & O_CLOEXEC))
+		return -EINVAL;
+
+	open->nofile = rlimit(RLIMIT_NOFILE);
+
+	return 0;
+}
+
+static int __io_openat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+	struct io_open *open = io_kiocb_to_cmd(req, struct io_open);
+	const char __user *fname;
+	int ret;
+
+	ret = __io_open_prep(req, sqe);
+	if (ret)
+		return ret;
+
 	fname = u64_to_user_ptr(READ_ONCE(sqe->addr));
 	open->filename = getname(fname);
 	if (IS_ERR(open->filename)) {
@@ -82,11 +101,6 @@ static int __io_openat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe
 		return ret;
 	}
 
-	open->file_slot = READ_ONCE(sqe->file_index);
-	if (open->file_slot && (open->how.flags & O_CLOEXEC))
-		return -EINVAL;
-
-	open->nofile = rlimit(RLIMIT_NOFILE);
 	req->flags |= REQ_F_NEED_CLEANUP;
 	if (io_openat_force_async(open))
 		req->flags |= REQ_F_FORCE_ASYNC;
-- 
2.50.1

[PATCH 6/6] io_uring: add support for IORING_OP_OPEN_BY_HANDLE_AT

[Thomas Bertschinger]

This adds support for open_by_handle_at(2) to io_uring.

Non-blocking open by handle is not yet supported, so this always runs in
async context.

Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
---
 include/uapi/linux/io_uring.h |  1 +
 io_uring/opdef.c              |  7 ++++
 io_uring/openclose.c          | 64 ++++++++++++++++++++++++++++++++++-
 io_uring/openclose.h          |  2 ++
 4 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 596bae788b48..946da13e1454 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -290,6 +290,7 @@ enum io_uring_op {
 	IORING_OP_WRITEV_FIXED,
 	IORING_OP_PIPE,
 	IORING_OP_NAME_TO_HANDLE_AT,
+	IORING_OP_OPEN_BY_HANDLE_AT,
 
 	/* this goes last, obviously */
 	IORING_OP_LAST,
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index ff2672bbd583..e2e0f4ed0d9d 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -578,6 +578,10 @@ const struct io_issue_def io_issue_defs[] = {
 		.prep			= io_name_to_handle_at_prep,
 		.issue			= io_name_to_handle_at,
 	},
+	[IORING_OP_OPEN_BY_HANDLE_AT] = {
+		.prep			= io_open_by_handle_at_prep,
+		.issue			= io_open_by_handle_at,
+	},
 };
 
 const struct io_cold_def io_cold_defs[] = {
@@ -831,6 +835,9 @@ const struct io_cold_def io_cold_defs[] = {
 	[IORING_OP_NAME_TO_HANDLE_AT] = {
 		.name			= "NAME_TO_HANDLE_AT",
 	},
+	[IORING_OP_OPEN_BY_HANDLE_AT] = {
+		.name			= "OPEN_BY_HANDLE_AT",
+	}
 };
 
 const char *io_uring_get_opcode(u8 opcode)
diff --git a/io_uring/openclose.c b/io_uring/openclose.c
index 8be061783207..5be17d7a46e0 100644
--- a/io_uring/openclose.c
+++ b/io_uring/openclose.c
@@ -22,7 +22,13 @@ struct io_open {
 	struct file			*file;
 	int				dfd;
 	u32				file_slot;
-	struct filename			*filename;
+	union {
+		/* For openat(), openat2() */
+		struct filename		*filename;
+
+		/* For open_by_handle_at() */
+		struct file_handle __user *ufh;
+	};
 	struct open_how			how;
 	unsigned long			nofile;
 };
@@ -244,6 +250,62 @@ int io_name_to_handle_at(struct io_kiocb *req, unsigned int issue_flags)
 	return IOU_COMPLETE;
 }
 
+int io_open_by_handle_at_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+	struct io_open *open = io_kiocb_to_cmd(req, struct io_open);
+	u64 flags;
+
+	flags = READ_ONCE(sqe->open_flags);
+	open->how = build_open_how(flags, 0);
+	open->ufh = u64_to_user_ptr(READ_ONCE(sqe->addr));
+
+	__io_open_prep(req, sqe);
+
+	return 0;
+}
+
+int io_open_by_handle_at(struct io_kiocb *req, unsigned int issue_flags)
+{
+	struct io_open *open = io_kiocb_to_cmd(req, struct io_open);
+	struct file *file;
+	bool fixed = !!open->file_slot;
+	int ret;
+
+	/*
+	 * Always try again if we aren't supposed to block, because there is no
+	 * way of preventing the FS implementation from blocking.
+	 */
+	if (issue_flags & IO_URING_F_NONBLOCK)
+		return -EAGAIN;
+
+	if (!fixed) {
+		ret = __get_unused_fd_flags(open->how.flags, open->nofile);
+		if (ret < 0)
+			goto err;
+	}
+
+	file = __do_handle_open(open->dfd, open->ufh, open->how.flags);
+
+	if (IS_ERR(file)) {
+		if (!fixed)
+			put_unused_fd(ret);
+		ret = PTR_ERR(file);
+		goto err;
+	}
+
+	if (!fixed)
+		fd_install(ret, file);
+	else
+		ret = io_fixed_fd_install(req, issue_flags, file,
+					  open->file_slot);
+
+err:
+	if (ret < 0)
+		req_set_fail(req);
+	io_req_set_res(req, ret, 0);
+	return IOU_COMPLETE;
+}
+
 int __io_close_fixed(struct io_ring_ctx *ctx, unsigned int issue_flags,
 		     unsigned int offset)
 {
diff --git a/io_uring/openclose.h b/io_uring/openclose.h
index 3d1096abffac..a6304fa856bf 100644
--- a/io_uring/openclose.h
+++ b/io_uring/openclose.h
@@ -12,6 +12,8 @@ int io_openat2(struct io_kiocb *req, unsigned int issue_flags);
 
 int io_name_to_handle_at_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
 int io_name_to_handle_at(struct io_kiocb *req, unsigned int issue_flags);
+int io_open_by_handle_at_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_open_by_handle_at(struct io_kiocb *req, unsigned int issue_flags);
 
 int io_close_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
 int io_close(struct io_kiocb *req, unsigned int issue_flags);
-- 
2.50.1

Re: [PATCH 3/6] fhandle: do_handle_open() should get FD with user flags

[Amir Goldstein]

On Fri, Aug 15, 2025 at 1:52 AM Thomas Bertschinger
<tahbertschinger@gmail.com> wrote:
>
> In f07c7cc4684a, do_handle_open() was switched to use the automatic
> cleanup method for getting a FD. In that change it was also switched
> to pass O_CLOEXEC unconditionally to get_unused_fd_flags() instead
> of passing the user-specified flags.
>
> I don't see anything in that commit description that indicates this was
> intentional, so I am assuming it was an oversight.
>
> With this fix, the FD will again be opened with, or without, O_CLOEXEC
> according to what the user requested.
>
> Fixes: f07c7cc4684a ("fhandle: simplify error handling")
> Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>

This patch does not seem to be conflicting with earlier patches in the series
but it is still preferred to start the series with the backportable fix patch.

Fee free to add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.

> ---
>  fs/fhandle.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/fhandle.c b/fs/fhandle.c
> index 57da648ca866..dbb273a26214 100644
> --- a/fs/fhandle.c
> +++ b/fs/fhandle.c
> @@ -409,7 +409,7 @@ static long do_handle_open(int mountdirfd, struct file_handle __user *ufh,
>         if (retval)
>                 return retval;
>
> -       CLASS(get_unused_fd, fd)(O_CLOEXEC);
> +       CLASS(get_unused_fd, fd)(open_flag);
>         if (fd < 0)
>                 return fd;
>
> --
> 2.50.1
>
>

Re: [PATCHSET RFC 0/6] add support for name_to, open_by_handle_at(2) to io_uring

[Amir Goldstein]

On Fri, Aug 15, 2025 at 1:50 AM Thomas Bertschinger
<tahbertschinger@gmail.com> wrote:
>
> This series adds support for name_to_handle_at() and open_by_handle_at()
> to io_uring. The idea is for these opcodes to be useful for userspace
> NFS servers that want to use io_uring.
>
> name_to_handle_at()
> ===================
>
> Support for name_to_handle_at() is added in patches 1 and 2.
>
> In order to do a non-blocking name_to_handle_at(), a new helper
> do_name_to_handle_at() is created that takes a lookup_flags argument.
>
> This is to support non-blocking lookup when called with
> IO_URING_F_NONBLOCK--user_path_at() will be called with LOOKUP_CACHED
> in that case.
>
> Aside from the lookup, I don't think there is anything else that
> do_name_to_handle_at() does that would be a problem in the non-blocking
> case. There is a GFP_KERNEL allocation:
>
> do_name_to_handle_at()
>   -> do_path_to_handle()
>     -> kzalloc(..., GFP_KERNEL)
>
> But I think that's OK? Let me know if there's anything else I'm
> missing...
>
> open_by_handle_at()
> ===================
>
> Patch 3 is a fixup to fhandle.c:do_handle_open() that (I believe) fixes
> a bug and can exist independently of this series, but it fits in with
> these changes so I'm including it here.
>
> Support for open_by_handle_at() is added in patches 4 - 6.
>
> A helper __do_handle_open() is created that does the file open without
> installing a file descriptor for it. This is needed because io_uring
> needs to decide between using a file descriptor or a fixed file.
>
> No attempt is made to support a non-blocking open_by_handle_at()--the
> attempt is always immediately returned with -EAGAIN if
> IO_URING_F_NONBLOCK is set.
>
> This isn't ideal and it would be nice to add support for non-blocking
> open by handle in the future. This would presumably require updates to
> the ->encode_fh() implementation for filesystems that want to
> support this.

Correction: ->encode_fh() is for name_to_handle()
You want to say that ->fh_to_dentry() need to support cached lookup,
but FWIW, the blocking code is more likely to come from the
lookup in exportfs_decode_fh_raw() => ... reconnect_one()
not from the filesystem code.

The fs would "only" need to be taught to return an alias to a
cached inode and generic code would "only" need to be taught
to give up on a disconnected dir dentry.

Doesn't sound too hard (famous last words).

Thanks,
Amir.

>
> I see that lack of support for non-blocking operation was a dealbreaker
> for adding getdents to io_uring previously:
>
> https://lore.kernel.org/io-uring/20230428050640.GA1969623@dread.disaster.area/
>
> On the other hand, AFAICT, support for openat() was originally added in
> 15b71abe7b52 (io_uring: add support for IORING_OP_OPENAT) without a non-
> blocking lookup, and the possibility of non-blocking lookup later added
> in 3a81fd02045c (io_uring: enable LOOKUP_CACHED path resolution for
> filename lookups).
>
> (To be honest I'm a little confused by the history here. The commit
> message of 15b71abe7b52 says
>
> > For the normal case of a non-blocking path lookup this will complete
> > inline. If we have to do IO to perform the open, it'll be done from
> > async context.
>
> but from the commit contents this would NOT appear to be the case:
>
> > +       if (force_nonblock) {
> > +               req->work.flags |= IO_WQ_WORK_NEEDS_FILES;
> > +               return -EAGAIN;
> > +       }
>
> until the support is really added in the later commit. Am I confused or
> is the commit message wrong?)
>
> In any event, based on my reading of the history, it would appear to be
> OK to add open_by_handle_at() initially without support for inline
> completion, and then later add that when the filesystem implementations
> can be updated to support this.
>
> Please let me know if I am wrong on my interpretation of the history or
> if anyone disagrees with the conclusion.
>
> Testing
> =======
>
> A liburing branch that includes support for the new opcodes, as well as
> a test, is available at:
>
> https://github.com/bertschingert/liburing/tree/open_by_handle_at
>
> To run the test:
>
> $ ./test/open_by_handle_at.t
>
> Thomas Bertschinger (6):
>   fhandle: create helper for name_to_handle_at(2)
>   io_uring: add support for IORING_OP_NAME_TO_HANDLE_AT
>   fhandle: do_handle_open() should get FD with user flags
>   fhandle: create __do_handle_open() helper
>   io_uring: add __io_open_prep() helper
>   io_uring: add support for IORING_OP_OPEN_BY_HANDLE_AT
>
>  fs/fhandle.c                  |  85 ++++++++++++---------
>  fs/internal.h                 |   9 +++
>  include/uapi/linux/io_uring.h |   2 +
>  io_uring/opdef.c              |  14 ++++
>  io_uring/openclose.c          | 137 +++++++++++++++++++++++++++++++---
>  io_uring/openclose.h          |   5 ++
>  6 files changed, 209 insertions(+), 43 deletions(-)
>
> --
> 2.50.1
>
>

Re: [PATCH 1/6] fhandle: create helper for name_to_handle_at(2)

[Amir Goldstein]

On Fri, Aug 15, 2025 at 1:51 AM Thomas Bertschinger
<tahbertschinger@gmail.com> wrote:
>
> Create a helper do_name_to_handle_at() that takes an additional argument,
> lookup_flags, beyond the syscall arguments.
>
> Because name_to_handle_at(2) doesn't take any lookup flags, it always
> passes 0 for this argument.
>
> Future callers like io_uring may pass LOOKUP_CACHED in order to request
> a non-blocking lookup.
>
> This helper's name is confusingly similar to do_sys_name_to_handle()
> which takes care of returning the file handle, once the filename has
> been turned into a struct path. To distinguish the names more clearly,
> rename the latter to do_path_to_handle().
>
> Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
> ---
>  fs/fhandle.c  | 61 ++++++++++++++++++++++++++++-----------------------
>  fs/internal.h |  7 ++++++
>  2 files changed, 41 insertions(+), 27 deletions(-)
>
> diff --git a/fs/fhandle.c b/fs/fhandle.c
> index 7c236f64cdea..57da648ca866 100644
> --- a/fs/fhandle.c
> +++ b/fs/fhandle.c
> @@ -14,10 +14,10 @@
>  #include "internal.h"
>  #include "mount.h"
>
> -static long do_sys_name_to_handle(const struct path *path,
> -                                 struct file_handle __user *ufh,
> -                                 void __user *mnt_id, bool unique_mntid,
> -                                 int fh_flags)
> +static long do_path_to_handle(const struct path *path,
> +                             struct file_handle __user *ufh,
> +                             void __user *mnt_id, bool unique_mntid,
> +                             int fh_flags)
>  {
>         long retval;
>         struct file_handle f_handle;
> @@ -111,27 +111,11 @@ static long do_sys_name_to_handle(const struct path *path,
>         return retval;
>  }
>
> -/**
> - * sys_name_to_handle_at: convert name to handle
> - * @dfd: directory relative to which name is interpreted if not absolute
> - * @name: name that should be converted to handle.
> - * @handle: resulting file handle
> - * @mnt_id: mount id of the file system containing the file
> - *          (u64 if AT_HANDLE_MNT_ID_UNIQUE, otherwise int)
> - * @flag: flag value to indicate whether to follow symlink or not
> - *        and whether a decodable file handle is required.
> - *
> - * @handle->handle_size indicate the space available to store the
> - * variable part of the file handle in bytes. If there is not
> - * enough space, the field is updated to return the minimum
> - * value required.
> - */
> -SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
> -               struct file_handle __user *, handle, void __user *, mnt_id,
> -               int, flag)
> +long do_name_to_handle_at(int dfd, const char __user *name,
> +                         struct file_handle __user *handle, void __user *mnt_id,
> +                         int flag, int lookup_flags)
>  {
>         struct path path;
> -       int lookup_flags;
>         int fh_flags = 0;
>         int err;
>
> @@ -155,19 +139,42 @@ SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
>         else if (flag & AT_HANDLE_CONNECTABLE)
>                 fh_flags |= EXPORT_FH_CONNECTABLE;
>
> -       lookup_flags = (flag & AT_SYMLINK_FOLLOW) ? LOOKUP_FOLLOW : 0;
> +       if (flag & AT_SYMLINK_FOLLOW)
> +               lookup_flags |= LOOKUP_FOLLOW;
>         if (flag & AT_EMPTY_PATH)
>                 lookup_flags |= LOOKUP_EMPTY;
>         err = user_path_at(dfd, name, lookup_flags, &path);
>         if (!err) {
> -               err = do_sys_name_to_handle(&path, handle, mnt_id,
> -                                           flag & AT_HANDLE_MNT_ID_UNIQUE,
> -                                           fh_flags);
> +               err = do_path_to_handle(&path, handle, mnt_id,
> +                                       flag & AT_HANDLE_MNT_ID_UNIQUE,
> +                                       fh_flags);
>                 path_put(&path);
>         }
>         return err;
>  }
>
> +/**
> + * sys_name_to_handle_at: convert name to handle
> + * @dfd: directory relative to which name is interpreted if not absolute
> + * @name: name that should be converted to handle.
> + * @handle: resulting file handle
> + * @mnt_id: mount id of the file system containing the file
> + *          (u64 if AT_HANDLE_MNT_ID_UNIQUE, otherwise int)
> + * @flag: flag value to indicate whether to follow symlink or not
> + *        and whether a decodable file handle is required.
> + *
> + * @handle->handle_size indicate the space available to store the
> + * variable part of the file handle in bytes. If there is not
> + * enough space, the field is updated to return the minimum
> + * value required.
> + */
> +SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
> +               struct file_handle __user *, handle, void __user *, mnt_id,
> +               int, flag)
> +{
> +       return do_name_to_handle_at(dfd, name, handle, mnt_id, flag, 0);
> +}
> +
>  static int get_path_anchor(int fd, struct path *root)
>  {
>         if (fd >= 0) {
> diff --git a/fs/internal.h b/fs/internal.h
> index 38e8aab27bbd..af7e0810a90d 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -355,3 +355,10 @@ int anon_inode_getattr(struct mnt_idmap *idmap, const struct path *path,
>  int anon_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>                        struct iattr *attr);
>  void pidfs_get_root(struct path *path);
> +
> +/*
> + * fs/fhandle.c
> + */
> +long do_name_to_handle_at(int dfd, const char __user *name,
> +                         struct file_handle __user *handle,
> +                         void __user *mnt_id, int flag, int lookup_flags);

I really dislike do_XXX() helpers because we use them interchangeably
sometimes to wrap vfs_XXX() helpers and sometimes the other way around,
so exporting them in the vfs internal interface is a very bad pattern IMO.

io_uring has a common pattern that requires a helper with all the syscall
args and for that purpose, it uses do_renameat2(), do_unlinkat(), ...

I would much rather that we stop this pattern and start with following
the do_sys_XXX() pattern as in the do_sys_ftruncate() helper.

Lucky for us, you just renamed the confusing helper named
do_sys_name_to_handle(), so you are free to reuse this name
(+ _at) in a non confusing placement.

Thanks,
Amir.

Re: [PATCH 4/6] fhandle: create __do_handle_open() helper

[Amir Goldstein]

On Fri, Aug 15, 2025 at 1:52 AM Thomas Bertschinger
<tahbertschinger@gmail.com> wrote:
>
> do_handle_open() takes care of both opening a file via its file handle,
> and associating it with a file descriptor.
>
> For io_uring, it is useful to do just the opening part separately,
> because io_uring might not want to install it into the main open files
> table when using fixed descriptors.
>
> This creates a helper which will enable io_uring to do that.
>
> Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
> ---
>  fs/fhandle.c  | 24 +++++++++++++++++-------
>  fs/internal.h |  2 ++
>  2 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/fs/fhandle.c b/fs/fhandle.c
> index dbb273a26214..b14884a93867 100644
> --- a/fs/fhandle.c
> +++ b/fs/fhandle.c
> @@ -397,8 +397,8 @@ static int handle_to_path(int mountdirfd, struct file_handle __user *ufh,
>         return retval;
>  }
>
> -static long do_handle_open(int mountdirfd, struct file_handle __user *ufh,
> -                          int open_flag)
> +struct file *__do_handle_open(int mountdirfd, struct file_handle __user *ufh,
> +                             int open_flag)
>  {
>         long retval = 0;
>         struct path path __free(path_put) = {};
> @@ -407,17 +407,27 @@ static long do_handle_open(int mountdirfd, struct file_handle __user *ufh,
>
>         retval = handle_to_path(mountdirfd, ufh, &path, open_flag);
>         if (retval)
> -               return retval;
> -
> -       CLASS(get_unused_fd, fd)(open_flag);
> -       if (fd < 0)
> -               return fd;
> +               return ERR_PTR(retval);
>
>         eops = path.mnt->mnt_sb->s_export_op;
>         if (eops->open)
>                 file = eops->open(&path, open_flag);
>         else
>                 file = file_open_root(&path, "", open_flag, 0);
> +
> +       return file;
> +}
> +
> +static long do_handle_open(int mountdirfd, struct file_handle __user *ufh,
> +                          int open_flag)
> +{
> +       struct file *file;
> +
> +       CLASS(get_unused_fd, fd)(open_flag);
> +       if (fd < 0)
> +               return fd;
> +
> +       file = __do_handle_open(mountdirfd, ufh, open_flag);
>         if (IS_ERR(file))
>                 return PTR_ERR(file);
>
> diff --git a/fs/internal.h b/fs/internal.h
> index af7e0810a90d..26ac6f356313 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -362,3 +362,5 @@ void pidfs_get_root(struct path *path);
>  long do_name_to_handle_at(int dfd, const char __user *name,
>                           struct file_handle __user *handle,
>                           void __user *mnt_id, int flag, int lookup_flags);
> +struct file *__do_handle_open(int mountdirfd, struct file_handle __user *ufh,
> +                             int open_flag);

As I said I displike exported do_XXX() helpers, but I __do_XXX() exported
helpers are much worse.

How about do_filp_handle_open()?

Thanks,
Amir.

Re: [PATCH 2/6] io_uring: add support for IORING_OP_NAME_TO_HANDLE_AT

[Amir Goldstein]

On Fri, Aug 15, 2025 at 1:51 AM Thomas Bertschinger
<tahbertschinger@gmail.com> wrote:
>
> Add support for name_to_handle_at(2) to io_uring.
>
> Like openat*(), this tries to do a non-blocking lookup first and resorts
> to async lookup when that fails.
>
> This uses sqe->addr for the path, ->addr2 for the file handle which is
> filled in by the kernel, and ->addr3 for the mouint_id which is filled
> in by the kernel.
>
> Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
> ---
>  include/uapi/linux/io_uring.h |  1 +
>  io_uring/opdef.c              |  7 ++++++
>  io_uring/openclose.c          | 43 +++++++++++++++++++++++++++++++++++
>  io_uring/openclose.h          |  3 +++
>  4 files changed, 54 insertions(+)
>
> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index 6957dc539d83..596bae788b48 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -289,6 +289,7 @@ enum io_uring_op {
>         IORING_OP_READV_FIXED,
>         IORING_OP_WRITEV_FIXED,
>         IORING_OP_PIPE,
> +       IORING_OP_NAME_TO_HANDLE_AT,
>
>         /* this goes last, obviously */
>         IORING_OP_LAST,
> diff --git a/io_uring/opdef.c b/io_uring/opdef.c
> index 9568785810d9..ff2672bbd583 100644
> --- a/io_uring/opdef.c
> +++ b/io_uring/opdef.c
> @@ -574,6 +574,10 @@ const struct io_issue_def io_issue_defs[] = {
>                 .prep                   = io_pipe_prep,
>                 .issue                  = io_pipe,
>         },
> +       [IORING_OP_NAME_TO_HANDLE_AT] = {
> +               .prep                   = io_name_to_handle_at_prep,
> +               .issue                  = io_name_to_handle_at,
> +       },
>  };
>
>  const struct io_cold_def io_cold_defs[] = {
> @@ -824,6 +828,9 @@ const struct io_cold_def io_cold_defs[] = {
>         [IORING_OP_PIPE] = {
>                 .name                   = "PIPE",
>         },
> +       [IORING_OP_NAME_TO_HANDLE_AT] = {
> +               .name                   = "NAME_TO_HANDLE_AT",
> +       },
>  };
>
>  const char *io_uring_get_opcode(u8 opcode)
> diff --git a/io_uring/openclose.c b/io_uring/openclose.c
> index d70700e5cef8..f15a9307f811 100644
> --- a/io_uring/openclose.c
> +++ b/io_uring/openclose.c
> @@ -27,6 +27,15 @@ struct io_open {
>         unsigned long                   nofile;
>  };
>
> +struct io_name_to_handle {
> +       struct file                     *file;
> +       int                             dfd;
> +       int                             open_flag;

These are not open_flags, there are handle_flags
(e.g. AT_HANDLE_FID)

> +       struct file_handle __user       *ufh;
> +       char __user                     *path;
> +       void __user                     *mount_id;
> +};
> +
>  struct io_close {
>         struct file                     *file;
>         int                             fd;
> @@ -187,6 +196,40 @@ void io_open_cleanup(struct io_kiocb *req)
>                 putname(open->filename);
>  }
>
> +int io_name_to_handle_at_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> +{
> +       struct io_name_to_handle *nh = io_kiocb_to_cmd(req, struct io_name_to_handle);
> +
> +       nh->dfd = READ_ONCE(sqe->fd);
> +       nh->open_flag = READ_ONCE(sqe->open_flags);

Please also create union member sqe->handle_flags
or name_to_handle_flags.

Thanks,
Amir.

Re: [PATCH 3/6] fhandle: do_handle_open() should get FD with user flags

[Christian Brauner]

On Fri, Aug 15, 2025 at 11:17:26AM +0200, Amir Goldstein wrote:
> On Fri, Aug 15, 2025 at 1:52 AM Thomas Bertschinger
> <tahbertschinger@gmail.com> wrote:
> >
> > In f07c7cc4684a, do_handle_open() was switched to use the automatic
> > cleanup method for getting a FD. In that change it was also switched
> > to pass O_CLOEXEC unconditionally to get_unused_fd_flags() instead
> > of passing the user-specified flags.
> >
> > I don't see anything in that commit description that indicates this was
> > intentional, so I am assuming it was an oversight.
> >
> > With this fix, the FD will again be opened with, or without, O_CLOEXEC
> > according to what the user requested.
> >
> > Fixes: f07c7cc4684a ("fhandle: simplify error handling")
> > Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
> 
> This patch does not seem to be conflicting with earlier patches in the series
> but it is still preferred to start the series with the backportable fix patch.
> 
> Fee free to add:
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>

I'm kinda tempted to last let it slide because I think that's how it
should actually be... But ofc, we'll fix.

Re: (subset) [PATCH 3/6] fhandle: do_handle_open() should get FD with user flags

[Christian Brauner]

On Thu, 14 Aug 2025 17:54:28 -0600, Thomas Bertschinger wrote:
> In f07c7cc4684a, do_handle_open() was switched to use the automatic
> cleanup method for getting a FD. In that change it was also switched
> to pass O_CLOEXEC unconditionally to get_unused_fd_flags() instead
> of passing the user-specified flags.
> 
> I don't see anything in that commit description that indicates this was
> intentional, so I am assuming it was an oversight.
> 
> [...]

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

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

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

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

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

[3/6] fhandle: do_handle_open() should get FD with user flags
      https://git.kernel.org/vfs/vfs/c/b5ca88927e35

Re: [PATCH 3/6] fhandle: do_handle_open() should get FD with user flags

[Amir Goldstein]

On Fri, Aug 15, 2025 at 3:46 PM Christian Brauner <brauner@kernel.org> wrote:
>
> On Fri, Aug 15, 2025 at 11:17:26AM +0200, Amir Goldstein wrote:
> > On Fri, Aug 15, 2025 at 1:52 AM Thomas Bertschinger
> > <tahbertschinger@gmail.com> wrote:
> > >
> > > In f07c7cc4684a, do_handle_open() was switched to use the automatic
> > > cleanup method for getting a FD. In that change it was also switched
> > > to pass O_CLOEXEC unconditionally to get_unused_fd_flags() instead
> > > of passing the user-specified flags.
> > >
> > > I don't see anything in that commit description that indicates this was
> > > intentional, so I am assuming it was an oversight.
> > >
> > > With this fix, the FD will again be opened with, or without, O_CLOEXEC
> > > according to what the user requested.
> > >
> > > Fixes: f07c7cc4684a ("fhandle: simplify error handling")
> > > Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
> >
> > This patch does not seem to be conflicting with earlier patches in the series
> > but it is still preferred to start the series with the backportable fix patch.
> >
> > Fee free to add:
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
>
> I'm kinda tempted to last let it slide because I think that's how it
> should actually be... But ofc, we'll fix.

You mean forcing O_CLOEXEC. right?
Not ignoring the rest of O_ flags...

Thanks,
Amir.

Re: [PATCH 1/6] fhandle: create helper for name_to_handle_at(2)

[Thomas Bertschinger]

On Fri Aug 15, 2025 at 4:21 AM MDT, Amir Goldstein wrote:
> On Fri, Aug 15, 2025 at 1:51 AM Thomas Bertschinger
> <tahbertschinger@gmail.com> wrote:
>> +/*
>> + * fs/fhandle.c
>> + */
>> +long do_name_to_handle_at(int dfd, const char __user *name,
>> +                         struct file_handle __user *handle,
>> +                         void __user *mnt_id, int flag, int lookup_flags);
>
> I really dislike do_XXX() helpers because we use them interchangeably
> sometimes to wrap vfs_XXX() helpers and sometimes the other way around,
> so exporting them in the vfs internal interface is a very bad pattern IMO.
>
> io_uring has a common pattern that requires a helper with all the syscall
> args and for that purpose, it uses do_renameat2(), do_unlinkat(), ...
>
> I would much rather that we stop this pattern and start with following
> the do_sys_XXX() pattern as in the do_sys_ftruncate() helper.
>
> Lucky for us, you just renamed the confusing helper named
> do_sys_name_to_handle(), so you are free to reuse this name
> (+ _at) in a non confusing placement.
>
> Thanks,
> Amir.

That makes sense, I can adjust it like that in v2. Thanks!

Re: [PATCHSET RFC 0/6] add support for name_to, open_by_handle_at(2) to io_uring

[Thomas Bertschinger]

On Fri Aug 15, 2025 at 3:52 AM MDT, Amir Goldstein wrote:
> On Fri, Aug 15, 2025 at 1:50 AM Thomas Bertschinger
> <tahbertschinger@gmail.com> wrote:
>> No attempt is made to support a non-blocking open_by_handle_at()--the
>> attempt is always immediately returned with -EAGAIN if
>> IO_URING_F_NONBLOCK is set.
>>
>> This isn't ideal and it would be nice to add support for non-blocking
>> open by handle in the future. This would presumably require updates to
>> the ->encode_fh() implementation for filesystems that want to
>> support this.
>
> Correction: ->encode_fh() is for name_to_handle()
> You want to say that ->fh_to_dentry() need to support cached lookup,
Yes, that is what I meant, whoops.

> but FWIW, the blocking code is more likely to come from the
> lookup in exportfs_decode_fh_raw() => ... reconnect_one()
> not from the filesystem code.
>
> The fs would "only" need to be taught to return an alias to a
> cached inode and generic code would "only" need to be taught
> to give up on a disconnected dir dentry.
>
> Doesn't sound too hard (famous last words).
>
> Thanks,
> Amir.
>

Thanks for the ideas on how to support this!

Re: [PATCH 2/6] io_uring: add support for IORING_OP_NAME_TO_HANDLE_AT

[kernel test robot]

Hi Thomas,

kernel test robot noticed the following build errors:

[auto build test ERROR on brauner-vfs/vfs.all]
[also build test ERROR on linus/master v6.17-rc1 next-20250815]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Thomas-Bertschinger/fhandle-create-helper-for-name_to_handle_at-2/20250815-075417
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link:    https://lore.kernel.org/r/20250814235431.995876-3-tahbertschinger%40gmail.com
patch subject: [PATCH 2/6] io_uring: add support for IORING_OP_NAME_TO_HANDLE_AT
config: parisc-randconfig-r132-20250816 (https://download.01.org/0day-ci/archive/20250816/202508161506.L391RGhx-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 11.5.0
reproduce: (https://download.01.org/0day-ci/archive/20250816/202508161506.L391RGhx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508161506.L391RGhx-lkp@intel.com/

All errors (new ones prefixed by >>):

   hppa-linux-ld: io_uring/openclose.o: in function `io_name_to_handle_at':
>> io_uring/openclose.c:221:(.text+0x838): undefined reference to `do_name_to_handle_at'


vim +221 io_uring/openclose.c

   211	
   212	int io_name_to_handle_at(struct io_kiocb *req, unsigned int issue_flags)
   213	{
   214		struct io_name_to_handle *nh = io_kiocb_to_cmd(req, struct io_name_to_handle);
   215		int lookup_flags = 0;
   216		long ret;
   217	
   218		if (issue_flags & IO_URING_F_NONBLOCK)
   219			lookup_flags = LOOKUP_CACHED;
   220	
 > 221		ret = do_name_to_handle_at(nh->dfd, nh->path, nh->ufh, nh->mount_id,
   222					   nh->open_flag, lookup_flags);
   223	
   224		if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK))
   225			return -EAGAIN;
   226	
   227		if (ret < 0)
   228			req_set_fail(req);
   229		io_req_set_res(req, ret, 0);
   230		return IOU_COMPLETE;
   231	}
   232	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 6/6] io_uring: add support for IORING_OP_OPEN_BY_HANDLE_AT

[kernel test robot]

Hi Thomas,

kernel test robot noticed the following build errors:

[auto build test ERROR on brauner-vfs/vfs.all]
[also build test ERROR on linus/master v6.17-rc1 next-20250815]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Thomas-Bertschinger/fhandle-create-helper-for-name_to_handle_at-2/20250815-075417
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link:    https://lore.kernel.org/r/20250814235431.995876-7-tahbertschinger%40gmail.com
patch subject: [PATCH 6/6] io_uring: add support for IORING_OP_OPEN_BY_HANDLE_AT
config: parisc-randconfig-r132-20250816 (https://download.01.org/0day-ci/archive/20250816/202508161702.2Ey3J7m9-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 11.5.0
reproduce: (https://download.01.org/0day-ci/archive/20250816/202508161702.2Ey3J7m9-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508161702.2Ey3J7m9-lkp@intel.com/

All errors (new ones prefixed by >>):

   hppa-linux-ld: io_uring/openclose.o: in function `io_name_to_handle_at':
   io_uring/openclose.c:241:(.text+0x870): undefined reference to `do_name_to_handle_at'
   hppa-linux-ld: io_uring/openclose.o: in function `io_open_by_handle_at':
>> io_uring/openclose.c:287:(.text+0x990): undefined reference to `__do_handle_open'


vim +287 io_uring/openclose.c

   266	
   267	int io_open_by_handle_at(struct io_kiocb *req, unsigned int issue_flags)
   268	{
   269		struct io_open *open = io_kiocb_to_cmd(req, struct io_open);
   270		struct file *file;
   271		bool fixed = !!open->file_slot;
   272		int ret;
   273	
   274		/*
   275		 * Always try again if we aren't supposed to block, because there is no
   276		 * way of preventing the FS implementation from blocking.
   277		 */
   278		if (issue_flags & IO_URING_F_NONBLOCK)
   279			return -EAGAIN;
   280	
   281		if (!fixed) {
   282			ret = __get_unused_fd_flags(open->how.flags, open->nofile);
   283			if (ret < 0)
   284				goto err;
   285		}
   286	
 > 287		file = __do_handle_open(open->dfd, open->ufh, open->how.flags);
   288	
   289		if (IS_ERR(file)) {
   290			if (!fixed)
   291				put_unused_fd(ret);
   292			ret = PTR_ERR(file);
   293			goto err;
   294		}
   295	
   296		if (!fixed)
   297			fd_install(ret, file);
   298		else
   299			ret = io_fixed_fd_install(req, issue_flags, file,
   300						  open->file_slot);
   301	
   302	err:
   303		if (ret < 0)
   304			req_set_fail(req);
   305		io_req_set_res(req, ret, 0);
   306		return IOU_COMPLETE;
   307	}
   308	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 3/6] fhandle: do_handle_open() should get FD with user flags

[Christian Brauner]

On Fri, Aug 15, 2025 at 03:51:53PM +0200, Amir Goldstein wrote:
> On Fri, Aug 15, 2025 at 3:46 PM Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Fri, Aug 15, 2025 at 11:17:26AM +0200, Amir Goldstein wrote:
> > > On Fri, Aug 15, 2025 at 1:52 AM Thomas Bertschinger
> > > <tahbertschinger@gmail.com> wrote:
> > > >
> > > > In f07c7cc4684a, do_handle_open() was switched to use the automatic
> > > > cleanup method for getting a FD. In that change it was also switched
> > > > to pass O_CLOEXEC unconditionally to get_unused_fd_flags() instead
> > > > of passing the user-specified flags.
> > > >
> > > > I don't see anything in that commit description that indicates this was
> > > > intentional, so I am assuming it was an oversight.
> > > >
> > > > With this fix, the FD will again be opened with, or without, O_CLOEXEC
> > > > according to what the user requested.
> > > >
> > > > Fixes: f07c7cc4684a ("fhandle: simplify error handling")
> > > > Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
> > >
> > > This patch does not seem to be conflicting with earlier patches in the series
> > > but it is still preferred to start the series with the backportable fix patch.
> > >
> > > Fee free to add:
> > > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> >
> > I'm kinda tempted to last let it slide because I think that's how it
> > should actually be... But ofc, we'll fix.
> 
> You mean forcing O_CLOEXEC. right?

Yes, of course. :)

> Not ignoring the rest of O_ flags...

No, I think that would be unwise. :)

Re: [PATCHSET RFC 0/6] add support for name_to, open_by_handle_at(2) to io_uring

[Jens Axboe]

I'll take a look at this, but wanted to mention that I dabbled in this
too a while ago, here's what I had:

https://git.kernel.dk/cgit/linux/log/?h=io_uring-handle

Probably pretty incomplete, but I did try and handle some of the
cases that won't block to avoid spurious -EAGAIN and io-wq usage.

Anyway, take a look at that too, and I'll take a look at your probably
more complete set.

-- 
Jens Axboe

Re: [PATCHSET RFC 0/6] add support for name_to, open_by_handle_at(2) to io_uring

[Thomas Bertschinger]

On Tue Aug 19, 2025 at 9:11 AM MDT, Jens Axboe wrote:
> I'll take a look at this, but wanted to mention that I dabbled in this
> too a while ago, here's what I had:
>
> https://git.kernel.dk/cgit/linux/log/?h=io_uring-handle

Thanks! That is helpful. Right away I see something you included that I
missed: requiring CONFIG_FHANDLE. Missing that would explain the build
failure emails I got on this series.

I'll include that in v2, when I get around to that--hopefully soon.

>
> Probably pretty incomplete, but I did try and handle some of the
> cases that won't block to avoid spurious -EAGAIN and io-wq usage.

So for the non-blocking case, what I am concerned about is code paths
like this:

do_handle_to_path()
  -> exportfs_decode_fh_raw()
    -> fh_to_dentry()
      -> xfs_fs_fh_to_dentry()
        ... -> xfs_iget()
      OR
      -> ext4_fh_to_dentry()
        ... -> ext4_iget()

Where there doesn't seem to be any existing way to tell the FS
implementation to give up and return -EAGAIN when appropriate. I wasn't
sure how to do that without modifying the signature of fh_to_dentry()
(and fh_to_parent()) which seems awfully invasive for this.

(Using a flag in task_struct to signify "don't block" was previously
discussed:
https://lore.kernel.org/io-uring/22630618-40fc-5668-078d-6cefcb2e4962@kernel.dk/
and that could allow not needing to pass a flag via function argument,
but I agree with the conclusion in that email chain that it's an ugly
solution.)

Any thoughts on that? This seemed to me like there wasn't an obvious
easy solution, hence why I just didn't attempt it at all in v1.
Maybe I'm missing something, though.

Aside from fh_to_dentry(), there is I/O that may arise from
reconnecting the dentry, as Amir pointed out earlier (like in
reconnect_one()). Handling that would, I think, be simpler because it
would only require modifying the generic code under reconnect_path() and
not each filesystem implementation.

Re: [PATCHSET RFC 0/6] add support for name_to, open_by_handle_at(2) to io_uring

[Amir Goldstein]

On Wed, Aug 20, 2025 at 4:57 AM Thomas Bertschinger
<tahbertschinger@gmail.com> wrote:
>
> On Tue Aug 19, 2025 at 9:11 AM MDT, Jens Axboe wrote:
> > I'll take a look at this, but wanted to mention that I dabbled in this
> > too a while ago, here's what I had:
> >
> > https://git.kernel.dk/cgit/linux/log/?h=io_uring-handle
>
> Thanks! That is helpful. Right away I see something you included that I
> missed: requiring CONFIG_FHANDLE. Missing that would explain the build
> failure emails I got on this series.
>
> I'll include that in v2, when I get around to that--hopefully soon.
>
> >
> > Probably pretty incomplete, but I did try and handle some of the
> > cases that won't block to avoid spurious -EAGAIN and io-wq usage.
>
> So for the non-blocking case, what I am concerned about is code paths
> like this:
>
> do_handle_to_path()
>   -> exportfs_decode_fh_raw()
>     -> fh_to_dentry()
>       -> xfs_fs_fh_to_dentry()
>         ... -> xfs_iget()
>       OR
>       -> ext4_fh_to_dentry()
>         ... -> ext4_iget()
>
> Where there doesn't seem to be any existing way to tell the FS
> implementation to give up and return -EAGAIN when appropriate. I wasn't
> sure how to do that without modifying the signature of fh_to_dentry()
> (and fh_to_parent()) which seems awfully invasive for this.
>
> (Using a flag in task_struct to signify "don't block" was previously
> discussed:
> https://lore.kernel.org/io-uring/22630618-40fc-5668-078d-6cefcb2e4962@kernel.dk/
> and that could allow not needing to pass a flag via function argument,
> but I agree with the conclusion in that email chain that it's an ugly
> solution.)
>
> Any thoughts on that? This seemed to me like there wasn't an obvious
> easy solution, hence why I just didn't attempt it at all in v1.
> Maybe I'm missing something, though.
>

Since FILEID_IS_CONNECTABLE, we started using the high 16 bits of
fh_type for FILEID_USER_FLAGS, since fs is not likely expecting a fh_type
beyond 0xff (Documentation/filesystems/nfs/exporting.rst):
"A filehandle fragment consists of an array of 1 or more 4byte words,
together with a one byte "type"."

The name FILEID_USER_FLAGS may be a bit misleading - it was
never the intention for users to manipulate those flags, although they
certainly can and there is no real harm in that.

These flags are used in the syscall interface only, but
->fh_to_{dentry,parent}() function signature also take an int fh_flags
argument, so we can use that to express the non-blocking request.

Untested patch follows (easier than explaining):


diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index d3e55de4a2a2a..a46c97af4dfb1 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -391,7 +391,7 @@ int exportfs_encode_inode_fh(struct inode *inode,
struct fid *fid,
        else
                type = nop->encode_fh(inode, fid->raw, max_len, parent);

-       if (type > 0 && FILEID_USER_FLAGS(type)) {
+       if (type > 0 && type & ~FILEID_HANDLE_TYPE_MASK) {
                pr_warn_once("%s: unexpected fh type value 0x%x from
fstype %s.\n",
                             __func__, type, inode->i_sb->s_type->name);
                return -EINVAL;
@@ -443,9 +443,12 @@ exportfs_decode_fh_raw(struct vfsmount *mnt,
struct fid *fid, int fh_len,
        const struct export_operations *nop = mnt->mnt_sb->s_export_op;
        struct dentry *result, *alias;
        char nbuf[NAME_MAX+1];
+       int fh_type = fileid_type | FILEID_FS_FLAGS(flags);
        int err;

-       if (fileid_type < 0 || FILEID_USER_FLAGS(fileid_type))
+       BUILD_BUG_ON(FILEID_HANDLE_TYPE_MASK & FILEID_FS_FLAGS_MASK);
+       BUILD_BUG_ON(FILEID_USER_FLAGS_MASK & FILEID_FS_FLAGS_MASK);
+       if (fileid_type < 0 || fileid_type & ~FILEID_HANDLE_TYPE_MASK)
                return ERR_PTR(-EINVAL);

        /*
@@ -453,7 +456,7 @@ exportfs_decode_fh_raw(struct vfsmount *mnt,
struct fid *fid, int fh_len,
         */
        if (!exportfs_can_decode_fh(nop))
                return ERR_PTR(-ESTALE);
-       result = nop->fh_to_dentry(mnt->mnt_sb, fid, fh_len, fileid_type);
+       result = nop->fh_to_dentry(mnt->mnt_sb, fid, fh_len, fh_type);
        if (IS_ERR_OR_NULL(result))
                return result;

@@ -481,6 +484,10 @@ exportfs_decode_fh_raw(struct vfsmount *mnt,
struct fid *fid, int fh_len,
                 * filesystem root.
                 */
                if (result->d_flags & DCACHE_DISCONNECTED) {
+                       err = -EAGAIN;
+                       if (flags & EXPORT_FH_CACHED)
+                               goto err_result;
+
                        err = reconnect_path(mnt, result, nbuf);
                        if (err)
                                goto err_result;
@@ -511,6 +518,10 @@ exportfs_decode_fh_raw(struct vfsmount *mnt,
struct fid *fid, int fh_len,
                if (alias)
                        return alias;

+               err = -EAGAIN;
+               if (flags & EXPORT_FH_CACHED)
+                       goto err_result;
+
                /*
                 * Try to extract a dentry for the parent directory from the
                 * file handle.  If this fails we'll have to give up.
diff --git a/fs/fhandle.c b/fs/fhandle.c
index 7c236f64cdeac..228512424ad65 100644
--- a/fs/fhandle.c
+++ b/fs/fhandle.c
@@ -339,7 +339,7 @@ static int handle_to_path(int mountdirfd, struct
file_handle __user *ufh,
            (f_handle.handle_bytes == 0))
                return -EINVAL;

-       if (f_handle.handle_type < 0 ||
+       if (f_handle.handle_type < 0 || FILEID_FS_FLAGS(f_handle.handle_type) ||
            FILEID_USER_FLAGS(f_handle.handle_type) & ~FILEID_VALID_USER_FLAGS)
                return -EINVAL;

@@ -382,7 +382,10 @@ static int handle_to_path(int mountdirfd, struct
file_handle __user *ufh,
        if (f_handle.handle_type & FILEID_IS_DIR)
                ctx.fh_flags |= EXPORT_FH_DIR_ONLY;
        /* Filesystem code should not be exposed to user flags */
-       handle->handle_type &= ~FILEID_USER_FLAGS_MASK;
+       BUILD_BUG_ON(FILEID_HANDLE_TYPE_MASK & FILEID_USER_FLAGS_MASK);
+       handle->handle_type &= ~FILEID_HANDLE_TYPE_MASK;
+       if (o_flags & O_NONBLOCK)
+               ctx.fh_flags |= EXPORT_FH_CACHED;
        retval = do_handle_to_path(handle, path, &ctx);

 out_path:
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index cfb0dd1ea49c7..331d28093b568 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -174,6 +174,18 @@ struct handle_to_path_ctx {

 /*
  * Filesystems use only lower 8 bits of file_handle type for fid_type.
+ */
+#define FILEID_HANDLE_TYPE_MASK        0xff
+
+/*
+ * vfs uses bits 8..15 of @fh_type arg of fh_to_dentry/fh_to_parent methods
+ * as misc. flags to pass to filesystems.
+ */
+#define EXPORT_FH_CACHED       0x100 /* Non-blocking encode/decode */
+#define FILEID_FS_FLAGS_MASK   0xff00
+#define FILEID_FS_FLAGS(flags) ((flags) & FILEID_FS_FLAGS_MASK)
+
+/*
  * name_to_handle_at() uses upper 16 bits of type as user flags to be
  * interpreted by open_by_handle_at().
  */

Thanks,
Amir.

Re: [PATCHSET RFC 0/6] add support for name_to, open_by_handle_at(2) to io_uring

[Thomas Bertschinger]

On Wed Aug 20, 2025 at 2:34 AM MDT, Amir Goldstein wrote:
> On Wed, Aug 20, 2025 at 4:57 AM Thomas Bertschinger
> <tahbertschinger@gmail.com> wrote:
>> Any thoughts on that? This seemed to me like there wasn't an obvious
>> easy solution, hence why I just didn't attempt it at all in v1.
>> Maybe I'm missing something, though.
>>
>
> Since FILEID_IS_CONNECTABLE, we started using the high 16 bits of
> fh_type for FILEID_USER_FLAGS, since fs is not likely expecting a fh_type
> beyond 0xff (Documentation/filesystems/nfs/exporting.rst):
> "A filehandle fragment consists of an array of 1 or more 4byte words,
> together with a one byte "type"."
>
> The name FILEID_USER_FLAGS may be a bit misleading - it was
> never the intention for users to manipulate those flags, although they
> certainly can and there is no real harm in that.
>
> These flags are used in the syscall interface only, but
> ->fh_to_{dentry,parent}() function signature also take an int fh_flags
> argument, so we can use that to express the non-blocking request.
>
> Untested patch follows (easier than explaining):

Ah, that makes sense and makes this seem feasible. Thanks for pointing
that out!

It also seems that each FS could opt in to this with a new EXPORT_OP
flag so that the FSes that want to support this can be updated
individually. Then, updating most or every exportable FS isn't a
requirement for this.

Do you have an opinion on that, versus expecting every ->fh_to_dentry()
implementation to respect the new flag?

Re: [PATCHSET RFC 0/6] add support for name_to, open_by_handle_at(2) to io_uring

[Amir Goldstein]

On Wed, Aug 20, 2025 at 5:00 PM Thomas Bertschinger
<tahbertschinger@gmail.com> wrote:
>
> On Wed Aug 20, 2025 at 2:34 AM MDT, Amir Goldstein wrote:
> > On Wed, Aug 20, 2025 at 4:57 AM Thomas Bertschinger
> > <tahbertschinger@gmail.com> wrote:
> >> Any thoughts on that? This seemed to me like there wasn't an obvious
> >> easy solution, hence why I just didn't attempt it at all in v1.
> >> Maybe I'm missing something, though.
> >>
> >
> > Since FILEID_IS_CONNECTABLE, we started using the high 16 bits of
> > fh_type for FILEID_USER_FLAGS, since fs is not likely expecting a fh_type
> > beyond 0xff (Documentation/filesystems/nfs/exporting.rst):
> > "A filehandle fragment consists of an array of 1 or more 4byte words,
> > together with a one byte "type"."
> >
> > The name FILEID_USER_FLAGS may be a bit misleading - it was
> > never the intention for users to manipulate those flags, although they
> > certainly can and there is no real harm in that.
> >
> > These flags are used in the syscall interface only, but
> > ->fh_to_{dentry,parent}() function signature also take an int fh_flags
> > argument, so we can use that to express the non-blocking request.
> >
> > Untested patch follows (easier than explaining):
>
> Ah, that makes sense and makes this seem feasible. Thanks for pointing
> that out!
>
> It also seems that each FS could opt in to this with a new EXPORT_OP
> flag so that the FSes that want to support this can be updated
> individually. Then, updating most or every exportable FS isn't a
> requirement for this.

Makes a lot of sense. yes.

>
> Do you have an opinion on that, versus expecting every ->fh_to_dentry()
> implementation to respect the new flag?

Technically, you do not need every fs to respect this flag, you only need them
to not ignore it.

Generally, if you pass (fileid_type | EXPORT_FH_CACHED) as the type
argument, most filesystems will not accept this value anyway and return
NULL or PTR_ERR(-ESTALE), so not ignoring.

But I think it is much preferred to check the opt-in EXPORT_OP
flag and return EAGAIN from generic code in the case that fs does
not support non-blocking decode.

And fs that do opt in should probably return PTR_ERR(-EAGAIN)
when the file type is correct but non-blocking decode is not possible.

Thanks,
Amir.

Re: [PATCHSET RFC 0/6] add support for name_to, open_by_handle_at(2) to io_uring

[Christian Brauner]

On Wed, Aug 20, 2025 at 09:58:15PM +0200, Amir Goldstein wrote:
> On Wed, Aug 20, 2025 at 5:00 PM Thomas Bertschinger
> <tahbertschinger@gmail.com> wrote:
> >
> > On Wed Aug 20, 2025 at 2:34 AM MDT, Amir Goldstein wrote:
> > > On Wed, Aug 20, 2025 at 4:57 AM Thomas Bertschinger
> > > <tahbertschinger@gmail.com> wrote:
> > >> Any thoughts on that? This seemed to me like there wasn't an obvious
> > >> easy solution, hence why I just didn't attempt it at all in v1.
> > >> Maybe I'm missing something, though.
> > >>
> > >
> > > Since FILEID_IS_CONNECTABLE, we started using the high 16 bits of
> > > fh_type for FILEID_USER_FLAGS, since fs is not likely expecting a fh_type
> > > beyond 0xff (Documentation/filesystems/nfs/exporting.rst):
> > > "A filehandle fragment consists of an array of 1 or more 4byte words,
> > > together with a one byte "type"."
> > >
> > > The name FILEID_USER_FLAGS may be a bit misleading - it was
> > > never the intention for users to manipulate those flags, although they
> > > certainly can and there is no real harm in that.
> > >
> > > These flags are used in the syscall interface only, but
> > > ->fh_to_{dentry,parent}() function signature also take an int fh_flags
> > > argument, so we can use that to express the non-blocking request.
> > >
> > > Untested patch follows (easier than explaining):
> >
> > Ah, that makes sense and makes this seem feasible. Thanks for pointing
> > that out!
> >
> > It also seems that each FS could opt in to this with a new EXPORT_OP
> > flag so that the FSes that want to support this can be updated
> > individually. Then, updating most or every exportable FS isn't a
> > requirement for this.
> 
> Makes a lot of sense. yes.
> 
> >
> > Do you have an opinion on that, versus expecting every ->fh_to_dentry()
> > implementation to respect the new flag?
> 
> Technically, you do not need every fs to respect this flag, you only need them
> to not ignore it.
> 
> Generally, if you pass (fileid_type | EXPORT_FH_CACHED) as the type
> argument, most filesystems will not accept this value anyway and return
> NULL or PTR_ERR(-ESTALE), so not ignoring.
> 
> But I think it is much preferred to check the opt-in EXPORT_OP
> flag and return EAGAIN from generic code in the case that fs does
> not support non-blocking decode.
> 
> And fs that do opt in should probably return PTR_ERR(-EAGAIN)
> when the file type is correct but non-blocking decode is not possible.

I like your idea as it's in line with other extensions we've done
recently to open_by_handle_at().