Skip to contentSkip to conversation

open/accept directly into io_uring fixed file table

RFC Superseded Pavel Begunkov 5 participants 11 messages Jul 7, 2021 – Aug 13, 2021

Subsystems (5)
Networking Network drivers VFS io_uring +1 more subsystems

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Pavel Begunkov

author
#0 lore raw

cover open/accept directly into io_uring fixed file table

Implement an old idea allowing open/accept io_uring requests to register a newly created file as a io_uring's fixed file instead of placing it into a task's file table. The switching is encoded in io_uring's SQEs by setting sqe->buf_index/file_index, so restricted to 2^16-1. Don't think we need more, but may be a good idea to scrap u32 somewhere instead.

From the net side only needs a function doing __sys_accept4_file() but not installing fd, see 2/4.

Only RFC for now, the new functionality is tested only for open yet. I hope we can remember the author of the idea to add attribution.

Pavel Begunkov (4):
io_uring: allow open directly into fixed fd table
net: add an accept helper not installing fd
io_uring: hand code io_accept()' fd installing
io_uring: accept directly into fixed file table

 fs/io_uring.c                 | 113 +++++++++++++++++++++++++++++-----
 include/linux/socket.h        |   3 +
 include/uapi/linux/io_uring.h |   2 +
 net/socket.c                  |  71 +++++++++++----------
 4 files changed, 138 insertions(+), 51 deletions(-)
Signature · 1 line
2.32.0
author
#1 lore raw

PATCH 1/4 io_uring: allow open directly into fixed fd table

Instead of opening a file into a process's file table as usual and then registering the fd within io_uring, we can skip the first step and place it directly into io_uring's fixed file table. Add support for that for both OPENAT opcodes.

The behaviour is controlled by setting sqe->file_index, where 0 implies the old behaviour, and non-0 values is translated into index (file_index-1). The field is u16, hence sets a limit on how many slots it covers.

Signed-off-by Pavel Begunkov <[email protected]>
---
 fs/io_uring.c                 | 75 ++++++++++++++++++++++++++++++-----
 include/uapi/linux/io_uring.h |  2 +
 2 files changed, 68 insertions(+), 9 deletions(-)
fs/io_uring.c +66 -9
index 599b11152505..17d2c0eb89dc 100644
@@ -1075,6 +1075,8 @@ static bool io_poll_remove_waitqs(struct io_kiocb *req);
 static int io_req_prep_async(struct io_kiocb *req);
 
 static void io_fallback_req_func(struct work_struct *unused);
+static int io_install_fixed_file(struct io_kiocb *req, struct file *file,
+				 unsigned int issue_flags);
 
 static struct kmem_cache *req_cachep;
 

@@ -3745,11 +3747,10 @@ static int __io_openat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe
 	const char __user *fname;
 	int ret;
 
-	if (unlikely(sqe->ioprio || sqe->buf_index))
+	if (unlikely(sqe->ioprio))
 		return -EINVAL;
 	if (unlikely(req->flags & REQ_F_FIXED_FILE))
 		return -EBADF;
-
 	/* open.how should be already initialised */
 	if (!(req->open.how.flags & O_PATH) && force_o_largefile())
 		req->open.how.flags |= O_LARGEFILE;

@@ -3762,6 +3763,10 @@ static int __io_openat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe
 		req->open.filename = NULL;
 		return ret;
 	}
+	req->buf_index = READ_ONCE(sqe->file_index);
+	if (req->buf_index && (req->open.how.flags & O_CLOEXEC))
+		return -EINVAL;
+
 	req->open.nofile = rlimit(RLIMIT_NOFILE);
 	req->flags |= REQ_F_NEED_CLEANUP;
 	return 0;

@@ -3804,8 +3809,8 @@ static int io_openat2(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct open_flags op;
 	struct file *file;
-	bool nonblock_set;
-	bool resolve_nonblock;
+	bool resolve_nonblock, nonblock_set;
+	bool fixed = !!req->buf_index;
 	int ret;
 
 	ret = build_open_flags(&req->open.how, &op);

@@ -3824,9 +3829,11 @@ static int io_openat2(struct io_kiocb *req, unsigned int issue_flags)
 		op.open_flag |= O_NONBLOCK;
 	}
 
