On 05/02/2021 14:42, Jens Axboe wrote:
> On 2/5/21 5:46 AM, Pavel Begunkov wrote:
>> On 04/02/2021 16:50, Jens Axboe wrote:
[...]
>>> Hence my suggestion is to just patch this in a trivial kind of fashion,
>>> even if it doesn't really make the function any prettier. But it'll be
>>> safer for a release, and then we can rework the function after.
>>>
>>> With that in mind, here's my suggestion. The premise is if we go through
>>> the loop and don't do io_uring_enter(), then there's no point in
>>> continuing. That's the trivial fix.
>>
>> Your idea but imho cleaner below.
>> +1 comment inline
>
> Shouldn't be hard, it was just a quick hack :-)
Yes, hopefully. That comment came straight from my ever failing
attempts to clean it up :) We will need to test well the final
version -- with and without IORING_FEAT_EXT_ARG.
[...]
>> diff --git a/src/queue.c b/src/queue.c
>> index 94f791e..7d6f31d 100644
>> --- a/src/queue.c
>> +++ b/src/queue.c
>> @@ -112,11 +112,15 @@ static int _io_uring_get_cqe(struct io_uring *ring, struct io_uring_cqe **cqe_pt
>> flags = IORING_ENTER_GETEVENTS | data->get_flags;
>> if (data->submit)
>> sq_ring_needs_enter(ring, &flags);
>> - if (data->wait_nr > nr_available || data->submit ||
>> - cq_overflow_flush)
>> - ret = __sys_io_uring_enter2(ring->ring_fd, data->submit,
>> - data->wait_nr, flags, data->arg,
>> - data->sz);
>> +
>> + if (data->wait_nr <= nr_available && !data->submit &&
>> + !cq_overflow_flush) {
>> + err = ?;
>
> which I guess is the actual error missing from here?
As a way to say "not tested at all". I just believe it's not all to that.
e.g. user calls wait/peek(nr=1, cqe);
__io_uring_peek_cqe() well succeeds, then
if (data->wait_nr && cqe)
data->wait_nr--;
That might make us to skip enter, and we return -EAGAIN.
>
>> + break;
>> + }
>> + ret = __sys_io_uring_enter2(ring->ring_fd, data->submit,
>> + data->wait_nr, flags, data->arg,
>> + data->sz);
>> if (ret < 0) {
>> err = -errno;
>> } else if (ret == (int)data->submit) {
>>
--
Pavel Begunkov
