[PATCH v1 1/3] thermal/debugfs: Avoid excessive updates of trip point statistics

Posted by Rafael J. Wysocki 2 weeks ago

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Since thermal_debug_update_temp() is called before invoking
thermal_debug_tz_trip_down() for the trips that were crossed by the
zone temperature on the way up, it updates the statistics for them
as though the current zone temperature was above the low temperature
of each of them.  However, if a given trip has just been crossed on the
way down, the zone temperature is in fact below its low temperature,
but this is handled by thermal_debug_tz_trip_down() running after the
update of the trip statistics.

The remedy is to call thermal_debug_update_temp() after
thermal_debug_tz_trip_down() has been invoked for all of the
trips in question, but then thermal_debug_tz_trip_up() needs to
be adjusted, so it does not update the statistics for the trips
that has just been crossed on the way up, as that will be taken
care of by thermal_debug_update_temp() down the road.

Modify the code accordingly.

Fixes: 7ef01f228c9f ("thermal/debugfs: Add thermal debugfs information for mitigation episodes")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c    |    3 ++-
 drivers/thermal/thermal_debugfs.c |    7 -------
 2 files changed, 2 insertions(+), 8 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -427,7 +427,6 @@ static void update_temperature(struct th
 	trace_thermal_temperature(tz);
 
 	thermal_genl_sampling_temp(tz->id, temp);
-	thermal_debug_update_temp(tz);
 }
 
 static void thermal_zone_device_check(struct work_struct *work)
@@ -505,6 +504,8 @@ void __thermal_zone_device_update(struct
 	if (governor->manage)
 		governor->manage(tz);
 
+	thermal_debug_update_temp(tz);
+
 	monitor_thermal_zone(tz);
 }
 
Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.c
+++ linux-pm/drivers/thermal/thermal_debugfs.c
@@ -545,7 +545,6 @@ void thermal_debug_tz_trip_up(struct the
 	struct tz_episode *tze;
 	struct tz_debugfs *tz_dbg;
 	struct thermal_debugfs *thermal_dbg = tz->debugfs;
-	int temperature = tz->temperature;
 	int trip_id = thermal_zone_trip_id(tz, trip);
 	ktime_t now = ktime_get();
 
@@ -614,12 +613,6 @@ void thermal_debug_tz_trip_up(struct the
 
 	tze = list_first_entry(&tz_dbg->tz_episodes, struct tz_episode, node);
 	tze->trip_stats[trip_id].timestamp = now;
-	tze->trip_stats[trip_id].max = max(tze->trip_stats[trip_id].max, temperature);
-	tze->trip_stats[trip_id].min = min(tze->trip_stats[trip_id].min, temperature);
-	tze->trip_stats[trip_id].count++;
-	tze->trip_stats[trip_id].avg = tze->trip_stats[trip_id].avg +
-		(temperature - tze->trip_stats[trip_id].avg) /
-		tze->trip_stats[trip_id].count;
 
 unlock:
 	mutex_unlock(&thermal_dbg->lock);

Re: [PATCH v1 1/3] thermal/debugfs: Avoid excessive updates of trip point statistics

Posted by Daniel Lezcano 1 week, 1 day ago

On 17/04/2024 15:09, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Since thermal_debug_update_temp() is called before invoking
> thermal_debug_tz_trip_down() for the trips that were crossed by the
> zone temperature on the way up, it updates the statistics for them
> as though the current zone temperature was above the low temperature
> of each of them.  However, if a given trip has just been crossed on the
> way down, the zone temperature is in fact below its low temperature,
> but this is handled by thermal_debug_tz_trip_down() running after the
> update of the trip statistics.
> 
> The remedy is to call thermal_debug_update_temp() after
> thermal_debug_tz_trip_down() has been invoked for all of the
> trips in question, but then thermal_debug_tz_trip_up() needs to
> be adjusted, so it does not update the statistics for the trips
> that has just been crossed on the way up, as that will be taken
> care of by thermal_debug_update_temp() down the road.
> 
> Modify the code accordingly.
> 
> Fixes: 7ef01f228c9f ("thermal/debugfs: Add thermal debugfs information for mitigation episodes")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v1 1/3] thermal/debugfs: Avoid excessive updates of trip point statistics

Posted by Lukasz Luba 1 week, 2 days ago


On 4/17/24 14:09, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Since thermal_debug_update_temp() is called before invoking
> thermal_debug_tz_trip_down() for the trips that were crossed by the
> zone temperature on the way up, it updates the statistics for them
> as though the current zone temperature was above the low temperature
> of each of them.  However, if a given trip has just been crossed on the
> way down, the zone temperature is in fact below its low temperature,
> but this is handled by thermal_debug_tz_trip_down() running after the
> update of the trip statistics.
> 
> The remedy is to call thermal_debug_update_temp() after
> thermal_debug_tz_trip_down() has been invoked for all of the
> trips in question, but then thermal_debug_tz_trip_up() needs to
> be adjusted, so it does not update the statistics for the trips
> that has just been crossed on the way up, as that will be taken
> care of by thermal_debug_update_temp() down the road.
> 
> Modify the code accordingly.
> 
> Fixes: 7ef01f228c9f ("thermal/debugfs: Add thermal debugfs information for mitigation episodes")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/thermal/thermal_core.c    |    3 ++-
>   drivers/thermal/thermal_debugfs.c |    7 -------
>   2 files changed, 2 insertions(+), 8 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -427,7 +427,6 @@ static void update_temperature(struct th
>   	trace_thermal_temperature(tz);
>   
>   	thermal_genl_sampling_temp(tz->id, temp);
> -	thermal_debug_update_temp(tz);
>   }
>   
>   static void thermal_zone_device_check(struct work_struct *work)
> @@ -505,6 +504,8 @@ void __thermal_zone_device_update(struct
>   	if (governor->manage)
>   		governor->manage(tz);
>   
> +	thermal_debug_update_temp(tz);
> +
>   	monitor_thermal_zone(tz);
>   }
>   
> Index: linux-pm/drivers/thermal/thermal_debugfs.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_debugfs.c
> +++ linux-pm/drivers/thermal/thermal_debugfs.c
> @@ -545,7 +545,6 @@ void thermal_debug_tz_trip_up(struct the
>   	struct tz_episode *tze;
>   	struct tz_debugfs *tz_dbg;
>   	struct thermal_debugfs *thermal_dbg = tz->debugfs;
> -	int temperature = tz->temperature;
>   	int trip_id = thermal_zone_trip_id(tz, trip);
>   	ktime_t now = ktime_get();
>   
> @@ -614,12 +613,6 @@ void thermal_debug_tz_trip_up(struct the
>   
>   	tze = list_first_entry(&tz_dbg->tz_episodes, struct tz_episode, node);
>   	tze->trip_stats[trip_id].timestamp = now;
> -	tze->trip_stats[trip_id].max = max(tze->trip_stats[trip_id].max, temperature);
> -	tze->trip_stats[trip_id].min = min(tze->trip_stats[trip_id].min, temperature);
> -	tze->trip_stats[trip_id].count++;
> -	tze->trip_stats[trip_id].avg = tze->trip_stats[trip_id].avg +
> -		(temperature - tze->trip_stats[trip_id].avg) /
> -		tze->trip_stats[trip_id].count;
>   
>   unlock:
>   	mutex_unlock(&thermal_dbg->lock);
> 
> 
> 
> 

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>