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

[Jens Axboe <axboe@kernel.dk>]

On 11/14/19 2:24 PM, Pavel Begunkov wrote:
> 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.

I agree, it could do with a bit of cleanup. Incrementals would be
appreciated!

> 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.

What cases would that apply to, what would the timeout even do in this
case? The point of the linked timeout is to abort the previous command.
Maybe I'm not following what you mean here.

> 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

Logically it need not be the second, it just has to follow another
request. Is there a bug there?

> 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.

But that's how links work, though. If you keep linking, then everything
that depends on X will fail, if X itself isn't succesful.

> 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?

I just think we need to make sure the ground rules are sane. I'm going
to write a few test cases to make sure we do the right thing.

-- 
Jens Axboe