Re: bug with fastpoll accept and sqpoll + IOSQE_FIXED_FILE

[Pavel Begunkov <asml.silence@gmail.com>]

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