[PATCH for-next 0/7] small zc improvements

[PATCH for-next 0/7] small zc improvements

[Pavel Begunkov]

Remove some cycles in a couple of places for zc sends. Touches a bunch of
bits here and there but the main theme is adding additional set of callbacks
for slower path and move in there some optional features.

Pavel Begunkov (7):
  io_uring: move kbuf put out of generic tw complete
  io_uring/net: remove extra notif rsrc setup
  io_uring/net: preset notif tw handler
  io_uring/net: rename io_uring_tx_zerocopy_callback
  io_uring/net: inline io_notif_flush()
  io_uring: move zc reporting from the hot path
  io_uring/net: move mm accounting to a slower path

 io_uring/io_uring.c |  6 -----
 io_uring/net.c      | 25 +++++++++++++-------
 io_uring/notif.c    | 57 ++++++++++++++++++++++++---------------------
 io_uring/notif.h    | 12 +++++++++-
 io_uring/rw.c       |  6 +++++
 5 files changed, 64 insertions(+), 42 deletions(-)

-- 
2.38.0

[PATCH for-next 1/7] io_uring: move kbuf put out of generic tw complete

[Pavel Begunkov]

There are multiple users of io_req_task_complete() including zc
notifications, but only read requests use selected buffers. As we
already have an rw specific tw function, move io_put_kbuf() in there.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.c | 6 ------
 io_uring/rw.c       | 6 ++++++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index ac8c488e3077..db0dec120f09 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1471,12 +1471,6 @@ static int io_iopoll_check(struct io_ring_ctx *ctx, long min)
 
 void io_req_task_complete(struct io_kiocb *req, bool *locked)
 {
-	if (req->flags & (REQ_F_BUFFER_SELECTED|REQ_F_BUFFER_RING)) {
-		unsigned issue_flags = *locked ? 0 : IO_URING_F_UNLOCKED;
-
-		req->cqe.flags |= io_put_kbuf(req, issue_flags);
-	}
-
 	if (*locked)
 		io_req_complete_defer(req);
 	else
diff --git a/io_uring/rw.c b/io_uring/rw.c
index bb47cc4da713..1ce065709724 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -286,6 +286,12 @@ static inline int io_fixup_rw_res(struct io_kiocb *req, long res)
 static void io_req_rw_complete(struct io_kiocb *req, bool *locked)
 {
 	io_req_io_end(req);
+
+	if (req->flags & (REQ_F_BUFFER_SELECTED|REQ_F_BUFFER_RING)) {
+		unsigned issue_flags = *locked ? 0 : IO_URING_F_UNLOCKED;
+
+		req->cqe.flags |= io_put_kbuf(req, issue_flags);
+	}
 	io_req_task_complete(req, locked);
 }
 
-- 
2.38.0

[PATCH for-next 2/7] io_uring/net: remove extra notif rsrc setup

[Pavel Begunkov]

io_send_zc_prep() sets up notification's rsrc_node when needed, don't
unconditionally install it on notif alloc.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/notif.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/io_uring/notif.c b/io_uring/notif.c
index 4bfef10161fa..59dafc42b8e0 100644
--- a/io_uring/notif.c
+++ b/io_uring/notif.c
@@ -60,7 +60,6 @@ struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx)
 	notif->task = current;
 	io_get_task_refs(1);
 	notif->rsrc_node = NULL;
-	io_req_set_rsrc_node(notif, ctx, 0);
 
 	nd = io_notif_to_data(notif);
 	nd->account_pages = 0;
-- 
2.38.0

[PATCH for-next 3/7] io_uring/net: preset notif tw handler

[Pavel Begunkov]

We're going to have multiple notification tw functions. In preparation
for future changes default the tw callback in advance so later we can
replace it with other versions.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/notif.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/io_uring/notif.c b/io_uring/notif.c
index 59dafc42b8e0..6afb58b94297 100644
--- a/io_uring/notif.c
+++ b/io_uring/notif.c
@@ -39,10 +39,8 @@ static void io_uring_tx_zerocopy_callback(struct sk_buff *skb,
 			WRITE_ONCE(nd->zc_copied, true);
 	}
 
-	if (refcount_dec_and_test(&uarg->refcnt)) {
-		notif->io_task_work.func = __io_notif_complete_tw;
+	if (refcount_dec_and_test(&uarg->refcnt))
 		io_req_task_work_add(notif);
-	}
 }
 
 struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx)
