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

[Pavel Begunkov <asml.silence@gmail.com>]

On 11/3/24 22:51, Jens Axboe wrote:
> On 11/3/24 3:47 PM, Pavel Begunkov wrote:
>> On 11/3/24 22:40, Jens Axboe wrote:
...
>>> Right, but:
>>>
>>> if (current->flags & (PF_EXITING | PF_KTHREAD))
>>>      ...
>>>
>>> should be fine as it'll catch both cases with the single check.
>>
>> Was thinking to mention it, it should be fine buf feels wrong. Instead
>> of directly checking what we want, i.e. whether the task we want to run
>> the request from is dead, we are now doing "let's check if the task
>> is dead. Ah yes, let's also see if it's PF_KTHREAD which indirectly
>> implies that the task is dead because of implementation details."
>>
>> Should be fine to leave that, but why not just leave the check
>> how it was? Even if it now requires an extra deref through tctx.
> 
> I think it'd be better with a comment, I added one that says:
> 
> /* exiting original task or fallback work, cancel */
> 
> We can retain the original check, but it's actually a data race to check
> ->flags from a different task. Yes for this case we're in fallback work
> and the value should be long since stable, but seems prudent to just
> check for the two criteria we care about. At least the comment will be
> correct now ;-)

I don't think whack-a-mole'ing all cases is a good thing,
but at least it can get moved into a helper and be reused in
all other places.

if (io_tw_should_terminate(req, tw))
	fail;

should be more readable

-- 
Pavel Begunkov