On 4/12/2020 6:14 PM, Hrvoje Zeba wrote: > On Sun, Apr 12, 2020 at 5:15 AM Pavel Begunkov <asml.silence@xxxxxxxxx> wrote: >> >> On 4/12/2020 5:07 AM, Jens Axboe wrote: >>> On 4/11/20 5:00 PM, Hrvoje Zeba wrote: >>>> Hi, >>>> >>>> I've been looking at timeouts and found a case I can't wrap my head around. >>>> >>>> Basically, If you submit OPs in a certain order, timeout fires before >>>> time elapses where I wouldn't expect it to. The order is as follows: >>>> >>>> poll(listen_socket, POLLIN) <- this never fires >>>> nop(async) >>>> timeout(1s, count=X) >>>> >>>> If you set X to anything but 0xffffffff/(unsigned)-1, the timeout does >>>> not fire (at least not immediately). This is expected apart from maybe >>>> setting X=1 which would potentially allow the timeout to fire if nop >>>> executes after the timeout is setup. >>>> >>>> If you set it to 0xffffffff, it will always fire (at least on my >>>> machine). Test program I'm using is attached. >>>> >>>> The funny thing is that, if you remove the poll, timeout will not fire. >>>> >>>> I'm using Linus' tree (v5.6-12604-gab6f762f0f53). >>>> >>>> Could anybody shine a bit of light here? >>> >>> Thinking about this, I think the mistake here is using the SQ side for >>> the timeouts. Let's say you queue up N requests that are waiting, like >>> the poll. Then you arm a timeout, it'll now be at N + count before it >>> fires. We really should be using the CQ side for the timeouts. >> >> As I get it, the problem is that timeout(off=0xffffffff, 1s) fires >> __immediately__ (i.e. not waiting 1s). > > Correct. > >> And still, the described behaviour is out of the definition. It's sounds >> like int overflow. Ok, I'll debug it, rest assured. I already see a >> couple of flaws anyway. > > For this particular case, > > req->sequence = ctx->cached_sq_head + count - 1; > > ends up being 1 which triggers in __req_need_defer() for nop sq. Right, that's it. The timeout's seq counter wraps around and triggers on previously submitted but still inflight requests. Jens, could you remind, do we limit number of inflight requests? We discussed it before, but can't find the thread. If we don't, vile stuff can happen with sequences. -- Pavel Begunkov
