Re: [PATCH v2 2/2] io_uring: flush timeouts that should already have expired

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

On 13/01/2021 14:41, Marcelo Diop-Gonzalez wrote:
> On Tue, Jan 12, 2021 at 08:47:11PM +0000, Pavel Begunkov wrote:
>> What about the first patch about overflows and cq_timeouts? I
>> assume that problem is still there, isn't it?
> 
> Yeah it's still there I think, I just couldn't think of a good way
> to fix it. So I figured I would just send this one since at least
> it doesn't make that problem worse. Maybe could send a fix for that
> one later if I think of something

sounds good to me

>>> +		events_needed = req->timeout.target_seq - ctx->cq_last_tm_flush;
>>> +		events_got = seq - ctx->cq_last_tm_flush;
>>> +		if (events_got < events_needed) 
>>
>> probably <=
> 
> Won't that make it break too early though? If you submit a timeout
> with off = 1 when {seq == 0, last_flush == 0}, then target_seq ==
> 1. Then let's say there's 1 cqe added, so the timeout should trigger.
> Then events_needed == 1 and events_got == 1, right?

Yep, you're right. I had in mind

@target in [last_flush, cur_flush], and so

(!(target - last_flush <= cur_flush - last_flush))
	break;

but that's same but reshuffled. Thanks for double checking. 

>> basically it checks that @target is in [last_flush, cur_seq],
>> it can use such a comment + a note about underflows and using
>> the modulus arithmetic, like with algebraic rings
-- 
Pavel Begunkov