[PATCHSET for-next 0/2] Deduplicate fd-to-ctx conversions

[PATCHSET for-next 0/2] Deduplicate fd-to-ctx conversions

[Jens Axboe]

Hi,

io_uring_register_get_file() exists to return a file associated with
a struct io_ring_ctx, dealing with both registered ring descriptors
and with normal file descriptors. There are a few users of this, but
the main io_uring_enter(2) syscall handrolls its own.

Deduplicate that by avoiding the fget/fput for the registered ring
descriptor, as that cannot go away since it's private to the task
doing the operation. Unify how the file is put because of that, and
then we can convert io_uring_enter(2) to use this helper too and move
it closer to the hot path of the actual syscall. Then we only have a
single way of doing the ctx retrieval, rather than two separate ones.

 io_uring/bpf-ops.c  |  2 +-
 io_uring/io_uring.c | 57 ++++++++++++++++++++++++++++-----------------
 io_uring/io_uring.h |  1 +
 io_uring/register.c | 39 +++----------------------------
 io_uring/register.h |  1 -
 io_uring/rsrc.c     |  5 ++--
 6 files changed, 44 insertions(+), 61 deletions(-)

-- 
Jens Axboe

[PATCH 1/2] io_uring/register: don't get a reference to the registered ring fd

[Jens Axboe]

This isn't necessary and was only done because the register path isn't a
hot path and hence the extra ref/put doesn't matter, and to have the
exit path be able to unconditionally put whatever file was gotten
regardless of the type.

In preparation for sharing this code with the main io_uring_enter(2)
syscall, drop the reference and have the caller conditionally put the
file if it was a normal file descriptor.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 io_uring/register.c | 8 ++++----
 io_uring/rsrc.c     | 3 ++-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/io_uring/register.c b/io_uring/register.c
index 35432471a550..95cfa88dc621 100644
--- a/io_uring/register.c
+++ b/io_uring/register.c
@@ -941,7 +941,8 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode,
 /*
  * Given an 'fd' value, return the ctx associated with if. If 'registered' is
  * true, then the registered index is used. Otherwise, the normal fd table.
- * Caller must call fput() on the returned file, unless it's an ERR_PTR.
+ * Caller must call fput() on the returned file if it isn't a registered file,
+ * unless it's an ERR_PTR.
  */
 struct file *io_uring_register_get_file(unsigned int fd, bool registered)
 {
@@ -958,8 +959,6 @@ struct file *io_uring_register_get_file(unsigned int fd, bool registered)
 			return ERR_PTR(-EINVAL);
 		fd = array_index_nospec(fd, IO_RINGFD_REG_MAX);
 		file = tctx->registered_rings[fd];
-		if (file)
-			get_file(file);
 	} else {
 		file = fget(fd);
 	}
@@ -1038,6 +1037,7 @@ SYSCALL_DEFINE4(io_uring_register, unsigned int, fd, unsigned int, opcode,
 				ctx->buf_table.nr, ret);
 	mutex_unlock(&ctx->uring_lock);
 
-	fput(file);
+	if (!use_registered_ring)
+		fput(file);
 	return ret;
 }
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index 2d8be5edbbf6..cb12194b35e8 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -1291,7 +1291,8 @@ int io_register_clone_buffers(struct io_ring_ctx *ctx, void __user *arg)
 	if (src_ctx != ctx)
 		mutex_unlock(&src_ctx->uring_lock);
 
-	fput(file);
+	if (!registered_src)
+		fput(file);
 	return ret;
 }
 
-- 
2.53.0

[PATCH 2/2] io_uring: unify getting ctx from passed in file descriptor

[Jens Axboe]

io_uring_enter() and io_uring_register() end up having duplicated code
for getting a ctx from a passed in file descriptor, for either a
registered ring descriptor or a normal file descriptor. Move the
io_uring_register_get_file() into io_uring.c and name it a bit more
generically, and use it from both callsites rather than have that logic
and handling duplicated.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 io_uring/bpf-ops.c  |  2 +-
 io_uring/io_uring.c | 57 ++++++++++++++++++++++++++++-----------------
 io_uring/io_uring.h |  1 +
 io_uring/register.c | 35 +---------------------------
 io_uring/register.h |  1 -
 io_uring/rsrc.c     |  2 +-
 6 files changed, 40 insertions(+), 58 deletions(-)

diff --git a/io_uring/bpf-ops.c b/io_uring/bpf-ops.c
index e4b244337aa9..937e48bef40b 100644
--- a/io_uring/bpf-ops.c
+++ b/io_uring/bpf-ops.c
@@ -181,7 +181,7 @@ static int bpf_io_reg(void *kdata, struct bpf_link *link)
 	struct file *file;
 	int ret = -EBUSY;
 
-	file = io_uring_register_get_file(ops->ring_fd, false);
+	file = io_uring_ctx_get_file(ops->ring_fd, false);
 	if (IS_ERR(file))
 		return PTR_ERR(file);
 	ctx = file->private_data;
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 16122f877aed..003f0e081d92 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -2543,39 +2543,54 @@ static int io_get_ext_arg(struct io_ring_ctx *ctx, unsigned flags,
 #endif
 }
 
-SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
-		u32, min_complete, u32, flags, const void __user *, argp,
-		size_t, argsz)
+/*
+ * Given an 'fd' value, return the ctx associated with if. If 'registered' is
+ * true, then the registered index is used. Otherwise, the normal fd table.
+ * Caller must call fput() on the returned file if it isn't a registered file,
+ * unless it's an ERR_PTR.
+ */
+struct file *io_uring_ctx_get_file(unsigned int fd, bool registered)
 {
-	struct io_ring_ctx *ctx;
 	struct file *file;
-	long ret;
-
-	if (unlikely(flags & ~IORING_ENTER_FLAGS))
-		return -EINVAL;
 
-	/*
-	 * Ring fd has been registered via IORING_REGISTER_RING_FDS, we
-	 * need only dereference our task private array to find it.
-	 */
-	if (flags & IORING_ENTER_REGISTERED_RING) {
+	if (registered) {
+		/*
+		 * Ring fd has been registered via IORING_REGISTER_RING_FDS, we
+		 * need only dereference our task private array to find it.
+		 */
 		struct io_uring_task *tctx = current->io_uring;
 
 		if (unlikely(!tctx || fd >= IO_RINGFD_REG_MAX))
-			return -EINVAL;
+			return ERR_PTR(-EINVAL);
 		fd = array_index_nospec(fd, IO_RINGFD_REG_MAX);
 		file = tctx->registered_rings[fd];
-		if (unlikely(!file))
-			return -EBADF;
 	} else {
 		file = fget(fd);
-		if (unlikely(!file))
-			return -EBADF;
-		ret = -EOPNOTSUPP;
-		if (unlikely(!io_is_uring_fops(file)))
-			goto out;
 	}
 
+	if (unlikely(!file))
+		return ERR_PTR(-EBADF);
+	if (io_is_uring_fops(file))
+		return file;
+	fput(file);
+	return ERR_PTR(-EOPNOTSUPP);
+}
+
+
+SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
+		u32, min_complete, u32, flags, const void __user *, argp,
+		size_t, argsz)
+{
+	struct io_ring_ctx *ctx;
+	struct file *file;
+	long ret;
+
+	if (unlikely(flags & ~IORING_ENTER_FLAGS))
+		return -EINVAL;
+
+	file = io_uring_ctx_get_file(fd, flags & IORING_ENTER_REGISTERED_RING);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
 	ctx = file->private_data;
 	ret = -EBADFD;
 	/*
diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index 91cf67b5d85b..e43995682c8b 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -173,6 +173,7 @@ void io_req_track_inflight(struct io_kiocb *req);
 struct file *io_file_get_normal(struct io_kiocb *req, int fd);
 struct file *io_file_get_fixed(struct io_kiocb *req, int fd,
 			       unsigned issue_flags);
+struct file *io_uring_ctx_get_file(unsigned int fd, bool registered);
 
 void io_req_task_queue(struct io_kiocb *req);
 void io_req_task_complete(struct io_tw_req tw_req, io_tw_token_t tw);
diff --git a/io_uring/register.c b/io_uring/register.c
index 95cfa88dc621..6260196929a7 100644
--- a/io_uring/register.c
+++ b/io_uring/register.c
@@ -938,39 +938,6 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode,
 	return ret;
 }
 
-/*
- * Given an 'fd' value, return the ctx associated with if. If 'registered' is
- * true, then the registered index is used. Otherwise, the normal fd table.
- * Caller must call fput() on the returned file if it isn't a registered file,
- * unless it's an ERR_PTR.
- */
-struct file *io_uring_register_get_file(unsigned int fd, bool registered)
-{
-	struct file *file;
-
-	if (registered) {
-		/*
-		 * Ring fd has been registered via IORING_REGISTER_RING_FDS, we
-		 * need only dereference our task private array to find it.
-		 */
-		struct io_uring_task *tctx = current->io_uring;
-
-		if (unlikely(!tctx || fd >= IO_RINGFD_REG_MAX))
-			return ERR_PTR(-EINVAL);
-		fd = array_index_nospec(fd, IO_RINGFD_REG_MAX);
-		file = tctx->registered_rings[fd];
-	} else {
-		file = fget(fd);
-	}
-
-	if (unlikely(!file))
-		return ERR_PTR(-EBADF);
-	if (io_is_uring_fops(file))
-		return file;
-	fput(file);
-	return ERR_PTR(-EOPNOTSUPP);
-}
-
 static int io_uring_register_send_msg_ring(void __user *arg, unsigned int nr_args)
 {
 	struct io_uring_sqe sqe;
@@ -1025,7 +992,7 @@ SYSCALL_DEFINE4(io_uring_register, unsigned int, fd, unsigned int, opcode,
 	if (fd == -1)
 		return io_uring_register_blind(opcode, arg, nr_args);
 
-	file = io_uring_register_get_file(fd, use_registered_ring);
+	file = io_uring_ctx_get_file(fd, use_registered_ring);
 	if (IS_ERR(file))
 		return PTR_ERR(file);
 	ctx = file->private_data;
diff --git a/io_uring/register.h b/io_uring/register.h
index a5f39d5ef9e0..c9da997d503c 100644
--- a/io_uring/register.h
+++ b/io_uring/register.h
@@ -4,6 +4,5 @@
 
 int io_eventfd_unregister(struct io_ring_ctx *ctx);
 int io_unregister_personality(struct io_ring_ctx *ctx, unsigned id);
-struct file *io_uring_register_get_file(unsigned int fd, bool registered);
 
 #endif
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index cb12194b35e8..57151c01da0f 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -1269,7 +1269,7 @@ int io_register_clone_buffers(struct io_ring_ctx *ctx, void __user *arg)
 		return -EINVAL;
 
 	registered_src = (buf.flags & IORING_REGISTER_SRC_REGISTERED) != 0;
-	file = io_uring_register_get_file(buf.src_fd, registered_src);
+	file = io_uring_ctx_get_file(buf.src_fd, registered_src);
 	if (IS_ERR(file))
 		return PTR_ERR(file);
 
-- 
2.53.0