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
