lkml.org 
[lkml]   [2017]   [Apr]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v6 2/2] thermal: core: Add a back up thermal shutdown mechanism
From
Date


On Tuesday 18 April 2017 11:45 AM, Ravikumar wrote:
>
>
> On Tuesday 18 April 2017 09:59 AM, 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.
> Care to add testing information?

I used THERMAL_EMULATION to fake temperature more than trip point.
If the delay is lesser (< 20S) then i see that backup poweroff is called
and the system shuts down immediately after the delay time expires else
orderly_poweroff gracefully shuts off the system. I do not have the logs
right now.

>> Reported-by: Nishanth Menon <nm@ti.com>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> Acked-by: Eduardo Valentin <edubezval@gmail.com>
>> ---
>>
>> Changes in v6:
>>
>> * Rephrased Kconfig description as per Eduardo's feedback.
>> * Added check to verify positive values of delay in milli Seconds.
>>
>> Changes in v5:
>>
>> * Mandated delay for thermal emergency poweroff to be a non-zero
>> value.
>>
>> Changes in v4:
>>
>> * Updated documentation
>> * changed emergency_poweroff_func to thermal_emergency_poweroff_func
>>
>> Changes in v3:
>>
>> * Removed unnecessary mutex init.
>> * Added WARN messages instead of a simple warning message.
>> * Added Documentation.
>>
>> Documentation/thermal/sysfs-api.txt | 21 +++++++++++++++
>> drivers/thermal/Kconfig | 17 ++++++++++++
>> drivers/thermal/thermal_core.c | 53
>> +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 91 insertions(+)
>>
>> diff --git a/Documentation/thermal/sysfs-api.txt
>> b/Documentation/thermal/sysfs-api.txt
>> index ef473dc..bb9a0a5 100644
>> --- a/Documentation/thermal/sysfs-api.txt
>> +++ b/Documentation/thermal/sysfs-api.txt
>> @@ -582,3 +582,24 @@ platform data is provided, this uses the
>> step_wise throttling policy.
>> This function serves as an arbitrator to set the state of a cooling
>> device. It sets the cooling device to the deepest cooling state if
>> possible.
>> +
>> +6. thermal_emergency_poweroff:
>> +
> Should this be in sysfs-api doc?
>> +On an event of critical trip temperature crossing. Thermal framework
>> +allows the system to shutdown gracefully by calling orderly_poweroff().
>> +In the event of a failure of orderly_poweroff() to shut down the system
>> +we are in danger of keeping the system alive at undesirably high
>> +temperatures. To mitigate this high risk scenario we program a work
>> +queue to fire after a pre-determined number of seconds to start
>> +an emergency shutdown of the device using the kernel_power_off()
>> +function. In case kernel_power_off() fails then finally
>> +emergency_restart() is called in the worst case.
>> +
>> +The delay should be carefully profiled so as to give adequate time for
>> +orderly_poweroff(). In case of failure of an orderly_poweroff() the
>> +emergency poweroff kicks in after the delay has elapsed and shuts down
>> +the system.
>> +
> In order to come up with an ideal delay, we need to strike a balance
> between
> being paranoid vs being too late.
> In a different patch, I tried to justify setting crit temp @120C by quoting
> we need to give some time to orderly_poweroff()
>
> So we got T = [3/temp change rate] seconds before the HW issues a reset.
>
> within this T sec we need to give a chance to orderly_poweroff() and
> when it
> fails, bring out the big weapons.
>
> crumb: we might actually be increasing the "temp rate change" by doing a
> lot of IO
> access for syncing.
> Let us hope someone is trying to cool the system down while we are
> trying to
> save the day..
>> +If set to 0 emergency poweroff will not be supported. So a carefully
>> +profiled non-zero positive value is a must for emergerncy poweroff to be
>> +triggered.
> Profiling should be done based on real data than emulation.
> That's when we get to know if the memory and IOs listen to the SoC
> when the lava is out.
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index 9347401..74bf92b 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -15,6 +15,23 @@ menuconfig THERMAL
>> if THERMAL
>> +config THERMAL_EMERGENCY_POWEROFF_DELAY_MS
>> + int "Emergency poweroff delay in milli-seconds"
>> + depends on THERMAL
>> + default 0
>> + help
>> + Thermal subsystem will issue a graceful shutdown when
>> + critical temperatures are reached using orderly_poweroff(). In
>> + case of failure of an orderly_poweroff(), the thermal emergency
>> + poweroff kicks in after a delay has elapsed and shuts down the
>> system.
>> + This config is number of milliseconds to delay before emergency
>> + poweroff kicks in. Similarly to the critical trip point,
>> + the delay should be carefully profiled so as to give adequate
>> + time for orderly_poweroff() to finish on regular execution.
>> + If set to 0 emergency poweroff will not be supported.
>> +
>> + In doubt, leave as 0.
>> +
>> config THERMAL_HWMON
>> bool
>> prompt "Expose thermal sensors as hwmon device"
>> diff --git a/drivers/thermal/thermal_core.c
>> b/drivers/thermal/thermal_core.c
>> index 8337c27..b21b9cc 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -324,6 +324,54 @@ static void handle_non_critical_trips(struct
>> thermal_zone_device *tz,
>> def_governor->throttle(tz, trip);
>> }
>> +/**
>> + * thermal_emergency_poweroff_func - emergency poweroff work after a
>> known delay
> may needs to be re-phrased as this func itself can't handle the delay.
>> + * @work: work_struct associated with the emergency poweroff function
>> + *
>> + * This function is called in very critical situations to force
>> + * a kernel poweroff after a configurable timeout value.
>> + */
>> +static void thermal_emergency_poweroff_func(struct work_struct *work)
>> +{
>> + /*
>> + * We have reached here after the emergency thermal shutdown
>> + * Waiting period has expired. This means orderly_poweroff has
>> + * not been able to shut off the system for some reason.
>> + * Try to shut down the system immediately using kernel_power_off
>> + * if populated
>> + */
>> + WARN(1, "Attempting kernel_power_off: Temperature too high\n");
>> + kernel_power_off();
>> +
>> + /*
>> + * Worst of the worst case trigger emergency restart
>> + */
>> + WARN(1, "Attempting emergency_restart: Temperature too high\n");
>> + emergency_restart();
>> +}
>> +
>> +static DECLARE_DELAYED_WORK(thermal_emergency_poweroff_work,
>> + thermal_emergency_poweroff_func);
>> +
>> +/**
>> + * thermal_emergency_poweroff - Trigger an emergency system poweroff
> Here you may say after a pre-set delay.
>> + *
>> + * This may be called from any critical situation to trigger a system
>> shutdown
>> + * after a known period of time. By default this is not scheduled.
> This will be called only on a critical temperature event, right?
>> + */
>> +void thermal_emergency_poweroff(void)
>> +{
>> + int poweroff_delay_ms = CONFIG_THERMAL_EMERGENCY_POWEROFF_DELAY_MS;
>> + /*
>> + * poweroff_delay_ms must be a carefully profiled positive value.
>> + * Its a must for thermal_emergency_poweroff_work to be scheduled
> typo %s/Its/It's/
>> + */
>> + if (poweroff_delay_ms <= 0)
>> + return;
> It may be helpful to provide hint before returning?
> "Back up thermal emergency poweroff service is not enabled, set
>
> CONFIG_THERMAL_EMERGENCY_POWEROFF_DELAY_MS to a carefully profiled value
> to enable this service"
>
>> + schedule_delayed_work(&thermal_emergency_poweroff_work,
>> + msecs_to_jiffies(poweroff_delay_ms));
>> +}
>> +
>> static void handle_critical_trips(struct thermal_zone_device *tz,
>> int trip, enum thermal_trip_type trip_type)
>> {
>> @@ -346,6 +394,11 @@ static void handle_critical_trips(struct
>> thermal_zone_device *tz,
>> tz->temperature / 1000);
>> mutex_lock(&poweroff_lock);
>> if (!power_off_triggered) {
>> + /*
>> + * Queue a backup emergency shutdown in the event of
>> + * orderly_poweroff failure
>> + */
>> + thermal_emergency_poweroff();
> This comment is misleading because calling the api is not enough to set
> a backup.
>> orderly_poweroff(true);
>> power_off_triggered = true;
>> }
> Over all, much needed functionality. Thanks.
>
> Regards,
> RK

\
 
 \ /
  Last update: 2017-04-18 08:19    [W:0.078 / U:12.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site