Re: [RFC PATCH 00/10] Complete setup before calling wake_up_new_task() and improve task->comm

[Stefan Metzmacher <metze@samba.org>]

Hi Jens,

>> now that we have an explicit wake_up_new_task() in order to start the
>> result from create_io_thread(), we should things up before calling
>> wake_up_new_task().
>>
>> There're also some improvements around task->comm:
>> - We return 0 bytes for /proc/<pid>/cmdline
>> - We no longer allow a userspace process to change
>>   /proc/<pid>/[task/<tid>]/comm
>> - We dynamically generate comm names (up to 63 chars)
>>   via io_wq_worker_comm(), similar to wq_worker_comm()
>>
>> While doing this I noticed a few places we check for
>> PF_KTHREAD, but not PF_IO_WORKER, maybe we should
>> have something like a PS_IS_KERNEL_THREAD_MASK() macro
>> that should be used in generic places and only
>> explicitly use PF_IO_WORKER or PF_KTHREAD checks where the
>> difference matters.
>>
>> There are also quite a number of cases where we use
>> same_thread_group(), I guess these need to be checked.
>> Should that return true if userspace threads and their iothreds
>> are compared?

Can you comment more deeply here and recheck this in the code
I just noticed possible problems by reading the code and playing
with git grep. I don't have time right now to build my own 5.12 kernel
and play with it. (I'm planing to do some livepath tricks to inject
backported io_uring into an older kernel...).

For a lot of things regarding PF_KTHREAD v. PF_IO_WORKER and
same_thread_group() I'm just unsure what the correct behavior would be.

It would help if you could post dumps of things like:
ps axf -o user,pid,tid,comm,cmd
ls -laR /proc/$pid/

Currently I can only guess how things will look like.

>> I've compiled but didn't test, I hope there's something useful...
> 
> Looks pretty good to me. Can I talk you into splitting this into
> a series for 5.12, and then a 5.13 on top? It looks a bit mixed
> right now. For 5.12, basically just things we absolutely need for
> release. Any cleanups or improvements on top should go to 5.13.

I'll rebase tomorrow. Actually I'd like to see all of them in 5.12
because it would means that do the admin visible change only once.

The WARN_ON() fixes are not strictly needed, but for me it would be
strange to defer them.
io_wq_worker_comm() patches are not strictly required,
but they would make the new design more consistent and
avoid changing things again in 5.13.

But I'll let you decide...

Thanks!
metze