[PATCH v1] kernel/reboot: Use static handler for register_platform_power_off()

[PATCH v1] kernel/reboot: Use static handler for register_platform_power_off()

[Dmitry Osipenko]

The register_platform_power_off() fails on m68k platform due to the
memory allocation error that happens at a very early boot time when
memory allocator isn't available yet. Fix it by using a static sys-off
handler for the platform-level power-off handlers.

Fixes: f0f7e5265b3b ("m68k: Switch to new sys-off handler API")
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 kernel/reboot.c | 43 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/kernel/reboot.c b/kernel/reboot.c
index a091145ee710..3b19b123efec 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -315,6 +315,37 @@ static int sys_off_notify(struct notifier_block *nb,
 	return handler->sys_off_cb(&data);
 }
 
+static struct sys_off_handler platform_sys_off_handler;
+
+static struct sys_off_handler *alloc_sys_off_handler(int priority)
+{
+	struct sys_off_handler *handler;
+
+	/*
+	 * Platforms like m68k can't allocate sys_off handler dynamically
+	 * at the early boot time because memory allocator isn't available yet.
+	 */
+	if (priority == SYS_OFF_PRIO_PLATFORM) {
+		handler = &platform_sys_off_handler;
+		if (handler->cb_data)
+			return ERR_PTR(-EBUSY);
+	} else {
+		handler = kzalloc(sizeof(*handler), GFP_KERNEL);
+		if (!handler)
+			return ERR_PTR(-ENOMEM);
+	}
+
+	return handler;
+}
+
+static void free_sys_off_handler(struct sys_off_handler *handler)
+{
+	if (handler == &platform_sys_off_handler)
+		memset(handler, 0, sizeof(*handler));
+	else
+		kfree(handler);
+}
+
 /**
  *	register_sys_off_handler - Register sys-off handler
  *	@mode: Sys-off mode
@@ -345,9 +376,9 @@ register_sys_off_handler(enum sys_off_mode mode,
 	struct sys_off_handler *handler;
 	int err;
 
-	handler = kzalloc(sizeof(*handler), GFP_KERNEL);
-	if (!handler)
-		return ERR_PTR(-ENOMEM);
+	handler = alloc_sys_off_handler(priority);
+	if (IS_ERR(handler))
+		return handler;
 
 	switch (mode) {
 	case SYS_OFF_MODE_POWER_OFF_PREPARE:
@@ -364,7 +395,7 @@ register_sys_off_handler(enum sys_off_mode mode,
 		break;
 
 	default:
-		kfree(handler);
+		free_sys_off_handler(handler);
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -391,7 +422,7 @@ register_sys_off_handler(enum sys_off_mode mode,
 	}
 
 	if (err) {
-		kfree(handler);
+		free_sys_off_handler(handler);
 		return ERR_PTR(err);
 	}
 
@@ -422,7 +453,7 @@ void unregister_sys_off_handler(struct sys_off_handler *handler)
 	/* sanity check, shall never happen */
 	WARN_ON(err);
 
-	kfree(handler);
+	free_sys_off_handler(handler);
 }
 EXPORT_SYMBOL_GPL(unregister_sys_off_handler);
 
-- 
2.35.3

Re: [PATCH v1] kernel/reboot: Use static handler for register_platform_power_off()

[Geert Uytterhoeven]

Hi Dmitry,

On Wed, Jun 1, 2022 at 12:11 AM Dmitry Osipenko
<dmitry.osipenko@collabora.com> wrote:
> The register_platform_power_off() fails on m68k platform due to the
> memory allocation error that happens at a very early boot time when
> memory allocator isn't available yet. Fix it by using a static sys-off
> handler for the platform-level power-off handlers.
>
> Fixes: f0f7e5265b3b ("m68k: Switch to new sys-off handler API")
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Thank you, that fixes the issue.
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v1] kernel/reboot: Use static handler for register_platform_power_off()

[Rafael J. Wysocki]

