io_uring 6.0 backports

PATCH In review Pavel Begunkov 2 participants 8 messages Oct 16, 2022 – Oct 17, 2022 Stable branch

lore

Share & tools

lore

Pavel Begunkov

author

2022-10-16 20:35 UTC #0 lore raw

cover io_uring 6.0 backports

io_uring patches that has failed to be applied today with their dependencies.

Pavel Begunkov (6):
io_uring/net: refactor io_sr_msg types io_uring/net: use io_sr_msg for sendzc io_uring/net: don't lose partial send_zc on fail io_uring/net: rename io_sendzc() io_uring/net: don't skip notifs for failed requests io_uring/net: fix notif cqe reordering

 io_uring/net.c   | 60 ++++++++++++++++++++++++++----------------------
 io_uring/net.h   |  7 +++---
 io_uring/opdef.c |  7 +++---
 3 files changed, 40 insertions(+), 34 deletions(-)

Signature · 1 line

2.38.0

Pavel Begunkov

author

2022-10-16 20:35 UTC #1 lore raw

PATCH 1/6 io_uring/net: refactor io_sr_msg types

[ upstream commit 0b048557db761d287777360a100e1d010760d209 ]

In preparation for using struct io_sr_msg for zerocopy sends, clean up types. First, flags can be u16 as it's provided by the userspace in u16 ioprio, as well as addr_len. This saves us 4 bytes. Also use unsigned for size and done_io, both are as well limited to u32.

Signed-off-by Pavel Begunkov <[email protected]>

Link https://lore.kernel.org/r/42c2639d6385b8b2181342d2af3a42d3b1c5bcd2.1662639236.git.asml.silence@gmail.com open here

Signed-off-by Jens Axboe <[email protected]>

---
 io_uring/net.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

io_uring/net.c +7 -7

index f1036f429156..bd9f686ba0a1 100644
@@ -55,21 +55,21 @@ struct io_sr_msg {
 		struct user_msghdr __user	*umsg;
 		void __user			*buf;
 	};
+	unsigned			len;
+	unsigned			done_io;
 	unsigned			msg_flags;
-	unsigned			flags;
-	size_t				len;
-	size_t				done_io;
+	u16				flags;
 };
 
 struct io_sendzc {
 	struct file			*file;
 	void __user			*buf;
-	size_t				len;
+	unsigned			len;
+	unsigned			done_io;
 	unsigned			msg_flags;
-	unsigned			flags;
-	unsigned			addr_len;
+	u16				flags;
+	u16				addr_len;
 	void __user			*addr;
-	size_t				done_io;
 	struct io_kiocb 		*notif;
 };

Signature · 1 line

2.38.0

Pavel Begunkov

author

2022-10-16 20:35 UTC #3 lore raw

PATCH 3/6 io_uring/net: don't lose partial send_zc on fail

In reply to Pavel Begunkov

[ upstream commit 5693bcce892d7b8b15a7a92b011d3d40a023b53c ]

Partial zc send may end up in io_req_complete_failed(), which not only would return invalid result but also mask out the notification leading to lifetime issues.

Cc [email protected]

Signed-off-by Pavel Begunkov <[email protected]>

Link https://lore.kernel.org/r/5673285b5e83e6ceca323727b4ddaa584b5cc91e.1663668091.git.asml.silence@gmail.com open here

Signed-off-by Jens Axboe <[email protected]>

---
 io_uring/net.c   | 16 ++++++++++++++++
 io_uring/net.h   |  1 +
 io_uring/opdef.c |  1 +
 3 files changed, 18 insertions(+)

io_uring/net.c +16 -0

index 3e9ab7a1abec..83cb8f1f6672 100644
@@ -1089,6 +1089,22 @@ void io_sendrecv_fail(struct io_kiocb *req)
 	io_req_set_res(req, res, req->cqe.flags);
 }
 
+void io_send_zc_fail(struct io_kiocb *req)
+{
+	struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
+	int res = req->cqe.res;
+
+	if (req->flags & REQ_F_PARTIAL_IO) {
+		if (req->flags & REQ_F_NEED_CLEANUP) {
+			io_notif_flush(sr->notif);
+			sr->notif = NULL;
+			req->flags &= ~REQ_F_NEED_CLEANUP;
+		}
+		res = sr->done_io;
+	}
+	io_req_set_res(req, res, req->cqe.flags);
+}
+
 int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	struct io_accept *accept = io_kiocb_to_cmd(req, struct io_accept);