@@ -60,6 +58,7 @@ struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx)
 	notif->task = current;
 	io_get_task_refs(1);
 	notif->rsrc_node = NULL;
+	notif->io_task_work.func = __io_notif_complete_tw;
 
 	nd = io_notif_to_data(notif);
 	nd->account_pages = 0;
@@ -76,8 +75,6 @@ void io_notif_flush(struct io_kiocb *notif)
 	struct io_notif_data *nd = io_notif_to_data(notif);
 
 	/* drop slot's master ref */
-	if (refcount_dec_and_test(&nd->uarg.refcnt)) {
-		notif->io_task_work.func = __io_notif_complete_tw;
+	if (refcount_dec_and_test(&nd->uarg.refcnt))
 		io_req_task_work_add(notif);
-	}
 }
-- 
2.38.0

[PATCH for-next 4/7] io_uring/net: rename io_uring_tx_zerocopy_callback

[Pavel Begunkov]

Just a simple renaming patch, io_uring_tx_zerocopy_callback() is too
bulky and doesn't follow usual naming style.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/notif.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/io_uring/notif.c b/io_uring/notif.c
index 6afb58b94297..5b0e7bb1198f 100644
--- a/io_uring/notif.c
+++ b/io_uring/notif.c
@@ -25,9 +25,8 @@ static void __io_notif_complete_tw(struct io_kiocb *notif, bool *locked)
 	io_req_task_complete(notif, locked);
 }
 
-static void io_uring_tx_zerocopy_callback(struct sk_buff *skb,
-					  struct ubuf_info *uarg,
-					  bool success)
+static void io_tx_ubuf_callback(struct sk_buff *skb, struct ubuf_info *uarg,
+				bool success)
 {
 	struct io_notif_data *nd = container_of(uarg, struct io_notif_data, uarg);
 	struct io_kiocb *notif = cmd_to_io_kiocb(nd);
@@ -63,7 +62,7 @@ struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx)
 	nd = io_notif_to_data(notif);
 	nd->account_pages = 0;
 	nd->uarg.flags = SKBFL_ZEROCOPY_FRAG | SKBFL_DONT_ORPHAN;
-	nd->uarg.callback = io_uring_tx_zerocopy_callback;
+	nd->uarg.callback = io_tx_ubuf_callback;
 	nd->zc_report = nd->zc_used = nd->zc_copied = false;
 	refcount_set(&nd->uarg.refcnt, 1);
 	return notif;
-- 
2.38.0

[PATCH for-next 5/7] io_uring/net: inline io_notif_flush()

[Pavel Begunkov]

io_notif_flush() is pretty simple, we can inline it.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/notif.c | 10 ----------
 io_uring/notif.h | 11 ++++++++++-
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/io_uring/notif.c b/io_uring/notif.c
index 5b0e7bb1198f..c287adf24e66 100644
--- a/io_uring/notif.c
+++ b/io_uring/notif.c
@@ -67,13 +67,3 @@ struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx)
 	refcount_set(&nd->uarg.refcnt, 1);
 	return notif;
 }
-
-void io_notif_flush(struct io_kiocb *notif)
-	__must_hold(&slot->notif->ctx->uring_lock)
-{
-	struct io_notif_data *nd = io_notif_to_data(notif);
-
-	/* drop slot's master ref */
-	if (refcount_dec_and_test(&nd->uarg.refcnt))
-		io_req_task_work_add(notif);
-}
diff --git a/io_uring/notif.h b/io_uring/notif.h
index 4ae696273c78..7f00176020d3 100644
--- a/io_uring/notif.h
+++ b/io_uring/notif.h
@@ -18,7 +18,6 @@ struct io_notif_data {
 	bool			zc_copied;
 };
 
-void io_notif_flush(struct io_kiocb *notif);
 struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx);
 
 static inline struct io_notif_data *io_notif_to_data(struct io_kiocb *notif)
@@ -26,6 +25,16 @@ static inline struct io_notif_data *io_notif_to_data(struct io_kiocb *notif)
 	return io_kiocb_to_cmd(notif, struct io_notif_data);
 }
 
