Re: [PATCH 0/2] reboot: Introduce emergency_poweroff function

From: Keerthy
Date: Thu Jan 28 2016 - 20:46:45 EST

Hi Alan,

On Thursday 28 January 2016 06:54 PM, One Thousand Gnomes wrote:
On Thu, 28 Jan 2016 18:36:27 +0530
Keerthy <j-keerthy@xxxxxx> 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.