Re: [PATCH v2 0/4] allow to skip CQE posting

[Jens Axboe <axboe@kernel.dk>]

On 11/10/21 9:42 AM, Pavel Begunkov wrote:
> On 11/10/21 16:14, Jens Axboe wrote:
>> On 11/10/21 8:49 AM, Pavel Begunkov wrote:
>>> It's expensive enough to post an CQE, and there are other
>>> reasons to want to ignore them, e.g. for link handling and
>>> it may just be more convenient for the userspace.
>>>
>>> Try to cover most of the use cases with one flag. The overhead
>>> is one "if (cqe->flags & IOSQE_CQE_SKIP_SUCCESS)" check per
>>> requests and a bit bloated req_set_fail(), should be bearable.
>>
>> I like the idea, one thing I'm struggling with is I think a normal use
>> case of this would be fast IO where we still need to know if a
>> completion event has happened, we just don't need to know the details of
>> it since we already know what those details would be if it ends up in
>> success.
>>
>> How about having a skip counter? That would supposedly also allow drain
>> to work, and it could be mapped with the other cq parts to allow the app
>> to see it as well.
> 
> It doesn't go through expensive io_cqring_ev_posted(), so the
> userspace can't really wait on it. It can do some linking tricks to
> alleviate that, but I don't see any new capabilities from the current
> approach.

I'm not talking about waiting, just reading the cqring entry to see how
many were skipped. If you ask for no cqe, by definition there would be
nothing to wait on for you. Though it'd probably be better as an sqring
entry, since we'd be accounting at that time. Only caveat there is then
if the sqe errors and we do end up posting a cqe..

> Also the locking is a problem, I was thinking about it, mainly hoping
> that I can adjust cq_extra and leave draining, but it didn't appear
> great to me. AFAIK, it's either an atomic, beating the purpose of the
> thing.

If we do submission side, then the ring mutex would cover it. No need
for any extra locking

> Another option is to split it in two, one counter is kept under
> ->uring_lock and another under ->completion_lock. But it'll be messy,
> shifting flushing part of draining to a work-queue for mutex locking,
> adding yet another bunch of counters that hard to maintain and so.

You'd only need the cqring counter for the unlikely event that the
request is failed and does post an cqe, though.

> And __io_submit_flush_completions() would also need to go through
> the request list one extra time to do the accounting, wouldn't
> want to grow massively inlined io_req_complete_state().

-- 
Jens Axboe