All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Victor Stewart <v@nametag.social>, io-uring <io-uring@vger.kernel.org>
Subject: Re: io_uring_prep_timeout_update on linked timeouts
Date: Fri, 27 Aug 2021 21:22:25 -0600	[thread overview]
Message-ID: <1b3865bd-f381-04f3-6e54-779fe6b43946@kernel.dk> (raw)
In-Reply-To: <CAM1kxwiAF3tmF8PxVf6KPV+Qsg_180sFvebxos5ySmU=TqxgmA@mail.gmail.com>

On 8/26/21 7:40 PM, Victor Stewart wrote:
> On Wed, Aug 25, 2021 at 2:27 AM Victor Stewart <v@nametag.social> wrote:
>>
>> On Tue, Aug 24, 2021 at 11:43 PM Victor Stewart <v@nametag.social> wrote:
>>>
>>> we're able to update timeouts with io_uring_prep_timeout_update
>>> without having to cancel
>>> and resubmit, has it ever been considered adding this ability to
>>> linked timeouts?
>>
>> whoops turns out this does work. just tested it.
> 
> doesn't work actually. missed that because of a bit of misdirection.
> returns -ENOENT.
> 
> the problem with the current way of cancelling then resubmitting
> a new a timeout linked op (let's use poll here) is you have 3 situations:
> 
> 1) the poll triggers and you get some positive value. all good.
> 
> 2) the linked timeout triggers and cancels the poll, so the poll
> operation returns -ECANCELED.
> 
> 3) you cancel the existing poll op, and submit a new one with
> the updated linked timeout. now the original poll op returns
> -ECANCELED.
> 
> so solely from looking at the return value of the poll op in 2) and 3)
> there is no way to disambiguate them. of course the linked timeout
> operation result will allow you to do so, but you'd have to persist state
> across cqe processings. you can also track the cancellations and know
> to skip the explicitly cancelled ops' cqes (which is what i chose).
> 
> there's also the problem of efficiency. you can imagine in a QUIC
> server where you're constantly updating that poll timeout in response
> to idle timeout and ACK scheduling, this extra work mounts.
> 
> so i think the ability to update linked timeouts via
> io_uring_prep_timeout_update would be fantastic.

Hmm, I'll need to dig a bit, but whether it's a linked timeout or not
should not matter. It's a timeout, it's queued and updated the same way.
And we even check this in some of the liburing tests.

Do you have a test case that doesn't work for you? Always easier to
reason about a test case.

-- 
Jens Axboe


  reply	other threads:[~2021-08-28  3:22 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24 22:43 io_uring_prep_timeout_update on linked timeouts Victor Stewart
2021-08-25  1:27 ` Victor Stewart
2021-08-27  1:40   ` Victor Stewart
2021-08-28  3:22     ` Jens Axboe [this message]
2021-08-28 13:39       ` Pavel Begunkov
2021-08-28 13:43         ` Jens Axboe
2021-08-28 21:38           ` Pavel Begunkov
2021-08-29  2:40             ` Jens Axboe
2021-08-31 11:36               ` Victor Stewart
2021-08-31 16:09                 ` Pavel Begunkov
2021-08-31 16:07               ` Pavel Begunkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1b3865bd-f381-04f3-6e54-779fe6b43946@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=io-uring@vger.kernel.org \
    --cc=v@nametag.social \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.