Allow IORING_OP_ASYNC_CANCEL to cancel requests on other rings

Allow IORING_OP_ASYNC_CANCEL to cancel requests on other rings

[Artyom Pavlov]

Greetings!

Right now when I want to cancel request which runs on a different ring I 
have to use IORING_OP_MSG_RING with a special len value. CQEs with res 
equal to this special value get intercepted by my code and 
IORING_OP_ASYNC_CANCEL SQE gets created in the receiver ring with 
user_data taken from the received message. This approach kind of works, 
but not efficient (it requires additional round trip through the ring) 
and somewhat fragile (it relies on lack of collisions between the 
special value and potential error codes).

I think it should be possible to add support for cancelling requests on 
other rings to IORING_OP_ASYNC_CANCEL by introducing a new flag. If the 
flag is enabled, then the fd field would be interpreted as fd of another 
ring to which cancellation request should be sent. Using the fd field 
would mean that the new flag would conflict with IORING_ASYNC_CANCEL_FD, 
so it could be worth to use a different field for receiver ring fd.

Best regards,
Artyom Pavlov.

Re: Allow IORING_OP_ASYNC_CANCEL to cancel requests on other rings

[Jens Axboe]

On 7/5/23 10:44?AM, Artyom Pavlov wrote:
> Greetings!
> 
> Right now when I want to cancel request which runs on a different ring
> I have to use IORING_OP_MSG_RING with a special len value. CQEs with
> res equal to this special value get intercepted by my code and
> IORING_OP_ASYNC_CANCEL SQE gets created in the receiver ring with
> user_data taken from the received message. This approach kind of
> works, but not efficient (it requires additional round trip through
> the ring) and somewhat fragile (it relies on lack of collisions
> between the special value and potential error codes).
> 
> I think it should be possible to add support for cancelling requests
> on other rings to IORING_OP_ASYNC_CANCEL by introducing a new flag. If
> the flag is enabled, then the fd field would be interpreted as fd of
> another ring to which cancellation request should be sent. Using the
> fd field would mean that the new flag would conflict with
> IORING_ASYNC_CANCEL_FD, so it could be worth to use a different field
> for receiver ring fd.

This could certainly work, though I think it'd be a good idea to use a
reserved field for the "other ring fd". As of right now, the
'splice_fd_in' descriptor field is not applicable to cancel requests, so
that'd probably be the right place to put it.

Some complications around locking here, as we'd need to grab the other
ring lock. If ring A and ring B both cancel requests for each other,
then there would be ordering concerns. But nothing that can't be worked
around.

Let me take a quick look at that.

-- 
Jens Axboe

Re: Allow IORING_OP_ASYNC_CANCEL to cancel requests on other rings

[Artyom Pavlov]

05.07.2023 21:32, Jens Axboe wrote:
> On 7/5/23 10:44?AM, Artyom Pavlov wrote:
>> Greetings!
>>
>> Right now when I want to cancel request which runs on a different ring
>> I have to use IORING_OP_MSG_RING with a special len value. CQEs with
>> res equal to this special value get intercepted by my code and
>> IORING_OP_ASYNC_CANCEL SQE gets created in the receiver ring with
>> user_data taken from the received message. This approach kind of
>> works, but not efficient (it requires additional round trip through
>> the ring) and somewhat fragile (it relies on lack of collisions
>> between the special value and potential error codes).
>>
>> I think it should be possible to add support for cancelling requests
>> on other rings to IORING_OP_ASYNC_CANCEL by introducing a new flag. If
>> the flag is enabled, then the fd field would be interpreted as fd of
>> another ring to which cancellation request should be sent. Using the
>> fd field would mean that the new flag would conflict with
>> IORING_ASYNC_CANCEL_FD, so it could be worth to use a different field
>> for receiver ring fd.
> This could certainly work, though I think it'd be a good idea to use a
> reserved field for the "other ring fd". As of right now, the
> 'splice_fd_in' descriptor field is not applicable to cancel requests, so
> that'd probably be the right place to put it.
>
> Some complications around locking here, as we'd need to grab the other
> ring lock. If ring A and ring B both cancel requests for each other,
> then there would be ordering concerns. But nothing that can't be worked
> around.
>
> Let me take a quick look at that.
Hi!

Any news?

 >If ring A and ring B both cancel requests for each other, then there 
would be ordering concerns.

I am not sure I understand the concern. Do you mean that task1 on ring1 
attempts to cancel task2 on ring2, while task2 attempts to cancel task1? 
I don't see how it's different when both tasks are on the same ring. 
Task2 may run when ring2 receives the cancellation request, but it looks 
similar to CQE for waking up task2 being already in competition ring. In 
both cases you would simply get -ENOENT in response to such SQE.

Best regards,
Artyom Pavlov.

Re: Allow IORING_OP_ASYNC_CANCEL to cancel requests on other rings

[Jens Axboe]

On 7/12/23 12:25?PM, Artyom Pavlov wrote:
> 05.07.2023 21:32, Jens Axboe wrote:
>> On 7/5/23 10:44?AM, Artyom Pavlov wrote:
>>> Greetings!
>>>
>>> Right now when I want to cancel request which runs on a different ring
>>> I have to use IORING_OP_MSG_RING with a special len value. CQEs with
>>> res equal to this special value get intercepted by my code and
>>> IORING_OP_ASYNC_CANCEL SQE gets created in the receiver ring with
>>> user_data taken from the received message. This approach kind of
>>> works, but not efficient (it requires additional round trip through
>>> the ring) and somewhat fragile (it relies on lack of collisions
>>> between the special value and potential error codes).
>>>
>>> I think it should be possible to add support for cancelling requests
>>> on other rings to IORING_OP_ASYNC_CANCEL by introducing a new flag. If
>>> the flag is enabled, then the fd field would be interpreted as fd of
>>> another ring to which cancellation request should be sent. Using the
>>> fd field would mean that the new flag would conflict with
>>> IORING_ASYNC_CANCEL_FD, so it could be worth to use a different field
>>> for receiver ring fd.
>> This could certainly work, though I think it'd be a good idea to use a
>> reserved field for the "other ring fd". As of right now, the
>> 'splice_fd_in' descriptor field is not applicable to cancel requests, so
>> that'd probably be the right place to put it.
>>
>> Some complications around locking here, as we'd need to grab the other
>> ring lock. If ring A and ring B both cancel requests for each other,
>> then there would be ordering concerns. But nothing that can't be worked
>> around.
>>
>> Let me take a quick look at that.
> Hi!
> 
> Any news?

Not yet, haven't had time to look into it yet.

>>If ring A and ring B both cancel requests for each other, then there would be ordering concerns.
> 
> I am not sure I understand the concern. Do you mean that task1 on
> ring1 attempts to cancel task2 on ring2, while task2 attempts to
> cancel task1? I don't see how it's different when both tasks are on
> the same ring. Task2 may run when ring2 receives the cancellation
> request, but it looks similar to CQE for waking up task2 being already
> in competition ring. In both cases you would simply get -ENOENT in
> response to such SQE.

It's just a locking concern, that is all.

-- 
Jens Axboe