Re: [PATCH] io_uring: ensure deferred completions are flushed for multishot

[Pavel Begunkov <asml.silence@gmail.com>]

On 5/7/25 15:58, Jens Axboe wrote:
> On 5/7/25 8:36 AM, Pavel Begunkov wrote:
>> On 5/7/25 14:56, Jens Axboe wrote:
>>> Multishot normally uses io_req_post_cqe() to post completions, but when
>>> stopping it, it may finish up with a deferred completion. This is fine,
>>> except if another multishot event triggers before the deferred completions
>>> get flushed. If this occurs, then CQEs may get reordered in the CQ ring,
>>> as new multishot completions get posted before the deferred ones are
>>> flushed. This can cause confusion on the application side, if strict
>>> ordering is required for the use case.
>>>
>>> When multishot posting via io_req_post_cqe(), flush any pending deferred
>>> completions first, if any.
>>>
>>> Cc: stable@vger.kernel.org # 6.1+
>>> Reported-by: Norman Maurer <norman_maurer@apple.com>
>>> Reported-by: Christian Mazakas <christian.mazakas@gmail.com>
>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>
>>> ---
>>>
>>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
>>> index 769814d71153..541e65a1eebf 100644
>>> --- a/io_uring/io_uring.c
>>> +++ b/io_uring/io_uring.c
>>> @@ -848,6 +848,14 @@ bool io_req_post_cqe(struct io_kiocb *req, s32 res, u32 cflags)
>>>        struct io_ring_ctx *ctx = req->ctx;
>>>        bool posted;
>>>    +    /*
>>> +     * If multishot has already posted deferred completions, ensure that
>>> +     * those are flushed first before posting this one. If not, CQEs
>>> +     * could get reordered.
>>> +     */
>>> +    if (!wq_list_empty(&ctx->submit_state.compl_reqs))
>>> +        __io_submit_flush_completions(ctx);
>>
>> A request is already dead if it's in compl_reqs, there should be no
>> way io_req_post_cqe() is called with it. Is it reordering of CQEs
>> belonging to the same request? And what do you mean by "deferred"
>> completions?
> 
> It's not the same req, it's different requests using the same
> provided buffer ring where it can be problematic.

Ok, and there has never been any ordering guarantees between them.
Is there any report describing the problem? Why it's ok if
io_req_post_cqe() produced CQEs of two multishot requests get
reordered, but it's not when one of them is finishing a request?
What's the problem with provided buffers?

It's a pretty bad spot to do such kinds of things, it disables any
mixed mshot with non-mshot batching, and nesting flushing under
an opcode handler is pretty nasty, it should rather stay top
level as it is. From what I hear it's a provided buffer specific
problem and should be fixed there.

-- 
Pavel Begunkov