2022-09-13 06:35:57

by Kai-Heng Feng

Subject: [PATCH v3 1/2] kernel/reboot: Add SYS_OFF_MODE_RESTART_PREPARE mode

Add SYS_OFF_MODE_RESTART_PREPARE callbacks can be invoked before system
restart.

This is a preparation for next patch.

Suggested-by: Dmitry Osipenko <[email protected]>
Signed-off-by: Kai-Heng Feng <[email protected]>
---
v3:
- New patch.

include/linux/reboot.h | 8 ++++++++
kernel/reboot.c | 17 +++++++++++++++++
2 files changed, 25 insertions(+)

diff --git a/include/linux/reboot.h b/include/linux/reboot.h
index e5d9ef886179c..ba87cdef2335a 100644
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -105,6 +105,14 @@ enum sys_off_mode {
*/
SYS_OFF_MODE_POWER_OFF,

+ /**
+ * @SYS_OFF_MODE_RESTART_PREPARE:
+ *
+ * Handlers prepare system to be powered off. Handlers are
+ * allowed to sleep.
+ */
+ SYS_OFF_MODE_RESTART_PREPARE,
+
/**
* @SYS_OFF_MODE_RESTART:
*
diff --git a/kernel/reboot.c b/kernel/reboot.c
index 3c35445bf5ad3..3bba88c7ffc6b 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -243,6 +243,17 @@ void migrate_to_reboot_cpu(void)
set_cpus_allowed_ptr(current, cpumask_of(cpu));
}

+/*
+ * Notifier list for kernel code which wants to be called
+ * to prepare system for restart.
+ */
+static BLOCKING_NOTIFIER_HEAD(restart_prep_handler_list);
+
+static void do_kernel_restart_prepare(void)
+{
+ blocking_notifier_call_chain(&restart_prep_handler_list, 0, NULL);
+}
+
/**
* kernel_restart - reboot the system
* @cmd: pointer to buffer containing command to execute for restart
@@ -254,6 +265,7 @@ void migrate_to_reboot_cpu(void)
void kernel_restart(char *cmd)
{
kernel_restart_prepare(cmd);
+ do_kernel_restart_prepare();
migrate_to_reboot_cpu();
syscore_shutdown();
if (!cmd)
@@ -396,6 +408,11 @@ register_sys_off_handler(enum sys_off_mode mode,
handler->list = &power_off_handler_list;
break;

+ case SYS_OFF_MODE_RESTART_PREPARE:
+ handler->list = &restart_prep_handler_list;
+ handler->blocking = true;
+ break;
+
case SYS_OFF_MODE_RESTART:
handler->list = &restart_handler_list;
break;
--
2.36.1

2022-09-13 06:59:22

by Kai-Heng Feng

Subject: [PATCH v3 2/2] PM: ACPI: reboot: Reinstate S5 for reboot

Commit d60cd06331a3 ("PM: ACPI: reboot: Use S5 for reboot") caused Dell
PowerEdge r440 hangs at reboot.

The issue is fixed by commit 2ca1c94ce0b6 ("tg3: Disable tg3 device on
system reboot to avoid triggering AER"), so use the new sysoff API to
reinstate S5 for reboot on ACPI-based systems.

Cc: Josef Bacik <[email protected]>
Suggested-by: Dmitry Osipenko <[email protected]>
Signed-off-by: Kai-Heng Feng <[email protected]>
---
v3:
- Use new API to invoke ACPI S5.

v2:
- Use do_kernel_power_off_prepare() instead.

drivers/acpi/sleep.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index ad4b2987b3d6e..dce5460902eed 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -1088,6 +1088,10 @@ int __init acpi_sleep_init(void)
register_sys_off_handler(SYS_OFF_MODE_POWER_OFF,
SYS_OFF_PRIO_FIRMWARE,
acpi_power_off, NULL);
+
+ register_sys_off_handler(SYS_OFF_MODE_RESTART_PREPARE,
+ SYS_OFF_PRIO_FIRMWARE,
+ acpi_power_off_prepare, NULL);
} else {
acpi_no_s5 = true;
}
--
2.36.1

2022-09-13 17:00:48

by Dmitry Osipenko

Subject: Re: [PATCH v3 2/2] PM: ACPI: reboot: Reinstate S5 for reboot

On 9/13/22 09:20, Kai-Heng Feng wrote:
> Commit d60cd06331a3 ("PM: ACPI: reboot: Use S5 for reboot") caused Dell
> PowerEdge r440 hangs at reboot.
>
> The issue is fixed by commit 2ca1c94ce0b6 ("tg3: Disable tg3 device on
> system reboot to avoid triggering AER"), so use the new sysoff API to
> reinstate S5 for reboot on ACPI-based systems.
>
> Cc: Josef Bacik <[email protected]>
> Suggested-by: Dmitry Osipenko <[email protected]>
> Signed-off-by: Kai-Heng Feng <[email protected]>
> ---
> v3:
> - Use new API to invoke ACPI S5.
>
> v2:
> - Use do_kernel_power_off_prepare() instead.
>
> drivers/acpi/sleep.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index ad4b2987b3d6e..dce5460902eed 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -1088,6 +1088,10 @@ int __init acpi_sleep_init(void)
> register_sys_off_handler(SYS_OFF_MODE_POWER_OFF,
> SYS_OFF_PRIO_FIRMWARE,
> acpi_power_off, NULL);
> +
> + register_sys_off_handler(SYS_OFF_MODE_RESTART_PREPARE,
> + SYS_OFF_PRIO_FIRMWARE,
> + acpi_power_off_prepare, NULL);

Maybe you could add a small comment to the code explaining why
acpi_power_off_prepare is used for restarting?

Is it safe to use S5 on restart for all devices in general?

--
Best regards,
Dmitry

2022-09-13 17:26:49

by Dmitry Osipenko

Subject: Re: [PATCH v3 1/2] kernel/reboot: Add SYS_OFF_MODE_RESTART_PREPARE mode

On 9/13/22 09:20, Kai-Heng Feng wrote:
> Add SYS_OFF_MODE_RESTART_PREPARE callbacks can be invoked before system
> restart.
>
> This is a preparation for next patch.
>
> Suggested-by: Dmitry Osipenko <[email protected]>
> Signed-off-by: Kai-Heng Feng <[email protected]>
> ---
> v3:
> - New patch.
>
> include/linux/reboot.h | 8 ++++++++
> kernel/reboot.c | 17 +++++++++++++++++
> 2 files changed, 25 insertions(+)
>
> diff --git a/include/linux/reboot.h b/include/linux/reboot.h
> index e5d9ef886179c..ba87cdef2335a 100644
> --- a/include/linux/reboot.h
> +++ b/include/linux/reboot.h
> @@ -105,6 +105,14 @@ enum sys_off_mode {
> */
> SYS_OFF_MODE_POWER_OFF,
>
> + /**
> + * @SYS_OFF_MODE_RESTART_PREPARE:
> + *
> + * Handlers prepare system to be powered off. Handlers are

s/powered off/restarted/

> + * allowed to sleep.
> + */
> + SYS_OFF_MODE_RESTART_PREPARE,
> +
> /**
> * @SYS_OFF_MODE_RESTART:
> *
> diff --git a/kernel/reboot.c b/kernel/reboot.c
> index 3c35445bf5ad3..3bba88c7ffc6b 100644
> --- a/kernel/reboot.c
> +++ b/kernel/reboot.c
> @@ -243,6 +243,17 @@ void migrate_to_reboot_cpu(void)
> set_cpus_allowed_ptr(current, cpumask_of(cpu));
> }
>
> +/*
> + * Notifier list for kernel code which wants to be called
> + * to prepare system for restart.
> + */
> +static BLOCKING_NOTIFIER_HEAD(restart_prep_handler_list);
> +
> +static void do_kernel_restart_prepare(void)
> +{
> + blocking_notifier_call_chain(&restart_prep_handler_list, 0, NULL);
> +}
> +
> /**
> * kernel_restart - reboot the system
> * @cmd: pointer to buffer containing command to execute for restart
> @@ -254,6 +265,7 @@ void migrate_to_reboot_cpu(void)
> void kernel_restart(char *cmd)
> {
> kernel_restart_prepare(cmd);
> + do_kernel_restart_prepare();
> migrate_to_reboot_cpu();
> syscore_shutdown();
> if (!cmd)
> @@ -396,6 +408,11 @@ register_sys_off_handler(enum sys_off_mode mode,
> handler->list = &power_off_handler_list;
> break;
>
> + case SYS_OFF_MODE_RESTART_PREPARE:
> + handler->list = &restart_prep_handler_list;
> + handler->blocking = true;
> + break;
> +
> case SYS_OFF_MODE_RESTART:
> handler->list = &restart_handler_list;
> break;

With the above comment addressed:

Reviewed-by: Dmitry Osipenko <[email protected]>

--
Best regards,
Dmitry

2022-09-16 00:47:50

by Kai-Heng Feng

Subject: Re: [PATCH v3 2/2] PM: ACPI: reboot: Reinstate S5 for reboot

On Tue, Sep 13, 2022 at 11:17 PM Dmitry Osipenko
<[email protected]> wrote:
>
> On 9/13/22 09:20, Kai-Heng Feng wrote:
> > Commit d60cd06331a3 ("PM: ACPI: reboot: Use S5 for reboot") caused Dell
> > PowerEdge r440 hangs at reboot.
> >
> > The issue is fixed by commit 2ca1c94ce0b6 ("tg3: Disable tg3 device on
> > system reboot to avoid triggering AER"), so use the new sysoff API to
> > reinstate S5 for reboot on ACPI-based systems.
> >
> > Cc: Josef Bacik <[email protected]>
> > Suggested-by: Dmitry Osipenko <[email protected]>
> > Signed-off-by: Kai-Heng Feng <[email protected]>
> > ---
> > v3:
> > - Use new API to invoke ACPI S5.
> >
> > v2:
> > - Use do_kernel_power_off_prepare() instead.
> >
> > drivers/acpi/sleep.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> > index ad4b2987b3d6e..dce5460902eed 100644
> > --- a/drivers/acpi/sleep.c
> > +++ b/drivers/acpi/sleep.c
> > @@ -1088,6 +1088,10 @@ int __init acpi_sleep_init(void)
> > register_sys_off_handler(SYS_OFF_MODE_POWER_OFF,
> > SYS_OFF_PRIO_FIRMWARE,
> > acpi_power_off, NULL);
> > +
> > + register_sys_off_handler(SYS_OFF_MODE_RESTART_PREPARE,
> > + SYS_OFF_PRIO_FIRMWARE,
> > + acpi_power_off_prepare, NULL);
>
> Maybe you could add a small comment to the code explaining why
> acpi_power_off_prepare is used for restarting?

Will do.

>
> Is it safe to use S5 on restart for all devices in general?

S5 should be used, but it may expose some driver bugs like the one
mentioned in the commit message.

Kai-Heng

>
> --
> Best regards,
> Dmitry
>

2022-09-16 00:49:25

by Kai-Heng Feng

Subject: Re: [PATCH v3 1/2] kernel/reboot: Add SYS_OFF_MODE_RESTART_PREPARE mode

On Tue, Sep 13, 2022 at 11:14 PM Dmitry Osipenko
<[email protected]> wrote:
>
> On 9/13/22 09:20, Kai-Heng Feng wrote:
> > Add SYS_OFF_MODE_RESTART_PREPARE callbacks can be invoked before system
> > restart.
> >
> > This is a preparation for next patch.
> >
> > Suggested-by: Dmitry Osipenko <[email protected]>
> > Signed-off-by: Kai-Heng Feng <[email protected]>
> > ---
> > v3:
> > - New patch.
> >
> > include/linux/reboot.h | 8 ++++++++
> > kernel/reboot.c | 17 +++++++++++++++++
> > 2 files changed, 25 insertions(+)
> >
> > diff --git a/include/linux/reboot.h b/include/linux/reboot.h
> > index e5d9ef886179c..ba87cdef2335a 100644
> > --- a/include/linux/reboot.h
> > +++ b/include/linux/reboot.h
> > @@ -105,6 +105,14 @@ enum sys_off_mode {
> > */
> > SYS_OFF_MODE_POWER_OFF,
> >
> > + /**
> > + * @SYS_OFF_MODE_RESTART_PREPARE:
> > + *
> > + * Handlers prepare system to be powered off. Handlers are
>
> s/powered off/restarted/

Will address this in next revision.

Kai-Heng

>
> > + * allowed to sleep.
> > + */
> > + SYS_OFF_MODE_RESTART_PREPARE,
> > +
> > /**
> > * @SYS_OFF_MODE_RESTART:
> > *
> > diff --git a/kernel/reboot.c b/kernel/reboot.c
> > index 3c35445bf5ad3..3bba88c7ffc6b 100644
> > --- a/kernel/reboot.c
> > +++ b/kernel/reboot.c
> > @@ -243,6 +243,17 @@ void migrate_to_reboot_cpu(void)
> > set_cpus_allowed_ptr(current, cpumask_of(cpu));
> > }
> >
> > +/*
> > + * Notifier list for kernel code which wants to be called
> > + * to prepare system for restart.
> > + */
> > +static BLOCKING_NOTIFIER_HEAD(restart_prep_handler_list);
> > +
> > +static void do_kernel_restart_prepare(void)
> > +{
> > + blocking_notifier_call_chain(&restart_prep_handler_list, 0, NULL);
> > +}
> > +
> > /**
> > * kernel_restart - reboot the system
> > * @cmd: pointer to buffer containing command to execute for restart
> > @@ -254,6 +265,7 @@ void migrate_to_reboot_cpu(void)
> > void kernel_restart(char *cmd)
> > {
> > kernel_restart_prepare(cmd);
> > + do_kernel_restart_prepare();
> > migrate_to_reboot_cpu();
> > syscore_shutdown();
> > if (!cmd)
> > @@ -396,6 +408,11 @@ register_sys_off_handler(enum sys_off_mode mode,
> > handler->list = &power_off_handler_list;
> > break;
> >
> > + case SYS_OFF_MODE_RESTART_PREPARE:
> > + handler->list = &restart_prep_handler_list;
> > + handler->blocking = true;
> > + break;
> > +
> > case SYS_OFF_MODE_RESTART:
> > handler->list = &restart_handler_list;
> > break;
>
> With the above comment addressed:
>
> Reviewed-by: Dmitry Osipenko <[email protected]>
>
> --
> Best regards,
> Dmitry
>