On 11/16/20 10:16 AM, Pavel Begunkov wrote:
> On 16/11/2020 16:57, Jens Axboe wrote:
>> On 11/16/20 9:48 AM, Pavel Begunkov wrote:
>>> On 16/11/2020 16:33, Jens Axboe wrote:
>>>> On 11/15/20 5:56 AM, Pavel Begunkov wrote:
>>>>> As tasks now cancel only theirs requests, and inflight_wait is awaited
>>>>> only in io_uring_cancel_files(), which should be called with ->in_idle
>>>>> set, instead of keeping a separate inflight_wait use tctx->wait.
>>>>>
>>>>> That will add some spurious wakeups but actually is safer from point of
>>>>> not hanging the task.
>>>>>
>>>>> e.g.
>>>>> task1 | IRQ
>>>>> | *start* io_complete_rw_common(link)
>>>>> | link: req1 -> req2 -> req3(with files)
>>>>> *cancel_files() |
>>>>> io_wq_cancel(), etc. |
>>>>> | put_req(link), adds to io-wq req2
>>>>> schedule() |
>>>>>
>>>>> So, task1 will never try to cancel req2 or req3. If req2 is
>>>>> long-standing (e.g. read(empty_pipe)), this may hang.
>>>>
>>>> This looks like it's against 5.11, but also looks like we should add
>>>> it for 5.10?
>>>
>>> Yeah, 5.10 completely slipped my mind, I'll resend
>>
>> I applied it to 5.10, and fixed up the 5.11 side of things. So all good,
>> just wanted to confirm.
>
> Hmm, this won't work with 5.10, at least without
>
> b7e7fb9960b03c ("io_uring: cancel only requests of current task")
> https://git.kernel.dk/cgit/linux-block/commit/?h=for-5.11/io_uring&id=b7e7fb9960b03ca07866b5c016ac3ce5373ef207
>
> That's because tctx->wait is kicked only by requests of current task,
> but 5.10 cancels everything with specified ->files, including owned
> by other tasks.
Ah good point, let's leave it 5.11 for now.
--
Jens Axboe
