Re: [PATCH v3 2/6] freezer,umh: Clean up freezer/initrd interaction

[Rafael J. Wysocki]

On Mon, Aug 22, 2022 at 1:48 PM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> handle_initrd() marks itself as PF_FREEZER_SKIP in order to ensure
> that the UMH, which is going to freeze the system, doesn't
> indefinitely wait for it's caller.
>
> Rework things by adding UMH_FREEZABLE to indicate the completion is
> freezable.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

> ---
>  include/linux/umh.h     |    9 +++++----
>  init/do_mounts_initrd.c |   10 +---------
>  kernel/umh.c            |    8 ++++++++
>  3 files changed, 14 insertions(+), 13 deletions(-)
>
> --- a/include/linux/umh.h
> +++ b/include/linux/umh.h
> @@ -11,10 +11,11 @@
>  struct cred;
>  struct file;
>
> -#define UMH_NO_WAIT    0       /* don't wait at all */
> -#define UMH_WAIT_EXEC  1       /* wait for the exec, but not the process */
> -#define UMH_WAIT_PROC  2       /* wait for the process to complete */
> -#define UMH_KILLABLE   4       /* wait for EXEC/PROC killable */
> +#define UMH_NO_WAIT    0x00    /* don't wait at all */
> +#define UMH_WAIT_EXEC  0x01    /* wait for the exec, but not the process */
> +#define UMH_WAIT_PROC  0x02    /* wait for the process to complete */
> +#define UMH_KILLABLE   0x04    /* wait for EXEC/PROC killable */
> +#define UMH_FREEZABLE  0x08    /* wait for EXEC/PROC freezable */
>
>  struct subprocess_info {
>         struct work_struct work;
> --- a/init/do_mounts_initrd.c
> +++ b/init/do_mounts_initrd.c
> @@ -79,19 +79,11 @@ static void __init handle_initrd(void)
>         init_mkdir("/old", 0700);
>         init_chdir("/old");
>
> -       /*
> -        * In case that a resume from disk is carried out by linuxrc or one of
> -        * its children, we need to tell the freezer not to wait for us.
> -        */
> -       current->flags |= PF_FREEZER_SKIP;
> -
>         info = call_usermodehelper_setup("/linuxrc", argv, envp_init,
>                                          GFP_KERNEL, init_linuxrc, NULL, NULL);
>         if (!info)
>                 return;
> -       call_usermodehelper_exec(info, UMH_WAIT_PROC);
> -
> -       current->flags &= ~PF_FREEZER_SKIP;
> +       call_usermodehelper_exec(info, UMH_WAIT_PROC|UMH_FREEZABLE);
>
>         /* move initrd to rootfs' /old */
>         init_mount("..", ".", NULL, MS_MOVE, NULL);
> --- a/kernel/umh.c
> +++ b/kernel/umh.c
> @@ -28,6 +28,7 @@
>  #include <linux/async.h>
>  #include <linux/uaccess.h>
>  #include <linux/initrd.h>
> +#include <linux/freezer.h>
>
>  #include <trace/events/module.h>
>
> @@ -436,6 +437,9 @@ int call_usermodehelper_exec(struct subp
>         if (wait == UMH_NO_WAIT)        /* task has freed sub_info */
>                 goto unlock;
>
> +       if (wait & UMH_FREEZABLE)
> +               freezer_do_not_count();
> +
>         if (wait & UMH_KILLABLE) {
>                 retval = wait_for_completion_killable(&done);
>                 if (!retval)
> @@ -448,6 +452,10 @@ int call_usermodehelper_exec(struct subp
>         }
>
>         wait_for_completion(&done);
> +
> +       if (wait & UMH_FREEZABLE)
> +               freezer_count();
> +
>  wait_done:
>         retval = sub_info->retval;
>  out:
>
>