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