Re: [PATCH v2 2/2] io_uring: io_queue_link*() right after submit

From: Bob Liu
Date: Wed Nov 06 2019 - 03:36:53 EST


On 11/6/19 5:22 AM, Pavel Begunkov wrote:
> After a call to io_submit_sqe(), it's already known whether it needs
> to queue a link or not. Do it there, as it's simplier and doesn't keep
> an extra variable across the loop.
>
> Signed-off-by: Pavel Begunkov <asml.silence@xxxxxxxxx>
> ---
> fs/io_uring.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index ebe2a4edd644..82c2da99cb5c 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -2687,7 +2687,6 @@ static int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr,
> struct io_submit_state state, *statep = NULL;
> struct io_kiocb *link = NULL;
> struct io_kiocb *shadow_req = NULL;
> - bool prev_was_link = false;
> int i, submitted = 0;
> bool mm_fault = false;
>
> @@ -2710,17 +2709,6 @@ static int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr,
> }
> }
>
> - /*
> - * If previous wasn't linked and we have a linked command,
> - * that's the end of the chain. Submit the previous link.
> - */
> - if (!prev_was_link && link) {
> - io_queue_link_head(ctx, link, &link->submit, shadow_req);
> - link = NULL;
> - shadow_req = NULL;
> - }
> - prev_was_link = (s.sqe->flags & IOSQE_IO_LINK) != 0;
> -
> if (link && (s.sqe->flags & IOSQE_IO_DRAIN)) {
> if (!shadow_req) {
> shadow_req = io_get_req(ctx, NULL);
> @@ -2741,6 +2729,16 @@ static int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr,
> trace_io_uring_submit_sqe(ctx, s.sqe->user_data, true, async);
> io_submit_sqe(ctx, &s, statep, &link);
> submitted++;
> +
> + /*
> + * If previous wasn't linked and we have a linked command,
> + * that's the end of the chain. Submit the previous link.
> + */
> + if (!(s.sqe->flags & IOSQE_IO_LINK) && link)
The behavior changed to 'current seq' instead of previous after dropping prev_was_link?

> + io_queue_link_head(ctx, link, &link->submit, shadow_req);
> + link = NULL;
> + shadow_req = NULL;
> + }
> }
>
> if (link)
>