| Message ID | 2185763.irdbgypaU6@kreacher |
|---|---|
| State | New |
| Headers | show |
| Series | thermal/debugfs: Fix and clean up trip point statistics updates | expand |
On 4/17/24 14:10, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Notice that it is not necessary to compute tze in every iteration of the > for () loop in thermal_debug_update_temp() because it is the same for all > trips, so compute it once before the loop starts. > > Also use a trip_stats local variable to make the code in that loop easier > to follow and move the trip_id variable definition into that loop because > it is not used elsewhere in the function. > > While at it, change to order of local variable definitions in the function > to follow the reverse-xmas-tree pattern. > > No intentional functional impact. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/thermal/thermal_debugfs.c | 21 +++++++++++---------- > 1 file changed, 11 insertions(+), 10 deletions(-) > > Index: linux-pm/drivers/thermal/thermal_debugfs.c > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_debugfs.c > +++ linux-pm/drivers/thermal/thermal_debugfs.c > @@ -679,9 +679,9 @@ out: > void thermal_debug_update_temp(struct thermal_zone_device *tz) > { > struct thermal_debugfs *thermal_dbg = tz->debugfs; > - struct tz_episode *tze; > struct tz_debugfs *tz_dbg; > - int trip_id, i; > + struct tz_episode *tze; > + int i; > > if (!thermal_dbg) > return; > @@ -693,15 +693,16 @@ void thermal_debug_update_temp(struct th > if (!tz_dbg->nr_trips) > goto out; > > + tze = list_first_entry(&tz_dbg->tz_episodes, struct tz_episode, node); > + > for (i = 0; i < tz_dbg->nr_trips; i++) { > - trip_id = tz_dbg->trips_crossed[i]; > - tze = list_first_entry(&tz_dbg->tz_episodes, struct tz_episode, node); > - tze->trip_stats[trip_id].count++; > - tze->trip_stats[trip_id].max = max(tze->trip_stats[trip_id].max, tz->temperature); > - tze->trip_stats[trip_id].min = min(tze->trip_stats[trip_id].min, tz->temperature); > - tze->trip_stats[trip_id].avg = tze->trip_stats[trip_id].avg + > - (tz->temperature - tze->trip_stats[trip_id].avg) / > - tze->trip_stats[trip_id].count; > + int trip_id = tz_dbg->trips_crossed[i]; > + struct trip_stats *trip_stats = &tze->trip_stats[trip_id]; > + > + trip_stats->max = max(trip_stats->max, tz->temperature); > + trip_stats->min = min(trip_stats->min, tz->temperature); > + trip_stats->avg += (tz->temperature - trip_stats->avg) / > + ++trip_stats->count; > } > out: > mutex_unlock(&thermal_dbg->lock); > > > > Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
On 17/04/2024 15:10, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Notice that it is not necessary to compute tze in every iteration of the > for () loop in thermal_debug_update_temp() because it is the same for all > trips, so compute it once before the loop starts. > > Also use a trip_stats local variable to make the code in that loop easier > to follow and move the trip_id variable definition into that loop because > it is not used elsewhere in the function. > > While at it, change to order of local variable definitions in the function > to follow the reverse-xmas-tree pattern. > > No intentional functional impact. > > 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_debugfs.c =================================================================== --- linux-pm.orig/drivers/thermal/thermal_debugfs.c +++ linux-pm/drivers/thermal/thermal_debugfs.c @@ -679,9 +679,9 @@ out: void thermal_debug_update_temp(struct thermal_zone_device *tz) { struct thermal_debugfs *thermal_dbg = tz->debugfs; - struct tz_episode *tze; struct tz_debugfs *tz_dbg; - int trip_id, i; + struct tz_episode *tze; + int i; if (!thermal_dbg) return; @@ -693,15 +693,16 @@ void thermal_debug_update_temp(struct th if (!tz_dbg->nr_trips) goto out; + tze = list_first_entry(&tz_dbg->tz_episodes, struct tz_episode, node); + for (i = 0; i < tz_dbg->nr_trips; i++) { - trip_id = tz_dbg->trips_crossed[i]; - tze = list_first_entry(&tz_dbg->tz_episodes, struct tz_episode, node); - tze->trip_stats[trip_id].count++; - tze->trip_stats[trip_id].max = max(tze->trip_stats[trip_id].max, tz->temperature); - tze->trip_stats[trip_id].min = min(tze->trip_stats[trip_id].min, tz->temperature); - tze->trip_stats[trip_id].avg = tze->trip_stats[trip_id].avg + - (tz->temperature - tze->trip_stats[trip_id].avg) / - tze->trip_stats[trip_id].count; + int trip_id = tz_dbg->trips_crossed[i]; + struct trip_stats *trip_stats = &tze->trip_stats[trip_id]; + + trip_stats->max = max(trip_stats->max, tz->temperature); + trip_stats->min = min(trip_stats->min, tz->temperature); + trip_stats->avg += (tz->temperature - trip_stats->avg) / + ++trip_stats->count; } out: mutex_unlock(&thermal_dbg->lock);