From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753648AbcAME0E (ORCPT ); Tue, 12 Jan 2016 23:26:04 -0500 Received: from devils.ext.ti.com ([198.47.26.153]:57325 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753474AbcAME0C (ORCPT ); Tue, 12 Jan 2016 23:26:02 -0500 Subject: Re: [PATCH] reboot: Backup orderly_poweroff To: Josh Triplett , Keerthy References: <1452507804-20886-1-git-send-email-j-keerthy@ti.com> <20160111145516.GA30206@x> CC: , , , , , , , , , From: Keerthy Message-ID: <5695D1AE.3010802@ti.com> Date: Wed, 13 Jan 2016 09:55:18 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20160111145516.GA30206@x> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Monday 11 January 2016 08:25 PM, Josh Triplett wrote: > On Mon, Jan 11, 2016 at 03:53:24PM +0530, Keerthy wrote: >> orderly_poweroff is triggered when a graceful shutdown >> of system is desired. This may be used in many critical states of the >> kernel such as when subsystems detects conditions such as critical >> temperature conditions. However, in certain conditions in system >> boot up sequences like those in the middle of driver probes being >> initiated, userspace will be unable to power off the system in a clean >> manner and leaves the system in a critical state. In cases like these, >> the /sbin/poweroff will return success (having forked off to attempt >> powering off the system. However, the system overall will fail to >> completely poweroff (since other modules will be probed) and the system >> is still functional with no userspace (since that would have shut itself >> off). >> >> However, there is no clean way of detecting such failure of userspace >> powering off the system. In such scenarios, it is necessary for a backup >> workqueue to be able to force a shutdown of the system when orderly >> shutdown is not successful after a configurable time period. > > One issue below, inline. > >> Signed-off-by: Keerthy >> Suggested-by: Eduardo Valentin >> Reported-by: Nishanth Menon >> --- >> Links to previous discussion can be found here: >> >> http://www.spinics.net/lists/linux-omap/msg124925.html >> >> Boot tested on DRA7. >> >> arch/Kconfig | 7 +++++++ >> kernel/reboot.c | 23 ++++++++++++++++++----- >> 2 files changed, 25 insertions(+), 5 deletions(-) >> >> Index: linux/arch/Kconfig >> =================================================================== >> --- linux.orig/arch/Kconfig 2016-01-11 15:26:07.732173131 +0530 >> +++ linux/arch/Kconfig 2016-01-11 15:26:07.728173205 +0530 >> @@ -37,6 +37,18 @@ >> def_bool y >> depends on PERF_EVENTS && HAVE_PERF_EVENTS_NMI && !PPC64 >> >> +config SHUTDOWN_BACKUP_DELAY_MS >> + int "Backup shutdown delay in milli-seconds" >> + default 0 > > With no dependencies, as far as I can tell this will always get defined > with a value, defaulting to 0. (I don't know of any special cases in > Kconfig for an int value of 0.) Thus, in the code below... Yes. I will fix this and post a new version. Thanks for the review. Regards, Keerthy > >> --- linux.orig/kernel/reboot.c 2016-01-11 15:26:07.732173131 +0530 >> +++ linux/kernel/reboot.c 2016-01-11 15:38:33.502341511 +0530 >> @@ -424,6 +424,38 @@ >> return ret; >> } >> >> +#ifdef CONFIG_SHUTDOWN_BACKUP_DELAY_MS > > ...this should use #if, not #ifdef. Otherwise this code will get > compiled into every kernel. > > - Josh Triplett >