From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keerthy Subject: Re: [PATCH 0/2] reboot: Introduce emergency_poweroff function Date: Fri, 29 Jan 2016 07:15:20 +0530 Message-ID: <56AAC430.3070107@ti.com> References: <1453986389-15887-1-git-send-email-j-keerthy@ti.com> <20160128132447.6b8bec2c@lxorguk.ukuu.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20160128132447.6b8bec2c@lxorguk.ukuu.org.uk> Sender: linux-kernel-owner@vger.kernel.org To: One Thousand Gnomes , Keerthy Cc: linux-kernel@vger.kernel.org, oleg@redhat.com, toshi.kani@hp.com, linux-omap@vger.kernel.org, linux-pm@vger.kernel.org, nm@ti.com, grygorii.strashko@ti.com, mingo@kernel.org, josh@joshtriplett.org, rui.zhang@intel.com, edubezval@gmail.com List-Id: linux-omap@vger.kernel.org Hi Alan, On Thursday 28 January 2016 06:54 PM, One Thousand Gnomes wrote: > On Thu, 28 Jan 2016 18:36:27 +0530 > Keerthy wrote: > >> The series introduces a new function emergency_poweroff which shuts >> down the system after a configurable period of time. emergency_poweroff >> is appropriate in case of thermal shutdown scenario. > > That depends upon the system. > > If your hardware has its own built in thermal reset protection then it > will merely make things worse by causing unneeded crashes. > > If your device doesn't have protection then you have bigger problems > because a kernel crash or spin in interrupt space could easily mean that > the thermal thermals but doesn't ever run any delayed work. On those that > have a watchdog as well it should be using the hardware watchdog for > protection not relying upon schedule_delayed_work to get work done. > > So IMHO this should get a resounding NAK as it stands. For most systems > it's a backward change, for most of those that need more protection it > doesn't look the right answer. > > In particular if you need to be sure the box goes off *right now* you > don't want to schedule work because there are so many ways that it might > never execute the work when the box is failing. Scheduling work was done to give a configurable delay before powering off. I get your point that it might never get scheduled when things go wrong. > > Do your devices actually *really* need this, are you saying that someone > who roots the device can disable this code and physically destroy the > product ? If they do then I'd much rather see thermal_core call > thermal_poweroff(), and define that on a platform basis - so for x86 it > would be orderly_poweroff(), for your platform it might well be a > function that right that instant bangs the registers to force power off, > devices with watchdogs might write the watchdog with a 5 second timer and > then try and do an orderly_poweroff and so forth. Thanks for the feedback. I apologize for Ccing the wrong list. I removed it from the thread. Regards, Keerthy > > Alan > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755425AbcA2Bqm (ORCPT ); Thu, 28 Jan 2016 20:46:42 -0500 Received: from devils.ext.ti.com ([198.47.26.153]:35527 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753843AbcA2Bpw (ORCPT ); Thu, 28 Jan 2016 20:45:52 -0500 Subject: Re: [PATCH 0/2] reboot: Introduce emergency_poweroff function To: One Thousand Gnomes , Keerthy References: <1453986389-15887-1-git-send-email-j-keerthy@ti.com> <20160128132447.6b8bec2c@lxorguk.ukuu.org.uk> CC: , , , , , , , , , , From: Keerthy Message-ID: <56AAC430.3070107@ti.com> Date: Fri, 29 Jan 2016 07:15:20 +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: <20160128132447.6b8bec2c@lxorguk.ukuu.org.uk> 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 Hi Alan, On Thursday 28 January 2016 06:54 PM, One Thousand Gnomes wrote: > On Thu, 28 Jan 2016 18:36:27 +0530 > Keerthy wrote: > >> The series introduces a new function emergency_poweroff which shuts >> down the system after a configurable period of time. emergency_poweroff >> is appropriate in case of thermal shutdown scenario. > > That depends upon the system. > > If your hardware has its own built in thermal reset protection then it > will merely make things worse by causing unneeded crashes. > > If your device doesn't have protection then you have bigger problems > because a kernel crash or spin in interrupt space could easily mean that > the thermal thermals but doesn't ever run any delayed work. On those that > have a watchdog as well it should be using the hardware watchdog for > protection not relying upon schedule_delayed_work to get work done. > > So IMHO this should get a resounding NAK as it stands. For most systems > it's a backward change, for most of those that need more protection it > doesn't look the right answer. > > In particular if you need to be sure the box goes off *right now* you > don't want to schedule work because there are so many ways that it might > never execute the work when the box is failing. Scheduling work was done to give a configurable delay before powering off. I get your point that it might never get scheduled when things go wrong. > > Do your devices actually *really* need this, are you saying that someone > who roots the device can disable this code and physically destroy the > product ? If they do then I'd much rather see thermal_core call > thermal_poweroff(), and define that on a platform basis - so for x86 it > would be orderly_poweroff(), for your platform it might well be a > function that right that instant bangs the registers to force power off, > devices with watchdogs might write the watchdog with a 5 second timer and > then try and do an orderly_poweroff and so forth. Thanks for the feedback. I apologize for Ccing the wrong list. I removed it from the thread. Regards, Keerthy > > Alan >