[PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot

[PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot

[Joel Stanley]

The kernel has orderly_poweroff which allows the kernel to initiate a
graceful shutdown of userspace, by running /sbin/poweroff. This adds
orderly_reboot that will cause userspace to shut itself down by calling
/sbin/reboot.

This will be used for shutdown initiated by a system controller on
platforms that do not use ACPI.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 include/linux/reboot.h |  1 +
 kernel/reboot.c        | 51 +++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/include/linux/reboot.h b/include/linux/reboot.h
index 48bf152..a4ffcd9 100644
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -68,6 +68,7 @@ void ctrl_alt_del(void);
 extern char poweroff_cmd[POWEROFF_CMD_PATH_LEN];
 
 extern int orderly_poweroff(bool force);
+extern int orderly_reboot(void);
 
 /*
  * Emergency restart, callable from an interrupt handler.
diff --git a/kernel/reboot.c b/kernel/reboot.c
index a3a9e24..d0aa1ec 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -306,8 +306,9 @@ void ctrl_alt_del(void)
 }
 
 char poweroff_cmd[POWEROFF_CMD_PATH_LEN] = "/sbin/poweroff";
+char reboot_cmd[POWEROFF_CMD_PATH_LEN] = "/sbin/reboot";
 
-static int __orderly_poweroff(bool force)
+static int run_cmd(const char *cmd)
 {
 	char **argv;
 	static char *envp[] = {
@@ -316,8 +317,7 @@ static int __orderly_poweroff(bool force)
 		NULL
 	};
 	int ret;
-
-	argv = argv_split(GFP_KERNEL, poweroff_cmd, NULL);
+	argv = argv_split(GFP_KERNEL, cmd, NULL);
 	if (argv) {
 		ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
 		argv_free(argv);
@@ -325,8 +325,33 @@ static int __orderly_poweroff(bool force)
 		ret = -ENOMEM;
 	}
 
+	return ret;
+}
+
+static int __orderly_reboot(void)
+{
+	int ret;
+
+	ret = run_cmd(reboot_cmd);
+
+	if (ret) {
+		pr_warn("Failed to start orderly reboot: forcing the issue\n");
+		emergency_sync();
+		kernel_restart(NULL);
+	}
+
+	return ret;
+}
+
+static int __orderly_poweroff(bool force)
+{
+	int ret;
+
+	ret = run_cmd(reboot_cmd);
+
 	if (ret && force) {
 		pr_warn("Failed to start orderly shutdown: forcing the issue\n");
+
 		/*
 		 * I guess this should try to kick off some daemon to sync and
 		 * poweroff asap.  Or not even bother syncing if we're doing an
@@ -364,6 +389,26 @@ int orderly_poweroff(bool force)
 }
 EXPORT_SYMBOL_GPL(orderly_poweroff);
 
+static void reboot_work_func(struct work_struct *work)
+{
+	__orderly_reboot();
+}
+
+static DECLARE_WORK(reboot_work, reboot_work_func);
+
+/**
+ * orderly_reboot - Trigger an orderly system reboot
+ *
+ * This may be called from any context to trigger a system reboot.
+ * If the orderly reboot fails, it will force an immediate reboot.
+ */
+int orderly_reboot(void)
+{
+	schedule_work(&reboot_work);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(orderly_reboot);
+
 static int __init reboot_setup(char *str)
 {
 	for (;;) {
-- 
2.1.4

[PATCH 2/2] powerpc/powernv: Reboot when requested by firmware

[Joel Stanley]

Use orderly_reboot so userspace will to shut itself down via the reboot
path. This is required for graceful reboot initiated by the BMC, such
as when a user uses ipmitool to issue a 'chassis power cycle' command.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 arch/powerpc/platforms/powernv/opal-power.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal-power.c b/arch/powerpc/platforms/powernv/opal-power.c
index 48bf5b0..ac46c2c 100644
--- a/arch/powerpc/platforms/powernv/opal-power.c
+++ b/arch/powerpc/platforms/powernv/opal-power.c
@@ -29,8 +29,9 @@ static int opal_power_control_event(struct notifier_block *nb,
 
 	switch (type) {
 	case SOFT_REBOOT:
-		/* Fall through. The service processor is responsible for
-		 * bringing the machine back up */
+		pr_info("OPAL: reboot requested\n");
+		orderly_reboot();
+		break;
 	case SOFT_OFF:
 		pr_info("OPAL: poweroff requested\n");
 		orderly_poweroff(true);
-- 
2.1.4

Re: [PATCH 2/2] powerpc/powernv: Reboot when requested by firmware

[Michael Ellerman]

On Mon, 2015-03-30 at 12:45 +1030, Joel Stanley wrote:
> Use orderly_reboot so userspace will to shut itself down via the reboot
> path. This is required for graceful reboot initiated by the BMC, such
> as when a user uses ipmitool to issue a 'chassis power cycle' command.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  arch/powerpc/platforms/powernv/opal-power.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/opal-power.c b/arch/powerpc/platforms/powernv/opal-power.c
> index 48bf5b0..ac46c2c 100644
> --- a/arch/powerpc/platforms/powernv/opal-power.c
> +++ b/arch/powerpc/platforms/powernv/opal-power.c
> @@ -29,8 +29,9 @@ static int opal_power_control_event(struct notifier_block *nb,
>  
>  	switch (type) {
>  	case SOFT_REBOOT:
> -		/* Fall through. The service processor is responsible for
> -		 * bringing the machine back up */
> +		pr_info("OPAL: reboot requested\n");
> +		orderly_reboot();
> +		break;

Acked-by: Michael Ellerman <mpe@ellerman.id.au>


Andrew, do you want to take these 2 via your tree? Assuming folks are OK with
patch 1.

cheers

Re: [PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot

[Andrew Morton]

On Mon, 30 Mar 2015 12:45:32 +1030 Joel Stanley <joel@jms.id.au> wrote:

> The kernel has orderly_poweroff which allows the kernel to initiate a
> graceful shutdown of userspace, by running /sbin/poweroff. This adds
> orderly_reboot that will cause userspace to shut itself down by calling
> /sbin/reboot.
> 
> This will be used for shutdown initiated by a system controller on
> platforms that do not use ACPI.

gee.  There are a lot of callers of emergency_restart().  Why is the
BMC reboot special, and how many of the emergency_restart() callers
really be using orderly_reboot()?

We have /proc/sys/kernel/poweroff_cmd.  Should we have
/proc/sys/kernel/reboot_cmd as well?  If not,
kernel/reboot.c:reboot_cmd[] can be made static ;) 

Re: [PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot

[Joel Stanley]

Hi Andrew,

On Wed, Apr 1, 2015 at 9:09 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Mon, 30 Mar 2015 12:45:32 +1030 Joel Stanley <joel@jms.id.au> wrote:
>
>> The kernel has orderly_poweroff which allows the kernel to initiate a
>> graceful shutdown of userspace, by running /sbin/poweroff. This adds
>> orderly_reboot that will cause userspace to shut itself down by calling
>> /sbin/reboot.
>>
>> This will be used for shutdown initiated by a system controller on
>> platforms that do not use ACPI.
>
> gee.  There are a lot of callers of emergency_restart().  Why is the
> BMC reboot special, and how many of the emergency_restart() callers
> really be using orderly_reboot()?

The BMC reboot is intended to be a graceful shutdown - let userspace
do it's thing before the system goes down.

Userspace may chose to stop and perform some long, slow teardown
before it gets around to shutting down. We don't want to move callers
over orderly_reboot() if they're shutting the system down due to a
critical failure, eg. printer on fire.

I had a read of the emergency_restart() callers and I didn't see any
obvious cases for moving over to orderly_reboot().

> We have /proc/sys/kernel/poweroff_cmd.  Should we have
> /proc/sys/kernel/reboot_cmd as well?  If not,
> kernel/reboot.c:reboot_cmd[] can be made static ;)

I don't think we need it. I'll make reboot_cmd[] static.

Cheers,

Joel

Re: [PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot

[Anshuman Khandual]

On 03/30/2015 07:45 AM, Joel Stanley wrote:
> The kernel has orderly_poweroff which allows the kernel to initiate a
> graceful shutdown of userspace, by running /sbin/poweroff. This adds
> orderly_reboot that will cause userspace to shut itself down by calling
> /sbin/reboot.
> 
> This will be used for shutdown initiated by a system controller on
> platforms that do not use ACPI.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  include/linux/reboot.h |  1 +
>  kernel/reboot.c        | 51 +++++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/reboot.h b/include/linux/reboot.h
> index 48bf152..a4ffcd9 100644
> --- a/include/linux/reboot.h
> +++ b/include/linux/reboot.h
> @@ -68,6 +68,7 @@ void ctrl_alt_del(void);
>  extern char poweroff_cmd[POWEROFF_CMD_PATH_LEN];
>  
>  extern int orderly_poweroff(bool force);
> +extern int orderly_reboot(void);
>  
>  /*
>   * Emergency restart, callable from an interrupt handler.
> diff --git a/kernel/reboot.c b/kernel/reboot.c
> index a3a9e24..d0aa1ec 100644
> --- a/kernel/reboot.c
> +++ b/kernel/reboot.c
> @@ -306,8 +306,9 @@ void ctrl_alt_del(void)
>  }
>  
>  char poweroff_cmd[POWEROFF_CMD_PATH_LEN] = "/sbin/poweroff";
> +char reboot_cmd[POWEROFF_CMD_PATH_LEN] = "/sbin/reboot";

Should not we declare one more REBOOT_CMD_PATH_LEN to make it cleaner.

>  
> -static int __orderly_poweroff(bool force)
> +static int run_cmd(const char *cmd)
>  {
>  	char **argv;
>  	static char *envp[] = {
> @@ -316,8 +317,7 @@ static int __orderly_poweroff(bool force)
>  		NULL
>  	};
>  	int ret;
> -
> -	argv = argv_split(GFP_KERNEL, poweroff_cmd, NULL);
> +	argv = argv_split(GFP_KERNEL, cmd, NULL);
>  	if (argv) {
>  		ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
>  		argv_free(argv);
> @@ -325,8 +325,33 @@ static int __orderly_poweroff(bool force)
>  		ret = -ENOMEM;
>  	}
>  
> +	return ret;
> +}
> +
> +static int __orderly_reboot(void)
> +{
> +	int ret;
> +
> +	ret = run_cmd(reboot_cmd);
> +
> +	if (ret) {
> +		pr_warn("Failed to start orderly reboot: forcing the issue\n");
> +		emergency_sync();
> +		kernel_restart(NULL);
> +	}
> +
> +	return ret;
> +}
> +
> +static int __orderly_poweroff(bool force)
> +{
> +	int ret;
> +
> +	ret = run_cmd(reboot_cmd);

Would it be poweroff_cmd instead of reboot_cmd ? Dont see poweroff_cmd getting used.

Re: [PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot

[Andrew Morton]

On Wed, 01 Apr 2015 10:22:08 +0530 Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:

> >  char poweroff_cmd[POWEROFF_CMD_PATH_LEN] = "/sbin/poweroff";
> > +char reboot_cmd[POWEROFF_CMD_PATH_LEN] = "/sbin/reboot";
> 
> Should not we declare one more REBOOT_CMD_PATH_LEN to make it cleaner.

It doesn't really seem necessary - they'll have the same value anyway.

But if you aren't going to implement the sysctl it isn't needed at all -
just do

static char reboot_cmd[] = "/sbin/reboot";

Re: [PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot

[Andrew Morton]

On Tue, 31 Mar 2015 22:03:26 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> static char reboot_cmd[] = "/sbin/reboot";

static const char, actually.

Re: [PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot

[Anshuman Khandual]

On 04/01/2015 08:47 AM, Joel Stanley wrote:
> Hi Andrew,
> 
> On Wed, Apr 1, 2015 at 9:09 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
>> > On Mon, 30 Mar 2015 12:45:32 +1030 Joel Stanley <joel@jms.id.au> wrote:
>> >
>>> >> The kernel has orderly_poweroff which allows the kernel to initiate a
>>> >> graceful shutdown of userspace, by running /sbin/poweroff. This adds
>>> >> orderly_reboot that will cause userspace to shut itself down by calling
>>> >> /sbin/reboot.
>>> >>
>>> >> This will be used for shutdown initiated by a system controller on
>>> >> platforms that do not use ACPI.
>> >
>> > gee.  There are a lot of callers of emergency_restart().  Why is the
>> > BMC reboot special, and how many of the emergency_restart() callers
>> > really be using orderly_reboot()?
> The BMC reboot is intended to be a graceful shutdown - let userspace
> do it's thing before the system goes down.
> 
> Userspace may chose to stop and perform some long, slow teardown
> before it gets around to shutting down. We don't want to move callers
> over orderly_reboot() if they're shutting the system down due to a
> critical failure, eg. printer on fire.
> 
> I had a read of the emergency_restart() callers and I didn't see any
> obvious cases for moving over to orderly_reboot().
> 
>> > We have /proc/sys/kernel/poweroff_cmd.  Should we have
>> > /proc/sys/kernel/reboot_cmd as well?  If not,
>> > kernel/reboot.c:reboot_cmd[] can be made static ;)
> I don't think we need it. I'll make reboot_cmd[] static.

Just to have parity with power off command, /proc/sys/kernel/reboot_cmd would
be nice to have.

Re: [PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot

[Joel Stanley]

On Wed, Apr 1, 2015 at 3:22 PM, Anshuman Khandual
<khandual@linux.vnet.ibm.com> wrote:
>> +static int __orderly_poweroff(bool force)
>> +{
>> +     int ret;
>> +
>> +     ret = run_cmd(reboot_cmd);
>
> Would it be poweroff_cmd instead of reboot_cmd ? Dont see poweroff_cmd getting used.

Yes, good catch. Thanks.

Joel