+static inline void io_notif_flush(struct io_kiocb *notif)
+	__must_hold(&notif->ctx->uring_lock)
+{
+	struct io_notif_data *nd = io_notif_to_data(notif);
+
+	/* drop slot's master ref */
+	if (refcount_dec_and_test(&nd->uarg.refcnt))
+		io_req_task_work_add(notif);
+}
+
 static inline int io_notif_account_mem(struct io_kiocb *notif, unsigned len)
 {
 	struct io_ring_ctx *ctx = notif->ctx;
-- 
2.38.0

[PATCH for-next 6/7] io_uring: move zc reporting from the hot path

[Pavel Begunkov]

Add custom tw and notif callbacks on top of usual bits also handling zc
reporting. That moves it from the hot path.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/net.c   | 22 ++++++++++++++--------
 io_uring/notif.c | 31 +++++++++++++++++++++++++++----
 io_uring/notif.h |  1 +
 3 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/io_uring/net.c b/io_uring/net.c
index 0a8cdc5ae7af..556a48bcdbe4 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -925,6 +925,9 @@ void io_send_zc_cleanup(struct io_kiocb *req)
 	}
 }
 
+#define IO_ZC_FLAGS_COMMON (IORING_RECVSEND_POLL_FIRST | IORING_RECVSEND_FIXED_BUF)
+#define IO_ZC_FLAGS_VALID  (IO_ZC_FLAGS_COMMON | IORING_SEND_ZC_REPORT_USAGE)
+
 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);
@@ -937,11 +940,6 @@ int io_send_zc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (req->flags & REQ_F_CQE_SKIP)
 		return -EINVAL;
 
-	zc->flags = READ_ONCE(sqe->ioprio);
-	if (zc->flags & ~(IORING_RECVSEND_POLL_FIRST |
-			  IORING_RECVSEND_FIXED_BUF |
-			  IORING_SEND_ZC_REPORT_USAGE))
-		return -EINVAL;
 	notif = zc->notif = io_alloc_notif(ctx);
 	if (!notif)
 		return -ENOMEM;
@@ -949,6 +947,17 @@ int io_send_zc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	notif->cqe.res = 0;
 	notif->cqe.flags = IORING_CQE_F_NOTIF;
 	req->flags |= REQ_F_NEED_CLEANUP;
+
+	zc->flags = READ_ONCE(sqe->ioprio);
+	if (unlikely(zc->flags & ~IO_ZC_FLAGS_COMMON)) {
+		if (zc->flags & ~IO_ZC_FLAGS_VALID)
+			return -EINVAL;
+		if (zc->flags & IORING_SEND_ZC_REPORT_USAGE) {
+			io_notif_set_extended(notif);
+			io_notif_to_data(notif)->zc_report = true;
+		}
+	}
+
 	if (zc->flags & IORING_RECVSEND_FIXED_BUF) {
 		unsigned idx = READ_ONCE(sqe->buf_index);
 
@@ -958,9 +967,6 @@ int io_send_zc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		req->imu = READ_ONCE(ctx->user_bufs[idx]);
 		io_req_set_rsrc_node(notif, ctx, 0);
 	}
-	if (zc->flags & IORING_SEND_ZC_REPORT_USAGE) {
-		io_notif_to_data(notif)->zc_report = true;
-	}
 
 	if (req->opcode == IORING_OP_SEND_ZC) {
 		if (READ_ONCE(sqe->__pad3[0]))
diff --git a/io_uring/notif.c b/io_uring/notif.c
index c287adf24e66..9864bde3e2ef 100644
--- a/io_uring/notif.c
+++ b/io_uring/notif.c
@@ -18,11 +18,17 @@ static void __io_notif_complete_tw(struct io_kiocb *notif, bool *locked)
 		__io_unaccount_mem(ctx->user, nd->account_pages);
 		nd->account_pages = 0;
 	}
