Date:   Sat, 25 Jun 2022 18:28:01 -0500
From:   "Eric W. Biederman" <ebiederm@...ssion.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Christian Brauner <brauner@...nel.org>, Tejun Heo <tj@...nel.org>,
        Petr Mladek <pmladek@...e.com>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Michal Hocko <mhocko@...e.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Oleg Nesterov <oleg@...hat.com>
Subject: Re: re. Spurious wakeup on a newly created kthread

Linus Torvalds <torvalds@...ux-foundation.org> writes:

> On Sat, Jun 25, 2022 at 11:25 AM Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
>>
>> And that's not at all what the kthread code wants. It wants to set
>> affinity masks, it wants to create a name for the thread, it wants to
>> do all those other things.
>>
>> That code really wants to just do copy_process().
>
> Honestly, I think kernel/kthread.c should be almost rewritten from scratch.
>
> I do not understand why it does all those odd keventd games at all,
> and why kthread_create_info exists in the first place.

I presume you mean kthreadd games?

> Why does kthread_create() not just create the thread directly itself,
> and instead does that odd queue it onto a work function?
>
> Some of that goes back to before the git history, and very little of
> it seems to make any sense. It's as if the code is meant to be able to
> run from interrupt context, but that can't be it: it's literally doing
> a GFP_KERNEL kmalloc, it's doing spin-locks without irq safety etc.
>
> So why is it calling kthreadd_task() to create the thread? Purely for
> some crazy odd "make that the parent" reason?
>
> I dunno.  The code is odd, unexplained, looks buggy, and most fo the
> reasons are probably entirely historical.

I can explain why kthreadd exists and why it creates the threads.

Very long ago in the context of random userspace processes people would
use kernel_thread to create threads and a helper function that I think
was called something like kernel_daemonize to scrub the userspace bits
off.

It was an unending sources of problems as the scrub was never complete
nor correct.

So with the introduction of kthreadd the kernel threads were moved
out of the userspace process tree, and userspace stopped being able to
influence the kernel threads.

AKA instead of doing the equivalent of a suid exec the code started
going the equivalent sshing into the local box.

We *need* to preserve that kind of separation.

I want to say that all that is required is that copy_process copies
from kthreadd.  Unfortunately that means that it needs to be kthreadd
doing the work, as copy_process does always copies from current.  It
would take quite a bit of work to untangle that mess.

It does appear possible to write a parallel function to copy_process
that is used only for creating kernel threads, and can streamline itself
because it knows it is creating kernel threads.

Short of that the code needs to keep routing through kthreadd.

Using create_io_thread or a dedicated wrapper around copy_process
certainly looks like it could simplify some of kthread creation.

> I'm adding Christian to this thread too, since I get the feeling that
> it really should be more tightly integrated with copy_process(), and
> that Christian might have comments.
>
> Christian, see some context in the thread here:
>
>   https://lore.kernel.org/all/CAHk-=wiC7rj1o7vTnYUPfD7YxAu09MZiZbahHqvLm9+Cgg1dFw@mail.gmail.com/
>
> for some of this.
>
>                Linus

Eric

Powered by blists - more mailing lists