在 2021/8/18 下午10:40, Pavel Begunkov 写道:
On 8/18/21 1:22 PM, Hao Xu wrote:
在 2021/8/18 下午6:20, Pavel Begunkov 写道:
On 8/18/21 8:43 AM, Hao Xu wrote:
Given a linkchain like this:
req0(link_flag)-->req1(link_flag)-->...-->reqn(no link_flag)
[...]
struct io_submit_link *link = &ctx->submit_state.link;
+ bool is_link = sqe->flags & (IOSQE_IO_LINK | IOSQE_IO_HARDLINK);
+ struct io_kiocb *head;
int ret;
+ /*
+ * we don't update link->last until we've done io_req_prep()
+ * since linked timeout uses old link->last
+ */
+ if (link->head)
+ link->last->link = req;
+ else if (is_link)
+ link->head = req;
+ head = link->head;
It's a horrorsome amount of overhead. How about to set the fail flag
if failed early and actually fail on io_queue_sqe(), as below. It's
not tested and a couple more bits added, but hopefully gives the idea.
I get the idea, it's truely with less change. But why do you think the
above code bring in more overhead, since anyway we have to link the req
to the linkchain. I tested it with fio-direct-4k-read-with/without-sqpoll, didn't see performance regression.
Well, it's an exaggeration :) However, we were cutting the overhead,
and there is no atomics or other heavy operations left in the hot path,
just pure number of instructions a request should go through. That's
just to clear the reason why I don't want extras on the path.
For the non-linked path, first it adds 2 ifs in front and removes one
at the end. Then there is is_link, which is most likely to be saved
on stack. And same with @head which I'd expect to be saved on stack.
Agree on most part except the @head, despite cache hit, a direct stack
variable head is better than a stack pointer link.
I'm excited to see io_uring is becoming faster and faster since we are
actively using it. Thanks for the great work, the recent refcount
optimization is amazing.
I'll send a v2 patchset.
Thanks,
Hao
If we have a way to avoid it, that's great, and it looks we can.
[...]
- ret = io_req_prep(req, sqe);
- if (unlikely(ret))
- goto fail_req;
/* don't need @sqe from now on */
trace_io_uring_submit_sqe(ctx, req, req->opcode, req->user_data,
@@ -6670,8 +6670,10 @@ static int io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req,
struct io_kiocb *head = link->head;
maybe better to add an if(head & FAIL) here, since we don't need to
prep_async if we know it will be cancelled.
Such an early fail is marginal enough to not care about performance,
but I agree that the check is needed as io_req_prep_async() won't be
able to handle it. E.g. if it failed to grab a file.
