On 11/24/21 11:02 AM, Pavel Begunkov wrote: > On 11/24/21 17:57, Jens Axboe wrote: >> 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. > > agree, but I can't find a fix without penalising performance I think we're OK as I don't DRAIN is used very much, and as long as it's adequately documented in terms of them not co-existing and what the error code is, then if we do find a way to make them work together we can relax them in the future. >> 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. > > yeah, and also need to rebase and resend tests Great thanks. -- Jens Axboe