| Message ID | 20240511001214.173711-6-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | io_uring: support sqe group and provide group kbuf | expand |
On Sat, May 11, 2024 at 08:12:08AM +0800, Ming Lei wrote: > SQE group is defined as one chain of SQEs starting with the first SQE that > has IOSQE_SQE_GROUP set, and ending with the first subsequent SQE that > doesn't have it set, and it is similar with chain of linked SQEs. > > Not like linked SQEs, each sqe is issued after the previous one is completed. > All SQEs in one group are submitted in parallel, so there isn't any dependency > among SQEs in one group. > > The 1st SQE is group leader, and the other SQEs are group member. The whole > group share single IOSQE_IO_LINK and IOSQE_IO_DRAIN from group leader, and > the two flags are ignored for group members. > > When the group is in one link chain, this group isn't submitted until the > previous SQE or group is completed. And the following SQE or group can't > be started if this group isn't completed. Failure from any group member will > fail the group leader, then the link chain can be terminated. > > When IOSQE_IO_DRAIN is set for group leader, all requests in this group and > previous requests submitted are drained. Given IOSQE_IO_DRAIN can be set for > group leader only, we respect IO_DRAIN by always completing group leader as > the last one in the group. > > Working together with IOSQE_IO_LINK, SQE group provides flexible way to > support N:M dependency, such as: > > - group A is chained with group B together > - group A has N SQEs > - group B has M SQEs > > then M SQEs in group B depend on N SQEs in group A. > > N:M dependency can support some interesting use cases in efficient way: > > 1) read from multiple files, then write the read data into single file > > 2) read from single file, and write the read data into multiple files > > 3) write same data into multiple files, and read data from multiple files and > compare if correct data is written > > Also IOSQE_SQE_GROUP takes the last bit in sqe->flags, but we still can > extend sqe->flags with one uring context flag, such as use __pad3 for > non-uring_cmd OPs and part of uring_cmd_flags for uring_cmd OP. > > Suggested-by: Kevin Wolf <kwolf@redhat.com> > Signed-off-by: Ming Lei <ming.lei@redhat.com> BTW, I wrote one link-grp-cp.c liburing/example which is based on sqe group, and keep QD not changed, just re-organize IOs in the following ways: - each group have 4 READ IOs, linked by one single write IO for writing the read data in sqe group to destination file - the 1st 12 groups have (4 + 1) IOs, and the last group have (3 + 1) IOs Run the example for copying two block device(from virtio-blk to virtio-scsi in my test VM): 1) buffered copy: - perf is improved by 5% 2) direct IO mode - perf is improved by 27% [1] link-grp-cp.c example https://github.com/ming1/liburing/commits/sqe_group_v2/ [2] one bug fixes(top commit) against V3 https://github.com/ming1/linux/commits/io_uring_sqe_group_v3/ Thanks, Ming
On 5/21/24 03:58, Ming Lei wrote: > On Sat, May 11, 2024 at 08:12:08AM +0800, Ming Lei wrote: >> SQE group is defined as one chain of SQEs starting with the first SQE that >> has IOSQE_SQE_GROUP set, and ending with the first subsequent SQE that >> doesn't have it set, and it is similar with chain of linked SQEs. >> >> Not like linked SQEs, each sqe is issued after the previous one is completed. >> All SQEs in one group are submitted in parallel, so there isn't any dependency >> among SQEs in one group. >> >> The 1st SQE is group leader, and the other SQEs are group member. The whole >> group share single IOSQE_IO_LINK and IOSQE_IO_DRAIN from group leader, and >> the two flags are ignored for group members. >> >> When the group is in one link chain, this group isn't submitted until the >> previous SQE or group is completed. And the following SQE or group can't >> be started if this group isn't completed. Failure from any group member will >> fail the group leader, then the link chain can be terminated. >> >> When IOSQE_IO_DRAIN is set for group leader, all requests in this group and >> previous requests submitted are drained. Given IOSQE_IO_DRAIN can be set for >> group leader only, we respect IO_DRAIN by always completing group leader as >> the last one in the group. >> >> Working together with IOSQE_IO_LINK, SQE group provides flexible way to >> support N:M dependency, such as: >> >> - group A is chained with group B together >> - group A has N SQEs >> - group B has M SQEs >> >> then M SQEs in group B depend on N SQEs in group A. >> >> N:M dependency can support some interesting use cases in efficient way: >> >> 1) read from multiple files, then write the read data into single file >> >> 2) read from single file, and write the read data into multiple files >> >> 3) write same data into multiple files, and read data from multiple files and >> compare if correct data is written >> >> Also IOSQE_SQE_GROUP takes the last bit in sqe->flags, but we still can >> extend sqe->flags with one uring context flag, such as use __pad3 for >> non-uring_cmd OPs and part of uring_cmd_flags for uring_cmd OP. >> >> Suggested-by: Kevin Wolf <kwolf@redhat.com> >> Signed-off-by: Ming Lei <ming.lei@redhat.com> > > BTW, I wrote one link-grp-cp.c liburing/example which is based on sqe group, > and keep QD not changed, just re-organize IOs in the following ways: > > - each group have 4 READ IOs, linked by one single write IO for writing > the read data in sqe group to destination file IIUC it's comparing 1 large write request with 4 small, and it's not exactly anything close to fair. And you can do same in userspace (without links). And having control in userspace you can do more fun tricks, like interleaving writes for one batch with reads from the next batch. > - the 1st 12 groups have (4 + 1) IOs, and the last group have (3 + 1) > IOs > > > Run the example for copying two block device(from virtio-blk to > virtio-scsi in my test VM): > > 1) buffered copy: > - perf is improved by 5% > > 2) direct IO mode > - perf is improved by 27% > > > [1] link-grp-cp.c example > > https://github.com/ming1/liburing/commits/sqe_group_v2/ > > > [2] one bug fixes(top commit) against V3 > > https://github.com/ming1/linux/commits/io_uring_sqe_group_v3/ > > > > Thanks, > Ming >
On 5/11/24 01:12, Ming Lei wrote: > SQE group is defined as one chain of SQEs starting with the first SQE that > has IOSQE_SQE_GROUP set, and ending with the first subsequent SQE that > doesn't have it set, and it is similar with chain of linked SQEs. The main concern stays same, it adds overhead nearly to every single hot function I can think of, as well as lots of complexity. Another minor issue is REQ_F_INFLIGHT, as explained before, cancellation has to be able to find all REQ_F_INFLIGHT requests. Requests you add to a group can have that flag but are not discoverable by core io_uring code. Another note, I'll be looking deeper into this patch, there is too much of random tossing around of requests / refcounting and other dependencies, as well as odd intertwinings with other parts. > Not like linked SQEs, each sqe is issued after the previous one is completed. > All SQEs in one group are submitted in parallel, so there isn't any dependency > among SQEs in one group. > > The 1st SQE is group leader, and the other SQEs are group member. The whole > group share single IOSQE_IO_LINK and IOSQE_IO_DRAIN from group leader, and > the two flags are ignored for group members. > > When the group is in one link chain, this group isn't submitted until the > previous SQE or group is completed. And the following SQE or group can't > be started if this group isn't completed. Failure from any group member will > fail the group leader, then the link chain can be terminated. > > When IOSQE_IO_DRAIN is set for group leader, all requests in this group and > previous requests submitted are drained. Given IOSQE_IO_DRAIN can be set for > group leader only, we respect IO_DRAIN by always completing group leader as > the last one in the group. > > Working together with IOSQE_IO_LINK, SQE group provides flexible way to > support N:M dependency, such as: > > - group A is chained with group B together > - group A has N SQEs > - group B has M SQEs > > then M SQEs in group B depend on N SQEs in group A. > > N:M dependency can support some interesting use cases in efficient way: > > 1) read from multiple files, then write the read data into single file > > 2) read from single file, and write the read data into multiple files > > 3) write same data into multiple files, and read data from multiple files and > compare if correct data is written > > Also IOSQE_SQE_GROUP takes the last bit in sqe->flags, but we still can > extend sqe->flags with one uring context flag, such as use __pad3 for > non-uring_cmd OPs and part of uring_cmd_flags for uring_cmd OP. > > Suggested-by: Kevin Wolf <kwolf@redhat.com> > Signed-off-by: Ming Lei <ming.lei@redhat.com> > --- > include/linux/io_uring_types.h | 12 ++ > include/uapi/linux/io_uring.h | 4 + > io_uring/io_uring.c | 255 +++++++++++++++++++++++++++++++-- > io_uring/io_uring.h | 16 +++ > io_uring/timeout.c | 2 + > 5 files changed, 277 insertions(+), 12 deletions(-) > > diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h > index 7a6b190c7da7..62311b0f0e0b 100644 > --- a/include/linux/io_uring_types.h > +++ b/include/linux/io_uring_types.h > @@ -666,6 +674,10 @@ struct io_kiocb { > u64 extra1; > u64 extra2; > } big_cqe; > + > + /* all SQE group members linked here for group lead */ > + struct io_kiocb *grp_link; > + int grp_refs; > }; > > struct io_overflow_cqe { > diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c > index c184c9a312df..b87c5452de43 100644 > --- a/io_uring/io_uring.c > +++ b/io_uring/io_uring.c > @@ -109,7 +109,8 @@ > IOSQE_IO_HARDLINK | IOSQE_ASYNC) > > #define SQE_VALID_FLAGS (SQE_COMMON_FLAGS | IOSQE_BUFFER_SELECT | \ > - IOSQE_IO_DRAIN | IOSQE_CQE_SKIP_SUCCESS) > + IOSQE_IO_DRAIN | IOSQE_CQE_SKIP_SUCCESS | \ > + IOSQE_SQE_GROUP) > > #define IO_REQ_CLEAN_FLAGS (REQ_F_BUFFER_SELECTED | REQ_F_NEED_CLEANUP | \ > REQ_F_POLLED | REQ_F_INFLIGHT | REQ_F_CREDS | \ > @@ -915,6 +916,13 @@ static __always_inline void io_req_commit_cqe(struct io_kiocb *req, > { > struct io_ring_ctx *ctx = req->ctx; > > + /* > + * For group leader, cqe has to be committed after all members are > + * committed, when the request becomes normal one. > + */ > + if (unlikely(req_is_group_leader(req))) > + return; The copy of it inlined into flush_completions should maintain a proper fast path. if (req->flags & (CQE_SKIP | GROUP)) { if (req->flags & CQE_SKIP) continue; if (req->flags & GROUP) {} } > + > if (unlikely(!io_fill_cqe_req(ctx, req))) { > if (lockless_cq) { > spin_lock(&ctx->completion_lock); > @@ -926,6 +934,116 @@ static __always_inline void io_req_commit_cqe(struct io_kiocb *req, > } > } > > +static inline bool need_queue_group_members(struct io_kiocb *req) > +{ > + return req_is_group_leader(req) && req->grp_link; > +} > + > +/* Can only be called after this request is issued */ > +static inline struct io_kiocb *get_group_leader(struct io_kiocb *req) > +{ > + if (req->flags & REQ_F_SQE_GROUP) { > + if (req_is_group_leader(req)) > + return req; > + return req->grp_link; I'm missing something, it seems io_group_sqe() adding all requests of a group into a singly linked list via ->grp_link, but here we return it as a leader. Confused. > + } > + return NULL; > +} > + > +void io_cancel_group_members(struct io_kiocb *req, bool ignore_cqes) > +{ > + struct io_kiocb *member = req->grp_link; > + > + while (member) { > + struct io_kiocb *next = member->grp_link; > + > + if (ignore_cqes) > + member->flags |= REQ_F_CQE_SKIP; > + if (!(member->flags & REQ_F_FAIL)) { > + req_set_fail(member); > + io_req_set_res(member, -ECANCELED, 0); > + } > + member = next; > + } > +} > + > +void io_queue_group_members(struct io_kiocb *req, bool async) > +{ > + struct io_kiocb *member = req->grp_link; > + > + if (!member) > + return; > + > + while (member) { > + struct io_kiocb *next = member->grp_link; > + > + member->grp_link = req; > + if (async) > + member->flags |= REQ_F_FORCE_ASYNC; > + > + if (unlikely(member->flags & REQ_F_FAIL)) { > + io_req_task_queue_fail(member, member->cqe.res); > + } else if (member->flags & REQ_F_FORCE_ASYNC) { > + io_req_task_queue(member); > + } else { > + io_queue_sqe(member); > + } > + member = next; > + } > + req->grp_link = NULL; > +} > + > +static inline bool __io_complete_group_req(struct io_kiocb *req, > + struct io_kiocb *lead) > +{ > + WARN_ON_ONCE(!(req->flags & REQ_F_SQE_GROUP)); > + > + if (WARN_ON_ONCE(lead->grp_refs <= 0)) > + return false; > + > + /* > + * Set linked leader as failed if any member is failed, so > + * the remained link chain can be terminated > + */ > + if (unlikely((req->flags & REQ_F_FAIL) && > + ((lead->flags & IO_REQ_LINK_FLAGS) && lead->link))) > + req_set_fail(lead); > + return !--lead->grp_refs; > +} > + > +/* Complete group request and collect completed leader for freeing */ > +static inline void io_complete_group_req(struct io_kiocb *req, > + struct io_wq_work_list *grp_list) > +{ > + struct io_kiocb *lead = get_group_leader(req); > + > + if (__io_complete_group_req(req, lead)) { > + req->flags &= ~REQ_F_SQE_GROUP; > + lead->flags &= ~REQ_F_SQE_GROUP_LEADER; > + if (!(lead->flags & REQ_F_CQE_SKIP)) > + io_req_commit_cqe(lead, lead->ctx->lockless_cq); > + > + if (req != lead) { > + /* > + * Add leader to free list if it isn't there > + * otherwise clearing group flag for freeing it > + * in current batch > + */ > + if (!(lead->flags & REQ_F_SQE_GROUP)) > + wq_list_add_tail(&lead->comp_list, grp_list); > + else > + lead->flags &= ~REQ_F_SQE_GROUP; > + } > + } else if (req != lead) { > + req->flags &= ~REQ_F_SQE_GROUP; > + } else { > + /* > + * Leader's group flag clearing is delayed until it is > + * removed from free list > + */ > + } > +} > + > static void io_req_complete_post(struct io_kiocb *req, unsigned issue_flags) > { > struct io_ring_ctx *ctx = req->ctx; > @@ -1427,6 +1545,17 @@ static void io_free_batch_list(struct io_ring_ctx *ctx, > comp_list); > > if (unlikely(req->flags & IO_REQ_CLEAN_SLOW_FLAGS)) { > + /* > + * Group leader may be removed twice, don't free it > + * if group flag isn't cleared, when some members > + * aren't completed yet > + */ > + if (req->flags & REQ_F_SQE_GROUP) { > + node = req->comp_list.next; > + req->flags &= ~REQ_F_SQE_GROUP; > + continue; > + } > + > if (req->flags & REQ_F_REFCOUNT) { > node = req->comp_list.next; > if (!req_ref_put_and_test(req)) > @@ -1459,6 +1588,7 @@ void __io_submit_flush_completions(struct io_ring_ctx *ctx) > __must_hold(&ctx->uring_lock) > { > struct io_submit_state *state = &ctx->submit_state; > + struct io_wq_work_list grp_list = {NULL}; > struct io_wq_work_node *node; > > __io_cq_lock(ctx); > @@ -1468,9 +1598,15 @@ void __io_submit_flush_completions(struct io_ring_ctx *ctx) > > if (!(req->flags & REQ_F_CQE_SKIP)) > io_req_commit_cqe(req, ctx->lockless_cq); > + > + if (req->flags & REQ_F_SQE_GROUP) Same note about hot path > + io_complete_group_req(req, &grp_list); > } > __io_cq_unlock_post(ctx); > > + if (!wq_list_empty(&grp_list)) > + __wq_list_splice(&grp_list, state->compl_reqs.first); What's the point of splicing it here insted of doing all that under REQ_F_SQE_GROUP above? > + > if (!wq_list_empty(&ctx->submit_state.compl_reqs)) { > io_free_batch_list(ctx, state->compl_reqs.first); > INIT_WQ_LIST(&state->compl_reqs); > @@ -1677,8 +1813,12 @@ static u32 io_get_sequence(struct io_kiocb *req) > struct io_kiocb *cur; > > /* need original cached_sq_head, but it was increased for each req */ > - io_for_each_link(cur, req) > - seq--; > + io_for_each_link(cur, req) { > + if (req_is_group_leader(cur)) > + seq -= cur->grp_refs; > + else > + seq--; > + } > return seq; > } > > @@ -1793,11 +1933,20 @@ struct io_wq_work *io_wq_free_work(struct io_wq_work *work) > struct io_kiocb *nxt = NULL; > > if (req_ref_put_and_test(req)) { > - if (req->flags & IO_REQ_LINK_FLAGS) > - nxt = io_req_find_next(req); > + /* > + * CQEs have been posted in io_req_complete_post() except > + * for group leader, and we can't advance the link for > + * group leader until its CQE is posted. > + * > + * TODO: try to avoid defer and complete leader in io_wq > + * context directly > + */ > + if (!req_is_group_leader(req)) { > + req->flags |= REQ_F_CQE_SKIP; > + if (req->flags & IO_REQ_LINK_FLAGS) > + nxt = io_req_find_next(req); > + } > > - /* we have posted CQEs in io_req_complete_post() */ > - req->flags |= REQ_F_CQE_SKIP; > io_free_req(req); > } > return nxt ? &nxt->work : NULL; > @@ -1863,6 +2012,8 @@ void io_wq_submit_work(struct io_wq_work *work) > } > } > > + if (need_queue_group_members(req)) > + io_queue_group_members(req, true); > do { > ret = io_issue_sqe(req, issue_flags); > if (ret != -EAGAIN) > @@ -1977,6 +2128,9 @@ static inline void io_queue_sqe(struct io_kiocb *req) > */ > if (unlikely(ret)) > io_queue_async(req, ret); > + > + if (need_queue_group_members(req)) > + io_queue_group_members(req, false); Request ownership is considered to be handed further at this point and requests should not be touched. Only ret==0 from io_issue_sqe it's still ours, but again it's handed somewhere by io_queue_async(). > } > > static void io_queue_sqe_fallback(struct io_kiocb *req) > @@ -2142,6 +2296,56 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, > return def->prep(req, sqe); > } > > +static struct io_kiocb *io_group_sqe(struct io_submit_link *group, > + struct io_kiocb *req) > +{ > + /* > + * Group chain is similar with link chain: starts with 1st sqe with > + * REQ_F_SQE_GROUP, and ends with the 1st sqe without REQ_F_SQE_GROUP > + */ > + if (group->head) { > + struct io_kiocb *lead = group->head; > + > + /* members can't be in link chain, can't be drained */ > + req->flags &= ~(IO_REQ_LINK_FLAGS | REQ_F_IO_DRAIN); > + lead->grp_refs += 1; > + group->last->grp_link = req; > + group->last = req; > + > + if (req->flags & REQ_F_SQE_GROUP) > + return NULL; > + > + req->grp_link = NULL; > + req->flags |= REQ_F_SQE_GROUP; > + group->head = NULL; > + return lead; > + } else if (req->flags & REQ_F_SQE_GROUP) { > + group->head = req; > + group->last = req; > + req->grp_refs = 1; > + req->flags |= REQ_F_SQE_GROUP_LEADER; > + return NULL; > + } else { > + return req; > + } > +} > + > +static __cold struct io_kiocb *io_submit_fail_group( > + struct io_submit_link *link, struct io_kiocb *req) > +{ > + struct io_kiocb *lead = link->head; > + > + /* > + * Instead of failing eagerly, continue assembling the group link > + * if applicable and mark the leader with REQ_F_FAIL. The group > + * flushing code should find the flag and handle the rest > + */ > + if (lead && (lead->flags & IO_REQ_LINK_FLAGS) && !(lead->flags & REQ_F_FAIL)) > + req_fail_link_node(lead, -ECANCELED); > + > + return io_group_sqe(link, req); > +} > + > static __cold int io_submit_fail_link(struct io_submit_link *link, > struct io_kiocb *req, int ret) > { > @@ -2180,11 +2384,18 @@ static __cold int io_submit_fail_init(const struct io_uring_sqe *sqe, > { > struct io_ring_ctx *ctx = req->ctx; > struct io_submit_link *link = &ctx->submit_state.link; > + struct io_submit_link *group = &ctx->submit_state.group; > > trace_io_uring_req_failed(sqe, req, ret); > > req_fail_link_node(req, ret); > > + if (group->head || (req->flags & REQ_F_SQE_GROUP)) { > + req = io_submit_fail_group(group, req); > + if (!req) > + return 0; > + } > + > /* cover both linked and non-linked request */ > return io_submit_fail_link(link, req, ret); > } > @@ -2232,7 +2443,7 @@ static inline int io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req, > const struct io_uring_sqe *sqe) > __must_hold(&ctx->uring_lock) > { > - struct io_submit_link *link = &ctx->submit_state.link; > + struct io_submit_state *state = &ctx->submit_state; > int ret; > > ret = io_init_req(ctx, req, sqe); > @@ -2241,9 +2452,17 @@ static inline int io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req, > > trace_io_uring_submit_req(req); > > - if (unlikely(link->head || (req->flags & (IO_REQ_LINK_FLAGS | > - REQ_F_FORCE_ASYNC | REQ_F_FAIL)))) { > - req = io_link_sqe(link, req); > + if (unlikely(state->group.head || A note rather to myself and for the future, all theese checks including links and groups can be folded under one common if. > + (req->flags & REQ_F_SQE_GROUP))) { > + req = io_group_sqe(&state->group, req); > + if (!req) > + return 0; > + } > + > + if (unlikely(state->link.head || > + (req->flags & (IO_REQ_LINK_FLAGS | REQ_F_FORCE_ASYNC | > + REQ_F_FAIL)))) { > + req = io_link_sqe(&state->link, req); > if (!req) > return 0; > } > @@ -2258,6 +2477,17 @@ static void io_submit_state_end(struct io_ring_ctx *ctx) > { > struct io_submit_state *state = &ctx->submit_state; > > + /* the last member must set REQ_F_SQE_GROUP */ > + if (unlikely(state->group.head)) { > + struct io_kiocb *lead = state->group.head; > + > + state->group.last->grp_link = NULL; > + if (lead->flags & IO_REQ_LINK_FLAGS) > + io_link_sqe(&state->link, lead); > + else > + io_queue_sqe_fallback(lead); > + } > + > if (unlikely(state->link.head)) > io_queue_sqe_fallback(state->link.head); > /* flush only after queuing links as they can generate completions */ > @@ -2277,6 +2507,7 @@ static void io_submit_state_start(struct io_submit_state *state, > state->submit_nr = max_ios; > /* set only head, no need to init link_last in advance */ > state->link.head = NULL; > + state->group.head = NULL; > } > > static void io_commit_sqring(struct io_ring_ctx *ctx) > diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h > index 624ca9076a50..b11db3bdd8d8 100644 > --- a/io_uring/io_uring.h > +++ b/io_uring/io_uring.h > @@ -67,6 +67,8 @@ void io_req_defer_failed(struct io_kiocb *req, s32 res); > bool io_post_aux_cqe(struct io_ring_ctx *ctx, u64 user_data, s32 res, u32 cflags); > bool io_req_post_cqe(struct io_kiocb *req, s32 res, u32 cflags); > void __io_commit_cqring_flush(struct io_ring_ctx *ctx); > +void io_queue_group_members(struct io_kiocb *req, bool async); > +void io_cancel_group_members(struct io_kiocb *req, bool ignore_cqes); > > struct file *io_file_get_normal(struct io_kiocb *req, int fd); > struct file *io_file_get_fixed(struct io_kiocb *req, int fd, > @@ -342,6 +344,16 @@ static inline void io_tw_lock(struct io_ring_ctx *ctx, struct io_tw_state *ts) > lockdep_assert_held(&ctx->uring_lock); > } > > +static inline bool req_is_group_leader(struct io_kiocb *req) > +{ > + return req->flags & REQ_F_SQE_GROUP_LEADER; > +} > + > +static inline bool req_is_group_member(struct io_kiocb *req) > +{ > + return !req_is_group_leader(req) && (req->flags & REQ_F_SQE_GROUP); > +} > + > /* > * Don't complete immediately but use deferred completion infrastructure. > * Protected by ->uring_lock and can only be used either with > @@ -355,6 +367,10 @@ static inline void io_req_complete_defer(struct io_kiocb *req) > lockdep_assert_held(&req->ctx->uring_lock); > > wq_list_add_tail(&req->comp_list, &state->compl_reqs); > + > + /* members may not be issued when leader is completed */ > + if (unlikely(req_is_group_leader(req) && req->grp_link)) > + io_queue_group_members(req, false); > } > > static inline void io_commit_cqring_flush(struct io_ring_ctx *ctx)
On Mon, Jun 10, 2024 at 02:55:22AM +0100, Pavel Begunkov wrote: > On 5/21/24 03:58, Ming Lei wrote: > > On Sat, May 11, 2024 at 08:12:08AM +0800, Ming Lei wrote: > > > SQE group is defined as one chain of SQEs starting with the first SQE that > > > has IOSQE_SQE_GROUP set, and ending with the first subsequent SQE that > > > doesn't have it set, and it is similar with chain of linked SQEs. > > > > > > Not like linked SQEs, each sqe is issued after the previous one is completed. > > > All SQEs in one group are submitted in parallel, so there isn't any dependency > > > among SQEs in one group. > > > > > > The 1st SQE is group leader, and the other SQEs are group member. The whole > > > group share single IOSQE_IO_LINK and IOSQE_IO_DRAIN from group leader, and > > > the two flags are ignored for group members. > > > > > > When the group is in one link chain, this group isn't submitted until the > > > previous SQE or group is completed. And the following SQE or group can't > > > be started if this group isn't completed. Failure from any group member will > > > fail the group leader, then the link chain can be terminated. > > > > > > When IOSQE_IO_DRAIN is set for group leader, all requests in this group and > > > previous requests submitted are drained. Given IOSQE_IO_DRAIN can be set for > > > group leader only, we respect IO_DRAIN by always completing group leader as > > > the last one in the group. > > > > > > Working together with IOSQE_IO_LINK, SQE group provides flexible way to > > > support N:M dependency, such as: > > > > > > - group A is chained with group B together > > > - group A has N SQEs > > > - group B has M SQEs > > > > > > then M SQEs in group B depend on N SQEs in group A. > > > > > > N:M dependency can support some interesting use cases in efficient way: > > > > > > 1) read from multiple files, then write the read data into single file > > > > > > 2) read from single file, and write the read data into multiple files > > > > > > 3) write same data into multiple files, and read data from multiple files and > > > compare if correct data is written > > > > > > Also IOSQE_SQE_GROUP takes the last bit in sqe->flags, but we still can > > > extend sqe->flags with one uring context flag, such as use __pad3 for > > > non-uring_cmd OPs and part of uring_cmd_flags for uring_cmd OP. > > > > > > Suggested-by: Kevin Wolf <kwolf@redhat.com> > > > Signed-off-by: Ming Lei <ming.lei@redhat.com> > > > > BTW, I wrote one link-grp-cp.c liburing/example which is based on sqe group, > > and keep QD not changed, just re-organize IOs in the following ways: > > > > - each group have 4 READ IOs, linked by one single write IO for writing > > the read data in sqe group to destination file > > IIUC it's comparing 1 large write request with 4 small, and It is actually reasonable from storage device viewpoint, concurrent small READs are often fast than single big READ, but concurrent small writes are usually slower. > it's not exactly anything close to fair. And you can do same > in userspace (without links). And having control in userspace No, you can't do it with single syscall. Thanks, Ming
On Mon, Jun 10, 2024 at 03:53:51AM +0100, Pavel Begunkov wrote: > On 5/11/24 01:12, Ming Lei wrote: > > SQE group is defined as one chain of SQEs starting with the first SQE that > > has IOSQE_SQE_GROUP set, and ending with the first subsequent SQE that > > doesn't have it set, and it is similar with chain of linked SQEs. > > The main concern stays same, it adds overhead nearly to every > single hot function I can think of, as well as lots of > complexity. Almost every sqe group change is covered by REQ_F_SQE_GROUP, so I am not clear what the added overhead is. > > Another minor issue is REQ_F_INFLIGHT, as explained before, > cancellation has to be able to find all REQ_F_INFLIGHT > requests. Requests you add to a group can have that flag > but are not discoverable by core io_uring code. OK, we can deal with it by setting leader as REQ_F_INFLIGHT if the flag is set for any member, since all members are guaranteed to be drained when leader is completed. Will do it in V4. > > Another note, I'll be looking deeper into this patch, there > is too much of random tossing around of requests / refcounting > and other dependencies, as well as odd intertwinings with > other parts. The only thing wrt. request refcount is for io-wq, since request reference is grabbed when the req is handled in io-wq context, and group leader need to be completed after all members are done. That is all special change wrt. request refcounting. > > > Not like linked SQEs, each sqe is issued after the previous one is completed. > > All SQEs in one group are submitted in parallel, so there isn't any dependency > > among SQEs in one group. > > > > The 1st SQE is group leader, and the other SQEs are group member. The whole > > group share single IOSQE_IO_LINK and IOSQE_IO_DRAIN from group leader, and > > the two flags are ignored for group members. > > > > When the group is in one link chain, this group isn't submitted until the > > previous SQE or group is completed. And the following SQE or group can't > > be started if this group isn't completed. Failure from any group member will > > fail the group leader, then the link chain can be terminated. > > > > When IOSQE_IO_DRAIN is set for group leader, all requests in this group and > > previous requests submitted are drained. Given IOSQE_IO_DRAIN can be set for > > group leader only, we respect IO_DRAIN by always completing group leader as > > the last one in the group. > > > > Working together with IOSQE_IO_LINK, SQE group provides flexible way to > > support N:M dependency, such as: > > > > - group A is chained with group B together > > - group A has N SQEs > > - group B has M SQEs > > > > then M SQEs in group B depend on N SQEs in group A. > > > > N:M dependency can support some interesting use cases in efficient way: > > > > 1) read from multiple files, then write the read data into single file > > > > 2) read from single file, and write the read data into multiple files > > > > 3) write same data into multiple files, and read data from multiple files and > > compare if correct data is written > > > > Also IOSQE_SQE_GROUP takes the last bit in sqe->flags, but we still can > > extend sqe->flags with one uring context flag, such as use __pad3 for > > non-uring_cmd OPs and part of uring_cmd_flags for uring_cmd OP. > > > > Suggested-by: Kevin Wolf <kwolf@redhat.com> > > Signed-off-by: Ming Lei <ming.lei@redhat.com> > > --- > > include/linux/io_uring_types.h | 12 ++ > > include/uapi/linux/io_uring.h | 4 + > > io_uring/io_uring.c | 255 +++++++++++++++++++++++++++++++-- > > io_uring/io_uring.h | 16 +++ > > io_uring/timeout.c | 2 + > > 5 files changed, 277 insertions(+), 12 deletions(-) > > > > diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h > > index 7a6b190c7da7..62311b0f0e0b 100644 > > --- a/include/linux/io_uring_types.h > > +++ b/include/linux/io_uring_types.h > > @@ -666,6 +674,10 @@ struct io_kiocb { > > u64 extra1; > > u64 extra2; > > } big_cqe; > > + > > + /* all SQE group members linked here for group lead */ > > + struct io_kiocb *grp_link; > > + int grp_refs; > > }; > > struct io_overflow_cqe { > > diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c > > index c184c9a312df..b87c5452de43 100644 > > --- a/io_uring/io_uring.c > > +++ b/io_uring/io_uring.c > > @@ -109,7 +109,8 @@ > > IOSQE_IO_HARDLINK | IOSQE_ASYNC) > > #define SQE_VALID_FLAGS (SQE_COMMON_FLAGS | IOSQE_BUFFER_SELECT | \ > > - IOSQE_IO_DRAIN | IOSQE_CQE_SKIP_SUCCESS) > > + IOSQE_IO_DRAIN | IOSQE_CQE_SKIP_SUCCESS | \ > > + IOSQE_SQE_GROUP) > > #define IO_REQ_CLEAN_FLAGS (REQ_F_BUFFER_SELECTED | REQ_F_NEED_CLEANUP | \ > > REQ_F_POLLED | REQ_F_INFLIGHT | REQ_F_CREDS | \ > > @@ -915,6 +916,13 @@ static __always_inline void io_req_commit_cqe(struct io_kiocb *req, > > { > > struct io_ring_ctx *ctx = req->ctx; > > + /* > > + * For group leader, cqe has to be committed after all members are > > + * committed, when the request becomes normal one. > > + */ > > + if (unlikely(req_is_group_leader(req))) > > + return; > > The copy of it inlined into flush_completions should > maintain a proper fast path. > > if (req->flags & (CQE_SKIP | GROUP)) { > if (req->flags & CQE_SKIP) > continue; > if (req->flags & GROUP) {} OK, I will try to do that in above way. > } > > > + > > if (unlikely(!io_fill_cqe_req(ctx, req))) { > > if (lockless_cq) { > > spin_lock(&ctx->completion_lock); > > @@ -926,6 +934,116 @@ static __always_inline void io_req_commit_cqe(struct io_kiocb *req, > > } > > } > > +static inline bool need_queue_group_members(struct io_kiocb *req) > > +{ > > + return req_is_group_leader(req) && req->grp_link; > > +} > > + > > +/* Can only be called after this request is issued */ > > +static inline struct io_kiocb *get_group_leader(struct io_kiocb *req) > > +{ > > + if (req->flags & REQ_F_SQE_GROUP) { > > + if (req_is_group_leader(req)) > > + return req; > > + return req->grp_link; > > I'm missing something, it seems io_group_sqe() adding all > requests of a group into a singly linked list via ->grp_link, > but here we return it as a leader. Confused. ->grp_link stores the singly linked list for group leader, and the same field stores the group leader pointer for group member requests. For later, we can add one union field to make code more readable. Will do that in V4. > > > + } > > + return NULL; > > +} > > + > > +void io_cancel_group_members(struct io_kiocb *req, bool ignore_cqes) > > +{ > > + struct io_kiocb *member = req->grp_link; > > + > > + while (member) { > > + struct io_kiocb *next = member->grp_link; > > + > > + if (ignore_cqes) > > + member->flags |= REQ_F_CQE_SKIP; > > + if (!(member->flags & REQ_F_FAIL)) { > > + req_set_fail(member); > > + io_req_set_res(member, -ECANCELED, 0); > > + } > > + member = next; > > + } > > +} > > + > > +void io_queue_group_members(struct io_kiocb *req, bool async) > > +{ > > + struct io_kiocb *member = req->grp_link; > > + > > + if (!member) > > + return; > > + > > + while (member) { > > + struct io_kiocb *next = member->grp_link; > > + > > + member->grp_link = req; > > + if (async) > > + member->flags |= REQ_F_FORCE_ASYNC; > > + > > + if (unlikely(member->flags & REQ_F_FAIL)) { > > + io_req_task_queue_fail(member, member->cqe.res); > > + } else if (member->flags & REQ_F_FORCE_ASYNC) { > > + io_req_task_queue(member); > > + } else { > > + io_queue_sqe(member); > > + } > > + member = next; > > + } > > + req->grp_link = NULL; > > +} > > + > > +static inline bool __io_complete_group_req(struct io_kiocb *req, > > + struct io_kiocb *lead) > > +{ > > + WARN_ON_ONCE(!(req->flags & REQ_F_SQE_GROUP)); > > + > > + if (WARN_ON_ONCE(lead->grp_refs <= 0)) > > + return false; > > + > > + /* > > + * Set linked leader as failed if any member is failed, so > > + * the remained link chain can be terminated > > + */ > > + if (unlikely((req->flags & REQ_F_FAIL) && > > + ((lead->flags & IO_REQ_LINK_FLAGS) && lead->link))) > > + req_set_fail(lead); > > + return !--lead->grp_refs; > > +} > > + > > +/* Complete group request and collect completed leader for freeing */ > > +static inline void io_complete_group_req(struct io_kiocb *req, > > + struct io_wq_work_list *grp_list) > > +{ > > + struct io_kiocb *lead = get_group_leader(req); > > + > > + if (__io_complete_group_req(req, lead)) { > > + req->flags &= ~REQ_F_SQE_GROUP; > > + lead->flags &= ~REQ_F_SQE_GROUP_LEADER; > > + if (!(lead->flags & REQ_F_CQE_SKIP)) > > + io_req_commit_cqe(lead, lead->ctx->lockless_cq); > > + > > + if (req != lead) { > > + /* > > + * Add leader to free list if it isn't there > > + * otherwise clearing group flag for freeing it > > + * in current batch > > + */ > > + if (!(lead->flags & REQ_F_SQE_GROUP)) > > + wq_list_add_tail(&lead->comp_list, grp_list); > > + else > > + lead->flags &= ~REQ_F_SQE_GROUP; > > + } > > + } else if (req != lead) { > > + req->flags &= ~REQ_F_SQE_GROUP; > > + } else { > > + /* > > + * Leader's group flag clearing is delayed until it is > > + * removed from free list > > + */ > > + } > > +} > > + > > static void io_req_complete_post(struct io_kiocb *req, unsigned issue_flags) > > { > > struct io_ring_ctx *ctx = req->ctx; > > @@ -1427,6 +1545,17 @@ static void io_free_batch_list(struct io_ring_ctx *ctx, > > comp_list); > > if (unlikely(req->flags & IO_REQ_CLEAN_SLOW_FLAGS)) { > > + /* > > + * Group leader may be removed twice, don't free it > > + * if group flag isn't cleared, when some members > > + * aren't completed yet > > + */ > > + if (req->flags & REQ_F_SQE_GROUP) { > > + node = req->comp_list.next; > > + req->flags &= ~REQ_F_SQE_GROUP; > > + continue; > > + } > > + > > if (req->flags & REQ_F_REFCOUNT) { > > node = req->comp_list.next; > > if (!req_ref_put_and_test(req)) > > @@ -1459,6 +1588,7 @@ void __io_submit_flush_completions(struct io_ring_ctx *ctx) > > __must_hold(&ctx->uring_lock) > > { > > struct io_submit_state *state = &ctx->submit_state; > > + struct io_wq_work_list grp_list = {NULL}; > > struct io_wq_work_node *node; > > __io_cq_lock(ctx); > > @@ -1468,9 +1598,15 @@ void __io_submit_flush_completions(struct io_ring_ctx *ctx) > > if (!(req->flags & REQ_F_CQE_SKIP)) > > io_req_commit_cqe(req, ctx->lockless_cq); > > + > > + if (req->flags & REQ_F_SQE_GROUP) > > Same note about hot path > > > + io_complete_group_req(req, &grp_list); > > } > > __io_cq_unlock_post(ctx); > > + if (!wq_list_empty(&grp_list)) > > + __wq_list_splice(&grp_list, state->compl_reqs.first); > > What's the point of splicing it here insted of doing all > that under REQ_F_SQE_GROUP above? As mentioned, group leader can't be completed until all members are done, so any leaders in the current list have to be moved to this local list for deferred completion. That should be the only tricky part of the whole sqe group implementation. > > > + > > if (!wq_list_empty(&ctx->submit_state.compl_reqs)) { > > io_free_batch_list(ctx, state->compl_reqs.first); > > INIT_WQ_LIST(&state->compl_reqs); > > @@ -1677,8 +1813,12 @@ static u32 io_get_sequence(struct io_kiocb *req) > > struct io_kiocb *cur; > > /* need original cached_sq_head, but it was increased for each req */ > > - io_for_each_link(cur, req) > > - seq--; > > + io_for_each_link(cur, req) { > > + if (req_is_group_leader(cur)) > > + seq -= cur->grp_refs; > > + else > > + seq--; > > + } > > return seq; > > } > > @@ -1793,11 +1933,20 @@ struct io_wq_work *io_wq_free_work(struct io_wq_work *work) > > struct io_kiocb *nxt = NULL; > > if (req_ref_put_and_test(req)) { > > - if (req->flags & IO_REQ_LINK_FLAGS) > > - nxt = io_req_find_next(req); > > + /* > > + * CQEs have been posted in io_req_complete_post() except > > + * for group leader, and we can't advance the link for > > + * group leader until its CQE is posted. > > + * > > + * TODO: try to avoid defer and complete leader in io_wq > > + * context directly > > + */ > > + if (!req_is_group_leader(req)) { > > + req->flags |= REQ_F_CQE_SKIP; > > + if (req->flags & IO_REQ_LINK_FLAGS) > > + nxt = io_req_find_next(req); > > + } > > - /* we have posted CQEs in io_req_complete_post() */ > > - req->flags |= REQ_F_CQE_SKIP; > > io_free_req(req); > > } > > return nxt ? &nxt->work : NULL; > > @@ -1863,6 +2012,8 @@ void io_wq_submit_work(struct io_wq_work *work) > > } > > } > > + if (need_queue_group_members(req)) > > + io_queue_group_members(req, true); > > do { > > ret = io_issue_sqe(req, issue_flags); > > if (ret != -EAGAIN) > > @@ -1977,6 +2128,9 @@ static inline void io_queue_sqe(struct io_kiocb *req) > > */ > > if (unlikely(ret)) > > io_queue_async(req, ret); > > + > > + if (need_queue_group_members(req)) > > + io_queue_group_members(req, false); > > Request ownership is considered to be handed further at this > point and requests should not be touched. Only ret==0 from > io_issue_sqe it's still ours, but again it's handed somewhere > by io_queue_async(). Yes, you are right. And it has been fixed in my local tree: @@ -2154,8 +2154,7 @@ static inline void io_queue_sqe(struct io_kiocb *req) */ if (unlikely(ret)) io_queue_async(req, ret); - - if (need_queue_group_members(req)) + else if (need_queue_group_members(req)) io_queue_group_members(req, false); } > > > } > > static void io_queue_sqe_fallback(struct io_kiocb *req) > > @@ -2142,6 +2296,56 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, > > return def->prep(req, sqe); > > } > > +static struct io_kiocb *io_group_sqe(struct io_submit_link *group, > > + struct io_kiocb *req) > > +{ > > + /* > > + * Group chain is similar with link chain: starts with 1st sqe with > > + * REQ_F_SQE_GROUP, and ends with the 1st sqe without REQ_F_SQE_GROUP > > + */ > > + if (group->head) { > > + struct io_kiocb *lead = group->head; > > + > > + /* members can't be in link chain, can't be drained */ > > + req->flags &= ~(IO_REQ_LINK_FLAGS | REQ_F_IO_DRAIN); > > + lead->grp_refs += 1; > > + group->last->grp_link = req; > > + group->last = req; > > + > > + if (req->flags & REQ_F_SQE_GROUP) > > + return NULL; > > + > > + req->grp_link = NULL; > > + req->flags |= REQ_F_SQE_GROUP; > > + group->head = NULL; > > + return lead; > > + } else if (req->flags & REQ_F_SQE_GROUP) { > > + group->head = req; > > + group->last = req; > > + req->grp_refs = 1; > > + req->flags |= REQ_F_SQE_GROUP_LEADER; > > + return NULL; > > + } else { > > + return req; > > + } > > +} > > + > > +static __cold struct io_kiocb *io_submit_fail_group( > > + struct io_submit_link *link, struct io_kiocb *req) > > +{ > > + struct io_kiocb *lead = link->head; > > + > > + /* > > + * Instead of failing eagerly, continue assembling the group link > > + * if applicable and mark the leader with REQ_F_FAIL. The group > > + * flushing code should find the flag and handle the rest > > + */ > > + if (lead && (lead->flags & IO_REQ_LINK_FLAGS) && !(lead->flags & REQ_F_FAIL)) > > + req_fail_link_node(lead, -ECANCELED); > > + > > + return io_group_sqe(link, req); > > +} > > + > > static __cold int io_submit_fail_link(struct io_submit_link *link, > > struct io_kiocb *req, int ret) > > { > > @@ -2180,11 +2384,18 @@ static __cold int io_submit_fail_init(const struct io_uring_sqe *sqe, > > { > > struct io_ring_ctx *ctx = req->ctx; > > struct io_submit_link *link = &ctx->submit_state.link; > > + struct io_submit_link *group = &ctx->submit_state.group; > > trace_io_uring_req_failed(sqe, req, ret); > > req_fail_link_node(req, ret); > > + if (group->head || (req->flags & REQ_F_SQE_GROUP)) { > > + req = io_submit_fail_group(group, req); > > + if (!req) > > + return 0; > > + } > > + > > /* cover both linked and non-linked request */ > > return io_submit_fail_link(link, req, ret); > > } > > @@ -2232,7 +2443,7 @@ static inline int io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req, > > const struct io_uring_sqe *sqe) > > __must_hold(&ctx->uring_lock) > > { > > - struct io_submit_link *link = &ctx->submit_state.link; > > + struct io_submit_state *state = &ctx->submit_state; > > int ret; > > ret = io_init_req(ctx, req, sqe); > > @@ -2241,9 +2452,17 @@ static inline int io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req, > > trace_io_uring_submit_req(req); > > - if (unlikely(link->head || (req->flags & (IO_REQ_LINK_FLAGS | > > - REQ_F_FORCE_ASYNC | REQ_F_FAIL)))) { > > - req = io_link_sqe(link, req); > > + if (unlikely(state->group.head || > > A note rather to myself and for the future, all theese checks > including links and groups can be folded under one common if. Sorry, I may not get the idea, can you provide one example? We need different logics for group and link, meantime group has to be handled first before linking, since only the group leader can be linked. Thanks, Ming
On 6/11/24 14:32, Ming Lei wrote: > On Mon, Jun 10, 2024 at 02:55:22AM +0100, Pavel Begunkov wrote: >> On 5/21/24 03:58, Ming Lei wrote: >>> On Sat, May 11, 2024 at 08:12:08AM +0800, Ming Lei wrote: >>>> SQE group is defined as one chain of SQEs starting with the first SQE that >>>> has IOSQE_SQE_GROUP set, and ending with the first subsequent SQE that >>>> doesn't have it set, and it is similar with chain of linked SQEs. >>>> >>>> Not like linked SQEs, each sqe is issued after the previous one is completed. >>>> All SQEs in one group are submitted in parallel, so there isn't any dependency >>>> among SQEs in one group. >>>> >>>> The 1st SQE is group leader, and the other SQEs are group member. The whole >>>> group share single IOSQE_IO_LINK and IOSQE_IO_DRAIN from group leader, and >>>> the two flags are ignored for group members. >>>> >>>> When the group is in one link chain, this group isn't submitted until the >>>> previous SQE or group is completed. And the following SQE or group can't >>>> be started if this group isn't completed. Failure from any group member will >>>> fail the group leader, then the link chain can be terminated. >>>> >>>> When IOSQE_IO_DRAIN is set for group leader, all requests in this group and >>>> previous requests submitted are drained. Given IOSQE_IO_DRAIN can be set for >>>> group leader only, we respect IO_DRAIN by always completing group leader as >>>> the last one in the group. >>>> >>>> Working together with IOSQE_IO_LINK, SQE group provides flexible way to >>>> support N:M dependency, such as: >>>> >>>> - group A is chained with group B together >>>> - group A has N SQEs >>>> - group B has M SQEs >>>> >>>> then M SQEs in group B depend on N SQEs in group A. >>>> >>>> N:M dependency can support some interesting use cases in efficient way: >>>> >>>> 1) read from multiple files, then write the read data into single file >>>> >>>> 2) read from single file, and write the read data into multiple files >>>> >>>> 3) write same data into multiple files, and read data from multiple files and >>>> compare if correct data is written >>>> >>>> Also IOSQE_SQE_GROUP takes the last bit in sqe->flags, but we still can >>>> extend sqe->flags with one uring context flag, such as use __pad3 for >>>> non-uring_cmd OPs and part of uring_cmd_flags for uring_cmd OP. >>>> >>>> Suggested-by: Kevin Wolf <kwolf@redhat.com> >>>> Signed-off-by: Ming Lei <ming.lei@redhat.com> >>> >>> BTW, I wrote one link-grp-cp.c liburing/example which is based on sqe group, >>> and keep QD not changed, just re-organize IOs in the following ways: >>> >>> - each group have 4 READ IOs, linked by one single write IO for writing >>> the read data in sqe group to destination file >> >> IIUC it's comparing 1 large write request with 4 small, and > > It is actually reasonable from storage device viewpoint, concurrent > small READs are often fast than single big READ, but concurrent small > writes are usually slower. It is, but that doesn't make the comparison apple to apple. Even what I described, even though it's better (same number of syscalls but better parallelism as you don't block next batch of reads by writes), you can argues it's not a completely fair comparison either since needs different number of buffers, etc. >> it's not exactly anything close to fair. And you can do same >> in userspace (without links). And having control in userspace > > No, you can't do it with single syscall. That's called you _can_ do it. And syscalls is not everything, context switching turned to be a bigger problem, and to execute links it does exactly that.
On 6/13/24 02:45, Ming Lei wrote: > On Mon, Jun 10, 2024 at 03:53:51AM +0100, Pavel Begunkov wrote: >> On 5/11/24 01:12, Ming Lei wrote: >>> SQE group is defined as one chain of SQEs starting with the first SQE that >>> has IOSQE_SQE_GROUP set, and ending with the first subsequent SQE that >>> doesn't have it set, and it is similar with chain of linked SQEs. >> >> The main concern stays same, it adds overhead nearly to every >> single hot function I can think of, as well as lots of >> complexity. > > Almost every sqe group change is covered by REQ_F_SQE_GROUP, so I am > not clear what the added overhead is. Yes, and there is a dozen of such in the hot path. >> Another minor issue is REQ_F_INFLIGHT, as explained before, >> cancellation has to be able to find all REQ_F_INFLIGHT >> requests. Requests you add to a group can have that flag >> but are not discoverable by core io_uring code. > > OK, we can deal with it by setting leader as REQ_F_INFLIGHT if the > flag is set for any member, since all members are guaranteed to > be drained when leader is completed. Will do it in V4. Or fail if see one, that's also fine. REQ_F_INFLIGHT is only set for POLL requests polling another io_uring. >> Another note, I'll be looking deeper into this patch, there >> is too much of random tossing around of requests / refcounting >> and other dependencies, as well as odd intertwinings with >> other parts. > > The only thing wrt. request refcount is for io-wq, since request > reference is grabbed when the req is handled in io-wq context, and > group leader need to be completed after all members are done. That > is all special change wrt. request refcounting. I rather mean refcounting the group leader, even if it's not atomic. >>> diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h >>> index 7a6b190c7da7..62311b0f0e0b 100644 >>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c >>> index c184c9a312df..b87c5452de43 100644 >>> --- a/io_uring/io_uring.c >>> +++ b/io_uring/io_uring.c ... >>> } >>> } >>> +static inline bool need_queue_group_members(struct io_kiocb *req) >>> +{ >>> + return req_is_group_leader(req) && req->grp_link; >>> +} >>> + >>> +/* Can only be called after this request is issued */ >>> +static inline struct io_kiocb *get_group_leader(struct io_kiocb *req) >>> +{ >>> + if (req->flags & REQ_F_SQE_GROUP) { >>> + if (req_is_group_leader(req)) >>> + return req; >>> + return req->grp_link; >> >> I'm missing something, it seems io_group_sqe() adding all >> requests of a group into a singly linked list via ->grp_link, >> but here we return it as a leader. Confused. > > ->grp_link stores the singly linked list for group leader, and > the same field stores the group leader pointer for group member requests. > For later, we can add one union field to make code more readable. > Will do that in V4. So you're repurposing it in io_queue_group_members(). Since it has different meaning at different stages of execution, it warrants a comment (unless there is one I missed). >>> + } >>> + return NULL; >>> +} >>> + >>> +void io_cancel_group_members(struct io_kiocb *req, bool ignore_cqes) >>> +{ >>> + struct io_kiocb *member = req->grp_link; >>> + >>> + while (member) { >>> + struct io_kiocb *next = member->grp_link; >>> + >>> + if (ignore_cqes) >>> + member->flags |= REQ_F_CQE_SKIP; >>> + if (!(member->flags & REQ_F_FAIL)) { >>> + req_set_fail(member); >>> + io_req_set_res(member, -ECANCELED, 0); >>> + } >>> + member = next; >>> + } >>> +} >>> + >>> +void io_queue_group_members(struct io_kiocb *req, bool async) >>> +{ >>> + struct io_kiocb *member = req->grp_link; >>> + >>> + if (!member) >>> + return; >>> + >>> + while (member) { >>> + struct io_kiocb *next = member->grp_link; >>> + >>> + member->grp_link = req; >>> + if (async) >>> + member->flags |= REQ_F_FORCE_ASYNC; >>> + >>> + if (unlikely(member->flags & REQ_F_FAIL)) { >>> + io_req_task_queue_fail(member, member->cqe.res); >>> + } else if (member->flags & REQ_F_FORCE_ASYNC) { >>> + io_req_task_queue(member); >>> + } else { >>> + io_queue_sqe(member); io_req_queue_tw_complete() please, just like links deal with it, so it's executed in a well known context without jumping ahead of other requests. >>> + } >>> + member = next; >>> + } >>> + req->grp_link = NULL; >>> +} >>> + >>> +static inline bool __io_complete_group_req(struct io_kiocb *req, >>> + struct io_kiocb *lead) >>> +{ >>> + WARN_ON_ONCE(!(req->flags & REQ_F_SQE_GROUP)); >>> + >>> + if (WARN_ON_ONCE(lead->grp_refs <= 0)) >>> + return false; >>> + >>> + /* >>> + * Set linked leader as failed if any member is failed, so >>> + * the remained link chain can be terminated >>> + */ >>> + if (unlikely((req->flags & REQ_F_FAIL) && >>> + ((lead->flags & IO_REQ_LINK_FLAGS) && lead->link))) >>> + req_set_fail(lead); >>> + return !--lead->grp_refs; >>> +} >>> + >>> +/* Complete group request and collect completed leader for freeing */ >>> +static inline void io_complete_group_req(struct io_kiocb *req, >>> + struct io_wq_work_list *grp_list) >>> +{ >>> + struct io_kiocb *lead = get_group_leader(req); >>> + >>> + if (__io_complete_group_req(req, lead)) { >>> + req->flags &= ~REQ_F_SQE_GROUP; >>> + lead->flags &= ~REQ_F_SQE_GROUP_LEADER; >>> + if (!(lead->flags & REQ_F_CQE_SKIP)) >>> + io_req_commit_cqe(lead, lead->ctx->lockless_cq); >>> + >>> + if (req != lead) { >>> + /* >>> + * Add leader to free list if it isn't there >>> + * otherwise clearing group flag for freeing it >>> + * in current batch >>> + */ >>> + if (!(lead->flags & REQ_F_SQE_GROUP)) >>> + wq_list_add_tail(&lead->comp_list, grp_list); >>> + else >>> + lead->flags &= ~REQ_F_SQE_GROUP; >>> + } >>> + } else if (req != lead) { >>> + req->flags &= ~REQ_F_SQE_GROUP; >>> + } else { >>> + /* >>> + * Leader's group flag clearing is delayed until it is >>> + * removed from free list >>> + */ >>> + } >>> +} >>> + >>> static void io_req_complete_post(struct io_kiocb *req, unsigned issue_flags) >>> { >>> struct io_ring_ctx *ctx = req->ctx; >>> @@ -1427,6 +1545,17 @@ static void io_free_batch_list(struct io_ring_ctx *ctx, >>> comp_list); >>> if (unlikely(req->flags & IO_REQ_CLEAN_SLOW_FLAGS)) { >>> + /* >>> + * Group leader may be removed twice, don't free it >>> + * if group flag isn't cleared, when some members >>> + * aren't completed yet >>> + */ >>> + if (req->flags & REQ_F_SQE_GROUP) { >>> + node = req->comp_list.next; >>> + req->flags &= ~REQ_F_SQE_GROUP; >>> + continue; >>> + } >>> + >>> if (req->flags & REQ_F_REFCOUNT) { >>> node = req->comp_list.next; >>> if (!req_ref_put_and_test(req)) >>> @@ -1459,6 +1588,7 @@ void __io_submit_flush_completions(struct io_ring_ctx *ctx) >>> __must_hold(&ctx->uring_lock) >>> { >>> struct io_submit_state *state = &ctx->submit_state; >>> + struct io_wq_work_list grp_list = {NULL}; >>> struct io_wq_work_node *node; >>> __io_cq_lock(ctx); >>> @@ -1468,9 +1598,15 @@ void __io_submit_flush_completions(struct io_ring_ctx *ctx) >>> if (!(req->flags & REQ_F_CQE_SKIP)) >>> io_req_commit_cqe(req, ctx->lockless_cq); >>> + >>> + if (req->flags & REQ_F_SQE_GROUP) >> >> Same note about hot path >> >>> + io_complete_group_req(req, &grp_list); >>> } >>> __io_cq_unlock_post(ctx); >>> + if (!wq_list_empty(&grp_list)) >>> + __wq_list_splice(&grp_list, state->compl_reqs.first); >> >> What's the point of splicing it here insted of doing all >> that under REQ_F_SQE_GROUP above? > > As mentioned, group leader can't be completed until all members are > done, so any leaders in the current list have to be moved to this > local list for deferred completion. That should be the only tricky > part of the whole sqe group implementation. > >> >>> + >>> if (!wq_list_empty(&ctx->submit_state.compl_reqs)) { >>> io_free_batch_list(ctx, state->compl_reqs.first); >>> INIT_WQ_LIST(&state->compl_reqs); ... >>> @@ -1863,6 +2012,8 @@ void io_wq_submit_work(struct io_wq_work *work) >>> } >>> } >>> + if (need_queue_group_members(req)) >>> + io_queue_group_members(req, true); >>> do { >>> ret = io_issue_sqe(req, issue_flags); >>> if (ret != -EAGAIN) >>> @@ -1977,6 +2128,9 @@ static inline void io_queue_sqe(struct io_kiocb *req) >>> */ >>> if (unlikely(ret)) >>> io_queue_async(req, ret); >>> + >>> + if (need_queue_group_members(req)) >>> + io_queue_group_members(req, false); >> >> Request ownership is considered to be handed further at this >> point and requests should not be touched. Only ret==0 from >> io_issue_sqe it's still ours, but again it's handed somewhere >> by io_queue_async(). > > Yes, you are right. > > And it has been fixed in my local tree: > > @@ -2154,8 +2154,7 @@ static inline void io_queue_sqe(struct io_kiocb *req) > */ > if (unlikely(ret)) > io_queue_async(req, ret); > - > - if (need_queue_group_members(req)) > + else if (need_queue_group_members(req)) > io_queue_group_members(req, false); > } In the else branch you don't own the request anymore and shouldn't be poking into it. It looks like you're trying to do io_queue_group_members() when previously the request would get completed. It's not the right place, and apart from whack'a'moled io_wq_submit_work() there is also io_poll_issue() missed. Seems __io_submit_flush_completions() / io_free_batch_list() would be more appropriate, and you already have a chunk with GROUP check in there handling the leader appearing in there twice. >>> } >>> static void io_queue_sqe_fallback(struct io_kiocb *req) ... >>> @@ -2232,7 +2443,7 @@ static inline int io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req, >>> const struct io_uring_sqe *sqe) >>> __must_hold(&ctx->uring_lock) >>> { >>> - struct io_submit_link *link = &ctx->submit_state.link; >>> + struct io_submit_state *state = &ctx->submit_state; >>> int ret; >>> ret = io_init_req(ctx, req, sqe); >>> @@ -2241,9 +2452,17 @@ static inline int io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req, >>> trace_io_uring_submit_req(req); >>> - if (unlikely(link->head || (req->flags & (IO_REQ_LINK_FLAGS | >>> - REQ_F_FORCE_ASYNC | REQ_F_FAIL)))) { >>> - req = io_link_sqe(link, req); >>> + if (unlikely(state->group.head || >> >> A note rather to myself and for the future, all theese checks >> including links and groups can be folded under one common if. > > Sorry, I may not get the idea, can you provide one example? To be clear, not suggesting you doing it. Simplifying: init_req() { if (req->flags & GROUP|LINK) { ctx->assembling; } } io_submit_sqe() { init_req(); if (ctx->assembling) { check_groups/links(); if (done); ctx->assembling = false; } } > > We need different logics for group and link, meantime group > has to be handled first before linking, since only the group leader > can be linked.
On Sun, Jun 16, 2024 at 07:14:37PM +0100, Pavel Begunkov wrote: > On 6/11/24 14:32, Ming Lei wrote: > > On Mon, Jun 10, 2024 at 02:55:22AM +0100, Pavel Begunkov wrote: > > > On 5/21/24 03:58, Ming Lei wrote: > > > > On Sat, May 11, 2024 at 08:12:08AM +0800, Ming Lei wrote: > > > > > SQE group is defined as one chain of SQEs starting with the first SQE that > > > > > has IOSQE_SQE_GROUP set, and ending with the first subsequent SQE that > > > > > doesn't have it set, and it is similar with chain of linked SQEs. > > > > > > > > > > Not like linked SQEs, each sqe is issued after the previous one is completed. > > > > > All SQEs in one group are submitted in parallel, so there isn't any dependency > > > > > among SQEs in one group. > > > > > > > > > > The 1st SQE is group leader, and the other SQEs are group member. The whole > > > > > group share single IOSQE_IO_LINK and IOSQE_IO_DRAIN from group leader, and > > > > > the two flags are ignored for group members. > > > > > > > > > > When the group is in one link chain, this group isn't submitted until the > > > > > previous SQE or group is completed. And the following SQE or group can't > > > > > be started if this group isn't completed. Failure from any group member will > > > > > fail the group leader, then the link chain can be terminated. > > > > > > > > > > When IOSQE_IO_DRAIN is set for group leader, all requests in this group and > > > > > previous requests submitted are drained. Given IOSQE_IO_DRAIN can be set for > > > > > group leader only, we respect IO_DRAIN by always completing group leader as > > > > > the last one in the group. > > > > > > > > > > Working together with IOSQE_IO_LINK, SQE group provides flexible way to > > > > > support N:M dependency, such as: > > > > > > > > > > - group A is chained with group B together > > > > > - group A has N SQEs > > > > > - group B has M SQEs > > > > > > > > > > then M SQEs in group B depend on N SQEs in group A. > > > > > > > > > > N:M dependency can support some interesting use cases in efficient way: > > > > > > > > > > 1) read from multiple files, then write the read data into single file > > > > > > > > > > 2) read from single file, and write the read data into multiple files > > > > > > > > > > 3) write same data into multiple files, and read data from multiple files and > > > > > compare if correct data is written > > > > > > > > > > Also IOSQE_SQE_GROUP takes the last bit in sqe->flags, but we still can > > > > > extend sqe->flags with one uring context flag, such as use __pad3 for > > > > > non-uring_cmd OPs and part of uring_cmd_flags for uring_cmd OP. > > > > > > > > > > Suggested-by: Kevin Wolf <kwolf@redhat.com> > > > > > Signed-off-by: Ming Lei <ming.lei@redhat.com> > > > > > > > > BTW, I wrote one link-grp-cp.c liburing/example which is based on sqe group, > > > > and keep QD not changed, just re-organize IOs in the following ways: > > > > > > > > - each group have 4 READ IOs, linked by one single write IO for writing > > > > the read data in sqe group to destination file > > > > > > IIUC it's comparing 1 large write request with 4 small, and > > > > It is actually reasonable from storage device viewpoint, concurrent > > small READs are often fast than single big READ, but concurrent small > > writes are usually slower. > > It is, but that doesn't make the comparison apple to apple. > Even what I described, even though it's better (same number > of syscalls but better parallelism as you don't block next > batch of reads by writes), you can argues it's not a > completely fair comparison either since needs different number > of buffers, etc. > > > > it's not exactly anything close to fair. And you can do same > > > in userspace (without links). And having control in userspace > > > > No, you can't do it with single syscall. > > That's called you _can_ do it. And syscalls is not everything, For ublk, syscall does mean something, because each ublk IO is handled by io_uring, if more syscalls are introduced for each ublk IO, performance definitely degrades a lot because IOPS can be million level. Now syscall PTI overhead does make difference, please see: https://lwn.net/Articles/752587/ > context switching turned to be a bigger problem, and to execute > links it does exactly that. If that is true, IO_LINK shouldn't have been needed, cause you can model dependency via io_uring syscall, unfortunately it isn't true. IO_LINK not only simplifies application programming, but also avoids extra syscall. If you compare io_uring-cp.c(282 LOC) with link-cp.c(193 LOC) in liburing/examples, you can see io_uring-cp.c is more complicated. Adding one extra syscall(wait point) makes application hard to write, especially in modern async/.await programming environment. Thanks, Ming
On Sun, Jun 16, 2024 at 08:13:26PM +0100, Pavel Begunkov wrote: > On 6/13/24 02:45, Ming Lei wrote: > > On Mon, Jun 10, 2024 at 03:53:51AM +0100, Pavel Begunkov wrote: > > > On 5/11/24 01:12, Ming Lei wrote: > > > > SQE group is defined as one chain of SQEs starting with the first SQE that > > > > has IOSQE_SQE_GROUP set, and ending with the first subsequent SQE that > > > > doesn't have it set, and it is similar with chain of linked SQEs. > > > > > > The main concern stays same, it adds overhead nearly to every > > > single hot function I can think of, as well as lots of > > > complexity. > > > > Almost every sqe group change is covered by REQ_F_SQE_GROUP, so I am > > not clear what the added overhead is. > > Yes, and there is a dozen of such in the hot path. req->flags is supposed to be L1-cached in all these hot paths, and the check is basically zero cost, so SQE_GROUP shouldn't add extra cost for existed io_uring core code path. > > > > Another minor issue is REQ_F_INFLIGHT, as explained before, > > > cancellation has to be able to find all REQ_F_INFLIGHT > > > requests. Requests you add to a group can have that flag > > > but are not discoverable by core io_uring code. > > > > OK, we can deal with it by setting leader as REQ_F_INFLIGHT if the > > flag is set for any member, since all members are guaranteed to > > be drained when leader is completed. Will do it in V4. > > Or fail if see one, that's also fine. REQ_F_INFLIGHT is > only set for POLL requests polling another io_uring. It is set for read-write/tee/splice op with normal file too, so looks not safe to fail. > > > > Another note, I'll be looking deeper into this patch, there > > > is too much of random tossing around of requests / refcounting > > > and other dependencies, as well as odd intertwinings with > > > other parts. > > > > The only thing wrt. request refcount is for io-wq, since request > > reference is grabbed when the req is handled in io-wq context, and > > group leader need to be completed after all members are done. That > > is all special change wrt. request refcounting. > > I rather mean refcounting the group leader, even if it's not > atomic. If you mean reusing req->refs for refcounting the group leader, it may not work, cause member can complete from io-wq, but leader may not. Meantime using dedicated ->grp_refs actually simplifies things a lot. > > > > > diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h > > > > index 7a6b190c7da7..62311b0f0e0b 100644 > > > > diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c > > > > index c184c9a312df..b87c5452de43 100644 > > > > --- a/io_uring/io_uring.c > > > > +++ b/io_uring/io_uring.c > ... > > > > } > > > > } > > > > +static inline bool need_queue_group_members(struct io_kiocb *req) > > > > +{ > > > > + return req_is_group_leader(req) && req->grp_link; > > > > +} > > > > + > > > > +/* Can only be called after this request is issued */ > > > > +static inline struct io_kiocb *get_group_leader(struct io_kiocb *req) > > > > +{ > > > > + if (req->flags & REQ_F_SQE_GROUP) { > > > > + if (req_is_group_leader(req)) > > > > + return req; > > > > + return req->grp_link; > > > > > > I'm missing something, it seems io_group_sqe() adding all > > > requests of a group into a singly linked list via ->grp_link, > > > but here we return it as a leader. Confused. > > > > ->grp_link stores the singly linked list for group leader, and > > the same field stores the group leader pointer for group member requests. > > For later, we can add one union field to make code more readable. > > Will do that in V4. > > So you're repurposing it in io_queue_group_members(). Since > it has different meaning at different stages of execution, > it warrants a comment (unless there is one I missed). OK, either adding comment or another union field for it. > > > > > + } > > > > + return NULL; > > > > +} > > > > + > > > > +void io_cancel_group_members(struct io_kiocb *req, bool ignore_cqes) > > > > +{ > > > > + struct io_kiocb *member = req->grp_link; > > > > + > > > > + while (member) { > > > > + struct io_kiocb *next = member->grp_link; > > > > + > > > > + if (ignore_cqes) > > > > + member->flags |= REQ_F_CQE_SKIP; > > > > + if (!(member->flags & REQ_F_FAIL)) { > > > > + req_set_fail(member); > > > > + io_req_set_res(member, -ECANCELED, 0); > > > > + } > > > > + member = next; > > > > + } > > > > +} > > > > + > > > > +void io_queue_group_members(struct io_kiocb *req, bool async) > > > > +{ > > > > + struct io_kiocb *member = req->grp_link; > > > > + > > > > + if (!member) > > > > + return; > > > > + > > > > + while (member) { > > > > + struct io_kiocb *next = member->grp_link; > > > > + > > > > + member->grp_link = req; > > > > + if (async) > > > > + member->flags |= REQ_F_FORCE_ASYNC; > > > > + > > > > + if (unlikely(member->flags & REQ_F_FAIL)) { > > > > + io_req_task_queue_fail(member, member->cqe.res); > > > > + } else if (member->flags & REQ_F_FORCE_ASYNC) { > > > > + io_req_task_queue(member); > > > > + } else { > > > > + io_queue_sqe(member); > > io_req_queue_tw_complete() please, just like links deal > with it, so it's executed in a well known context without > jumping ahead of other requests. members needn't to be queued until leader is completed for plain SQE_GROUP, otherwise perf can drop. > > > > > + } > > > > + member = next; > > > > + } > > > > + req->grp_link = NULL; > > > > +} > > > > + > > > > +static inline bool __io_complete_group_req(struct io_kiocb *req, > > > > + struct io_kiocb *lead) > > > > +{ > > > > + WARN_ON_ONCE(!(req->flags & REQ_F_SQE_GROUP)); > > > > + > > > > + if (WARN_ON_ONCE(lead->grp_refs <= 0)) > > > > + return false; > > > > + > > > > + /* > > > > + * Set linked leader as failed if any member is failed, so > > > > + * the remained link chain can be terminated > > > > + */ > > > > + if (unlikely((req->flags & REQ_F_FAIL) && > > > > + ((lead->flags & IO_REQ_LINK_FLAGS) && lead->link))) > > > > + req_set_fail(lead); > > > > + return !--lead->grp_refs; > > > > +} > > > > + > > > > +/* Complete group request and collect completed leader for freeing */ > > > > +static inline void io_complete_group_req(struct io_kiocb *req, > > > > + struct io_wq_work_list *grp_list) > > > > +{ > > > > + struct io_kiocb *lead = get_group_leader(req); > > > > + > > > > + if (__io_complete_group_req(req, lead)) { > > > > + req->flags &= ~REQ_F_SQE_GROUP; > > > > + lead->flags &= ~REQ_F_SQE_GROUP_LEADER; > > > > + if (!(lead->flags & REQ_F_CQE_SKIP)) > > > > + io_req_commit_cqe(lead, lead->ctx->lockless_cq); > > > > + > > > > + if (req != lead) { > > > > + /* > > > > + * Add leader to free list if it isn't there > > > > + * otherwise clearing group flag for freeing it > > > > + * in current batch > > > > + */ > > > > + if (!(lead->flags & REQ_F_SQE_GROUP)) > > > > + wq_list_add_tail(&lead->comp_list, grp_list); > > > > + else > > > > + lead->flags &= ~REQ_F_SQE_GROUP; > > > > + } > > > > + } else if (req != lead) { > > > > + req->flags &= ~REQ_F_SQE_GROUP; > > > > + } else { > > > > + /* > > > > + * Leader's group flag clearing is delayed until it is > > > > + * removed from free list > > > > + */ > > > > + } > > > > +} > > > > + > > > > static void io_req_complete_post(struct io_kiocb *req, unsigned issue_flags) > > > > { > > > > struct io_ring_ctx *ctx = req->ctx; > > > > @@ -1427,6 +1545,17 @@ static void io_free_batch_list(struct io_ring_ctx *ctx, > > > > comp_list); > > > > if (unlikely(req->flags & IO_REQ_CLEAN_SLOW_FLAGS)) { > > > > + /* > > > > + * Group leader may be removed twice, don't free it > > > > + * if group flag isn't cleared, when some members > > > > + * aren't completed yet > > > > + */ > > > > + if (req->flags & REQ_F_SQE_GROUP) { > > > > + node = req->comp_list.next; > > > > + req->flags &= ~REQ_F_SQE_GROUP; > > > > + continue; > > > > + } > > > > + > > > > if (req->flags & REQ_F_REFCOUNT) { > > > > node = req->comp_list.next; > > > > if (!req_ref_put_and_test(req)) > > > > @@ -1459,6 +1588,7 @@ void __io_submit_flush_completions(struct io_ring_ctx *ctx) > > > > __must_hold(&ctx->uring_lock) > > > > { > > > > struct io_submit_state *state = &ctx->submit_state; > > > > + struct io_wq_work_list grp_list = {NULL}; > > > > struct io_wq_work_node *node; > > > > __io_cq_lock(ctx); > > > > @@ -1468,9 +1598,15 @@ void __io_submit_flush_completions(struct io_ring_ctx *ctx) > > > > if (!(req->flags & REQ_F_CQE_SKIP)) > > > > io_req_commit_cqe(req, ctx->lockless_cq); > > > > + > > > > + if (req->flags & REQ_F_SQE_GROUP) > > > > > > Same note about hot path > > > > > > > + io_complete_group_req(req, &grp_list); > > > > } > > > > __io_cq_unlock_post(ctx); > > > > + if (!wq_list_empty(&grp_list)) > > > > + __wq_list_splice(&grp_list, state->compl_reqs.first); > > > > > > What's the point of splicing it here insted of doing all > > > that under REQ_F_SQE_GROUP above? > > > > As mentioned, group leader can't be completed until all members are > > done, so any leaders in the current list have to be moved to this > > local list for deferred completion. That should be the only tricky > > part of the whole sqe group implementation. > > > > > > > > > + > > > > if (!wq_list_empty(&ctx->submit_state.compl_reqs)) { > > > > io_free_batch_list(ctx, state->compl_reqs.first); > > > > INIT_WQ_LIST(&state->compl_reqs); > ... > > > > @@ -1863,6 +2012,8 @@ void io_wq_submit_work(struct io_wq_work *work) > > > > } > > > > } > > > > + if (need_queue_group_members(req)) > > > > + io_queue_group_members(req, true); > > > > do { > > > > ret = io_issue_sqe(req, issue_flags); > > > > if (ret != -EAGAIN) > > > > @@ -1977,6 +2128,9 @@ static inline void io_queue_sqe(struct io_kiocb *req) > > > > */ > > > > if (unlikely(ret)) > > > > io_queue_async(req, ret); > > > > + > > > > + if (need_queue_group_members(req)) > > > > + io_queue_group_members(req, false); > > > > > > Request ownership is considered to be handed further at this > > > point and requests should not be touched. Only ret==0 from > > > io_issue_sqe it's still ours, but again it's handed somewhere > > > by io_queue_async(). > > > > Yes, you are right. > > > > And it has been fixed in my local tree: > > > > @@ -2154,8 +2154,7 @@ static inline void io_queue_sqe(struct io_kiocb *req) > > */ > > if (unlikely(ret)) > > io_queue_async(req, ret); > > - > > - if (need_queue_group_members(req)) > > + else if (need_queue_group_members(req)) > > io_queue_group_members(req, false); > > } > > In the else branch you don't own the request anymore > and shouldn't be poking into it. In theory, it is yes, but now all requests won't be freed unless returning from io_queue_sqe(), and it needs to be commented carefully. > > It looks like you're trying to do io_queue_group_members() > when previously the request would get completed. It's not It is only true for REQ_F_SQE_GROUP_DEP, and there isn't such dependency for plain SQE_GROUP. > the right place, and apart from whack'a'moled > io_wq_submit_work() there is also io_poll_issue() missed. > > Seems __io_submit_flush_completions() / io_free_batch_list() > would be more appropriate, and you already have a chunk with > GROUP check in there handling the leader appearing in there > twice. As mentioned, we need to queue members with leader together if there isn't dependency among them. > > > > > > } > > > > static void io_queue_sqe_fallback(struct io_kiocb *req) > ... > > > > @@ -2232,7 +2443,7 @@ static inline int io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req, > > > > const struct io_uring_sqe *sqe) > > > > __must_hold(&ctx->uring_lock) > > > > { > > > > - struct io_submit_link *link = &ctx->submit_state.link; > > > > + struct io_submit_state *state = &ctx->submit_state; > > > > int ret; > > > > ret = io_init_req(ctx, req, sqe); > > > > @@ -2241,9 +2452,17 @@ static inline int io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req, > > > > trace_io_uring_submit_req(req); > > > > - if (unlikely(link->head || (req->flags & (IO_REQ_LINK_FLAGS | > > > > - REQ_F_FORCE_ASYNC | REQ_F_FAIL)))) { > > > > - req = io_link_sqe(link, req); > > > > + if (unlikely(state->group.head || > > > > > > A note rather to myself and for the future, all theese checks > > > including links and groups can be folded under one common if. > > > > Sorry, I may not get the idea, can you provide one example? > > To be clear, not suggesting you doing it. > > Simplifying: > > init_req() { > if (req->flags & GROUP|LINK) { > ctx->assembling; > } > } > > io_submit_sqe() { > init_req(); > > if (ctx->assembling) { > check_groups/links(); > if (done); > ctx->assembling = false; > } > } OK, I can work toward this way, and it is just to replace check over group.head/link.head & link/group flag with ->assembling, meantime with cost of setting ctx->assembling. Thanks, Ming
diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h index 7a6b190c7da7..62311b0f0e0b 100644 --- a/include/linux/io_uring_types.h +++ b/include/linux/io_uring_types.h @@ -202,6 +202,8 @@ struct io_submit_state { /* batch completion logic */ struct io_wq_work_list compl_reqs; struct io_submit_link link; + /* points to current group */ + struct io_submit_link group; bool plug_started; bool need_plug; @@ -442,6 +444,7 @@ enum { REQ_F_FORCE_ASYNC_BIT = IOSQE_ASYNC_BIT, REQ_F_BUFFER_SELECT_BIT = IOSQE_BUFFER_SELECT_BIT, REQ_F_CQE_SKIP_BIT = IOSQE_CQE_SKIP_SUCCESS_BIT, + REQ_F_SQE_GROUP_BIT = IOSQE_SQE_GROUP_BIT, /* first byte is taken by user flags, shift it to not overlap */ REQ_F_FAIL_BIT = 8, @@ -473,6 +476,7 @@ enum { REQ_F_BL_EMPTY_BIT, REQ_F_BL_NO_RECYCLE_BIT, REQ_F_BUFFERS_COMMIT_BIT, + REQ_F_SQE_GROUP_LEADER_BIT, /* not a real bit, just to check we're not overflowing the space */ __REQ_F_LAST_BIT, @@ -496,6 +500,8 @@ enum { REQ_F_BUFFER_SELECT = IO_REQ_FLAG(REQ_F_BUFFER_SELECT_BIT), /* IOSQE_CQE_SKIP_SUCCESS */ REQ_F_CQE_SKIP = IO_REQ_FLAG(REQ_F_CQE_SKIP_BIT), + /* IOSQE_SQE_GROUP */ + REQ_F_SQE_GROUP = IO_REQ_FLAG(REQ_F_SQE_GROUP_BIT), /* fail rest of links */ REQ_F_FAIL = IO_REQ_FLAG(REQ_F_FAIL_BIT), @@ -553,6 +559,8 @@ enum { REQ_F_BL_NO_RECYCLE = IO_REQ_FLAG(REQ_F_BL_NO_RECYCLE_BIT), /* buffer ring head needs incrementing on put */ REQ_F_BUFFERS_COMMIT = IO_REQ_FLAG(REQ_F_BUFFERS_COMMIT_BIT), + /* sqe group lead */ + REQ_F_SQE_GROUP_LEADER = IO_REQ_FLAG(REQ_F_SQE_GROUP_LEADER_BIT), }; typedef void (*io_req_tw_func_t)(struct io_kiocb *req, struct io_tw_state *ts); @@ -666,6 +674,10 @@ struct io_kiocb { u64 extra1; u64 extra2; } big_cqe; + + /* all SQE group members linked here for group lead */ + struct io_kiocb *grp_link; + int grp_refs; }; struct io_overflow_cqe { diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h index 994bf7af0efe..2b99d9d0b93e 100644 --- a/include/uapi/linux/io_uring.h +++ b/include/uapi/linux/io_uring.h @@ -124,6 +124,7 @@ enum io_uring_sqe_flags_bit { IOSQE_ASYNC_BIT, IOSQE_BUFFER_SELECT_BIT, IOSQE_CQE_SKIP_SUCCESS_BIT, + IOSQE_SQE_GROUP_BIT, }; /* @@ -143,6 +144,8 @@ enum io_uring_sqe_flags_bit { #define IOSQE_BUFFER_SELECT (1U << IOSQE_BUFFER_SELECT_BIT) /* don't post CQE if request succeeded */ #define IOSQE_CQE_SKIP_SUCCESS (1U << IOSQE_CQE_SKIP_SUCCESS_BIT) +/* defines sqe group */ +#define IOSQE_SQE_GROUP (1U << IOSQE_SQE_GROUP_BIT) /* * io_uring_setup() flags @@ -540,6 +543,7 @@ struct io_uring_params { #define IORING_FEAT_LINKED_FILE (1U << 12) #define IORING_FEAT_REG_REG_RING (1U << 13) #define IORING_FEAT_RECVSEND_BUNDLE (1U << 14) +#define IORING_FEAT_SQE_GROUP (1U << 15) /* * io_uring_register(2) opcodes and arguments diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c index c184c9a312df..b87c5452de43 100644 --- a/io_uring/io_uring.c +++ b/io_uring/io_uring.c @@ -109,7 +109,8 @@ IOSQE_IO_HARDLINK | IOSQE_ASYNC) #define SQE_VALID_FLAGS (SQE_COMMON_FLAGS | IOSQE_BUFFER_SELECT | \ - IOSQE_IO_DRAIN | IOSQE_CQE_SKIP_SUCCESS) + IOSQE_IO_DRAIN | IOSQE_CQE_SKIP_SUCCESS | \ + IOSQE_SQE_GROUP) #define IO_REQ_CLEAN_FLAGS (REQ_F_BUFFER_SELECTED | REQ_F_NEED_CLEANUP | \ REQ_F_POLLED | REQ_F_INFLIGHT | REQ_F_CREDS | \ @@ -915,6 +916,13 @@ static __always_inline void io_req_commit_cqe(struct io_kiocb *req, { struct io_ring_ctx *ctx = req->ctx; + /* + * For group leader, cqe has to be committed after all members are + * committed, when the request becomes normal one. + */ + if (unlikely(req_is_group_leader(req))) + return; + if (unlikely(!io_fill_cqe_req(ctx, req))) { if (lockless_cq) { spin_lock(&ctx->completion_lock); @@ -926,6 +934,116 @@ static __always_inline void io_req_commit_cqe(struct io_kiocb *req, } } +static inline bool need_queue_group_members(struct io_kiocb *req) +{ + return req_is_group_leader(req) && req->grp_link; +} + +/* Can only be called after this request is issued */ +static inline struct io_kiocb *get_group_leader(struct io_kiocb *req) +{ + if (req->flags & REQ_F_SQE_GROUP) { + if (req_is_group_leader(req)) + return req; + return req->grp_link; + } + return NULL; +} + +void io_cancel_group_members(struct io_kiocb *req, bool ignore_cqes) +{ + struct io_kiocb *member = req->grp_link; + + while (member) { + struct io_kiocb *next = member->grp_link; + + if (ignore_cqes) + member->flags |= REQ_F_CQE_SKIP; + if (!(member->flags & REQ_F_FAIL)) { + req_set_fail(member); + io_req_set_res(member, -ECANCELED, 0); + } + member = next; + } +} + +void io_queue_group_members(struct io_kiocb *req, bool async) +{ + struct io_kiocb *member = req->grp_link; + + if (!member) + return; + + while (member) { + struct io_kiocb *next = member->grp_link; + + member->grp_link = req; + if (async) + member->flags |= REQ_F_FORCE_ASYNC; + + if (unlikely(member->flags & REQ_F_FAIL)) { + io_req_task_queue_fail(member, member->cqe.res); + } else if (member->flags & REQ_F_FORCE_ASYNC) { + io_req_task_queue(member); + } else { + io_queue_sqe(member); + } + member = next; + } + req->grp_link = NULL; +} + +static inline bool __io_complete_group_req(struct io_kiocb *req, + struct io_kiocb *lead) +{ + WARN_ON_ONCE(!(req->flags & REQ_F_SQE_GROUP)); + + if (WARN_ON_ONCE(lead->grp_refs <= 0)) + return false; + + /* + * Set linked leader as failed if any member is failed, so + * the remained link chain can be terminated + */ + if (unlikely((req->flags & REQ_F_FAIL) && + ((lead->flags & IO_REQ_LINK_FLAGS) && lead->link))) + req_set_fail(lead); + return !--lead->grp_refs; +} + +/* Complete group request and collect completed leader for freeing */ +static inline void io_complete_group_req(struct io_kiocb *req, + struct io_wq_work_list *grp_list) +{ + struct io_kiocb *lead = get_group_leader(req); + + if (__io_complete_group_req(req, lead)) { + req->flags &= ~REQ_F_SQE_GROUP; + lead->flags &= ~REQ_F_SQE_GROUP_LEADER; + if (!(lead->flags & REQ_F_CQE_SKIP)) + io_req_commit_cqe(lead, lead->ctx->lockless_cq); + + if (req != lead) { + /* + * Add leader to free list if it isn't there + * otherwise clearing group flag for freeing it + * in current batch + */ + if (!(lead->flags & REQ_F_SQE_GROUP)) + wq_list_add_tail(&lead->comp_list, grp_list); + else + lead->flags &= ~REQ_F_SQE_GROUP; + } + } else if (req != lead) { + req->flags &= ~REQ_F_SQE_GROUP; + } else { + /* + * Leader's group flag clearing is delayed until it is + * removed from free list + */ + } +} + static void io_req_complete_post(struct io_kiocb *req, unsigned issue_flags) { struct io_ring_ctx *ctx = req->ctx; @@ -1427,6 +1545,17 @@ static void io_free_batch_list(struct io_ring_ctx *ctx, comp_list); if (unlikely(req->flags & IO_REQ_CLEAN_SLOW_FLAGS)) { + /* + * Group leader may be removed twice, don't free it + * if group flag isn't cleared, when some members + * aren't completed yet + */ + if (req->flags & REQ_F_SQE_GROUP) { + node = req->comp_list.next; + req->flags &= ~REQ_F_SQE_GROUP; + continue; + } + if (req->flags & REQ_F_REFCOUNT) { node = req->comp_list.next; if (!req_ref_put_and_test(req)) @@ -1459,6 +1588,7 @@ void __io_submit_flush_completions(struct io_ring_ctx *ctx) __must_hold(&ctx->uring_lock) { struct io_submit_state *state = &ctx->submit_state; + struct io_wq_work_list grp_list = {NULL}; struct io_wq_work_node *node; __io_cq_lock(ctx); @@ -1468,9 +1598,15 @@ void __io_submit_flush_completions(struct io_ring_ctx *ctx) if (!(req->flags & REQ_F_CQE_SKIP)) io_req_commit_cqe(req, ctx->lockless_cq); + + if (req->flags & REQ_F_SQE_GROUP) + io_complete_group_req(req, &grp_list); } __io_cq_unlock_post(ctx); + if (!wq_list_empty(&grp_list)) + __wq_list_splice(&grp_list, state->compl_reqs.first); + if (!wq_list_empty(&ctx->submit_state.compl_reqs)) { io_free_batch_list(ctx, state->compl_reqs.first); INIT_WQ_LIST(&state->compl_reqs); @@ -1677,8 +1813,12 @@ static u32 io_get_sequence(struct io_kiocb *req) struct io_kiocb *cur; /* need original cached_sq_head, but it was increased for each req */ - io_for_each_link(cur, req) - seq--; + io_for_each_link(cur, req) { + if (req_is_group_leader(cur)) + seq -= cur->grp_refs; + else + seq--; + } return seq; } @@ -1793,11 +1933,20 @@ struct io_wq_work *io_wq_free_work(struct io_wq_work *work) struct io_kiocb *nxt = NULL; if (req_ref_put_and_test(req)) { - if (req->flags & IO_REQ_LINK_FLAGS) - nxt = io_req_find_next(req); + /* + * CQEs have been posted in io_req_complete_post() except + * for group leader, and we can't advance the link for + * group leader until its CQE is posted. + * + * TODO: try to avoid defer and complete leader in io_wq + * context directly + */ + if (!req_is_group_leader(req)) { + req->flags |= REQ_F_CQE_SKIP; + if (req->flags & IO_REQ_LINK_FLAGS) + nxt = io_req_find_next(req); + } - /* we have posted CQEs in io_req_complete_post() */ - req->flags |= REQ_F_CQE_SKIP; io_free_req(req); } return nxt ? &nxt->work : NULL; @@ -1863,6 +2012,8 @@ void io_wq_submit_work(struct io_wq_work *work) } } + if (need_queue_group_members(req)) + io_queue_group_members(req, true); do { ret = io_issue_sqe(req, issue_flags); if (ret != -EAGAIN) @@ -1977,6 +2128,9 @@ static inline void io_queue_sqe(struct io_kiocb *req) */ if (unlikely(ret)) io_queue_async(req, ret); + + if (need_queue_group_members(req)) + io_queue_group_members(req, false); } static void io_queue_sqe_fallback(struct io_kiocb *req) @@ -2142,6 +2296,56 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, return def->prep(req, sqe); } +static struct io_kiocb *io_group_sqe(struct io_submit_link *group, + struct io_kiocb *req) +{ + /* + * Group chain is similar with link chain: starts with 1st sqe with + * REQ_F_SQE_GROUP, and ends with the 1st sqe without REQ_F_SQE_GROUP + */ + if (group->head) { + struct io_kiocb *lead = group->head; + + /* members can't be in link chain, can't be drained */ + req->flags &= ~(IO_REQ_LINK_FLAGS | REQ_F_IO_DRAIN); + lead->grp_refs += 1; + group->last->grp_link = req; + group->last = req; + + if (req->flags & REQ_F_SQE_GROUP) + return NULL; + + req->grp_link = NULL; + req->flags |= REQ_F_SQE_GROUP; + group->head = NULL; + return lead; + } else if (req->flags & REQ_F_SQE_GROUP) { + group->head = req; + group->last = req; + req->grp_refs = 1; + req->flags |= REQ_F_SQE_GROUP_LEADER; + return NULL; + } else { + return req; + } +} + +static __cold struct io_kiocb *io_submit_fail_group( + struct io_submit_link *link, struct io_kiocb *req) +{ + struct io_kiocb *lead = link->head; + + /* + * Instead of failing eagerly, continue assembling the group link + * if applicable and mark the leader with REQ_F_FAIL. The group + * flushing code should find the flag and handle the rest + */ + if (lead && (lead->flags & IO_REQ_LINK_FLAGS) && !(lead->flags & REQ_F_FAIL)) + req_fail_link_node(lead, -ECANCELED); + + return io_group_sqe(link, req); +} + static __cold int io_submit_fail_link(struct io_submit_link *link, struct io_kiocb *req, int ret) { @@ -2180,11 +2384,18 @@ static __cold int io_submit_fail_init(const struct io_uring_sqe *sqe, { struct io_ring_ctx *ctx = req->ctx; struct io_submit_link *link = &ctx->submit_state.link; + struct io_submit_link *group = &ctx->submit_state.group; trace_io_uring_req_failed(sqe, req, ret); req_fail_link_node(req, ret); + if (group->head || (req->flags & REQ_F_SQE_GROUP)) { + req = io_submit_fail_group(group, req); + if (!req) + return 0; + } + /* cover both linked and non-linked request */ return io_submit_fail_link(link, req, ret); } @@ -2232,7 +2443,7 @@ static inline int io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req, const struct io_uring_sqe *sqe) __must_hold(&ctx->uring_lock) { - struct io_submit_link *link = &ctx->submit_state.link; + struct io_submit_state *state = &ctx->submit_state; int ret; ret = io_init_req(ctx, req, sqe); @@ -2241,9 +2452,17 @@ static inline int io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req, trace_io_uring_submit_req(req); - if (unlikely(link->head || (req->flags & (IO_REQ_LINK_FLAGS | - REQ_F_FORCE_ASYNC | REQ_F_FAIL)))) { - req = io_link_sqe(link, req); + if (unlikely(state->group.head || + (req->flags & REQ_F_SQE_GROUP))) { + req = io_group_sqe(&state->group, req); + if (!req) + return 0; + } + + if (unlikely(state->link.head || + (req->flags & (IO_REQ_LINK_FLAGS | REQ_F_FORCE_ASYNC | + REQ_F_FAIL)))) { + req = io_link_sqe(&state->link, req); if (!req) return 0; } @@ -2258,6 +2477,17 @@ static void io_submit_state_end(struct io_ring_ctx *ctx) { struct io_submit_state *state = &ctx->submit_state; + /* the last member must set REQ_F_SQE_GROUP */ + if (unlikely(state->group.head)) { + struct io_kiocb *lead = state->group.head; + + state->group.last->grp_link = NULL; + if (lead->flags & IO_REQ_LINK_FLAGS) + io_link_sqe(&state->link, lead); + else + io_queue_sqe_fallback(lead); + } + if (unlikely(state->link.head)) io_queue_sqe_fallback(state->link.head); /* flush only after queuing links as they can generate completions */ @@ -2277,6 +2507,7 @@ static void io_submit_state_start(struct io_submit_state *state, state->submit_nr = max_ios; /* set only head, no need to init link_last in advance */ state->link.head = NULL; + state->group.head = NULL; } static void io_commit_sqring(struct io_ring_ctx *ctx) @@ -3601,7 +3832,7 @@ static __cold int io_uring_create(unsigned entries, struct io_uring_params *p, IORING_FEAT_EXT_ARG | IORING_FEAT_NATIVE_WORKERS | IORING_FEAT_RSRC_TAGS | IORING_FEAT_CQE_SKIP | IORING_FEAT_LINKED_FILE | IORING_FEAT_REG_REG_RING | - IORING_FEAT_RECVSEND_BUNDLE; + IORING_FEAT_RECVSEND_BUNDLE | IORING_FEAT_SQE_GROUP; if (copy_to_user(params, p, sizeof(*p))) { ret = -EFAULT; diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h index 624ca9076a50..b11db3bdd8d8 100644 --- a/io_uring/io_uring.h +++ b/io_uring/io_uring.h @@ -67,6 +67,8 @@ void io_req_defer_failed(struct io_kiocb *req, s32 res); bool io_post_aux_cqe(struct io_ring_ctx *ctx, u64 user_data, s32 res, u32 cflags); bool io_req_post_cqe(struct io_kiocb *req, s32 res, u32 cflags); void __io_commit_cqring_flush(struct io_ring_ctx *ctx); +void io_queue_group_members(struct io_kiocb *req, bool async); +void io_cancel_group_members(struct io_kiocb *req, bool ignore_cqes); struct file *io_file_get_normal(struct io_kiocb *req, int fd); struct file *io_file_get_fixed(struct io_kiocb *req, int fd, @@ -342,6 +344,16 @@ static inline void io_tw_lock(struct io_ring_ctx *ctx, struct io_tw_state *ts) lockdep_assert_held(&ctx->uring_lock); } +static inline bool req_is_group_leader(struct io_kiocb *req) +{ + return req->flags & REQ_F_SQE_GROUP_LEADER; +} + +static inline bool req_is_group_member(struct io_kiocb *req) +{ + return !req_is_group_leader(req) && (req->flags & REQ_F_SQE_GROUP); +} + /* * Don't complete immediately but use deferred completion infrastructure. * Protected by ->uring_lock and can only be used either with @@ -355,6 +367,10 @@ static inline void io_req_complete_defer(struct io_kiocb *req) lockdep_assert_held(&req->ctx->uring_lock); wq_list_add_tail(&req->comp_list, &state->compl_reqs); + + /* members may not be issued when leader is completed */ + if (unlikely(req_is_group_leader(req) && req->grp_link)) + io_queue_group_members(req, false); } static inline void io_commit_cqring_flush(struct io_ring_ctx *ctx) diff --git a/io_uring/timeout.c b/io_uring/timeout.c index 202f540aa314..2496bfa64948 100644 --- a/io_uring/timeout.c +++ b/io_uring/timeout.c @@ -171,6 +171,8 @@ static void io_fail_links(struct io_kiocb *req) link->flags |= REQ_F_CQE_SKIP; else link->flags &= ~REQ_F_CQE_SKIP; + if (req_is_group_leader(link)) + io_cancel_group_members(link, ignore_cqes); trace_io_uring_fail_link(req, link); link = link->link; }
SQE group is defined as one chain of SQEs starting with the first SQE that has IOSQE_SQE_GROUP set, and ending with the first subsequent SQE that doesn't have it set, and it is similar with chain of linked SQEs. Not like linked SQEs, each sqe is issued after the previous one is completed. All SQEs in one group are submitted in parallel, so there isn't any dependency among SQEs in one group. The 1st SQE is group leader, and the other SQEs are group member. The whole group share single IOSQE_IO_LINK and IOSQE_IO_DRAIN from group leader, and the two flags are ignored for group members. When the group is in one link chain, this group isn't submitted until the previous SQE or group is completed. And the following SQE or group can't be started if this group isn't completed. Failure from any group member will fail the group leader, then the link chain can be terminated. When IOSQE_IO_DRAIN is set for group leader, all requests in this group and previous requests submitted are drained. Given IOSQE_IO_DRAIN can be set for group leader only, we respect IO_DRAIN by always completing group leader as the last one in the group. Working together with IOSQE_IO_LINK, SQE group provides flexible way to support N:M dependency, such as: - group A is chained with group B together - group A has N SQEs - group B has M SQEs then M SQEs in group B depend on N SQEs in group A. N:M dependency can support some interesting use cases in efficient way: 1) read from multiple files, then write the read data into single file 2) read from single file, and write the read data into multiple files 3) write same data into multiple files, and read data from multiple files and compare if correct data is written Also IOSQE_SQE_GROUP takes the last bit in sqe->flags, but we still can extend sqe->flags with one uring context flag, such as use __pad3 for non-uring_cmd OPs and part of uring_cmd_flags for uring_cmd OP. Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- include/linux/io_uring_types.h | 12 ++ include/uapi/linux/io_uring.h | 4 + io_uring/io_uring.c | 255 +++++++++++++++++++++++++++++++-- io_uring/io_uring.h | 16 +++ io_uring/timeout.c | 2 + 5 files changed, 277 insertions(+), 12 deletions(-)