-	ret = __get_unused_fd_flags(req->open.how.flags, req->open.nofile);
-	if (ret < 0)
-		goto err;
+	if (!fixed) {
+		ret = __get_unused_fd_flags(req->open.how.flags, req->open.nofile);
+		if (ret < 0)
+			goto err;
+	}
 
 	file = do_filp_open(req->open.dfd, req->open.filename, &op);
 	if (IS_ERR(file)) {

@@ -3835,7 +3842,8 @@ static int io_openat2(struct io_kiocb *req, unsigned int issue_flags)
 		 * marginal gain for something that is now known to be a slower
 		 * path. So just put it, and we'll get a new one when we retry.
 		 */
-		put_unused_fd(ret);
+		if (!fixed)
+			put_unused_fd(ret);
 
 		ret = PTR_ERR(file);
 		/* only retry if RESOLVE_CACHED wasn't already set by application */

@@ -3848,7 +3856,11 @@ static int io_openat2(struct io_kiocb *req, unsigned int issue_flags)
 	if ((issue_flags & IO_URING_F_NONBLOCK) && !nonblock_set)
 		file->f_flags &= ~O_NONBLOCK;
 	fsnotify_open(file);
-	fd_install(ret, file);
+
+	if (!fixed)
+		fd_install(ret, file);
+	else
+		ret = io_install_fixed_file(req, file, issue_flags);
 err:
 	putname(req->open.filename);
 	req->flags &= ~REQ_F_NEED_CLEANUP;

@@ -7776,6 +7788,50 @@ static int io_sqe_file_register(struct io_ring_ctx *ctx, struct file *file,
 #endif
 }
 
+static int io_install_fixed_file(struct io_kiocb *req, struct file *file,
+				 unsigned int issue_flags)
+{
+	struct io_ring_ctx *ctx = req->ctx;
+	int i = req->buf_index - 1;
+	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
+	struct io_fixed_file *file_slot;
+	int ret = -EBADF;
+
+	if (WARN_ON_ONCE(req->buf_index == 0))
+		goto err;
+
+	io_ring_submit_lock(ctx, !force_nonblock);
+	if (file->f_op == &io_uring_fops)
+		goto err;
+	ret = -ENXIO;
+	if (!ctx->file_data)
+		goto err;
+	ret = -EINVAL;
+	if (i > ctx->nr_user_files)
+		goto err;
+
+	i = array_index_nospec(i, ctx->nr_user_files);
+	file_slot = io_fixed_file_slot(&ctx->file_table, i);
+	ret = -EEXIST;
+	if (file_slot->file_ptr)
+		goto err;
+
+	*io_get_tag_slot(ctx->file_data, i) = 0;
+	io_fixed_file_set(file_slot, file);
+	ret = io_sqe_file_register(ctx, file, i);
+	if (ret) {
+		file_slot->file_ptr = 0;
+		goto err;
+	}
+
+	ret = 0;
+err:
+	io_ring_submit_unlock(ctx, !force_nonblock);
+	if (ret)
+		fput(file);
+	return ret;
+}
+
 static int io_queue_rsrc_removal(struct io_rsrc_data *data, unsigned idx,
 				 struct io_rsrc_node *node, void *rsrc)
 {

@@ -10231,6 +10287,7 @@ static int __init io_uring_init(void)
 	BUILD_BUG_SQE_ELEM(32, __u64,  user_data);
 	BUILD_BUG_SQE_ELEM(40, __u16,  buf_index);
 	BUILD_BUG_SQE_ELEM(40, __u16,  buf_group);
+	BUILD_BUG_SQE_ELEM(40, __u16,  file_index);
 	BUILD_BUG_SQE_ELEM(42, __u16,  personality);
 	BUILD_BUG_SQE_ELEM(44, __s32,  splice_fd_in);
include/uapi/linux/io_uring.h +2 -0
index 79126d5cd289..f105deb4da1d 100644
@@ -52,6 +52,8 @@ struct io_uring_sqe {
 		__u16	buf_index;
 		/* for grouped buffer selection */
 		__u16	buf_group;
+		/* index into fixed files */
+		__u16	file_index;
 	} __attribute__((packed));
 	/* personality to use, if used */
 	__u16	personality;
Signature · 1 line
2.32.0
author
#2 lore raw

PATCH 2/4 net: add an accept helper not installing fd

In reply to Pavel Begunkov

Introduce and reuse a helper that acts similarly to __sys_accept4_file() but returns struct file instead of installing file descriptor. Will be used by io_uring.

Signed-off-by Pavel Begunkov <[email protected]>
---
 include/linux/socket.h |  3 ++
 net/socket.c           | 71 ++++++++++++++++++++++--------------------
 2 files changed, 40 insertions(+), 34 deletions(-)
include/linux/socket.h +3 -0
index 0d8e3dcb7f88..d3c1a42a2edd 100644
@@ -421,6 +421,9 @@ extern int __sys_accept4_file(struct file *file, unsigned file_flags,
 			struct sockaddr __user *upeer_sockaddr,
 			 int __user *upeer_addrlen, int flags,
 			 unsigned long nofile);
+extern struct file *do_accept(struct file *file, unsigned file_flags,
+			      struct sockaddr __user *upeer_sockaddr,
+			      int __user *upeer_addrlen, int flags);
 extern int __sys_accept4(int fd, struct sockaddr __user *upeer_sockaddr,
 			 int __user *upeer_addrlen, int flags);
 extern int __sys_socket(int family, int type, int protocol);
net/socket.c +37 -34

103 lines not on this page · raw message

Signature · 1 line
2.32.0
author
#3 lore raw

PATCH 3/4 io_uring: hand code io_accept()' fd installing

In reply to Pavel Begunkov

Make io_accept() to handle file descriptor allocations and installation. A preparation patch for bypassing file tables.

Signed-off-by Pavel Begunkov <[email protected]>
---
 fs/io_uring.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)
fs/io_uring.c +20 -7
index 17d2c0eb89dc..fd0dcee251b0 100644
@@ -4682,6 +4682,11 @@ static int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	accept->addr_len = u64_to_user_ptr(READ_ONCE(sqe->addr2));
 	accept->flags = READ_ONCE(sqe->accept_flags);
 	accept->nofile = rlimit(RLIMIT_NOFILE);
+
+	if (accept->flags & ~(SOCK_CLOEXEC | SOCK_NONBLOCK))
+		return -EINVAL;
+	if (SOCK_NONBLOCK != O_NONBLOCK && (accept->flags & SOCK_NONBLOCK))
+		accept->flags = (accept->flags & ~SOCK_NONBLOCK) | O_NONBLOCK;
 	return 0;
 }
 

@@ -4690,20 +4695,28 @@ static int io_accept(struct io_kiocb *req, unsigned int issue_flags)
 	struct io_accept *accept = &req->accept;
 	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
 	unsigned int file_flags = force_nonblock ? O_NONBLOCK : 0;
-	int ret;
+	struct file *file;
+	int ret, fd;
 
 	if (req->file->f_flags & O_NONBLOCK)
 		req->flags |= REQ_F_NOWAIT;
 
-	ret = __sys_accept4_file(req->file, file_flags, accept->addr,
-					accept->addr_len, accept->flags,
-					accept->nofile);
-	if (ret == -EAGAIN && force_nonblock)
-		return -EAGAIN;
-	if (ret < 0) {
+	fd = __get_unused_fd_flags(accept->flags, accept->nofile);
+	if (unlikely(fd < 0))
+		return fd;
+
+	file = do_accept(req->file, file_flags, accept->addr, accept->addr_len,
+			 accept->flags);
+	if (IS_ERR(file)) {
+		ret = PTR_ERR(file);
+		if (ret == -EAGAIN && force_nonblock)
+			return -EAGAIN;
 		if (ret == -ERESTARTSYS)
 			ret = -EINTR;
 		req_set_fail(req);
+	} else {
+		fd_install(fd, file);
+		ret = fd;
 	}
 	__io_req_complete(req, issue_flags, ret, 0);
 	return 0;
Signature · 1 line
2.32.0
author
#4 lore raw

PATCH 4/4 io_uring: accept directly into fixed file table

In reply to Pavel Begunkov

As done with open opcodes, allow accept to skip installing fd into processes' file tables and put it directly into io_uring's fixed file table. Same restrictions and design as for open.

Signed-off-by Pavel Begunkov <[email protected]>
---
 fs/io_uring.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)
fs/io_uring.c +15 -6
index fd0dcee251b0..f7db43bf7dad 100644
@@ -4675,14 +4675,17 @@ static int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 
 	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
 		return -EINVAL;
