All of lore.kernel.org
 help / color / mirror / Atom feed
From: Keerthy <a0393675@ti.com>
To: One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>,
	Keerthy <j-keerthy@ti.com>
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
Subject: Re: [PATCH 0/2] reboot: Introduce emergency_poweroff function
Date: Fri, 29 Jan 2016 07:15:20 +0530	[thread overview]
Message-ID: <56AAC430.3070107@ti.com> (raw)
In-Reply-To: <20160128132447.6b8bec2c@lxorguk.ukuu.org.uk>

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@ti.com> 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
>

WARNING: multiple messages have this Message-ID (diff)
From: Keerthy <a0393675@ti.com>
To: One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>,
	Keerthy <j-keerthy@ti.com>
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>
Subject: Re: [PATCH 0/2] reboot: Introduce emergency_poweroff function
Date: Fri, 29 Jan 2016 07:15:20 +0530	[thread overview]
Message-ID: <56AAC430.3070107@ti.com> (raw)
In-Reply-To: <20160128132447.6b8bec2c@lxorguk.ukuu.org.uk>

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@ti.com> 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
>

  reply	other threads:[~2016-01-29  1:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-28 13:06 [PATCH 0/2] reboot: Introduce emergency_poweroff function Keerthy
2016-01-28 13:06 ` Keerthy
2016-01-28 13:06 ` [PATCH 1/2] reboot: Introduce emergency poweroff function Keerthy
2016-01-28 13:06   ` Keerthy
2016-01-28 13:06 ` [PATCH 2/2] thermal: Use emergency_poweroff instead of orderly_poweroff for shutdown scenario Keerthy
2016-01-28 13:06   ` Keerthy
2016-01-28 13:24 ` [PATCH 0/2] reboot: Introduce emergency_poweroff function One Thousand Gnomes
2016-01-28 13:24   ` One Thousand Gnomes
2016-01-29  1:45   ` Keerthy [this message]
2016-01-29  1:45     ` Keerthy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56AAC430.3070107@ti.com \
    --to=a0393675@ti.com \
    --cc=edubezval@gmail.com \
    --cc=gnomes@lxorguk.ukuu.org.uk \
    --cc=grygorii.strashko@ti.com \
    --cc=j-keerthy@ti.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=nm@ti.com \
    --cc=oleg@redhat.com \
    --cc=rui.zhang@intel.com \
    --cc=toshi.kani@hp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.