+	io_req_task_complete(notif, locked);
+}
+
+static void io_notif_complete_tw_ext(struct io_kiocb *notif, bool *locked)
+{
+	struct io_notif_data *nd = io_notif_to_data(notif);
 
 	if (nd->zc_report && (nd->zc_copied || !nd->zc_used))
 		notif->cqe.res |= IORING_NOTIF_USAGE_ZC_COPIED;
 
-	io_req_task_complete(notif, locked);
+	__io_notif_complete_tw(notif, locked);
 }
 
 static void io_tx_ubuf_callback(struct sk_buff *skb, struct ubuf_info *uarg,
@@ -31,15 +37,33 @@ static void io_tx_ubuf_callback(struct sk_buff *skb, struct ubuf_info *uarg,
 	struct io_notif_data *nd = container_of(uarg, struct io_notif_data, uarg);
 	struct io_kiocb *notif = cmd_to_io_kiocb(nd);
 
+	if (refcount_dec_and_test(&uarg->refcnt))
+		io_req_task_work_add(notif);
+}
+
+static void io_tx_ubuf_callback_ext(struct sk_buff *skb, struct ubuf_info *uarg,
+			     bool success)
+{
+	struct io_notif_data *nd = container_of(uarg, struct io_notif_data, uarg);
+
 	if (nd->zc_report) {
 		if (success && !nd->zc_used && skb)
 			WRITE_ONCE(nd->zc_used, true);
 		else if (!success && !nd->zc_copied)
 			WRITE_ONCE(nd->zc_copied, true);
 	}
+	io_tx_ubuf_callback(skb, uarg, success);
+}
 
-	if (refcount_dec_and_test(&uarg->refcnt))
-		io_req_task_work_add(notif);
+void io_notif_set_extended(struct io_kiocb *notif)
+{
+	struct io_notif_data *nd = io_notif_to_data(notif);
+
+	nd->zc_report = false;
+	nd->zc_used = false;
+	nd->zc_copied = false;
+	notif->io_task_work.func = io_notif_complete_tw_ext;
+	io_notif_to_data(notif)->uarg.callback = io_tx_ubuf_callback_ext;
 }
 
 struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx)
@@ -63,7 +87,6 @@ struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx)
 	nd->account_pages = 0;
 	nd->uarg.flags = SKBFL_ZEROCOPY_FRAG | SKBFL_DONT_ORPHAN;
 	nd->uarg.callback = io_tx_ubuf_callback;
-	nd->zc_report = nd->zc_used = nd->zc_copied = false;
 	refcount_set(&nd->uarg.refcnt, 1);
 	return notif;
 }
diff --git a/io_uring/notif.h b/io_uring/notif.h
index 7f00176020d3..c88c800cd89d 100644
--- a/io_uring/notif.h
+++ b/io_uring/notif.h
@@ -19,6 +19,7 @@ struct io_notif_data {
 };
 
 struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx);