On Wed, Jun 1, 2022 at 12:11 AM Dmitry Osipenko
<dmitry.osipenko@collabora.com> wrote:
>
> The register_platform_power_off() fails on m68k platform due to the
> memory allocation error that happens at a very early boot time when
> memory allocator isn't available yet. Fix it by using a static sys-off
> handler for the platform-level power-off handlers.
>
> Fixes: f0f7e5265b3b ("m68k: Switch to new sys-off handler API")
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
>  kernel/reboot.c | 43 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/reboot.c b/kernel/reboot.c
> index a091145ee710..3b19b123efec 100644
> --- a/kernel/reboot.c
> +++ b/kernel/reboot.c
> @@ -315,6 +315,37 @@ static int sys_off_notify(struct notifier_block *nb,
>         return handler->sys_off_cb(&data);
>  }
>
> +static struct sys_off_handler platform_sys_off_handler;
> +
> +static struct sys_off_handler *alloc_sys_off_handler(int priority)
> +{
> +       struct sys_off_handler *handler;
> +
> +       /*
> +        * Platforms like m68k can't allocate sys_off handler dynamically
> +        * at the early boot time because memory allocator isn't available yet.
> +        */
> +       if (priority == SYS_OFF_PRIO_PLATFORM) {
> +               handler = &platform_sys_off_handler;
> +               if (handler->cb_data)
> +                       return ERR_PTR(-EBUSY);
> +       } else {
> +               handler = kzalloc(sizeof(*handler), GFP_KERNEL);
> +               if (!handler)
> +                       return ERR_PTR(-ENOMEM);
> +       }
> +
> +       return handler;
> +}
> +
> +static void free_sys_off_handler(struct sys_off_handler *handler)
> +{
> +       if (handler == &platform_sys_off_handler)
> +               memset(handler, 0, sizeof(*handler));
> +       else
> +               kfree(handler);
> +}
> +
>  /**
>   *     register_sys_off_handler - Register sys-off handler
>   *     @mode: Sys-off mode
> @@ -345,9 +376,9 @@ register_sys_off_handler(enum sys_off_mode mode,
>         struct sys_off_handler *handler;
>         int err;
>
> -       handler = kzalloc(sizeof(*handler), GFP_KERNEL);
> -       if (!handler)
> -               return ERR_PTR(-ENOMEM);
> +       handler = alloc_sys_off_handler(priority);
> +       if (IS_ERR(handler))
> +               return handler;
>
>         switch (mode) {
>         case SYS_OFF_MODE_POWER_OFF_PREPARE:
> @@ -364,7 +395,7 @@ register_sys_off_handler(enum sys_off_mode mode,
>                 break;
>
>         default:
> -               kfree(handler);
> +               free_sys_off_handler(handler);
>                 return ERR_PTR(-EINVAL);
>         }
>
> @@ -391,7 +422,7 @@ register_sys_off_handler(enum sys_off_mode mode,
>         }
>
>         if (err) {
> -               kfree(handler);
> +               free_sys_off_handler(handler);
>                 return ERR_PTR(err);
>         }
>
> @@ -422,7 +453,7 @@ void unregister_sys_off_handler(struct sys_off_handler *handler)
>         /* sanity check, shall never happen */
>         WARN_ON(err);
>
> -       kfree(handler);
> +       free_sys_off_handler(handler);
>  }
>  EXPORT_SYMBOL_GPL(unregister_sys_off_handler);
>
> --

Applied now (with the tags from Geert), but if there are any more
followup changes in that area, can you please CC them to linux-pm, so
people who have seen your original series can see them too?

Thanks!

Re: [PATCH v1] kernel/reboot: Use static handler for register_platform_power_off()

[Dmitry Osipenko]

On 6/2/22 21:40, Rafael J. Wysocki wrote:
..
> Applied now (with the tags from Geert), but if there are any more
> followup changes in that area, can you please CC them to linux-pm, so
> people who have seen your original series can see them too?
> 
> Thanks!

Sure, I actually wanted to CC linux-pm, but missed twice to update the
send-mail command from the bash history. Thank you all!

-- 
Best regards,
Dmitry