io_uring/net.h +1 -0

index 109ffb3a1a3f..e7366aac335c 100644
@@ -58,6 +58,7 @@ int io_connect(struct io_kiocb *req, unsigned int issue_flags);
 int io_sendzc(struct io_kiocb *req, unsigned int issue_flags);
 int io_sendzc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
 void io_sendzc_cleanup(struct io_kiocb *req);
+void io_send_zc_fail(struct io_kiocb *req);
 
 void io_netmsg_cache_free(struct io_cache_entry *entry);
 #else

io_uring/opdef.c +1 -0

index 5768620b075d..7f85e0fbd60b 100644
@@ -488,6 +488,7 @@ const struct io_op_def io_op_defs[] = {
 		.issue			= io_sendzc,
 		.prep_async		= io_sendzc_prep_async,
 		.cleanup		= io_sendzc_cleanup,
+		.fail			= io_send_zc_fail,
 #else
 		.prep			= io_eopnotsupp_prep,
 #endif

Signature · 1 line

2.38.0

Pavel Begunkov

author

2022-10-16 20:35 UTC #2 lore raw

PATCH 2/6 io_uring/net: use io_sr_msg for sendzc

In reply to Pavel Begunkov

[ upstream commit ac9e5784bbe72f4f603d1af84760ec09bc0b5ccd ]

Reuse struct io_sr_msg for zerocopy sends, which is handy. There is only one zerocopy specific field, namely .notif, and we have enough space for it.

Signed-off-by Pavel Begunkov <[email protected]>

Link https://lore.kernel.org/r/408c5b1b2d8869e1a12da5f5a78ed72cac112149.1662639236.git.asml.silence@gmail.com open here

Signed-off-by Jens Axboe <[email protected]>

---
 io_uring/net.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

io_uring/net.c +5 ~~-13~~

index bd9f686ba0a1..3e9ab7a1abec 100644
@@ -59,15 +59,7 @@ struct io_sr_msg {
 	unsigned			done_io;
 	unsigned			msg_flags;
 	u16				flags;
-};
-
-struct io_sendzc {
-	struct file			*file;
-	void __user			*buf;
-	unsigned			len;
-	unsigned			done_io;
-	unsigned			msg_flags;
-	u16				flags;
+	/* used only for sendzc */
 	u16				addr_len;
 	void __user			*addr;
 	struct io_kiocb 		*notif;

@@ -184,7 +176,7 @@ static int io_sendmsg_copy_hdr(struct io_kiocb *req,
 
 int io_sendzc_prep_async(struct io_kiocb *req)
 {
-	struct io_sendzc *zc = io_kiocb_to_cmd(req, struct io_sendzc);
+	struct io_sr_msg *zc = io_kiocb_to_cmd(req, struct io_sr_msg);
 	struct io_async_msghdr *io;
 	int ret;

@@ -881,7 +873,7 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
 
 void io_sendzc_cleanup(struct io_kiocb *req)
 {
-	struct io_sendzc *zc = io_kiocb_to_cmd(req, struct io_sendzc);
+	struct io_sr_msg *zc = io_kiocb_to_cmd(req, struct io_sr_msg);
 
 	zc->notif->flags |= REQ_F_CQE_SKIP;
 	io_notif_flush(zc->notif);

@@ -890,7 +882,7 @@ void io_sendzc_cleanup(struct io_kiocb *req)
 
 int io_sendzc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
-	struct io_sendzc *zc = io_kiocb_to_cmd(req, struct io_sendzc);
+	struct io_sr_msg *zc = io_kiocb_to_cmd(req, struct io_sr_msg);
 	struct io_ring_ctx *ctx = req->ctx;
 	struct io_kiocb *notif;

@@ -996,7 +988,7 @@ static int io_sg_from_iter(struct sock *sk, struct sk_buff *skb,
 int io_sendzc(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct sockaddr_storage __address, *addr = NULL;
-	struct io_sendzc *zc = io_kiocb_to_cmd(req, struct io_sendzc);
+	struct io_sr_msg *zc = io_kiocb_to_cmd(req, struct io_sr_msg);
 	struct msghdr msg;
 	struct iovec iov;
 	struct socket *sock;

Signature · 1 line

2.38.0

Pavel Begunkov

author

2022-10-16 20:35 UTC #4 lore raw

PATCH 4/6 io_uring/net: rename io_sendzc()

In reply to Pavel Begunkov

[ upstream commit b0e9b5517eb12fa80c72e205fe28534c2e2f39b9 ]

Simple renaming of io_sendzc*() functions in preparatio to adding a zerocopy sendmsg variant.

Signed-off-by Pavel Begunkov <[email protected]>

Link https://lore.kernel.org/r/265af46829e6076dd220011b1858dc3151969226.1663668091.git.asml.silence@gmail.com open here

Signed-off-by Jens Axboe <[email protected]>

---
 io_uring/net.c   | 6 +++---
 io_uring/net.h   | 6 +++---
 io_uring/opdef.c | 6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

io_uring/net.c +3 -3

index 83cb8f1f6672..3dbb2bf99b4d 100644
@@ -871,7 +871,7 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
 	return ret;
 }
 
-void io_sendzc_cleanup(struct io_kiocb *req)
+void io_send_zc_cleanup(struct io_kiocb *req)
 {
 	struct io_sr_msg *zc = io_kiocb_to_cmd(req, struct io_sr_msg);

@@ -880,7 +880,7 @@ void io_sendzc_cleanup(struct io_kiocb *req)
 	zc->notif = NULL;
 }
 
-int io_sendzc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+int io_send_zc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	struct io_sr_msg *zc = io_kiocb_to_cmd(req, struct io_sr_msg);
 	struct io_ring_ctx *ctx = req->ctx;

@@ -985,7 +985,7 @@ static int io_sg_from_iter(struct sock *sk, struct sk_buff *skb,
 	return ret;
 }
 
-int io_sendzc(struct io_kiocb *req, unsigned int issue_flags)
+int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct sockaddr_storage __address, *addr = NULL;
 	struct io_sr_msg *zc = io_kiocb_to_cmd(req, struct io_sr_msg);

io_uring/net.h +3 -3

index e7366aac335c..4090d008fd55 100644
@@ -55,9 +55,9 @@ int io_connect_prep_async(struct io_kiocb *req);
 int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
 int io_connect(struct io_kiocb *req, unsigned int issue_flags);
 
-int io_sendzc(struct io_kiocb *req, unsigned int issue_flags);
-int io_sendzc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
-void io_sendzc_cleanup(struct io_kiocb *req);
+int io_send_zc(struct io_kiocb *req, unsigned int issue_flags);
+int io_send_zc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+void io_send_zc_cleanup(struct io_kiocb *req);
 void io_send_zc_fail(struct io_kiocb *req);
 
 void io_netmsg_cache_free(struct io_cache_entry *entry);

io_uring/opdef.c +3 -3

index 7f85e0fbd60b..4f0f69482016 100644
@@ -484,10 +484,10 @@ const struct io_op_def io_op_defs[] = {
 		.manual_alloc		= 1,
 #if defined(CONFIG_NET)
 		.async_size		= sizeof(struct io_async_msghdr),
-		.prep			= io_sendzc_prep,
-		.issue			= io_sendzc,
+		.prep			= io_send_zc_prep,
+		.issue			= io_send_zc,
 		.prep_async		= io_sendzc_prep_async,
-		.cleanup		= io_sendzc_cleanup,
+		.cleanup		= io_send_zc_cleanup,
 		.fail			= io_send_zc_fail,
 #else
 		.prep			= io_eopnotsupp_prep,

Signature · 1 line

2.38.0

Pavel Begunkov

author

2022-10-16 20:35 UTC #5 lore raw

PATCH 5/6 io_uring/net: don't skip notifs for failed requests

In reply to Pavel Begunkov

[ upstream commit 6ae91ac9a6aa7d6005c3c6d0f4d263fbab9f377f ]

We currently only add a notification CQE when the send succeded, i.e. cqe.res >= 0. However, it'd be more robust to do buffer notifications for failed requests as well in case drivers decide do something fanky.

Always return a buffer notification after initial prep, don't hide it. This behaviour is better aligned with documentation and the patch also helps the userspace to respect it.

Cc [email protected] # 6.0

Signed-off-by Pavel Begunkov <[email protected]>

Link https://lore.kernel.org/r/9c8bead87b2b980fcec441b8faef52188b4a6588.1664292100.git.asml.silence@gmail.com open here

Signed-off-by Jens Axboe <[email protected]>

---
 io_uring/net.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

io_uring/net.c +6 ~~-16~~

index 3dbb2bf99b4d..b0324775e6ce 100644
@@ -875,7 +875,6 @@ void io_send_zc_cleanup(struct io_kiocb *req)
 {
 	struct io_sr_msg *zc = io_kiocb_to_cmd(req, struct io_sr_msg);
 
-	zc->notif->flags |= REQ_F_CQE_SKIP;
 	io_notif_flush(zc->notif);
 	zc->notif = NULL;
 }

@@ -992,7 +991,7 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
 	struct msghdr msg;
 	struct iovec iov;
 	struct socket *sock;
-	unsigned msg_flags, cflags;
+	unsigned msg_flags;
 	int ret, min_ret = 0;
 
 	sock = sock_from_file(req->file);

@@ -1060,8 +1059,6 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
 			req->flags |= REQ_F_PARTIAL_IO;
 			return io_setup_async_addr(req, addr, issue_flags);
 		}
-		if (ret < 0 && !zc->done_io)
-			zc->notif->flags |= REQ_F_CQE_SKIP;
 		if (ret == -ERESTARTSYS)
 			ret = -EINTR;
 		req_set_fail(req);

@@ -1074,8 +1071,7 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
 
 	io_notif_flush(zc->notif);
 	req->flags &= ~REQ_F_NEED_CLEANUP;
-	cflags = ret >= 0 ? IORING_CQE_F_MORE : 0;
-	io_req_set_res(req, ret, cflags);
+	io_req_set_res(req, ret, IORING_CQE_F_MORE);
 	return IOU_OK;
 }

@@ -1092,17 +1088,11 @@ void io_sendrecv_fail(struct io_kiocb *req)
 void io_send_zc_fail(struct io_kiocb *req)
 {
 	struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
-	int res = req->cqe.res;
 
-	if (req->flags & REQ_F_PARTIAL_IO) {
-		if (req->flags & REQ_F_NEED_CLEANUP) {
-			io_notif_flush(sr->notif);
-			sr->notif = NULL;
-			req->flags &= ~REQ_F_NEED_CLEANUP;
-		}
-		res = sr->done_io;
-	}
-	io_req_set_res(req, res, req->cqe.flags);
+	if (req->flags & REQ_F_PARTIAL_IO)
+		req->cqe.res = sr->done_io;
+	if (req->flags & REQ_F_NEED_CLEANUP)
+		req->cqe.flags |= IORING_CQE_F_MORE;
 }
 
 int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)

Signature · 1 line

2.38.0

Pavel Begunkov

author

2022-10-16 20:35 UTC #6 lore raw

PATCH 6/6 io_uring/net: fix notif cqe reordering

In reply to Pavel Begunkov

[ upstream commit 108893ddcc4d3aa0a4a02aeb02d478e997001227 ]

send zc is not restricted to !IO_URING_F_UNLOCKED anymore and so we can't use task-tw ordering trick to order notification cqes with requests completions. In this case leave it alone and let io_send_zc_cleanup() flush it.

Cc [email protected]

Fixes 53bdc88aac9a2 ("io_uring/notif: order notif vs send CQEs")

Signed-off-by Pavel Begunkov <[email protected]>

Link https://lore.kernel.org/r/0031f3a00d492e814a4a0935a2029a46d9c9ba06.1664486545.git.asml.silence@gmail.com open here

Signed-off-by Jens Axboe <[email protected]>

---
 io_uring/net.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

io_uring/net.c +8 -2

index b0324775e6ce..d2912c1550d4 100644
@@ -1069,8 +1069,14 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
 	else if (zc->done_io)
 		ret = zc->done_io;
 
-	io_notif_flush(zc->notif);
-	req->flags &= ~REQ_F_NEED_CLEANUP;
+	/*
+	 * If we're in io-wq we can't rely on tw ordering guarantees, defer
+	 * flushing notif to io_send_zc_cleanup()
+	 */
+	if (!(issue_flags & IO_URING_F_UNLOCKED)) {
+		io_notif_flush(zc->notif);
+		req->flags &= ~REQ_F_NEED_CLEANUP;
+	}
 	io_req_set_res(req, ret, IORING_CQE_F_MORE);
 	return IOU_OK;
 }

Signature · 1 line

2.38.0

Greg KH

maintainer

2022-10-17 09:37 UTC #7 lore raw

In reply to Pavel Begunkov

Quoted 2 lines · Pavel Begunkov

io_uring patches that has failed to be applied today with their dependencies.

Now queued up, thanks.

greg k-h