+void io_notif_set_extended(struct io_kiocb *notif);
 
 static inline struct io_notif_data *io_notif_to_data(struct io_kiocb *notif)
 {
-- 
2.38.0

[PATCH for-next 7/7] io_uring/net: move mm accounting to a slower path

[Pavel Begunkov]

We can also move mm accounting to the extended callbacks. It removes a
few cycles from the hot path including skipping one function call and
setting io_req_task_complete as a callback directly. For user backed I/O
it shouldn't make any difference taking into considering atomic mm
accounting and page pinning.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/net.c   |  3 +++
 io_uring/notif.c | 31 +++++++++++++------------------
 2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/io_uring/net.c b/io_uring/net.c
index 556a48bcdbe4..b7192e3e4d2d 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -1099,6 +1099,7 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
 			return ret;
 		msg.sg_from_iter = io_sg_from_iter;
 	} else {
+		io_notif_set_extended(zc->notif);
 		ret = import_single_range(WRITE, zc->buf, zc->len, &iov,
 					  &msg.msg_iter);
 		if (unlikely(ret))
@@ -1160,6 +1161,8 @@ int io_sendmsg_zc(struct io_kiocb *req, unsigned int issue_flags)
 	unsigned flags;
 	int ret, min_ret = 0;
 
+	io_notif_set_extended(sr->notif);
+
 	sock = sock_from_file(req->file);
 	if (unlikely(!sock))
 		return -ENOTSOCK;
diff --git a/io_uring/notif.c b/io_uring/notif.c
index 9864bde3e2ef..c4bb793ebf0e 100644
--- a/io_uring/notif.c
+++ b/io_uring/notif.c
@@ -9,11 +9,14 @@
 #include "notif.h"
 #include "rsrc.h"
 
-static void __io_notif_complete_tw(struct io_kiocb *notif, bool *locked)
+static void io_notif_complete_tw_ext(struct io_kiocb *notif, bool *locked)
 {
 	struct io_notif_data *nd = io_notif_to_data(notif);
 	struct io_ring_ctx *ctx = notif->ctx;
 
+	if (nd->zc_report && (nd->zc_copied || !nd->zc_used))
+		notif->cqe.res |= IORING_NOTIF_USAGE_ZC_COPIED;
+
 	if (nd->account_pages && ctx->user) {
 		__io_unaccount_mem(ctx->user, nd->account_pages);
 		nd->account_pages = 0;
@@ -21,16 +24,6 @@ static void __io_notif_complete_tw(struct io_kiocb *notif, bool *locked)
 	io_req_task_complete(notif, locked);
 }
 
-static void io_notif_complete_tw_ext(struct io_kiocb *notif, bool *locked)
-{
-	struct io_notif_data *nd = io_notif_to_data(notif);
-
-	if (nd->zc_report && (nd->zc_copied || !nd->zc_used))
-		notif->cqe.res |= IORING_NOTIF_USAGE_ZC_COPIED;
-
-	__io_notif_complete_tw(notif, locked);
-}
-
 static void io_tx_ubuf_callback(struct sk_buff *skb, struct ubuf_info *uarg,
 				bool success)
 {
@@ -59,11 +52,14 @@ void io_notif_set_extended(struct io_kiocb *notif)
 {
 	struct io_notif_data *nd = io_notif_to_data(notif);
 
-	nd->zc_report = false;
-	nd->zc_used = false;
-	nd->zc_copied = false;
-	notif->io_task_work.func = io_notif_complete_tw_ext;
-	io_notif_to_data(notif)->uarg.callback = io_tx_ubuf_callback_ext;
+	if (nd->uarg.callback != io_tx_ubuf_callback_ext) {
+		nd->account_pages = 0;
+		nd->zc_report = false;
+		nd->zc_used = false;
+		nd->zc_copied = false;
+		nd->uarg.callback = io_tx_ubuf_callback_ext;
+		notif->io_task_work.func = io_notif_complete_tw_ext;
+	}
 }
 
 struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx)
@@ -81,10 +77,9 @@ struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx)
 	notif->task = current;
 	io_get_task_refs(1);
 	notif->rsrc_node = NULL;
-	notif->io_task_work.func = __io_notif_complete_tw;
+	notif->io_task_work.func = io_req_task_complete;
 
 	nd = io_notif_to_data(notif);
-	nd->account_pages = 0;
 	nd->uarg.flags = SKBFL_ZEROCOPY_FRAG | SKBFL_DONT_ORPHAN;
 	nd->uarg.callback = io_tx_ubuf_callback;
 	refcount_set(&nd->uarg.refcnt, 1);
-- 
2.38.0

Re: [PATCH for-next 0/7] small zc improvements

[Jens Axboe]

On Fri, 4 Nov 2022 10:59:39 +0000, Pavel Begunkov wrote:
> Remove some cycles in a couple of places for zc sends. Touches a bunch of
> bits here and there but the main theme is adding additional set of callbacks
> for slower path and move in there some optional features.
> 
> Pavel Begunkov (7):
>   io_uring: move kbuf put out of generic tw complete
>   io_uring/net: remove extra notif rsrc setup
>   io_uring/net: preset notif tw handler
>   io_uring/net: rename io_uring_tx_zerocopy_callback
>   io_uring/net: inline io_notif_flush()
>   io_uring: move zc reporting from the hot path
>   io_uring/net: move mm accounting to a slower path
> 
> [...]

Applied, thanks!

[1/7] io_uring: move kbuf put out of generic tw complete
      commit: 4b72e6e5396b5b49b6a58fe6d674326375f0e0c5
[2/7] io_uring/net: remove extra notif rsrc setup
      commit: 69a47aaa5c9ad309c09c9a24a1045ca733f72a41
[3/7] io_uring/net: preset notif tw handler
      commit: b96a61e0a4dda80faef4cc553a97828915edfd60
[4/7] io_uring/net: rename io_uring_tx_zerocopy_callback
      commit: d546548227976445133ee229ab99c7b2ad933712
[5/7] io_uring/net: inline io_notif_flush()
      commit: 1e52225edde1cfbd764b61f0aa07e77372b2717c
[6/7] io_uring: move zc reporting from the hot path
      commit: 02912499a7a43c4d6ee3cfb164a9dd509c1f7f18
[7/7] io_uring/net: move mm accounting to a slower path
      commit: 1cc5a56e4b4cf4dbe1a7497e4bf9e0b92ff4803f

Best regards,
-- 
Jens Axboe