From: Kanchan Joshi <[email protected]>
To: Jens Axboe <[email protected]>
Cc: io-uring <[email protected]>, Kanchan Joshi <[email protected]>
Subject: Re: [PATCH] io_uring/uring_cmd: push IRQ based completions through task_work
Date: Mon, 20 Mar 2023 20:36:15 +0530 [thread overview]
Message-ID: <CA+1E3rKBNhmT63GMNpe-c+EVDpzvs4voTkL-efkdbJHdNZhZ7w@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
On Sun, Mar 19, 2023 at 8:51 PM Jens Axboe <[email protected]> wrote:
>
> This is similar to what we do on the non-passthrough read/write side,
> and helps take advantage of the completion batching we can do when we
> post CQEs via task_work. On top of that, this avoids a uring_lock
> grab/drop for every completion.
>
> In the normal peak IRQ based testing, this increases performance in
> my testing from ~75M to ~77M IOPS, or an increase of 2-3%.
>
> Signed-off-by: Jens Axboe <[email protected]>
>
> ---
>
> diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
> index 2e4c483075d3..b4fba5f0ab0d 100644
> --- a/io_uring/uring_cmd.c
> +++ b/io_uring/uring_cmd.c
> @@ -45,18 +45,21 @@ static inline void io_req_set_cqe32_extra(struct io_kiocb *req,
> void io_uring_cmd_done(struct io_uring_cmd *ioucmd, ssize_t ret, ssize_t res2)
> {
> struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
> + struct io_ring_ctx *ctx = req->ctx;
>
> if (ret < 0)
> req_set_fail(req);
>
> io_req_set_res(req, ret, 0);
> - if (req->ctx->flags & IORING_SETUP_CQE32)
> + if (ctx->flags & IORING_SETUP_CQE32)
> io_req_set_cqe32_extra(req, res2, 0);
> - if (req->ctx->flags & IORING_SETUP_IOPOLL)
> + if (ctx->flags & IORING_SETUP_IOPOLL) {
> /* order with io_iopoll_req_issued() checking ->iopoll_complete */
> smp_store_release(&req->iopoll_completed, 1);
> - else
> - io_req_complete_post(req, 0);
> + return;
> + }
> + req->io_task_work.func = io_req_task_complete;
> + io_req_task_work_add(req);
> }
Since io_uring_cmd_done itself would be executing in task-work often
(always in case of nvme), can this be further optimized by doing
directly what this new task-work (that is being set up here) would
have done?
Something like below on top of your patch -
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index e1929f6e5a24..7a764e04f309 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -58,8 +58,12 @@ void io_uring_cmd_done(struct io_uring_cmd *ioucmd,
ssize_t ret, ssize_t res2)
smp_store_release(&req->iopoll_completed, 1);
return;
}
- req->io_task_work.func = io_req_task_complete;
- io_req_task_work_add(req);
+ if (in_task()) {
+ io_req_complete_defer(req);
+ } else {
+ req->io_task_work.func = io_req_task_complete;
+ io_req_task_work_add(req);
+ }
}
EXPORT_SYMBOL_GPL(io_uring_cmd_done);
next prev parent reply other threads:[~2023-03-20 15:12 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-19 15:18 [PATCH] io_uring/uring_cmd: push IRQ based completions through task_work Jens Axboe
2023-03-20 15:06 ` Kanchan Joshi [this message]
2023-03-20 20:03 ` Jens Axboe
2023-03-20 20:42 ` Jens Axboe
2023-03-21 4:32 ` Kanchan Joshi
2023-03-21 4:38 ` Kanchan Joshi
2023-03-27 11:16 ` Pavel Begunkov
2023-03-20 23:35 ` Ming Lei
2023-03-21 1:39 ` Jens Axboe
2023-03-21 1:54 ` Ming Lei
2023-03-21 1:56 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CA+1E3rKBNhmT63GMNpe-c+EVDpzvs4voTkL-efkdbJHdNZhZ7w@mail.gmail.com \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox