Re: [PATCHSET 0/2] io_uring support for linked timeouts

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

[-- 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 --]