-	if (sqe->ioprio || sqe->len || sqe->buf_index)
+	if (sqe->ioprio || sqe->len)
 		return -EINVAL;
 
 	accept->addr = u64_to_user_ptr(READ_ONCE(sqe->addr));
 	accept->addr_len = u64_to_user_ptr(READ_ONCE(sqe->addr2));
 	accept->flags = READ_ONCE(sqe->accept_flags);
 	accept->nofile = rlimit(RLIMIT_NOFILE);
+	req->buf_index = READ_ONCE(sqe->file_index);
 
+	if (req->buf_index && (accept->flags & SOCK_CLOEXEC))
+		return -EINVAL;
 	if (accept->flags & ~(SOCK_CLOEXEC | SOCK_NONBLOCK))
 		return -EINVAL;
 	if (SOCK_NONBLOCK != O_NONBLOCK && (accept->flags & SOCK_NONBLOCK))

@@ -4695,28 +4698,34 @@ static int io_accept(struct io_kiocb *req, unsigned int issue_flags)
 	struct io_accept *accept = &req->accept;
 	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
 	unsigned int file_flags = force_nonblock ? O_NONBLOCK : 0;
+	bool fixed = !!req->buf_index;
 	struct file *file;
 	int ret, fd;
 
 	if (req->file->f_flags & O_NONBLOCK)
 		req->flags |= REQ_F_NOWAIT;
 
-	fd = __get_unused_fd_flags(accept->flags, accept->nofile);
-	if (unlikely(fd < 0))
-		return fd;
-
+	if (!fixed) {
+		fd = __get_unused_fd_flags(accept->flags, accept->nofile);
+		if (unlikely(fd < 0))
+			return fd;
+	}
 	file = do_accept(req->file, file_flags, accept->addr, accept->addr_len,
 			 accept->flags);
 	if (IS_ERR(file)) {
+		if (!fixed)
+			put_unused_fd(fd);
 		ret = PTR_ERR(file);
 		if (ret == -EAGAIN && force_nonblock)
 			return -EAGAIN;
 		if (ret == -ERESTARTSYS)
 			ret = -EINTR;
 		req_set_fail(req);
-	} else {
+	} else if (!fixed) {
 		fd_install(fd, file);
 		ret = fd;
+	} else {
+		ret = io_install_fixed_file(req, file, issue_flags);
 	}
 	__io_req_complete(req, issue_flags, ret, 0);
 	return 0;
Signature · 1 line
2.32.0
maintainer
#5 lore raw
In reply to Pavel Begunkov
Quoted 7 lines · Pavel Begunkov

Implement an old idea allowing open/accept io_uring requests to register a newly created file as a io_uring's fixed file instead of placing it into a task's file table. The switching is encoded in io_uring's SQEs by setting sqe->buf_index/file_index, so restricted to 2^16-1. Don't think we need more, but may be a good idea to scrap u32 somewhere instead.

Quoted 5 lines

From the net side only needs a function doing __sys_accept4_file() but not installing fd, see 2/4.

Only RFC for now, the new functionality is tested only for open yet. I hope we can remember the author of the idea to add attribution.

Pretty sure the original suggester of this as Josh, CC'ed.

Signature · 1 line
Jens Axboe
#6 lore raw

Am 07.07.21 um 15:07 schrieb Jens Axboe:

Quoted 8 lines · Pavel Begunkov
Quoted 7 lines

Implement an old idea allowing open/accept io_uring requests to register a newly created file as a io_uring's fixed file instead of placing it into a task's file table. The switching is encoded in io_uring's SQEs by setting sqe->buf_index/file_index, so restricted to 2^16-1. Don't think we need more, but may be a good idea to scrap u32 somewhere instead.

Quoted 7 lines
Quoted 5 lines

From the net side only needs a function doing __sys_accept4_file() but not installing fd, see 2/4.

Only RFC for now, the new functionality is tested only for open yet. I hope we can remember the author of the idea to add attribution.

Pretty sure the original suggester of this as Josh, CC'ed.

I also requested it for open :-)

metze

maintainer
#7 lore raw
Quoted 12 lines · Stefan Metzmacher

Am 07.07.21 um 15:07 schrieb Jens Axboe:

Quoted 11 lines · Pavel Begunkov
Quoted 10 lines

Implement an old idea allowing open/accept io_uring requests to register a newly created file as a io_uring's fixed file instead of placing it into a task's file table. The switching is encoded in io_uring's SQEs by setting sqe->buf_index/file_index, so restricted to 2^16-1. Don't think we need more, but may be a good idea to scrap u32 somewhere instead.

From the net side only needs a function doing __sys_accept4_file() but not installing fd, see 2/4.

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

Only RFC for now, the new functionality is tested only for open yet. I hope we can remember the author of the idea to add attribution.

Pretty sure the original suggester of this as Josh, CC'ed.

I also requested it for open :-)

Indeed! I honestly forget the details, as some of it is implementation detail. I think Josh was the first to suggest a private fd table could be used, but that's mostly implementation detail as the point was to be able to know which fd would be assigned.

But I think we're all in agreement here, it's a nifty feature :-)

Signature · 1 line
Jens Axboe
#8 lore raw
In reply to Jens Axboe
Quoted 8 lines · Jens Axboe
Quoted 7 lines · Pavel Begunkov

Implement an old idea allowing open/accept io_uring requests to register a newly created file as a io_uring's fixed file instead of placing it into a task's file table. The switching is encoded in io_uring's SQEs by setting sqe->buf_index/file_index, so restricted to 2^16-1. Don't think we need more, but may be a good idea to scrap u32 somewhere instead.

Quoted 7 lines
Quoted 5 lines

From the net side only needs a function doing __sys_accept4_file() but not installing fd, see 2/4.

Only RFC for now, the new functionality is tested only for open yet. I hope we can remember the author of the idea to add attribution.

Pretty sure the original suggester of this as Josh, CC'ed.

Thanks for working on this, Pavel!

Original thread at
https://lore.kernel.org/io-uring/20200715004209.GA334456@localhost/T/ in case that helps.

author
#9 lore raw
Quoted 12 lines · Josh Triplett
Quoted 11 lines · Jens Axboe
Quoted 10 lines · Pavel Begunkov

Implement an old idea allowing open/accept io_uring requests to register a newly created file as a io_uring's fixed file instead of placing it into a task's file table. The switching is encoded in io_uring's SQEs by setting sqe->buf_index/file_index, so restricted to 2^16-1. Don't think we need more, but may be a good idea to scrap u32 somewhere instead.

From the net side only needs a function doing __sys_accept4_file() but not installing fd, see 2/4.

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

Only RFC for now, the new functionality is tested only for open yet. I hope we can remember the author of the idea to add attribution.

Pretty sure the original suggester of this as Josh, CC'ed.

Thanks for working on this, Pavel!

Original thread at

Totally was thinking it was only a discussion but not an actual patch, and I even did comment on it! Sorry Josh, would have persuaded you to finish it, if I remembered that.

Quoted 3 lines

https://lore.kernel.org/io-uring/20200715004209.GA334456@localhost/T/ in case that helps.

Signature · 1 line
Pavel Begunkov
#10 lore raw
In reply to Pavel Begunkov
Quoted 7 lines · Pavel Begunkov

Implement an old idea allowing open/accept io_uring requests to register a newly created file as a io_uring's fixed file instead of placing it into a task's file table. The switching is encoded in io_uring's SQEs by setting sqe->buf_index/file_index, so restricted to 2^16-1. Don't think we need more, but may be a good idea to scrap u32 somewhere instead.

Quoted 6 lines

From the net side only needs a function doing __sys_accept4_file()
but not installing fd, see 2/4.

Only RFC for now, the new functionality is tested only for open yet. I hope we can remember the author of the idea to add attribution.

Great feature! I believe this one leverages linked sqes, we may need to remind users to be careful when they use this feature in shared sqthread mode since linked sqes may be splited.

Quoted 12 lines

Pavel Begunkov (4):
io_uring: allow open directly into fixed fd table
net: add an accept helper not installing fd
io_uring: hand code io_accept()' fd installing
io_uring: accept directly into fixed file table

  fs/io_uring.c                 | 113 +++++++++++++++++++++++++++++-----
  include/linux/socket.h        |   3 +
  include/uapi/linux/io_uring.h |   2 +
  net/socket.c                  |  71 +++++++++++----------
  4 files changed, 138 insertions(+), 51 deletions(-)