Freezer Rewrite

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

Posted by Peter Zijlstra 3 years, 3 months ago

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@infradead.org>
---
 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:

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

Posted by Rafael J. Wysocki 3 years, 3 months ago

On Mon, Aug 22, 2022 at 1:48 PM Peter Zijlstra <peterz@infradead.org> 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@infradead.org>

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

> ---
>  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:
>
>

[tip: sched/core] freezer,umh: Clean up freezer/initrd interaction

Posted by tip-bot2 for Peter Zijlstra 3 years, 3 months ago

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     1fbcaa923ce2d7e6de17abd74fa076dc1e0be1a2
Gitweb:        https://git.kernel.org/tip/1fbcaa923ce2d7e6de17abd74fa076dc1e0be1a2
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Mon, 22 Aug 2022 13:18:18 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 07 Sep 2022 21:53:48 +02:00

freezer,umh: Clean up freezer/initrd interaction

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@infradead.org>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Link: https://lore.kernel.org/r/20220822114648.791019324@infradead.org
---
 include/linux/umh.h     |  9 +++++----
 init/do_mounts_initrd.c | 10 +---------
 kernel/umh.c            |  8 ++++++++
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/include/linux/umh.h b/include/linux/umh.h
index 244aff6..5d1f612 100644
--- 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;
diff --git a/init/do_mounts_initrd.c b/init/do_mounts_initrd.c
index 327962e..3473124 100644
--- a/init/do_mounts_initrd.c
+++ b/init/do_mounts_initrd.c
@@ -99,19 +99,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);
diff --git a/kernel/umh.c b/kernel/umh.c
index b989736..8945eaf 100644
--- 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 subprocess_info *sub_info, int wait)
 	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 subprocess_info *sub_info, int wait)
 	}
 
 	wait_for_completion(&done);
+
+	if (wait & UMH_FREEZABLE)
+		freezer_count();
+
 wait_done:
 	retval = sub_info->retval;
 out:

[PATCH v3 1/6] freezer: Have {,un}lock_system_sleep() save/restore flags
[PATCH v3 2/6] freezer,umh: Clean up freezer/initrd interaction
[PATCH v3 3/6] sched: Change wait_task_inactive()s match_state
[PATCH v3 4/6] sched/completion: Add wait_for_completion_state()
[PATCH v3 5/6] sched/wait: Add wait_event_state()
[PATCH v3 6/6] freezer,sched: Rewrite core freezer logic