| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 10 Jul 2023 12:41:23 +0800
From: Di Shen <cindygm567@...il.com>
To: Di Shen <di.shen@...soc.com>
Cc: lukasz.luba@....com, rafael@...nel.org, daniel.lezcano@...aro.org,
amitk@...nel.org, rui.zhang@...el.com, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org, xuewen.yan@...soc.com,
jeson.gao@...soc.com, orsonzhai@...il.com, zhanglyra@...il.com
Subject: Re: [PATCH V5] thermal/core/power_allocator: reset thermal governor
when trip point is changed
On Mon, Jul 10, 2023 at 11:38 AM Di Shen <di.shen@...soc.com> wrote:
>
> When the thermal trip point is changed, the governor should
> be reset so that the policy algorithm can be updated to adapt
> to the new trip point.
>
> 1.Thermal governor is working for the passive trip point or active
> trip point. Therefore, when the trip point is changed it should
> reset the governor only for passic/active trip points.
>
> 2.For "power_allocator" governor reset, the parameters of pid
> controller and the states of power cooling devices should be reset.
>
> 2.1
> The IPA controls temperature using PID mechanism. It is a closed-
> loop feedback monitoring system. It uses the gap between target
> temperature and current temperature which says "error" as the
> input of the PID controller:
>
> err = desired_temperature - current_temperature
> P_max =
> k_p * err + k_i * err_integral + k_d * diff_err + sustainable_power
>
> That algorithm can 'learn' from the 'observed' in the past reaction
> for it's control decisions and accumulates that information in the
> I(Integral) part so that it can conpensate for those 'learned'
> mistakes.
>
> Based on the above, the most important is the desired temperature
> comes from the trip point. When the trip point is changed, all the
> previous learned errors won't help for the IPA. So the pid parameters
> should be reset for IPA governor reset.
>
> 2.2
> Other wise, the cooling devices should also be reset when the trip
> point is changed.
>
> This patch adds an ops for the thermal_governor structure to reset
> the governor and give the 'reset' function definition for power
> allocator. The ops is called when the trip points are changed via
> sysfs interface.
>
> Signed-off-by: Di Shen <di.shen@...soc.com>
> ---
> drivers/thermal/gov_power_allocator.c | 9 +++++++++
> drivers/thermal/thermal_trip.c | 5 +++++
> include/linux/thermal.h | 3 +++
> 3 files changed, 17 insertions(+)
>
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> index 8642f1096b91..8d17a10671e4 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -729,10 +729,19 @@ static int power_allocator_throttle(struct thermal_zone_device *tz, int trip_id)
> return allocate_power(tz, trip.temperature);
> }
>
> +static void power_allocator_reset(struct thermal_zone_device *tz)
> +{
> + struct power_allocator_params *params = tz->governor_data;
> +
> + reset_pid_controller(params);
> + allow_maximum_power(tz, true);
> +}
> +
> static struct thermal_governor thermal_gov_power_allocator = {
> .name = "power_allocator",
> .bind_to_tz = power_allocator_bind,
> .unbind_from_tz = power_allocator_unbind,
> .throttle = power_allocator_throttle,
> + .reset = power_allocator_reset,
> };
> THERMAL_GOVERNOR_DECLARE(thermal_gov_power_allocator);
> diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c
> index 907f3a4d7bc8..13bbe029c6ab 100644
> --- a/drivers/thermal/thermal_trip.c
> +++ b/drivers/thermal/thermal_trip.c
> @@ -173,6 +173,11 @@ int thermal_zone_set_trip(struct thermal_zone_device *tz, int trip_id,
> if (tz->trips && (t.temperature != trip->temperature || t.hysteresis != trip->hysteresis))
> tz->trips[trip_id] = *trip;
>
> + /* Reset the governor only when the trip type is active or passive. */
> + ret = (trip->type == THERMAL_TRIP_PASSIVE || trip->type == THERMAL_TRIP_ACTIVE);
> + if (ret && t.temperature != trip->temperature && tz->governor && tz->governor->reset)
> + tz->governor->reset(tz);
> +
> thermal_notify_tz_trip_change(tz->id, trip_id, trip->type,
> trip->temperature, trip->hysteresis);
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 87837094d549..d27d053319bf 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -197,6 +197,8 @@ struct thermal_zone_device {
> * thermal zone.
> * @throttle: callback called for every trip point even if temperature is
> * below the trip point temperature
> + * @reset: callback called for governor reset
> + *
> * @governor_list: node in thermal_governor_list (in thermal_core.c)
> */
> struct thermal_governor {
> @@ -204,6 +206,7 @@ struct thermal_governor {
> int (*bind_to_tz)(struct thermal_zone_device *tz);
> void (*unbind_from_tz)(struct thermal_zone_device *tz);
> int (*throttle)(struct thermal_zone_device *tz, int trip);
> + void (*reset)(struct thermal_zone_device *tz);
> struct list_head governor_list;
> };
>
> --
> 2.17.1
>
Sorry, the change log was not successfully saved to the commit message.
Add it as following here:
---
V5:
- Simplify the reset ops, make it no return value and no specific
trip ID as argument.
- Extend the commit message.
V4: [4]
- Compared to V3, handle it in thermal core instead of in governor.
- Add an ops to the governor structure, and call it when a trip
point is changed
- Define reset ops for power allocator.
V3: [3]
- Add fix tag.
V2: [2]
- Compared to v1, do not revert.
- Add a variable(last_switch_on_temp) in power_allocator_params
to record the last switch_on_temp value.
- Adds a function to renew the update flag and update the
last_switch_on_temp when thermal trips are writable.
V1: [1]
- Revert commit 0952177f2a1f.
[1] https://lore.kernel.org/all/20230309135515.1232-1-di.shen@unisoc.com/
[2] https://lore.kernel.org/all/20230315093008.17489-1-di.shen@unisoc.com/
[3] https://lore.kernel.org/all/20230320095620.7480-1-di.shen@unisoc.com/
[4] https://lore.kernel.org/all/20230619063534.12831-1-di.shen@unisoc.com/
---
Powered by blists - more mailing lists