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

[Jens Axboe <axboe@kernel.dk>]

On 11/24/21 10:55 AM, Pavel Begunkov wrote:
> On 11/10/21 16:47, Jens Axboe wrote:
>> 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
> 
> Jens, let's decide what we're going to do with this feature

Only weird bit is the drain, but apart from that I think it looks sane.
Are you going to send a documentation update to liburing as well? Should
be detailed in terms of what it does and the usability of it.

-- 
Jens Axboe