public inbox for [email protected]
 help / color / mirror / Atom feed
From: Pavel Begunkov <[email protected]>
To: Jens Axboe <[email protected]>,
	[email protected], [email protected]
Cc: [email protected], [email protected]
Subject: Re: [PATCHSET 0/2] io_uring support for linked timeouts
Date: Fri, 15 Nov 2019 00:24:29 +0300	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>


[-- Attachment #1.1: Type: text/plain, Size: 2868 bytes --]

On 06/11/2019 00:11, Jens Axboe wrote:
> Anyway, this is support for IORING_OP_LINK_TIMEOUT. Unlike the timeouts
> we have now that work on purely the CQ ring, these timeouts are
> specifically tied to a specific command. They are meant to be used to
> auto-cancel a request, if it hasn't finished in X amount of time. The
> way to use then is to setup your command as you usually would, but then
> mark is IOSQE_IO_LINK and add an IORING_OP_LINK_TIMEOUT right after it.
> That's how linked commands work to begin with. The main difference here
> is that links are normally only started once the dependent request
> completes, but for IORING_OP_LINK_TIMEOUT they are armed as soon as we
> start the dependent request.
> 
> If the dependent request finishes before the linked timeout, the timeout
> is canceled. If the timeout finishes before the dependent request, the
> dependent request is attempted canceled.
> 
> IORING_OP_LINK_TIMEOUT is setup just like IORING_OP_TIMEOUT in terms
> of passing in the timespec associated with it.
> 
> I added a bunch of test cases to liburing, currently residing in a
> link-timeout branch. View them here:
> 

Finally got to this patch. I think, find it adding too many edge cases
and it isn't integrated consistently into what we have now. I would love
to hear your vision, but I'd try to implement them in such a way, that it
doesn't need to modify the framework, at least for some particular case.
In other words, as opcodes could have been added from the outside with a
function table.

Also, it's not so consistent with the userspace API as well.

1. If we specified drain for the timeout, should its start be delayed
until then? I would prefer so.

E.g. send_msg + drained linked_timeout, which would set a timeout from the
start of the send.

2. Why it could be only the second one in a link? May we want to cancel
from a certain point?
e.g. "op1 -> op2 -> timeout -> op3" cancels op2 and op3

3. It's a bit strange, that the timeout affects a request from the left,
and after as an consequence cancels everything on the right (i.e. chain).
Could we place it in the head? So it would affect all requests on the right
from it.

4. I'd prefer to handle it as a new generic command and setting a timer
in __io_submit_sqe().

I believe we can do it more gracefully, and at the same moment giving
more freedom to the user. What do you think?


> https://git.kernel.dk/cgit/liburing/commit/?h=link-timeout&id=bc1bd5e97e2c758d6fd975bd35843b9b2c770c5a
> 
> Patches are against for-5.5/io_uring, and can currently also be found in
> my for-5.5/io_uring-test branch.
> 
>  fs/io_uring.c                 | 203 +++++++++++++++++++++++++++++++---
>  include/uapi/linux/io_uring.h |   1 +
>  2 files changed, 187 insertions(+), 17 deletions(-)
> 

-- 
Pavel Begunkov


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2019-11-14 21:25 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-05 21:11 [PATCHSET 0/2] io_uring support for linked timeouts Jens Axboe
2019-11-05 21:11 ` [PATCH 1/2] io_uring: abstract out io_async_cancel_one() helper Jens Axboe
2019-11-05 21:11 ` [PATCH 2/2] io_uring: add support for linked SQE timeouts Jens Axboe
2019-11-14 21:24 ` Pavel Begunkov [this message]
2019-11-14 22:37   ` [PATCHSET 0/2] io_uring support for linked timeouts Jens Axboe
2019-11-15  9:40     ` Pavel Begunkov
2019-11-15 14:21       ` Pavel Begunkov
2019-11-15 15:13         ` Jens Axboe
2019-11-15 17:11           ` Pavel Begunkov
2019-11-15 19:34             ` Jens Axboe
2019-11-15 21:16               ` Jens Axboe
2019-11-15 21:38                 ` Pavel Begunkov
2019-11-15 22:15                   ` Jens Axboe
2019-11-15 22:19                     ` Pavel Begunkov
2019-11-15 22:23                       ` Pavel Begunkov
2019-11-15 22:25                         ` Jens Axboe
2019-11-15 21:22               ` Pavel Begunkov
2019-11-15 21:26                 ` Jens Axboe
2019-11-19 21:11                   ` 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 \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox