| Message ID | 4890052.GXAFRqVoOG@kreacher (mailing list archive) |
|---|---|
| State | In Next |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | thermal/debugfs: Fix and clean up trip point statistics updates | expand |
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>
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>
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);