Registering the single issuer task from the first submit adds unnecesary
complications to the API as well as the implementation. Where simply
registering it at creation should not impose any barriers to getting the
same performance wins.
There is another problem in 6.1, with IORING_SETUP_DEFER_TASKRUN. That
would like to check the submitter_task from unlocked contexts, which would
be racy. If upfront the submitter_task is set at creation time it will
simplify the logic there and probably increase performance (though this is
unmeasured).
Patch 1 registers the task at creation of the io_uring, this works
standalone in case you want to only merge this part for 6.0
Patch 2/3 cleans up the code from the old style
Dylan Yudaken (3):
io_uring: register single issuer task at creation
io_uring: simplify __io_uring_add_tctx_node
io_uring: remove io_register_submitter
io_uring/io_uring.c | 5 ++++-
io_uring/tctx.c | 42 ++++++++++++++++++------------------------
io_uring/tctx.h | 6 ++++--
3 files changed, 26 insertions(+), 27 deletions(-)
base-commit: f76349cf41451c5c42a99f18a9163377e4b364ff
--
2.30.2
Remove submitter parameter from __io_uring_add_tctx_node.
It was only called from one place, and we can do that logic in that one
place.
Signed-off-by: Dylan Yudaken <[email protected]>
---
io_uring/io_uring.c | 2 +-
io_uring/tctx.c | 30 ++++++++++++++++++++----------
io_uring/tctx.h | 6 ++++--
3 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 3f40defd721d..57315a49b85c 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3183,7 +3183,7 @@ static int io_uring_install_fd(struct io_ring_ctx *ctx, struct file *file)
if (fd < 0)
return fd;
- ret = __io_uring_add_tctx_node(ctx, false);
+ ret = __io_uring_add_tctx_node(ctx);
if (ret) {
put_unused_fd(fd);
return ret;
diff --git a/io_uring/tctx.c b/io_uring/tctx.c
index 7f97d97fef0a..dd0205fcdb13 100644
--- a/io_uring/tctx.c
+++ b/io_uring/tctx.c
@@ -105,18 +105,12 @@ static int io_register_submitter(struct io_ring_ctx *ctx)
return ret;
}
-int __io_uring_add_tctx_node(struct io_ring_ctx *ctx, bool submitter)
+int __io_uring_add_tctx_node(struct io_ring_ctx *ctx)
{
struct io_uring_task *tctx = current->io_uring;
struct io_tctx_node *node;
int ret;
- if ((ctx->flags & IORING_SETUP_SINGLE_ISSUER) && submitter) {
- ret = io_register_submitter(ctx);
- if (ret)
- return ret;
- }
-
if (unlikely(!tctx)) {
ret = io_uring_alloc_task_context(current, ctx);
if (unlikely(ret))
@@ -150,8 +144,24 @@ int __io_uring_add_tctx_node(struct io_ring_ctx *ctx, bool submitter)
list_add(&node->ctx_node, &ctx->tctx_list);
mutex_unlock(&ctx->uring_lock);
}
- if (submitter)
- tctx->last = ctx;
+ return 0;
+}
+
+int __io_uring_add_tctx_node_from_submit(struct io_ring_ctx *ctx)
+{
+ int ret;
+
+ if (ctx->flags & IORING_SETUP_SINGLE_ISSUER) {
+ ret = io_register_submitter(ctx);
+ if (ret)
+ return ret;
+ }
+
+ ret = __io_uring_add_tctx_node(ctx);
+ if (ret)
+ return ret;
+
+ current->io_uring->last = ctx;
return 0;
}
@@ -259,7 +269,7 @@ int io_ringfd_register(struct io_ring_ctx *ctx, void __user *__arg,
return -EINVAL;
mutex_unlock(&ctx->uring_lock);
- ret = __io_uring_add_tctx_node(ctx, false);
+ ret = __io_uring_add_tctx_node(ctx);
mutex_lock(&ctx->uring_lock);
if (ret)
return ret;
diff --git a/io_uring/tctx.h b/io_uring/tctx.h
index 25974beed4d6..608e96de70a2 100644
--- a/io_uring/tctx.h
+++ b/io_uring/tctx.h
@@ -9,7 +9,8 @@ struct io_tctx_node {
int io_uring_alloc_task_context(struct task_struct *task,
struct io_ring_ctx *ctx);
void io_uring_del_tctx_node(unsigned long index);
-int __io_uring_add_tctx_node(struct io_ring_ctx *ctx, bool submitter);
+int __io_uring_add_tctx_node(struct io_ring_ctx *ctx);
+int __io_uring_add_tctx_node_from_submit(struct io_ring_ctx *ctx);
void io_uring_clean_tctx(struct io_uring_task *tctx);
void io_uring_unreg_ringfd(void);
@@ -27,5 +28,6 @@ static inline int io_uring_add_tctx_node(struct io_ring_ctx *ctx)
if (likely(tctx && tctx->last == ctx))
return 0;
- return __io_uring_add_tctx_node(ctx, true);
+
+ return __io_uring_add_tctx_node_from_submit(ctx);
}
--
2.30.2
On 9/26/22 8:03 AM, Dylan Yudaken wrote:
> Registering the single issuer task from the first submit adds unnecesary
> complications to the API as well as the implementation. Where simply
> registering it at creation should not impose any barriers to getting the
> same performance wins.
>
> There is another problem in 6.1, with IORING_SETUP_DEFER_TASKRUN. That
> would like to check the submitter_task from unlocked contexts, which would
> be racy. If upfront the submitter_task is set at creation time it will
> simplify the logic there and probably increase performance (though this is
> unmeasured).
>
> Patch 1 registers the task at creation of the io_uring, this works
> standalone in case you want to only merge this part for 6.0
>
> Patch 2/3 cleans up the code from the old style
Applied 1/3 for 6.0, and then created a new branch for 6.1 that holds
2-3/3. Thanks!
--
Jens Axboe