[PATCH 5/7] thermal: stats: introduce tz time in trip

[Eduardo Valentin]

On Fri, Jun 23, 2023 at 06:40:20PM +0200, Rafael J. Wysocki wrote:
> 
> 
> 
> On Wed, Jun 21, 2023 at 6:45 AM Eduardo Valentin <evalenti@xxxxxxxxxx> wrote:
> >
> > On Tue, Jun 20, 2023 at 07:27:57PM +0200, Rafael J. Wysocki wrote:
> > >
> > >
> > >
> > > On Fri, May 19, 2023 at 5:27 AM Eduardo Valentin <evalenti@xxxxxxxxxx> wrote:
> > > >
> > > > From: Eduardo Valentin <eduval@xxxxxxxxxx>
> > > >
> > > > This patch adds a statistic to report how long
> > > > the thermal zone spent on temperature intervals
> > > > created by each trip point. The first interval
> > > > is the range below the first trip point. All
> > > > subsequent intervals are accounted when temperature
> > > > is above the trip point temperature value.
> > > >
> > > > Samples:
> > > > $ cat /sys//class/thermal/thermal_zone0/stats/time_in_trip_ms
> > > > trip-1  0       0
> > >
> > > The above line is confusing.
> > >
> > > > trip0   -10000  35188
> > > > trip1   25000   0
> > >
> > > And the format violates the "one value per attribute" sysfs rule.
> > >
> > > > $ cat /sys//class/thermal/thermal_zone0/stats/time_in_trip_ms
> > > > trip-1  0       0
> > > > trip0   -10000  36901
> > > > trip1   25000   0
> > > > $ echo 25001 > /sys//class/thermal/thermal_zone0/emul_temp
> > > > $ cat /sys//class/thermal/thermal_zone0/stats/time_in_trip_ms
> > > > trip-1  0       0
> > > > trip0   -10000  47810
> > > > trip1   25000   2259
> > > > $ cat /sys//class/thermal/thermal_zone0/stats/time_in_trip_ms
> > > > trip-1  0       0
> > > > trip0   -10000  47810
> > > > trip1   25000   3224
> > > > $ echo 24001 > /sys//class/thermal/thermal_zone0/emul_temp
> > > > $ cat /sys//class/thermal/thermal_zone0/stats/time_in_trip_ms
> > > > trip-1  0       0
> > > > trip0   -10000  48960
> > > > trip1   25000   10080
> > > > $ cat /sys//class/thermal/thermal_zone0/stats/time_in_trip_ms
> > > > trip-1  0       0
> > > > trip0   -10000  49844
> > > > trip1   25000   10080
> > > >
> > > > Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx> (supporter:THERMAL)
> > > > Cc: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> (supporter:THERMAL)
> > > > Cc: Amit Kucheria <amitk@xxxxxxxxxx> (reviewer:THERMAL)
> > > > Cc: Zhang Rui <rui.zhang@xxxxxxxxx> (reviewer:THERMAL)
> > > > Cc: Jonathan Corbet <corbet@xxxxxxx> (maintainer:DOCUMENTATION)
> > > > Cc: linux-pm@xxxxxxxxxxxxxxx (open list:THERMAL)
> > > > Cc: linux-doc@xxxxxxxxxxxxxxx (open list:DOCUMENTATION)
> > > > Cc: linux-kernel@xxxxxxxxxxxxxxx (open list)
> > > >
> > > > Signed-off-by: Eduardo Valentin <eduval@xxxxxxxxxx>
> > > > ---
> > > >  .../driver-api/thermal/sysfs-api.rst          |  2 +
> > > >  drivers/thermal/thermal_sysfs.c               | 86 +++++++++++++++++++
> > > >  2 files changed, 88 insertions(+)
> > > >
> > > > diff --git a/Documentation/driver-api/thermal/sysfs-api.rst b/Documentation/driver-api/thermal/sysfs-api.rst
> > > > index ed5e6ba4e0d7..4a2b92a7488c 100644
> > > > --- a/Documentation/driver-api/thermal/sysfs-api.rst
> > > > +++ b/Documentation/driver-api/thermal/sysfs-api.rst
> > > > @@ -359,6 +359,8 @@ Thermal zone device sys I/F, created once it's registered::
> > > >      |---stats/reset_tz_stats:  Writes to this file resets the statistics.
> > > >      |---stats/max_gradient:    The maximum recorded dT/dt in uC/ms.
> > > >      |---stats/min_gradient:    The minimum recorded dT/dt in uC/ms.
> > > > +    |---stats/time_in_trip_ms: Time spent on each temperature interval of
> > > > +                               trip points.
> > >
> > > I would write "in each temperature interval between consecutive trip points".
> >
> > Ok
> >
> > >
> > > Doesn't this assume a specific temperature ordering of trip points?
> > > And so what if they are not ordered?
> >
> > It does. I believe other things will break if they are not ordered.
> 
> But there's no guarantee that they will be ordered, so it looks like
> those other things are already broken.

Correct. (1) there is no guarantee, it works by construction, and (2) current
code does assume ascending order, so yes, if they come unsorted, the core
code will not properly work.

Ensuring the order is likely beyond the original intention of this patch, but
we do need to improve there, for sure.

-- 
All the best,
Eduardo Valentin