linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Eric W. Biederman" <ebiederm@xmission.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Christian Brauner <brauner@kernel.org>, Tejun Heo <tj@kernel.org>,
	Petr Mladek <pmladek@suse.com>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Michal Hocko <mhocko@suse.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>
Subject: Re: re. Spurious wakeup on a newly created kthread
Date: Sat, 25 Jun 2022 18:28:01 -0500	[thread overview]
Message-ID: <87pmiw1fy6.fsf@email.froward.int.ebiederm.org> (raw)
In-Reply-To: <CAHk-=whLsaRKaFKS0UffeCYYCVyP0bbiB4BTYTaXtScgu6R9yA@mail.gmail.com> (Linus Torvalds's message of "Sat, 25 Jun 2022 11:43:15 -0700")

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

> On Sat, Jun 25, 2022 at 11:25 AM Linus Torvalds
> <torvalds@linux-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

  reply	other threads:[~2022-06-25 23:28 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-22 14:08 [PATCH] workqueue: Make create_worker() safe against spurious wakeups Petr Mladek
2022-06-23  7:00 ` Petr Mladek
2022-06-23  7:14   ` Michal Hocko
2022-06-25  5:00 ` re. Spurious wakeup on a newly created kthread Tejun Heo
2022-06-25 17:01   ` Linus Torvalds
2022-06-25 17:36     ` Eric W. Biederman
2022-06-25 18:25       ` Linus Torvalds
2022-06-25 18:43         ` Linus Torvalds
2022-06-25 23:28           ` Eric W. Biederman [this message]
2022-06-25 23:41             ` Eric W. Biederman
2022-06-25 23:43             ` Linus Torvalds
2022-06-25 23:48               ` Linus Torvalds
2022-06-26  0:19                 ` Eric W. Biederman
2022-06-27  0:01                   ` Wedson Almeida Filho
2022-06-27  7:11                     ` Peter Zijlstra
2022-06-27 18:23                       ` Wedson Almeida Filho
2022-06-27 18:45                         ` Linus Torvalds
2022-06-26 19:14                 ` [PATCH 0/3] kthread: Stop using TASK_UNINTERRUPTIBLE Eric W. Biederman
2022-06-26 19:15                   ` [PATCH 1/3] kthread: Remove the flags argument from kernel_thread Eric W. Biederman
2022-06-26 21:20                     ` Linus Torvalds
2022-06-26 19:16                   ` [PATCH 2/3] kthread: Replace kernel_thread with new_kthread Eric W. Biederman
2022-06-26 19:16                   ` [PATCH 3/3] kthread: Stop abusing TASK_UNINTERRUPTIBLE (INCOMPLETE) Eric W. Biederman
2022-06-26 19:59                     ` Linus Torvalds
2022-06-26 20:23                       ` Tejun Heo
2022-06-26 20:55                         ` Linus Torvalds
2022-06-27  7:22                         ` Peter Zijlstra
2022-06-27  8:11                           ` Tejun Heo
2022-06-27 18:04                             ` Wedson Almeida Filho
2022-06-27 22:06                               ` Peter Zijlstra
2022-06-27 22:34                                 ` Linus Torvalds
2022-06-27 22:45                                 ` Wedson Almeida Filho
2022-06-28  0:32                                 ` Wedson Almeida Filho
2022-06-28  7:58                                   ` Peter Zijlstra
2022-06-30  0:57                                     ` Wedson Almeida Filho
2022-06-26 22:14                     ` kernel test robot
2022-06-26 22:34                     ` kernel test robot
2022-06-26  0:21               ` re. Spurious wakeup on a newly created kthread Eric W. Biederman
2022-06-28 14:16           ` Christian Brauner
2022-06-26  0:26         ` Eric W. Biederman
2022-06-26  1:58     ` Tejun Heo
2022-06-26  2:53       ` Linus Torvalds
2022-06-26  6:09         ` Tejun Heo
2022-06-27 12:04         ` Michal Hocko
2022-06-28  9:51     ` Petr Mladek
2022-06-28 10:07       ` Tejun Heo
2022-06-27  8:07   ` Michal Hocko
2022-06-27  8:21     ` Tejun Heo
2022-06-27 10:18       ` Michal Hocko
2022-06-28 15:08     ` Petr Mladek
2022-08-04  8:57 ` [PATCH] workqueue: Make create_worker() safe against spurious wakeups Lai Jiangshan
2022-08-04 10:19   ` Lai Jiangshan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87pmiw1fy6.fsf@email.froward.int.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=akpm@linux-foundation.org \
    --cc=brauner@kernel.org \
    --cc=jiangshanlai@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhocko@suse.com \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).