On Wednesday 12 April 2017 08:50 AM, Zhang Rui wrote: > On Wed, 2017-04-12 at 08:19 +0530, Keerthy wrote: >> >> On Tuesday 11 April 2017 10:59 PM, Eduardo Valentin wrote: >>> >>> Hey, >>> >>> On Fri, Mar 31, 2017 at 12:00:20PM +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). >>> OK... This seams to me, still a corner case supposed to be fixed at >>> orderly_power_off, not at thermal. But.. >>> >>>> >>>> >>>> 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. >>>> >>> Given that system running hot is a thermal issue, I guess we care >>> more >>> on this matter then.. >> Yes! >> > I just read this thread again https://patchwork.kernel.org/patch/802458 > 1/ to recall the previous discussion. > > https://patchwork.kernel.org/patch/8149891/ > https://patchwork.kernel.org/patch/8149861/ > should be the solution made based on Ingo' suggestion, right? > > And to me, this sounds like the right direction to go, thermal does not > need a back up shutdown solution, it just needs a kernel function call > which guarantees the system can be shutdown/reboot immediately. > > is there any reason that patch 1/2 is not accepted?
Zhang, http://www.serverphorums.com/read.php?12,1400964 I got a NAK from Alan and was given this direction on a thermal_poweroff which is more or less what is done in this patch. Thanks, Keerthy > > thanks, > rui >>> >>> >>>> >>>> Reported-by: Nishanth Menon <[email protected]> >>>> Signed-off-by: Keerthy <[email protected]> >>>> --- >>>> drivers/thermal/Kconfig | 13 +++++++++++++ >>>> drivers/thermal/thermal_core.c | 42 >>>> ++++++++++++++++++++++++++++++++++++++++++ >>>> 2 files changed, 55 insertions(+) >>>> >>>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig >>>> index 0a16cf4..4cc55f9 100644 >>>> --- a/drivers/thermal/Kconfig >>>> +++ b/drivers/thermal/Kconfig >>>> @@ -15,6 +15,19 @@ menuconfig THERMAL >>>> >>>> if THERMAL >>>> >>>> +config THERMAL_EMERGENCY_POWEROFF_DELAY_MS >>>> + int "Emergency poweroff delay in milli-seconds" >>>> + depends on THERMAL >>>> + default 0 >>>> + help >>>> + The number of milliseconds to delay before emergency >>>> + poweroff kicks in. 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. >>>> + >>>> + If set to 0 poweroff will happen immediately. >>>> + >>>> 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 11f0675..dc7fdd4 100644 >>>> --- a/drivers/thermal/thermal_core.c >>>> +++ b/drivers/thermal/thermal_core.c >>>> @@ -322,6 +322,47 @@ static void handle_non_critical_trips(struct >>>> thermal_zone_device *tz, >>>> def_governor->throttle(tz, trip); >>>> } >>>> >>>> +/** >>>> + * emergency_poweroff_func - emergency poweroff work after a >>>> known 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 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 >>>> pm_power_off >>>> + * if populated >>>> + */ >>> The above is not a kernel doc entry... >> I will fix that. >> >>> >>> >>>> >>>> + pr_warn("Attempting kernel_power_off\n"); >>>> + if (pm_power_off) >>>> + pm_power_off(); >>> Why not calling kernel_power_off() directly instead? That is what >>> is called by orderly >>> power off in case it fails, which seams to be the missing part >>> when >>> user land returns success, and therefore we don't call >>> kernel_power_off(). That path goes through the machine_power_off(), >>> which seams to be the default for pm_power_off() anyway. >>> >>> kernel_power_off() handles the power off system call too. >> Yes. This is after orderly_poweroff fails so i felt why go through >> kernel_power_off and directly call pm_power_off which directly pulls >> out >> the power plug. This is in dire straits situation. Hence preferred to >> call the last piece directly. >> >>> >>> >>>> >>>> + >>>> + /** >>> not a kernel doc entry... >> Okay. >> >>> >>> >>>> >>>> + * Worst of the worst case trigger emergency restart >>>> + */ >>>> + pr_warn("kernel_power_off has failed! Attempting >>>> emergency_restart\n"); >>>> + emergency_restart(); >>>> +} >>>> + >>>> +static DECLARE_DELAYED_WORK(emergency_poweroff_work, >>>> emergency_poweroff_func); >>>> + >>>> +/** >>>> + * emergency_poweroff - Trigger an emergency system poweroff >>>> + * >>>> + * This may be called from any critical situation to trigger a >>>> system shutdown >>>> + * after a known period of time. By default the delay is 0 >>>> millisecond >>>> + */ >>>> +void thermal_emergency_poweroff(void) >>>> +{ >>>> + schedule_delayed_work(&emergency_poweroff_work, >>>> + msecs_to_jiffies(CONFIG_THERMAL_EM >>>> ERGENCY_POWEROFF_DELAY_MS)); >>>> +} >>>> + >>>> static void handle_critical_trips(struct thermal_zone_device >>>> *tz, >>>> int trip, enum >>>> thermal_trip_type trip_type) >>>> { >>>> @@ -343,6 +384,7 @@ static void handle_critical_trips(struct >>>> thermal_zone_device *tz, >>>> "critical temperature reached(%d >>>> C),shutting down\n", >>>> tz->temperature / 1000); >>>> orderly_poweroff(true); >>>> + thermal_emergency_poweroff(); >>> Shouldn't we start count the timeout before calling >>> orderly_poweroff? >> Okay yes. That makes more sense. Queue the emergency function, start >> the >> countdown and immediately call the orderly_poweroff. I will fix the >> above comments and send a v2. I still want to go with pm_power_off >> over >> kernel_poweroff as we have already elapsed the time out and the first >> thing we want is to shut off the SoC! Let me know. >> >> >>> >>> >>>> >>>> } >>>> } >>>>
