Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230615121016.3731983-1-chenhuacai@loongson.cn>
In-Reply-To: <20230615121016.3731983-1-chenhuacai@loongson.cn>
From:   Huacai Chen <chenhuacai@kernel.org>
Date:   Sun, 25 Jun 2023 16:55:33 +0800
Message-ID: <CAAhV-H49AyrG-sH2SXLNg_cX-Wv8vS4Qno_2N4v6ccGGciv-+A@mail.gmail.com>
Subject: Re: [PATCH] kthread: Rename user_mode_thread() to kmuser_thread()
To:     Huacai Chen <chenhuacai@loongson.cn>
Cc:     Luis Chamberlain <mcgrof@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        "Eric W . Biederman" <ebiederm@xmission.com>,
        Kees Cook <keescook@chromium.org>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Friendly ping?

Huacai

On Thu, Jun 15, 2023 at 8:10=E2=80=AFPM Huacai Chen <chenhuacai@loongson.cn=
> wrote:
>
> Commit 343f4c49f2438d8 ("kthread: Don't allocate kthread_struct for init
> and umh") introduces a new function user_mode_thread() for init and umh.
>
> init and umh are different from typical kernel threads since the don't
> need a "kthread" struct and they will finally become user processes by
> calling kernel_execve(), but on the other hand, they are also different
> from typical user mode threads (they have no "mm" structs at creation
> time, which is traditionally used to distinguish a user thread and a
> kernel thread).
>
> In a former patch I treat init and umh as "special kernel threads" and
> unify kernel_thread() and user_mode_thread() to kernel_thread() again.
> However, the patch has been nacked because init and umh are essentially
> "special user threads".
>
> Nevertheless, I still agree with Andrews' comment "But the naming isn't
> very good anyway. They should have been usermode_thread/kernel_thread or
> user_thread/kernel_thread.".
>
> Since Eric describes init and umh as "user threads run in kernel mode",
> in this patch I rename user_mode_thread() as kmuser_thread(), which is
> a little better than just user_thread().
>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
>  include/linux/sched/task.h | 2 +-
>  init/main.c                | 2 +-
>  kernel/fork.c              | 4 ++--
>  kernel/umh.c               | 4 ++--
>  4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
> index e0f5ac90a228..c774d604b0a3 100644
> --- a/include/linux/sched/task.h
> +++ b/include/linux/sched/task.h
> @@ -98,7 +98,7 @@ struct task_struct *create_io_thread(int (*fn)(void *),=
 void *arg, int node);
>  struct task_struct *fork_idle(int);
>  extern pid_t kernel_thread(int (*fn)(void *), void *arg, const char *nam=
e,
>                             unsigned long flags);
> -extern pid_t user_mode_thread(int (*fn)(void *), void *arg, unsigned lon=
g flags);
> +extern pid_t kmuser_thread(int (*fn)(void *), void *arg, unsigned long f=
lags);
>  extern long kernel_wait4(pid_t, int __user *, int, struct rusage *);
>  int kernel_wait(pid_t pid, int *stat);
>
> diff --git a/init/main.c b/init/main.c
> index af50044deed5..362ba90d6f73 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -697,7 +697,7 @@ noinline void __ref __noreturn rest_init(void)
>          * the init task will end up wanting to create kthreads, which, i=
f
>          * we schedule it before we create kthreadd, will OOPS.
>          */
> -       pid =3D user_mode_thread(kernel_init, NULL, CLONE_FS);
> +       pid =3D kmuser_thread(kernel_init, NULL, CLONE_FS);
>         /*
>          * Pin init on the boot CPU. Task migration is not properly worki=
ng
>          * until sched_init_smp() has been run. It will set the allowed
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 41c964104b58..57d5c8c1766e 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2978,9 +2978,9 @@ pid_t kernel_thread(int (*fn)(void *), void *arg, c=
onst char *name,
>  }
>
>  /*
> - * Create a user mode thread.
> + * Create a kernel mode user thread.
>   */
> -pid_t user_mode_thread(int (*fn)(void *), void *arg, unsigned long flags=
)
> +pid_t kmuser_thread(int (*fn)(void *), void *arg, unsigned long flags)
>  {
>         struct kernel_clone_args args =3D {
>                 .flags          =3D ((lower_32_bits(flags) | CLONE_VM |
> diff --git a/kernel/umh.c b/kernel/umh.c
> index 60aa9e764a38..28c0cf0da7be 100644
> --- a/kernel/umh.c
> +++ b/kernel/umh.c
> @@ -130,7 +130,7 @@ static void call_usermodehelper_exec_sync(struct subp=
rocess_info *sub_info)
>
>         /* If SIGCLD is ignored do_wait won't populate the status. */
>         kernel_sigaction(SIGCHLD, SIG_DFL);
> -       pid =3D user_mode_thread(call_usermodehelper_exec_async, sub_info=
, SIGCHLD);
> +       pid =3D kmuser_thread(call_usermodehelper_exec_async, sub_info, S=
IGCHLD);
>         if (pid < 0)
>                 sub_info->retval =3D pid;
>         else
> @@ -169,7 +169,7 @@ static void call_usermodehelper_exec_work(struct work=
_struct *work)
>                  * want to pollute current->children, and we need a paren=
t
>                  * that always ignores SIGCHLD to ensure auto-reaping.
>                  */
> -               pid =3D user_mode_thread(call_usermodehelper_exec_async, =
sub_info,
> +               pid =3D kmuser_thread(call_usermodehelper_exec_async, sub=
_info,
>                                        CLONE_PARENT | SIGCHLD);
>                 if (pid < 0) {
>                         sub_info->retval =3D pid;
> --
> 2.39.3
>