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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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

--
Pavel Begunkov



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux