Re: [PATCH RFC v4 12/15] io_uring/cmd: let cmds to know about dying task

[Bernd Schubert <bschubert@ddn.com>]

On 11/6/24 01:14, Pavel Begunkov wrote:
> On 11/5/24 23:02, Bernd Schubert wrote:
>> On 11/5/24 02:08, Pavel Begunkov wrote:
>>> On 11/4/24 22:15, Bernd Schubert wrote:
>>>> On 11/4/24 01:28, Pavel Begunkov wrote:
>>> ...
>>>>> In general if you need to change something, either stick your
>>>>> name, so that I know it might be a derivative, or reflect it in
>>>>> the commit message, e.g.
>>>>>
>>>>> Signed-off-by: initial author
>>>>> [Person 2: changed this and that]
>>>>> Signed-off-by: person 2
>>>>
>>>> Oh sorry, for sure. I totally forgot to update the commit message.
>>>>
>>>> Somehow the initial version didn't trigger. I need to double check to
>>>
>>> "Didn't trigger" like in "kernel was still crashing"?
>>
>> My initial problem was a crash in iov_iter_get_pages2() on process
>> kill. And when I tested your initial patch IO_URING_F_TASK_DEAD didn't
>> get set. Jens then asked to test with the version that I have in my
>> branch and that worked fine. Although in the mean time I wonder if
>> I made test mistake (like just fuse.ko reload instead of reboot with
>> new kernel). Just fixed a couple of issues in my branch (basically
>> ready for the next version send), will test the initial patch
>> again as first thing in the morning.
> 
> I see. Please let know if it doesn't work, it's not specific
> to fuse, if there is a problem it'd also affects other core
> io_uring parts.

In my current branch getting that situation is rather hard, but 
eventually got IO_URING_F_TASK_DEAD (>40 test rounds vs. every time
before...) - with the initial patch version - I think my testing was
flawed back that time.


> 
>>> FWIW, the original version is how it's handled in several places
>>> across io_uring, and the difference is a gap for !DEFER_TASKRUN
>>> when a task_work is queued somewhere in between when a task is
>>> started going through exit() but haven't got PF_EXITING set yet.
>>> IOW, should be harder to hit.
>>>
>>
>> Does that mean that the test for PF_EXITING is racy and we cannot
>> entirely rely on it?
> 
> No, the PF_EXITING check was fine, even though it'll look
> different now for unrelated reasons. What I'm saying is that the
> callback can get executed from the desired task, i.e.
> req->task == current, but it can happen from a late exit(2)/etc.
> path where the task is botched and likely doesn't have ->mm.
> 

Ah ok, thanks for the info!



Thanks,
Bernd