Re: [PATCH 3/3] io_uring: move struct io_kiocb from task_struct to io_uring_task

[Jens Axboe <axboe@kernel.dk>]

On 11/3/24 3:36 PM, Pavel Begunkov wrote:
> On 11/3/24 22:18, Jens Axboe wrote:
>> On 11/3/24 3:05 PM, Pavel Begunkov wrote:
>>> On 11/3/24 21:54, Jens Axboe wrote:
>>>> On 11/3/24 2:47 PM, Pavel Begunkov wrote:
>>>>> On 11/3/24 17:49, Jens Axboe wrote:
>>>>> ...
>>>>>> diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
>>>>> ...
>>>>>>         nd->head = prev_nd->head;
>>>>>> @@ -115,7 +115,7 @@ struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx)
>>>>>>         notif->opcode = IORING_OP_NOP;
>>>>>>         notif->flags = 0;
>>>>>>         notif->file = NULL;
>>>>>> -    notif->task = current;
>>>>>> +    notif->tctx = current->io_uring;
>>>>>>         io_get_task_refs(1);
>>>>>>         notif->file_node = NULL;
>>>>>>         notif->buf_node = NULL;
>>>>>> diff --git a/io_uring/poll.c b/io_uring/poll.c
>>>>>> index 7db3010b5733..56332893a4b0 100644
>>>>>> --- a/io_uring/poll.c
>>>>>> +++ b/io_uring/poll.c
>>>>>> @@ -224,8 +224,7 @@ static int io_poll_check_events(struct io_kiocb *req, struct io_tw_state *ts)
>>>>>>     {
>>>>>>         int v;
>>>>>>     -    /* req->task == current here, checking PF_EXITING is safe */
>>>>>> -    if (unlikely(req->task->flags & PF_EXITING))
>>>>>> +    if (unlikely(current->flags & PF_EXITING))
>>>>>>             return -ECANCELED
>>>>>
>>>>> Unlike what the comment says, req->task doesn't have to match current,
>>>>> in which case the new check does nothing and it'll break in many very
>>>>> interesting ways.
>>>>
>>>> In which cases does it not outside of fallback?
>>>
>>> I think it can only be fallback path
>>
>> I think so too, that's what I was getting at. Hence I think we should just
>> change these PF_EXITING checks to be PF_KTHREAD instead. If we're invoked
>> from that kind of context, cancel.
> 
> Replacing with just a PF_KTHREAD check won't be right, you can
> get here with the right task but after it has been half killed and
> marked PF_EXITING.

Right, but:

if (current->flags & (PF_EXITING | PF_KTHREAD))
	...

should be fine as it'll catch both cases with the single check.

-- 
Jens Axboe