From: Eduardo Valentin <[email protected]>
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
trip0 -10000 35188
trip1 25000 0
$ 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" <[email protected]> (supporter:THERMAL)
Cc: Daniel Lezcano <[email protected]> (supporter:THERMAL)
Cc: Amit Kucheria <[email protected]> (reviewer:THERMAL)
Cc: Zhang Rui <[email protected]> (reviewer:THERMAL)
Cc: Jonathan Corbet <[email protected]> (maintainer:DOCUMENTATION)
Cc: [email protected] (open list:THERMAL)
Cc: [email protected] (open list:DOCUMENTATION)
Cc: [email protected] (open list)
Signed-off-by: Eduardo Valentin <[email protected]>
---
.../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.
Thermal cooling device sys I/F, created once it's registered::
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index f89ec9a7e8c8..25851fe073c3 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -544,6 +544,7 @@ struct thermal_zone_device_stats {
s64 max_gradient;
s64 min_gradient;
ktime_t last_time;
+ ktime_t *time_in_trip;
};
#define DELTA_MILLI_C_TO_MICRO_C(t0, t1) (((t0) - (t1)) * 1000)
@@ -552,6 +553,7 @@ static void temperature_stats_update(struct thermal_zone_device *tz)
struct thermal_zone_device_stats *stats = tz->stats;
ktime_t now = ktime_get(), delta;
s64 cur_gradient, delta_temp;
+ int i, trip_id = -1;
delta = ktime_sub(now, stats->last_time);
stats->last_time = now;
@@ -567,6 +569,29 @@ static void temperature_stats_update(struct thermal_zone_device *tz)
if (tz->last_temperature == 0)
cur_gradient = 0;
+ for (i = 0; i < tz->num_trips; i++) {
+ struct thermal_trip trip;
+ int ret;
+
+ ret = __thermal_zone_get_trip(tz, i, &trip);
+ if (ret)
+ continue;
+
+ if (tz->temperature > trip.temperature)
+ trip_id = i;
+ }
+
+ /*
+ * Update how long we spend in each temperature interval.
+ * This array is like as follows:
+ * time_in_trip[0] == time spent with temperature <= trip[0]
+ * time_in_trip[1] == time spent with temperature > trip[0]
+ * time_in_trip[2] == time spent with temperature > trip[1]
+ * ...
+ */
+ stats->time_in_trip[trip_id + 1] =
+ ktime_add(stats->time_in_trip[trip_id + 1], delta);
+
/* update fastest temperature rise from our perspective */
if (cur_gradient > stats->max_gradient)
stats->max_gradient = cur_gradient;
@@ -615,12 +640,66 @@ static ssize_t min_gradient_show(struct device *dev,
return ret;
}
+static ssize_t
+time_in_trip_ms_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct thermal_zone_device *tz = to_thermal_zone(dev);
+ struct thermal_zone_device_stats *stats = tz->stats;
+ ssize_t len = 0, ret = 0;
+ int i;
+
+ spin_lock(&stats->lock);
+ temperature_stats_update(tz);
+
+ /*
+ * This should look like this:
+ * trip-1 X
+ * trip0 Y
+ * trip1 Z
+ * ...
+ * and the way to read is. This thermal zone temperature was seen
+ * X ms with tz->temperature <= trip0, Y ms with tz->temperature > trip0
+ * and Z ms with tz->temperature > trip1.
+ */
+ for (i = 0; i <= tz->num_trips; i++) {
+ int trip_temp = 0, r = 0;
+
+ if (i > 0) {
+ struct thermal_trip trip;
+
+ r = __thermal_zone_get_trip(tz, i - 1, &trip);
+ if (r < 0) {
+ ret = r;
+ break;
+ }
+ trip_temp = trip.temperature;
+ }
+
+ ret = snprintf(buf + len, PAGE_SIZE - len, "trip%d\t%d\t%llu\n",
+ i - 1, trip_temp,
+ ktime_to_ms(stats->time_in_trip[i]));
+
+ if (ret == 0)
+ ret = -EOVERFLOW;
+
+ if (ret < 0)
+ break;
+
+ len += ret;
+ }
+ spin_unlock(&stats->lock);
+
+ return ret < 0 ? ret : len;
+}
+
static ssize_t
reset_tz_stats_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct thermal_zone_device *tz = to_thermal_zone(dev);
struct thermal_zone_device_stats *stats = tz->stats;
+ int i;
spin_lock(&stats->lock);
@@ -628,6 +707,9 @@ reset_tz_stats_store(struct device *dev, struct device_attribute *attr,
stats->max_gradient = 0;
stats->last_time = ktime_get();
+ for (i = 0; i <= tz->num_trips; i++)
+ stats->time_in_trip[i] = ktime_set(0, 0);
+
spin_unlock(&stats->lock);
return count;
@@ -635,11 +717,13 @@ reset_tz_stats_store(struct device *dev, struct device_attribute *attr,
static DEVICE_ATTR_RO(min_gradient);
static DEVICE_ATTR_RO(max_gradient);
+static DEVICE_ATTR_RO(time_in_trip_ms);
static DEVICE_ATTR_WO(reset_tz_stats);
static struct attribute *thermal_zone_device_stats_attrs[] = {
&dev_attr_min_gradient.attr,
&dev_attr_max_gradient.attr,
+ &dev_attr_time_in_trip_ms.attr,
&dev_attr_reset_tz_stats.attr,
NULL
};
@@ -657,11 +741,13 @@ thermal_zone_device_stats_setup(struct thermal_zone_device *tz,
int var;
var = sizeof(*stats);
+ var += sizeof(*stats->time_in_trip) * (tz->num_trips + 1);
stats = kzalloc(var, GFP_KERNEL);
if (!stats)
return;
+ stats->time_in_trip = (ktime_t *)(stats + 1);
stats->last_time = ktime_get();
spin_lock_init(&stats->lock);
tz->stats = stats;
--
2.34.1
On Fri, May 19, 2023 at 5:27 AM Eduardo Valentin <[email protected]> wrote:
>
> From: Eduardo Valentin <[email protected]>
>
> 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" <[email protected]> (supporter:THERMAL)
> Cc: Daniel Lezcano <[email protected]> (supporter:THERMAL)
> Cc: Amit Kucheria <[email protected]> (reviewer:THERMAL)
> Cc: Zhang Rui <[email protected]> (reviewer:THERMAL)
> Cc: Jonathan Corbet <[email protected]> (maintainer:DOCUMENTATION)
> Cc: [email protected] (open list:THERMAL)
> Cc: [email protected] (open list:DOCUMENTATION)
> Cc: [email protected] (open list)
>
> Signed-off-by: Eduardo Valentin <[email protected]>
> ---
> .../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".
Doesn't this assume a specific temperature ordering of trip points?
And so what if they are not ordered?
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 <[email protected]> wrote:
> >
> > From: Eduardo Valentin <[email protected]>
> >
> > 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" <[email protected]> (supporter:THERMAL)
> > Cc: Daniel Lezcano <[email protected]> (supporter:THERMAL)
> > Cc: Amit Kucheria <[email protected]> (reviewer:THERMAL)
> > Cc: Zhang Rui <[email protected]> (reviewer:THERMAL)
> > Cc: Jonathan Corbet <[email protected]> (maintainer:DOCUMENTATION)
> > Cc: [email protected] (open list:THERMAL)
> > Cc: [email protected] (open list:DOCUMENTATION)
> > Cc: [email protected] (open list)
> >
> > Signed-off-by: Eduardo Valentin <[email protected]>
> > ---
> > .../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. Like the temperature update
against the governor throttle callback invocation in the thermal core.
--
All the best,
Eduardo Valentin
On Wed, Jun 21, 2023 at 6:45 AM Eduardo Valentin <[email protected]> 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 <[email protected]> wrote:
> > >
> > > From: Eduardo Valentin <[email protected]>
> > >
> > > 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" <[email protected]> (supporter:THERMAL)
> > > Cc: Daniel Lezcano <[email protected]> (supporter:THERMAL)
> > > Cc: Amit Kucheria <[email protected]> (reviewer:THERMAL)
> > > Cc: Zhang Rui <[email protected]> (reviewer:THERMAL)
> > > Cc: Jonathan Corbet <[email protected]> (maintainer:DOCUMENTATION)
> > > Cc: [email protected] (open list:THERMAL)
> > > Cc: [email protected] (open list:DOCUMENTATION)
> > > Cc: [email protected] (open list)
> > >
> > > Signed-off-by: Eduardo Valentin <[email protected]>
> > > ---
> > > .../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.
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 <[email protected]> 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 <[email protected]> wrote:
> > > >
> > > > From: Eduardo Valentin <[email protected]>
> > > >
> > > > 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" <[email protected]> (supporter:THERMAL)
> > > > Cc: Daniel Lezcano <[email protected]> (supporter:THERMAL)
> > > > Cc: Amit Kucheria <[email protected]> (reviewer:THERMAL)
> > > > Cc: Zhang Rui <[email protected]> (reviewer:THERMAL)
> > > > Cc: Jonathan Corbet <[email protected]> (maintainer:DOCUMENTATION)
> > > > Cc: [email protected] (open list:THERMAL)
> > > > Cc: [email protected] (open list:DOCUMENTATION)
> > > > Cc: [email protected] (open list)
> > > >
> > > > Signed-off-by: Eduardo Valentin <[email protected]>
> > > > ---
> > > > .../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