Re: [PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot
On Wed, Apr 1, 2015 at 3:22 PM, Anshuman Khandual
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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot
On 04/01/2015 08:47 AM, Joel Stanley wrote: > Hi Andrew, > > On Wed, Apr 1, 2015 at 9:09 AM, Andrew Morton > wrote: >> > On Mon, 30 Mar 2015 12:45:32 +1030 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. >> > >> > 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. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot
On Tue, 31 Mar 2015 22:03:26 -0700 Andrew Morton wrote: > static char reboot_cmd[] = "/sbin/reboot"; static const char, actually. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot
On Wed, 01 Apr 2015 10:22:08 +0530 Anshuman Khandual 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"; ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot
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
> ---
> 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.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot
Hi Andrew, On Wed, Apr 1, 2015 at 9:09 AM, Andrew Morton wrote: > On Mon, 30 Mar 2015 12:45:32 +1030 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. > > 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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] kernel/reboot.c: Add orderly_reboot for graceful reboot
On Mon, 30 Mar 2015 12:45:32 +1030 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. 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 ;) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
