Hi linux-pm,
The linux thermal framework doesn't have any support for tracing. This
makes it hard to run workloads and observe the thermal behaviour of
the system without actively polling files in sysfs or enabling debug
builds.
This patch set introduces trace events in the framework to allow
observing the behaviour of the different components in the
framework. The events added trace temperature changes, trip points and
cooling device state changes.
The patches are based on v3.15-rc8.
Cheers,
Punit
Punit Agrawal (3):
thermal: trace: Trace temperature changes
thermal: trace: Trace when a cooling device's state is updated
thermal: trace: Trace when temperature is above a trip point
drivers/thermal/fair_share.c | 7 +++-
drivers/thermal/step_wise.c | 5 ++-
drivers/thermal/thermal_core.c | 7 ++++
include/trace/events/thermal.h | 87 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 104 insertions(+), 2 deletions(-)
create mode 100644 include/trace/events/thermal.h
--
1.7.10.4
Create a new event to trace the temperature of a thermal zone. Using
this event trace the temperature changes of the thermal zone every-time
it is updated.
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Signed-off-by: Punit Agrawal <[email protected]>
---
drivers/thermal/thermal_core.c | 4 ++++
include/trace/events/thermal.h | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+)
create mode 100644 include/trace/events/thermal.h
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 71b0ec0..6b32391 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -38,6 +38,9 @@
#include <net/netlink.h>
#include <net/genetlink.h>
+#define CREATE_TRACE_POINTS
+#include <trace/events/thermal.h>
+
#include "thermal_core.h"
#include "thermal_hwmon.h"
@@ -463,6 +466,7 @@ static void update_temperature(struct thermal_zone_device *tz)
tz->temperature = temp;
mutex_unlock(&tz->lock);
+ trace_thermal_temperature(tz);
dev_dbg(&tz->device, "last_temperature=%d, current_temperature=%d\n",
tz->last_temperature, tz->temperature);
}
diff --git a/include/trace/events/thermal.h b/include/trace/events/thermal.h
new file mode 100644
index 0000000..8c5ca96
--- /dev/null
+++ b/include/trace/events/thermal.h
@@ -0,0 +1,38 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM thermal
+
+#if !defined(_TRACE_THERMAL_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_THERMAL_H
+
+#include <linux/thermal.h>
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(thermal_temperature,
+
+ TP_PROTO(struct thermal_zone_device *tz),
+
+ TP_ARGS(tz),
+
+ TP_STRUCT__entry(
+ __string(thermal_zone, tz->type)
+ __field(int, id)
+ __field(int, temp_prev)
+ __field(int, temp)
+ ),
+
+ TP_fast_assign(
+ __assign_str(thermal_zone, tz->type);
+ __entry->id = tz->id;
+ __entry->temp_prev = tz->last_temperature;
+ __entry->temp = tz->temperature;
+ ),
+
+ TP_printk("thermal_zone=%s id=%d temp_prev=%d temp=%d",
+ __get_str(thermal_zone), __entry->id, __entry->temp_prev,
+ __entry->temp)
+);
+
+#endif /* _TRACE_THERMAL_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
1.7.10.4
Create a new event to trace when the temperature is above a trip
point. Use the trace-point when handling non-critical and critical
trip pionts.
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Signed-off-by: Punit Agrawal <[email protected]>
---
Hi Steven,
I am facing an issue with partial trace being emitted when using
__print_symbolic in this patch.
When the trip_type is THERMAL_TRIP_ACTIVE (i.e., the first value in
the symbol map), the emitted trace contains the corresponding string
("active"). But for other values of trip_type an empty string is
emitted in the trace.
I've looked at other uses of __print_symbolic in the kernel and don't
see any difference in usage. Do you know what could be causing this or
alternately have any pointers on how to debug this behaviour?
Thanks.
Punit
drivers/thermal/fair_share.c | 7 ++++++-
drivers/thermal/step_wise.c | 5 ++++-
drivers/thermal/thermal_core.c | 2 ++
include/trace/events/thermal.h | 30 ++++++++++++++++++++++++++++++
4 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
index 944ba2f..2cddd68 100644
--- a/drivers/thermal/fair_share.c
+++ b/drivers/thermal/fair_share.c
@@ -23,6 +23,7 @@
*/
#include <linux/thermal.h>
+#include <trace/events/thermal.h>
#include "thermal_core.h"
@@ -34,14 +35,18 @@ static int get_trip_level(struct thermal_zone_device *tz)
{
int count = 0;
unsigned long trip_temp;
+ enum thermal_trip_type trip_type;
if (tz->trips == 0 || !tz->ops->get_trip_temp)
return 0;
for (count = 0; count < tz->trips; count++) {
tz->ops->get_trip_temp(tz, count, &trip_temp);
- if (tz->temperature < trip_temp)
+ if (tz->temperature < trip_temp) {
+ tz->ops->get_trip_type(tz, count, &trip_type);
+ trace_thermal_zone_trip(tz, count, trip_type);
break;
+ }
}
return count;
}
diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
index f251521..3b54c2c 100644
--- a/drivers/thermal/step_wise.c
+++ b/drivers/thermal/step_wise.c
@@ -23,6 +23,7 @@
*/
#include <linux/thermal.h>
+#include <trace/events/thermal.h>
#include "thermal_core.h"
@@ -129,8 +130,10 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
trend = get_tz_trend(tz, trip);
- if (tz->temperature >= trip_temp)
+ if (tz->temperature >= trip_temp) {
throttle = true;
+ trace_thermal_zone_trip(tz, trip, trip_type);
+ }
dev_dbg(&tz->device, "Trip%d[type=%d,temp=%ld]:trend=%d,throttle=%d\n",
trip, trip_type, trip_temp, trend, throttle);
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index c74c78d..454884a 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -371,6 +371,8 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
if (tz->temperature < trip_temp)
return;
+ trace_thermal_zone_trip(tz, trip, trip_type);
+
if (tz->ops->notify)
tz->ops->notify(tz, trip, trip_type);
diff --git a/include/trace/events/thermal.h b/include/trace/events/thermal.h
index 894a79e..5eeb1e7 100644
--- a/include/trace/events/thermal.h
+++ b/include/trace/events/thermal.h
@@ -51,6 +51,36 @@ TRACE_EVENT(cdev_update,
TP_printk("type=%s target=%lu", __get_str(type), __entry->target)
);
+TRACE_EVENT(thermal_zone_trip,
+
+ TP_PROTO(struct thermal_zone_device *tz, int trip,
+ enum thermal_trip_type trip_type),
+
+ TP_ARGS(tz, trip, trip_type),
+
+ TP_STRUCT__entry(
+ __string(thermal_zone, tz->type)
+ __field(int, id)
+ __field(int, trip)
+ __field(enum thermal_trip_type, trip_type)
+ ),
+
+ TP_fast_assign(
+ __assign_str(thermal_zone, tz->type);
+ __entry->id = tz->id;
+ __entry->trip = trip;
+ __entry->trip_type = trip_type;
+ ),
+
+ TP_printk("thermal_zone=%s id=%d trip=%d trip_type=%s",
+ __get_str(thermal_zone), __entry->id, __entry->trip,
+ __print_symbolic(__entry->trip_type,
+ { THERMAL_TRIP_ACTIVE, "active" },
+ { THERMAL_TRIP_PASSIVE, "passive" },
+ { THERMAL_TRIP_HOT, "hot" },
+ { THERMAL_TRIP_CRITICAL, "critical" }))
+);
+
#endif /* _TRACE_THERMAL_H */
/* This part must be outside protection */
--
1.7.10.4
Introduce and use an event to trace when a cooling device's state is
updated. This is useful to follow the effect of governor decisions on
cooling devices.
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Signed-off-by: Punit Agrawal <[email protected]>
---
drivers/thermal/thermal_core.c | 1 +
include/trace/events/thermal.h | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 6b32391..c74c78d 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1291,6 +1291,7 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev)
mutex_unlock(&cdev->lock);
cdev->ops->set_cur_state(cdev, target);
cdev->updated = true;
+ trace_cdev_update(cdev, target);
dev_dbg(&cdev->device, "set to state %lu\n", target);
}
EXPORT_SYMBOL(thermal_cdev_update);
diff --git a/include/trace/events/thermal.h b/include/trace/events/thermal.h
index 8c5ca96..894a79e 100644
--- a/include/trace/events/thermal.h
+++ b/include/trace/events/thermal.h
@@ -32,6 +32,25 @@ TRACE_EVENT(thermal_temperature,
__entry->temp)
);
+TRACE_EVENT(cdev_update,
+
+ TP_PROTO(struct thermal_cooling_device *cdev, unsigned long target),
+
+ TP_ARGS(cdev, target),
+
+ TP_STRUCT__entry(
+ __string(type, cdev->type)
+ __field(unsigned long, target)
+ ),
+
+ TP_fast_assign(
+ __assign_str(type, cdev->type);
+ __entry->target = target;
+ ),
+
+ TP_printk("type=%s target=%lu", __get_str(type), __entry->target)
+);
+
#endif /* _TRACE_THERMAL_H */
/* This part must be outside protection */
--
1.7.10.4
On Wed, 11 Jun 2014 12:31:44 +0100
Punit Agrawal <[email protected]> wrote:
> Create a new event to trace when the temperature is above a trip
> point. Use the trace-point when handling non-critical and critical
> trip pionts.
>
> Cc: Zhang Rui <[email protected]>
> Cc: Eduardo Valentin <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Signed-off-by: Punit Agrawal <[email protected]>
> ---
> Hi Steven,
>
> I am facing an issue with partial trace being emitted when using
> __print_symbolic in this patch.
>
> When the trip_type is THERMAL_TRIP_ACTIVE (i.e., the first value in
> the symbol map), the emitted trace contains the corresponding string
> ("active"). But for other values of trip_type an empty string is
> emitted in the trace.
>
> I've looked at other uses of __print_symbolic in the kernel and don't
> see any difference in usage. Do you know what could be causing this or
> alternately have any pointers on how to debug this behaviour?
>
If you can use trace-cmd to record your events then we can look at the
raw data too.
trace-cmd record -e thermal_zone_trip <some-command>
where <some-command> would trigger your tracepoint.
Then do: trace-cmd report -R
You should see the raw value of trip_type.
Make sure that it matches the enum values that you have listed.
-- Steve
> Thanks.
> Punit
>
> drivers/thermal/fair_share.c | 7 ++++++-
> drivers/thermal/step_wise.c | 5 ++++-
> drivers/thermal/thermal_core.c | 2 ++
> include/trace/events/thermal.h | 30 ++++++++++++++++++++++++++++++
> 4 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
> index 944ba2f..2cddd68 100644
> --- a/drivers/thermal/fair_share.c
> +++ b/drivers/thermal/fair_share.c
> @@ -23,6 +23,7 @@
> */
>
> #include <linux/thermal.h>
> +#include <trace/events/thermal.h>
>
> #include "thermal_core.h"
>
> @@ -34,14 +35,18 @@ static int get_trip_level(struct thermal_zone_device *tz)
> {
> int count = 0;
> unsigned long trip_temp;
> + enum thermal_trip_type trip_type;
>
> if (tz->trips == 0 || !tz->ops->get_trip_temp)
> return 0;
>
> for (count = 0; count < tz->trips; count++) {
> tz->ops->get_trip_temp(tz, count, &trip_temp);
> - if (tz->temperature < trip_temp)
> + if (tz->temperature < trip_temp) {
> + tz->ops->get_trip_type(tz, count, &trip_type);
> + trace_thermal_zone_trip(tz, count, trip_type);
> break;
> + }
> }
> return count;
> }
> diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
> index f251521..3b54c2c 100644
> --- a/drivers/thermal/step_wise.c
> +++ b/drivers/thermal/step_wise.c
> @@ -23,6 +23,7 @@
> */
>
> #include <linux/thermal.h>
> +#include <trace/events/thermal.h>
>
> #include "thermal_core.h"
>
> @@ -129,8 +130,10 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
>
> trend = get_tz_trend(tz, trip);
>
> - if (tz->temperature >= trip_temp)
> + if (tz->temperature >= trip_temp) {
> throttle = true;
> + trace_thermal_zone_trip(tz, trip, trip_type);
> + }
>
> dev_dbg(&tz->device, "Trip%d[type=%d,temp=%ld]:trend=%d,throttle=%d\n",
> trip, trip_type, trip_temp, trend, throttle);
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index c74c78d..454884a 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -371,6 +371,8 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
> if (tz->temperature < trip_temp)
> return;
>
> + trace_thermal_zone_trip(tz, trip, trip_type);
> +
> if (tz->ops->notify)
> tz->ops->notify(tz, trip, trip_type);
>
> diff --git a/include/trace/events/thermal.h b/include/trace/events/thermal.h
> index 894a79e..5eeb1e7 100644
> --- a/include/trace/events/thermal.h
> +++ b/include/trace/events/thermal.h
> @@ -51,6 +51,36 @@ TRACE_EVENT(cdev_update,
> TP_printk("type=%s target=%lu", __get_str(type), __entry->target)
> );
>
> +TRACE_EVENT(,
> +
> + TP_PROTO(struct thermal_zone_device *tz, int trip,
> + enum thermal_trip_type trip_type),
> +
> + TP_ARGS(tz, trip, trip_type),
> +
> + TP_STRUCT__entry(
> + __string(thermal_zone, tz->type)
> + __field(int, id)
> + __field(int, trip)
> + __field(enum thermal_trip_type, trip_type)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(thermal_zone, tz->type);
> + __entry->id = tz->id;
> + __entry->trip = trip;
> + __entry->trip_type = trip_type;
> + ),
> +
> + TP_printk("thermal_zone=%s id=%d trip=%d trip_type=%s",
> + __get_str(thermal_zone), __entry->id, __entry->trip,
> + __print_symbolic(__entry->trip_type,
> + { THERMAL_TRIP_ACTIVE, "active" },
> + { THERMAL_TRIP_PASSIVE, "passive" },
> + { THERMAL_TRIP_HOT, "hot" },
> + { THERMAL_TRIP_CRITICAL, "critical" }))
> +);
> +
> #endif /* _TRACE_THERMAL_H */
>
> /* This part must be outside protection */
Thanks for the quick response.
Steven Rostedt <[email protected]> writes:
> On Wed, 11 Jun 2014 12:31:44 +0100
> Punit Agrawal <[email protected]> wrote:
>
>> Create a new event to trace when the temperature is above a trip
>> point. Use the trace-point when handling non-critical and critical
>> trip pionts.
>>
>> Cc: Zhang Rui <[email protected]>
>> Cc: Eduardo Valentin <[email protected]>
>> Cc: Steven Rostedt <[email protected]>
>> Cc: Frederic Weisbecker <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Signed-off-by: Punit Agrawal <[email protected]>
>> ---
>> Hi Steven,
>>
>> I am facing an issue with partial trace being emitted when using
>> __print_symbolic in this patch.
>>
>> When the trip_type is THERMAL_TRIP_ACTIVE (i.e., the first value in
>> the symbol map), the emitted trace contains the corresponding string
>> ("active"). But for other values of trip_type an empty string is
>> emitted in the trace.
>>
>> I've looked at other uses of __print_symbolic in the kernel and don't
>> see any difference in usage. Do you know what could be causing this or
>> alternately have any pointers on how to debug this behaviour?
>>
>
> If you can use trace-cmd to record your events then we can look at the
> raw data too.
>
> trace-cmd record -e thermal_zone_trip <some-command>
>
> where <some-command> would trigger your tracepoint.
>
> Then do: trace-cmd report -R
>
> You should see the raw value of trip_type.
>
> Make sure that it matches the enum values that you have listed.
>
I do indeed see the value of trip_type and it matches what's being
traced.
~# trace-cmd report | grep thermal_zone_trip | tail -n 5
kworker/2:2-1014 [002] 125.623213: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
kworker/2:2-1014 [002] 125.743174: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
kworker/2:2-1014 [002] 125.863196: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
kworker/2:2-1014 [002] 125.983175: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
kworker/2:2-1014 [002] 126.103173: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
~# trace-cmd report -R | grep thermal_zone_trip | tail -n 5
kworker/2:2-1014 [002] 125.623213: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
kworker/2:2-1014 [002] 125.743174: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
kworker/2:2-1014 [002] 125.863196: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
kworker/2:2-1014 [002] 125.983175: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
kworker/2:2-1014 [002] 126.103173: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
Is there something I should be doing to enable the translation to the
string representation when reporting using trace-cmd?
Cheers,
Punit
> -- Steve
>
>
>> Thanks.
>> Punit
>>
>> drivers/thermal/fair_share.c | 7 ++++++-
>> drivers/thermal/step_wise.c | 5 ++++-
>> drivers/thermal/thermal_core.c | 2 ++
>> include/trace/events/thermal.h | 30 ++++++++++++++++++++++++++++++
>> 4 files changed, 42 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
>> index 944ba2f..2cddd68 100644
>> --- a/drivers/thermal/fair_share.c
>> +++ b/drivers/thermal/fair_share.c
>> @@ -23,6 +23,7 @@
>> */
>>
>> #include <linux/thermal.h>
>> +#include <trace/events/thermal.h>
>>
>> #include "thermal_core.h"
>>
>> @@ -34,14 +35,18 @@ static int get_trip_level(struct thermal_zone_device *tz)
>> {
>> int count = 0;
>> unsigned long trip_temp;
>> + enum thermal_trip_type trip_type;
>>
>> if (tz->trips == 0 || !tz->ops->get_trip_temp)
>> return 0;
>>
>> for (count = 0; count < tz->trips; count++) {
>> tz->ops->get_trip_temp(tz, count, &trip_temp);
>> - if (tz->temperature < trip_temp)
>> + if (tz->temperature < trip_temp) {
>> + tz->ops->get_trip_type(tz, count, &trip_type);
>> + trace_thermal_zone_trip(tz, count, trip_type);
>> break;
>> + }
>> }
>> return count;
>> }
>> diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
>> index f251521..3b54c2c 100644
>> --- a/drivers/thermal/step_wise.c
>> +++ b/drivers/thermal/step_wise.c
>> @@ -23,6 +23,7 @@
>> */
>>
>> #include <linux/thermal.h>
>> +#include <trace/events/thermal.h>
>>
>> #include "thermal_core.h"
>>
>> @@ -129,8 +130,10 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
>>
>> trend = get_tz_trend(tz, trip);
>>
>> - if (tz->temperature >= trip_temp)
>> + if (tz->temperature >= trip_temp) {
>> throttle = true;
>> + trace_thermal_zone_trip(tz, trip, trip_type);
>> + }
>>
>> dev_dbg(&tz->device, "Trip%d[type=%d,temp=%ld]:trend=%d,throttle=%d\n",
>> trip, trip_type, trip_temp, trend, throttle);
>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>> index c74c78d..454884a 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -371,6 +371,8 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
>> if (tz->temperature < trip_temp)
>> return;
>>
>> + trace_thermal_zone_trip(tz, trip, trip_type);
>> +
>> if (tz->ops->notify)
>> tz->ops->notify(tz, trip, trip_type);
>>
>> diff --git a/include/trace/events/thermal.h b/include/trace/events/thermal.h
>> index 894a79e..5eeb1e7 100644
>> --- a/include/trace/events/thermal.h
>> +++ b/include/trace/events/thermal.h
>> @@ -51,6 +51,36 @@ TRACE_EVENT(cdev_update,
>> TP_printk("type=%s target=%lu", __get_str(type), __entry->target)
>> );
>>
>> +TRACE_EVENT(,
>> +
>> + TP_PROTO(struct thermal_zone_device *tz, int trip,
>> + enum thermal_trip_type trip_type),
>> +
>> + TP_ARGS(tz, trip, trip_type),
>> +
>> + TP_STRUCT__entry(
>> + __string(thermal_zone, tz->type)
>> + __field(int, id)
>> + __field(int, trip)
>> + __field(enum thermal_trip_type, trip_type)
>> + ),
>> +
>> + TP_fast_assign(
>> + __assign_str(thermal_zone, tz->type);
>> + __entry->id = tz->id;
>> + __entry->trip = trip;
>> + __entry->trip_type = trip_type;
>> + ),
>> +
>> + TP_printk("thermal_zone=%s id=%d trip=%d trip_type=%s",
>> + __get_str(thermal_zone), __entry->id, __entry->trip,
>> + __print_symbolic(__entry->trip_type,
>> + { THERMAL_TRIP_ACTIVE, "active" },
>> + { THERMAL_TRIP_PASSIVE, "passive" },
>> + { THERMAL_TRIP_HOT, "hot" },
>> + { THERMAL_TRIP_CRITICAL, "critical" }))
>> +);
>> +
>> #endif /* _TRACE_THERMAL_H */
>>
>> /* This part must be outside protection */
On Wed, 11 Jun 2014 15:11:02 +0100
Punit Agrawal <[email protected]> wrote:
> I do indeed see the value of trip_type and it matches what's being
> traced.
>
> ~# trace-cmd report | grep thermal_zone_trip | tail -n 5
> kworker/2:2-1014 [002] 125.623213: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
> kworker/2:2-1014 [002] 125.743174: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
> kworker/2:2-1014 [002] 125.863196: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
> kworker/2:2-1014 [002] 125.983175: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
> kworker/2:2-1014 [002] 126.103173: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
> ~# trace-cmd report -R | grep thermal_zone_trip | tail -n 5
> kworker/2:2-1014 [002] 125.623213: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
> kworker/2:2-1014 [002] 125.743174: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
> kworker/2:2-1014 [002] 125.863196: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
> kworker/2:2-1014 [002] 125.983175: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
> kworker/2:2-1014 [002] 126.103173: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
>
> Is there something I should be doing to enable the translation to the
> string representation when reporting using trace-cmd?
Out of curiosity, what does the format file for it look like?
/sys/kernel/debug/thermal/thermal_zone_trip/format
-- Steve
Steven Rostedt <[email protected]> writes:
> On Wed, 11 Jun 2014 15:11:02 +0100
> Punit Agrawal <[email protected]> wrote:
>
>> I do indeed see the value of trip_type and it matches what's being
>> traced.
>>
>> ~# trace-cmd report | grep thermal_zone_trip | tail -n 5
>> kworker/2:2-1014 [002] 125.623213: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
>> kworker/2:2-1014 [002] 125.743174: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
>> kworker/2:2-1014 [002] 125.863196: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
>> kworker/2:2-1014 [002] 125.983175: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
>> kworker/2:2-1014 [002] 126.103173: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=
>> ~# trace-cmd report -R | grep thermal_zone_trip | tail -n 5
>> kworker/2:2-1014 [002] 125.623213: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
>> kworker/2:2-1014 [002] 125.743174: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
>> kworker/2:2-1014 [002] 125.863196: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
>> kworker/2:2-1014 [002] 125.983175: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
>> kworker/2:2-1014 [002] 126.103173: thermal_zone_trip: thermal_zone=soc_thermal id=0 trip=0 trip_type=1
>>
>> Is there something I should be doing to enable the translation to the
>> string representation when reporting using trace-cmd?
>
> Out of curiosity, what does the format file for it look like?
I didn't know this was being exported. Thanks for the pointer.
>
> /sys/kernel/debug/thermal/thermal_zone_trip/format
I don't have this file but found the following which seems to contain
the format.
# pwd /sys/kernel/debug/tracing/events/thermal/thermal_zone_trip
# cat format
name: thermal_zone_trip
ID: 463
format:
field:unsigned short common_type; offset:0; size:2; signed:0;
field:unsigned char common_flags; offset:2; size:1; signed:0;
field:unsigned char common_preempt_count; offset:3; size:1; signed:0;
field:int common_pid; offset:4; size:4; signed:1;
field:__data_loc char[] thermal_zone; offset:8; size:4; signed:0;
field:int id; offset:12; size:4; signed:1;
field:int trip; offset:16; size:4; signed:1;
field:enum thermal_trip_type trip_type; offset:20; size:4; signed:0;
print fmt: "thermal_zone=%s id=%d trip=%d trip_type=%s", __get_str(thermal_zone), REC->id, REC->trip, __print_symbolic(REC->trip_type, { THERMAL_TRIP_ACTIVE, "active" }, { THERMAL_TRIP_PASSIVE, "passive" }, { THERMAL_TRIP_HOT, "hot" }, { THERMAL_TRIP_CRITICAL, "critical" })
Cheers,
Punit
>
> -- Steve
On Wed, 11 Jun 2014 15:53:42 +0100
Punit Agrawal <[email protected]> wrote:
> >
> > /sys/kernel/debug/thermal/thermal_zone_trip/format
>
> I don't have this file but found the following which seems to contain
> the format.
Yeah, that was typed manually, forgot "tracing". I mount the debugfs
system at /debug for easier typing. I hate the /sys/kernel/debug path.
Too much typing.
>
> # pwd /sys/kernel/debug/tracing/events/thermal/thermal_zone_trip
> # cat format
> name: thermal_zone_trip
> ID: 463
> format:
> field:unsigned short common_type; offset:0; size:2; signed:0;
> field:unsigned char common_flags; offset:2; size:1; signed:0;
> field:unsigned char common_preempt_count; offset:3; size:1; signed:0;
> field:int common_pid; offset:4; size:4; signed:1;
>
> field:__data_loc char[] thermal_zone; offset:8; size:4; signed:0;
> field:int id; offset:12; size:4; signed:1;
> field:int trip; offset:16; size:4; signed:1;
> field:enum thermal_trip_type trip_type; offset:20; size:4; signed:0;
>
> print fmt: "thermal_zone=%s id=%d trip=%d trip_type=%s", __get_str(thermal_zone), REC->id, REC->trip, __print_symbolic(REC->trip_type, { THERMAL_TRIP_ACTIVE, "active" }, { THERMAL_TRIP_PASSIVE, "passive" }, { THERMAL_TRIP_HOT, "hot" }, { THERMAL_TRIP_CRITICAL, "critical" })
>
For it to work with trace-cmd (and perf) you'll need to add a plugin to
define what those enums are. This is the file that trace-cmd uses to
foramat. But it has no idea how to convert something like
THERMAL_TRIP_PASSIVE into a number.
-- Steve
Steven Rostedt <[email protected]> writes:
[...]
>>
>> # pwd /sys/kernel/debug/tracing/events/thermal/thermal_zone_trip
>> # cat format
>> name: thermal_zone_trip
>> ID: 463
>> format:
>> field:unsigned short common_type; offset:0; size:2; signed:0;
>> field:unsigned char common_flags; offset:2; size:1; signed:0;
>> field:unsigned char common_preempt_count; offset:3; size:1; signed:0;
>> field:int common_pid; offset:4; size:4; signed:1;
>>
>> field:__data_loc char[] thermal_zone; offset:8; size:4; signed:0;
>> field:int id; offset:12; size:4; signed:1;
>> field:int trip; offset:16; size:4; signed:1;
>> field:enum thermal_trip_type trip_type; offset:20; size:4; signed:0;
>>
>> print fmt: "thermal_zone=%s id=%d trip=%d trip_type=%s",
>> __get_str(thermal_zone), REC->id, REC->trip,
>> __print_symbolic(REC->trip_type, { THERMAL_TRIP_ACTIVE, "active" },
>> { THERMAL_TRIP_PASSIVE, "passive" }, { THERMAL_TRIP_HOT, "hot" }, {
>> THERMAL_TRIP_CRITICAL, "critical" })
>>
>
> For it to work with trace-cmd (and perf) you'll need to add a plugin to
> define what those enums are. This is the file that trace-cmd uses to
> foramat. But it has no idea how to convert something like
> THERMAL_TRIP_PASSIVE into a number.
>
Hmm, right. I was working under the assumption that the enum values will
be converted to their numeric representation when compiled.
I think it's better to convert the enum to int in the trace event - the
changes are localised and the tool will work as well.
Rui, Eduardo what do you think?
Punit
> -- Steve
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Punit,
On Wed, Jun 11, 2014 at 12:31:44PM +0100, Punit Agrawal wrote:
> Create a new event to trace when the temperature is above a trip
> point. Use the trace-point when handling non-critical and critical
> trip pionts.
>
> Cc: Zhang Rui <[email protected]>
> Cc: Eduardo Valentin <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Signed-off-by: Punit Agrawal <[email protected]>
> ---
> Hi Steven,
>
> I am facing an issue with partial trace being emitted when using
> __print_symbolic in this patch.
>
> When the trip_type is THERMAL_TRIP_ACTIVE (i.e., the first value in
> the symbol map), the emitted trace contains the corresponding string
> ("active"). But for other values of trip_type an empty string is
> emitted in the trace.
>
> I've looked at other uses of __print_symbolic in the kernel and don't
> see any difference in usage. Do you know what could be causing this or
> alternately have any pointers on how to debug this behaviour?
>
> Thanks.
> Punit
>
> drivers/thermal/fair_share.c | 7 ++++++-
> drivers/thermal/step_wise.c | 5 ++++-
> drivers/thermal/thermal_core.c | 2 ++
> include/trace/events/thermal.h | 30 ++++++++++++++++++++++++++++++
> 4 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
> index 944ba2f..2cddd68 100644
> --- a/drivers/thermal/fair_share.c
> +++ b/drivers/thermal/fair_share.c
> @@ -23,6 +23,7 @@
> */
>
> #include <linux/thermal.h>
> +#include <trace/events/thermal.h>
>
> #include "thermal_core.h"
>
> @@ -34,14 +35,18 @@ static int get_trip_level(struct thermal_zone_device *tz)
> {
> int count = 0;
> unsigned long trip_temp;
> + enum thermal_trip_type trip_type;
>
> if (tz->trips == 0 || !tz->ops->get_trip_temp)
> return 0;
>
> for (count = 0; count < tz->trips; count++) {
> tz->ops->get_trip_temp(tz, count, &trip_temp);
> - if (tz->temperature < trip_temp)
> + if (tz->temperature < trip_temp) {
> + tz->ops->get_trip_type(tz, count, &trip_type);
> + trace_thermal_zone_trip(tz, count, trip_type);
This should be outside the if condition. You want to report when trip
points have been hit, like in the step_wise code below.
> break;
> + }
> }
> return count;
> }
> diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
> index f251521..3b54c2c 100644
> --- a/drivers/thermal/step_wise.c
> +++ b/drivers/thermal/step_wise.c
> @@ -23,6 +23,7 @@
> */
>
> #include <linux/thermal.h>
> +#include <trace/events/thermal.h>
>
> #include "thermal_core.h"
>
> @@ -129,8 +130,10 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
>
> trend = get_tz_trend(tz, trip);
>
> - if (tz->temperature >= trip_temp)
> + if (tz->temperature >= trip_temp) {
> throttle = true;
> + trace_thermal_zone_trip(tz, trip, trip_type);
> + }
>
> dev_dbg(&tz->device, "Trip%d[type=%d,temp=%ld]:trend=%d,throttle=%d\n",
> trip, trip_type, trip_temp, trend, throttle);
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index c74c78d..454884a 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -371,6 +371,8 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
> if (tz->temperature < trip_temp)
> return;
>
> + trace_thermal_zone_trip(tz, trip, trip_type);
> +
> if (tz->ops->notify)
> tz->ops->notify(tz, trip, trip_type);
>
Cheers,
Javi
"Javi Merino" <[email protected]> writes:
> Hi Punit,
>
> On Wed, Jun 11, 2014 at 12:31:44PM +0100, Punit Agrawal wrote:
>> Create a new event to trace when the temperature is above a trip
>> point. Use the trace-point when handling non-critical and critical
>> trip pionts.
>>
>> Cc: Zhang Rui <[email protected]>
>> Cc: Eduardo Valentin <[email protected]>
>> Cc: Steven Rostedt <[email protected]>
>> Cc: Frederic Weisbecker <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Signed-off-by: Punit Agrawal <[email protected]>
>> ---
>> Hi Steven,
>>
>> I am facing an issue with partial trace being emitted when using
>> __print_symbolic in this patch.
>>
>> When the trip_type is THERMAL_TRIP_ACTIVE (i.e., the first value in
>> the symbol map), the emitted trace contains the corresponding string
>> ("active"). But for other values of trip_type an empty string is
>> emitted in the trace.
>>
>> I've looked at other uses of __print_symbolic in the kernel and don't
>> see any difference in usage. Do you know what could be causing this or
>> alternately have any pointers on how to debug this behaviour?
>>
>> Thanks.
>> Punit
>>
>> drivers/thermal/fair_share.c | 7 ++++++-
>> drivers/thermal/step_wise.c | 5 ++++-
>> drivers/thermal/thermal_core.c | 2 ++
>> include/trace/events/thermal.h | 30 ++++++++++++++++++++++++++++++
>> 4 files changed, 42 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
>> index 944ba2f..2cddd68 100644
>> --- a/drivers/thermal/fair_share.c
>> +++ b/drivers/thermal/fair_share.c
>> @@ -23,6 +23,7 @@
>> */
>>
>> #include <linux/thermal.h>
>> +#include <trace/events/thermal.h>
>>
>> #include "thermal_core.h"
>>
>> @@ -34,14 +35,18 @@ static int get_trip_level(struct thermal_zone_device *tz)
>> {
>> int count = 0;
>> unsigned long trip_temp;
>> + enum thermal_trip_type trip_type;
>>
>> if (tz->trips == 0 || !tz->ops->get_trip_temp)
>> return 0;
>>
>> for (count = 0; count < tz->trips; count++) {
>> tz->ops->get_trip_temp(tz, count, &trip_temp);
>> - if (tz->temperature < trip_temp)
>> + if (tz->temperature < trip_temp) {
>> + tz->ops->get_trip_type(tz, count, &trip_type);
>> + trace_thermal_zone_trip(tz, count, trip_type);
>
> This should be outside the if condition. You want to report when trip
> points have been hit, like in the step_wise code below.
>
It turned out to be a bit more subtle than moving the trace outside the
if. I have the below fixup with an added comment. Let me know if that
doesn't solve the problem.
-- >8 --
Subject: [PATCH] fixup! thermal: trace: Trace when temperature is above a
trip point
---
drivers/thermal/fair_share.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
index 2cddd68..6e0a3fb 100644
--- a/drivers/thermal/fair_share.c
+++ b/drivers/thermal/fair_share.c
@@ -42,12 +42,19 @@ static int get_trip_level(struct thermal_zone_device *tz)
for (count = 0; count < tz->trips; count++) {
tz->ops->get_trip_temp(tz, count, &trip_temp);
- if (tz->temperature < trip_temp) {
- tz->ops->get_trip_type(tz, count, &trip_type);
- trace_thermal_zone_trip(tz, count, trip_type);
+ if (tz->temperature < trip_temp)
break;
- }
}
+
+ /*
+ * count > 0 only if temperature is greater than first trip
+ * point, in which case, trip_point = count - 1
+ */
+ if (count > 0) {
+ tz->ops->get_trip_type(tz, count - 1, &trip_type);
+ trace_thermal_zone_trip(tz, count - 1, trip_type);
+ }
+
return count;
}
--
1.7.10.4
>> break;
>> + }
>> }
>> return count;
>> }
>> diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
>> index f251521..3b54c2c 100644
>> --- a/drivers/thermal/step_wise.c
>> +++ b/drivers/thermal/step_wise.c
>> @@ -23,6 +23,7 @@
>> */
>>
>> #include <linux/thermal.h>
>> +#include <trace/events/thermal.h>
>>
>> #include "thermal_core.h"
>>
>> @@ -129,8 +130,10 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
>>
>> trend = get_tz_trend(tz, trip);
>>
>> - if (tz->temperature >= trip_temp)
>> + if (tz->temperature >= trip_temp) {
>> throttle = true;
>> + trace_thermal_zone_trip(tz, trip, trip_type);
>> + }
>>
>> dev_dbg(&tz->device, "Trip%d[type=%d,temp=%ld]:trend=%d,throttle=%d\n",
>> trip, trip_type, trip_temp, trend, throttle);
>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>> index c74c78d..454884a 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -371,6 +371,8 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
>> if (tz->temperature < trip_temp)
>> return;
>>
>> + trace_thermal_zone_trip(tz, trip, trip_type);
>> +
>> if (tz->ops->notify)
>> tz->ops->notify(tz, trip, trip_type);
>>
>
> Cheers,
> Javi
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Tue, Jun 24, 2014 at 11:41:38AM +0100, Punit Agrawal wrote:
> "Javi Merino" <[email protected]> writes:
>
> > Hi Punit,
> >
> > On Wed, Jun 11, 2014 at 12:31:44PM +0100, Punit Agrawal wrote:
> >> Create a new event to trace when the temperature is above a trip
> >> point. Use the trace-point when handling non-critical and critical
> >> trip pionts.
> >>
> >> Cc: Zhang Rui <[email protected]>
> >> Cc: Eduardo Valentin <[email protected]>
> >> Cc: Steven Rostedt <[email protected]>
> >> Cc: Frederic Weisbecker <[email protected]>
> >> Cc: Ingo Molnar <[email protected]>
> >> Signed-off-by: Punit Agrawal <[email protected]>
> >> ---
> >> Hi Steven,
> >>
> >> I am facing an issue with partial trace being emitted when using
> >> __print_symbolic in this patch.
> >>
> >> When the trip_type is THERMAL_TRIP_ACTIVE (i.e., the first value in
> >> the symbol map), the emitted trace contains the corresponding string
> >> ("active"). But for other values of trip_type an empty string is
> >> emitted in the trace.
> >>
> >> I've looked at other uses of __print_symbolic in the kernel and don't
> >> see any difference in usage. Do you know what could be causing this or
> >> alternately have any pointers on how to debug this behaviour?
> >>
> >> Thanks.
> >> Punit
> >>
> >> drivers/thermal/fair_share.c | 7 ++++++-
> >> drivers/thermal/step_wise.c | 5 ++++-
> >> drivers/thermal/thermal_core.c | 2 ++
> >> include/trace/events/thermal.h | 30 ++++++++++++++++++++++++++++++
> >> 4 files changed, 42 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
> >> index 944ba2f..2cddd68 100644
> >> --- a/drivers/thermal/fair_share.c
> >> +++ b/drivers/thermal/fair_share.c
> >> @@ -23,6 +23,7 @@
> >> */
> >>
> >> #include <linux/thermal.h>
> >> +#include <trace/events/thermal.h>
> >>
> >> #include "thermal_core.h"
> >>
> >> @@ -34,14 +35,18 @@ static int get_trip_level(struct thermal_zone_device *tz)
> >> {
> >> int count = 0;
> >> unsigned long trip_temp;
> >> + enum thermal_trip_type trip_type;
> >>
> >> if (tz->trips == 0 || !tz->ops->get_trip_temp)
> >> return 0;
> >>
> >> for (count = 0; count < tz->trips; count++) {
> >> tz->ops->get_trip_temp(tz, count, &trip_temp);
> >> - if (tz->temperature < trip_temp)
> >> + if (tz->temperature < trip_temp) {
> >> + tz->ops->get_trip_type(tz, count, &trip_type);
> >> + trace_thermal_zone_trip(tz, count, trip_type);
> >
> > This should be outside the if condition. You want to report when trip
> > points have been hit, like in the step_wise code below.
> >
>
> It turned out to be a bit more subtle than moving the trace outside the
> if.
True, it was more difficult than what I said.
> I have the below fixup with an added comment. Let me know if that
> doesn't solve the problem.
I don't have a reproducer, I just spotted it while reading the code.
The below fix seems to be the right thing.
> -- >8 --
> Subject: [PATCH] fixup! thermal: trace: Trace when temperature is above a
> trip point
>
> ---
> drivers/thermal/fair_share.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
> index 2cddd68..6e0a3fb 100644
> --- a/drivers/thermal/fair_share.c
> +++ b/drivers/thermal/fair_share.c
> @@ -42,12 +42,19 @@ static int get_trip_level(struct thermal_zone_device *tz)
>
> for (count = 0; count < tz->trips; count++) {
> tz->ops->get_trip_temp(tz, count, &trip_temp);
> - if (tz->temperature < trip_temp) {
> - tz->ops->get_trip_type(tz, count, &trip_type);
> - trace_thermal_zone_trip(tz, count, trip_type);
> + if (tz->temperature < trip_temp)
> break;
> - }
> }
> +
> + /*
> + * count > 0 only if temperature is greater than first trip
> + * point, in which case, trip_point = count - 1
> + */
> + if (count > 0) {
> + tz->ops->get_trip_type(tz, count - 1, &trip_type);
> + trace_thermal_zone_trip(tz, count - 1, trip_type);
> + }
> +
> return count;
> }
>
> --
> 1.7.10.4
Punit,
On Tue, Jun 24, 2014 at 6:41 AM, Punit Agrawal <[email protected]> wrote:
> "Javi Merino" <[email protected]> writes:
>
>> Hi Punit,
>>
>> On Wed, Jun 11, 2014 at 12:31:44PM +0100, Punit Agrawal wrote:
>>> Create a new event to trace when the temperature is above a trip
>>> point. Use the trace-point when handling non-critical and critical
>>> trip pionts.
>>>
>>> Cc: Zhang Rui <[email protected]>
>>> Cc: Eduardo Valentin <[email protected]>
>>> Cc: Steven Rostedt <[email protected]>
>>> Cc: Frederic Weisbecker <[email protected]>
>>> Cc: Ingo Molnar <[email protected]>
>>> Signed-off-by: Punit Agrawal <[email protected]>
>>> ---
>>> Hi Steven,
>>>
>>> I am facing an issue with partial trace being emitted when using
>>> __print_symbolic in this patch.
>>>
>>> When the trip_type is THERMAL_TRIP_ACTIVE (i.e., the first value in
>>> the symbol map), the emitted trace contains the corresponding string
>>> ("active"). But for other values of trip_type an empty string is
>>> emitted in the trace.
>>>
>>> I've looked at other uses of __print_symbolic in the kernel and don't
>>> see any difference in usage. Do you know what could be causing this or
>>> alternately have any pointers on how to debug this behaviour?
>>>
>>> Thanks.
>>> Punit
>>>
>>> drivers/thermal/fair_share.c | 7 ++++++-
>>> drivers/thermal/step_wise.c | 5 ++++-
>>> drivers/thermal/thermal_core.c | 2 ++
>>> include/trace/events/thermal.h | 30 ++++++++++++++++++++++++++++++
>>> 4 files changed, 42 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
>>> index 944ba2f..2cddd68 100644
>>> --- a/drivers/thermal/fair_share.c
>>> +++ b/drivers/thermal/fair_share.c
>>> @@ -23,6 +23,7 @@
>>> */
>>>
>>> #include <linux/thermal.h>
>>> +#include <trace/events/thermal.h>
>>>
>>> #include "thermal_core.h"
>>>
>>> @@ -34,14 +35,18 @@ static int get_trip_level(struct thermal_zone_device *tz)
>>> {
>>> int count = 0;
>>> unsigned long trip_temp;
>>> + enum thermal_trip_type trip_type;
>>>
>>> if (tz->trips == 0 || !tz->ops->get_trip_temp)
>>> return 0;
>>>
>>> for (count = 0; count < tz->trips; count++) {
>>> tz->ops->get_trip_temp(tz, count, &trip_temp);
>>> - if (tz->temperature < trip_temp)
>>> + if (tz->temperature < trip_temp) {
>>> + tz->ops->get_trip_type(tz, count, &trip_type);
>>> + trace_thermal_zone_trip(tz, count, trip_type);
>>
>> This should be outside the if condition. You want to report when trip
>> points have been hit, like in the step_wise code below.
>>
>
> It turned out to be a bit more subtle than moving the trace outside the
> if. I have the below fixup with an added comment. Let me know if that
> doesn't solve the problem.
>
> -- >8 --
> Subject: [PATCH] fixup! thermal: trace: Trace when temperature is above a
> trip point
Can you please repost the patch with the fixup merged?
Cheers,
>
> ---
> drivers/thermal/fair_share.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
> index 2cddd68..6e0a3fb 100644
> --- a/drivers/thermal/fair_share.c
> +++ b/drivers/thermal/fair_share.c
> @@ -42,12 +42,19 @@ static int get_trip_level(struct thermal_zone_device *tz)
>
> for (count = 0; count < tz->trips; count++) {
> tz->ops->get_trip_temp(tz, count, &trip_temp);
> - if (tz->temperature < trip_temp) {
> - tz->ops->get_trip_type(tz, count, &trip_type);
> - trace_thermal_zone_trip(tz, count, trip_type);
> + if (tz->temperature < trip_temp)
> break;
> - }
> }
> +
> + /*
> + * count > 0 only if temperature is greater than first trip
> + * point, in which case, trip_point = count - 1
> + */
> + if (count > 0) {
> + tz->ops->get_trip_type(tz, count - 1, &trip_type);
> + trace_thermal_zone_trip(tz, count - 1, trip_type);
> + }
> +
> return count;
> }
>
> --
> 1.7.10.4
>
>>> break;
>>> + }
>>> }
>>> return count;
>>> }
>>> diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
>>> index f251521..3b54c2c 100644
>>> --- a/drivers/thermal/step_wise.c
>>> +++ b/drivers/thermal/step_wise.c
>>> @@ -23,6 +23,7 @@
>>> */
>>>
>>> #include <linux/thermal.h>
>>> +#include <trace/events/thermal.h>
>>>
>>> #include "thermal_core.h"
>>>
>>> @@ -129,8 +130,10 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
>>>
>>> trend = get_tz_trend(tz, trip);
>>>
>>> - if (tz->temperature >= trip_temp)
>>> + if (tz->temperature >= trip_temp) {
>>> throttle = true;
>>> + trace_thermal_zone_trip(tz, trip, trip_type);
>>> + }
>>>
>>> dev_dbg(&tz->device, "Trip%d[type=%d,temp=%ld]:trend=%d,throttle=%d\n",
>>> trip, trip_type, trip_temp, trend, throttle);
>>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>>> index c74c78d..454884a 100644
>>> --- a/drivers/thermal/thermal_core.c
>>> +++ b/drivers/thermal/thermal_core.c
>>> @@ -371,6 +371,8 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
>>> if (tz->temperature < trip_temp)
>>> return;
>>>
>>> + trace_thermal_zone_trip(tz, trip, trip_type);
>>> +
>>> if (tz->ops->notify)
>>> tz->ops->notify(tz, trip, trip_type);
>>>
>>
>> Cheers,
>> Javi
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
--
Eduardo Bezerra Valentin
Hi linux-pm,
This is the second posting of the patches to add trace to the thermal
framework.
The linux thermal framework doesn't have any support for tracing. This
makes it hard to run workloads and observe the thermal behaviour of
the system without actively polling files in sysfs or enabling debug
builds.
This patch set introduces trace events in the framework to allow
observing the behaviour of the different components in the
framework. The events added trace temperature changes, trip points and
cooling device state changes.
changes since rfc:
* Fixed an issue where incorrect trip point was traced when using fair
share
* Trace the numeric value of trip_type instead of using __print_symbolic
Punit Agrawal (3):
thermal: trace: Trace temperature changes
thermal: trace: Trace when a cooling device's state is updated
thermal: trace: Trace when temperature is above a trip point
drivers/thermal/fair_share.c | 12 ++++++
drivers/thermal/step_wise.c | 5 ++-
drivers/thermal/thermal_core.c | 7 ++++
include/trace/events/thermal.h | 87 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 110 insertions(+), 1 deletion(-)
create mode 100644 include/trace/events/thermal.h
--
1.7.10.4
Create a new event to trace the temperature of a thermal zone. Using
this event trace the temperature changes of the thermal zone every-time
it is updated.
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Signed-off-by: Punit Agrawal <[email protected]>
---
drivers/thermal/thermal_core.c | 4 ++++
include/trace/events/thermal.h | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+)
create mode 100644 include/trace/events/thermal.h
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 71b0ec0..6b32391 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -38,6 +38,9 @@
#include <net/netlink.h>
#include <net/genetlink.h>
+#define CREATE_TRACE_POINTS
+#include <trace/events/thermal.h>
+
#include "thermal_core.h"
#include "thermal_hwmon.h"
@@ -463,6 +466,7 @@ static void update_temperature(struct thermal_zone_device *tz)
tz->temperature = temp;
mutex_unlock(&tz->lock);
+ trace_thermal_temperature(tz);
dev_dbg(&tz->device, "last_temperature=%d, current_temperature=%d\n",
tz->last_temperature, tz->temperature);
}
diff --git a/include/trace/events/thermal.h b/include/trace/events/thermal.h
new file mode 100644
index 0000000..8c5ca96
--- /dev/null
+++ b/include/trace/events/thermal.h
@@ -0,0 +1,38 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM thermal
+
+#if !defined(_TRACE_THERMAL_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_THERMAL_H
+
+#include <linux/thermal.h>
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(thermal_temperature,
+
+ TP_PROTO(struct thermal_zone_device *tz),
+
+ TP_ARGS(tz),
+
+ TP_STRUCT__entry(
+ __string(thermal_zone, tz->type)
+ __field(int, id)
+ __field(int, temp_prev)
+ __field(int, temp)
+ ),
+
+ TP_fast_assign(
+ __assign_str(thermal_zone, tz->type);
+ __entry->id = tz->id;
+ __entry->temp_prev = tz->last_temperature;
+ __entry->temp = tz->temperature;
+ ),
+
+ TP_printk("thermal_zone=%s id=%d temp_prev=%d temp=%d",
+ __get_str(thermal_zone), __entry->id, __entry->temp_prev,
+ __entry->temp)
+);
+
+#endif /* _TRACE_THERMAL_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
1.7.10.4
Create a new event to trace when the temperature is above a trip
point. Use the trace-point when handling non-critical and critical
trip pionts.
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Signed-off-by: Punit Agrawal <[email protected]>
---
drivers/thermal/fair_share.c | 12 ++++++++++++
drivers/thermal/step_wise.c | 5 ++++-
drivers/thermal/thermal_core.c | 2 ++
include/trace/events/thermal.h | 26 ++++++++++++++++++++++++++
4 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c
index 944ba2f..6e0a3fb 100644
--- a/drivers/thermal/fair_share.c
+++ b/drivers/thermal/fair_share.c
@@ -23,6 +23,7 @@
*/
#include <linux/thermal.h>
+#include <trace/events/thermal.h>
#include "thermal_core.h"
@@ -34,6 +35,7 @@ static int get_trip_level(struct thermal_zone_device *tz)
{
int count = 0;
unsigned long trip_temp;
+ enum thermal_trip_type trip_type;
if (tz->trips == 0 || !tz->ops->get_trip_temp)
return 0;
@@ -43,6 +45,16 @@ static int get_trip_level(struct thermal_zone_device *tz)
if (tz->temperature < trip_temp)
break;
}
+
+ /*
+ * count > 0 only if temperature is greater than first trip
+ * point, in which case, trip_point = count - 1
+ */
+ if (count > 0) {
+ tz->ops->get_trip_type(tz, count - 1, &trip_type);
+ trace_thermal_zone_trip(tz, count - 1, trip_type);
+ }
+
return count;
}
diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
index f251521..3b54c2c 100644
--- a/drivers/thermal/step_wise.c
+++ b/drivers/thermal/step_wise.c
@@ -23,6 +23,7 @@
*/
#include <linux/thermal.h>
+#include <trace/events/thermal.h>
#include "thermal_core.h"
@@ -129,8 +130,10 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
trend = get_tz_trend(tz, trip);
- if (tz->temperature >= trip_temp)
+ if (tz->temperature >= trip_temp) {
throttle = true;
+ trace_thermal_zone_trip(tz, trip, trip_type);
+ }
dev_dbg(&tz->device, "Trip%d[type=%d,temp=%ld]:trend=%d,throttle=%d\n",
trip, trip_type, trip_temp, trend, throttle);
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index c74c78d..454884a 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -371,6 +371,8 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
if (tz->temperature < trip_temp)
return;
+ trace_thermal_zone_trip(tz, trip, trip_type);
+
if (tz->ops->notify)
tz->ops->notify(tz, trip, trip_type);
diff --git a/include/trace/events/thermal.h b/include/trace/events/thermal.h
index 894a79e..0f4f95d 100644
--- a/include/trace/events/thermal.h
+++ b/include/trace/events/thermal.h
@@ -51,6 +51,32 @@ TRACE_EVENT(cdev_update,
TP_printk("type=%s target=%lu", __get_str(type), __entry->target)
);
+TRACE_EVENT(thermal_zone_trip,
+
+ TP_PROTO(struct thermal_zone_device *tz, int trip,
+ enum thermal_trip_type trip_type),
+
+ TP_ARGS(tz, trip, trip_type),
+
+ TP_STRUCT__entry(
+ __string(thermal_zone, tz->type)
+ __field(int, id)
+ __field(int, trip)
+ __field(enum thermal_trip_type, trip_type)
+ ),
+
+ TP_fast_assign(
+ __assign_str(thermal_zone, tz->type);
+ __entry->id = tz->id;
+ __entry->trip = trip;
+ __entry->trip_type = trip_type;
+ ),
+
+ TP_printk("thermal_zone=%s id=%d trip=%d trip_type=%d",
+ __get_str(thermal_zone), __entry->id, __entry->trip,
+ __entry->trip_type)
+);
+
#endif /* _TRACE_THERMAL_H */
/* This part must be outside protection */
--
1.7.10.4
Introduce and use an event to trace when a cooling device's state is
updated. This is useful to follow the effect of governor decisions on
cooling devices.
Cc: Zhang Rui <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Signed-off-by: Punit Agrawal <[email protected]>
---
drivers/thermal/thermal_core.c | 1 +
include/trace/events/thermal.h | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 6b32391..c74c78d 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1291,6 +1291,7 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev)
mutex_unlock(&cdev->lock);
cdev->ops->set_cur_state(cdev, target);
cdev->updated = true;
+ trace_cdev_update(cdev, target);
dev_dbg(&cdev->device, "set to state %lu\n", target);
}
EXPORT_SYMBOL(thermal_cdev_update);
diff --git a/include/trace/events/thermal.h b/include/trace/events/thermal.h
index 8c5ca96..894a79e 100644
--- a/include/trace/events/thermal.h
+++ b/include/trace/events/thermal.h
@@ -32,6 +32,25 @@ TRACE_EVENT(thermal_temperature,
__entry->temp)
);
+TRACE_EVENT(cdev_update,
+
+ TP_PROTO(struct thermal_cooling_device *cdev, unsigned long target),
+
+ TP_ARGS(cdev, target),
+
+ TP_STRUCT__entry(
+ __string(type, cdev->type)
+ __field(unsigned long, target)
+ ),
+
+ TP_fast_assign(
+ __assign_str(type, cdev->type);
+ __entry->target = target;
+ ),
+
+ TP_printk("type=%s target=%lu", __get_str(type), __entry->target)
+);
+
#endif /* _TRACE_THERMAL_H */
/* This part must be outside protection */
--
1.7.10.4
On Tue, Jul 29, 2014 at 11:50:47AM +0100, Punit Agrawal wrote:
> Hi linux-pm,
>
> This is the second posting of the patches to add trace to the thermal
> framework.
>
> The linux thermal framework doesn't have any support for tracing. This
> makes it hard to run workloads and observe the thermal behaviour of
> the system without actively polling files in sysfs or enabling debug
> builds.
>
> This patch set introduces trace events in the framework to allow
> observing the behaviour of the different components in the
> framework. The events added trace temperature changes, trip points and
> cooling device state changes.
>
> changes since rfc:
> * Fixed an issue where incorrect trip point was traced when using fair
> share
> * Trace the numeric value of trip_type instead of using __print_symbolic
Pulled the series.
Thanks.
>
> Punit Agrawal (3):
> thermal: trace: Trace temperature changes
> thermal: trace: Trace when a cooling device's state is updated
> thermal: trace: Trace when temperature is above a trip point
>
> drivers/thermal/fair_share.c | 12 ++++++
> drivers/thermal/step_wise.c | 5 ++-
> drivers/thermal/thermal_core.c | 7 ++++
> include/trace/events/thermal.h | 87 ++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 110 insertions(+), 1 deletion(-)
> create mode 100644 include/trace/events/thermal.h
>
> --
> 1.7.10.4
>
Eduardo Valentin <[email protected]> writes:
> On Tue, Jul 29, 2014 at 11:50:47AM +0100, Punit Agrawal wrote:
>> Hi linux-pm,
>>
>> This is the second posting of the patches to add trace to the thermal
>> framework.
>>
>> The linux thermal framework doesn't have any support for tracing. This
>> makes it hard to run workloads and observe the thermal behaviour of
>> the system without actively polling files in sysfs or enabling debug
>> builds.
>>
>> This patch set introduces trace events in the framework to allow
>> observing the behaviour of the different components in the
>> framework. The events added trace temperature changes, trip points and
>> cooling device state changes.
>>
>> changes since rfc:
>> * Fixed an issue where incorrect trip point was traced when using fair
>> share
>> * Trace the numeric value of trip_type instead of using __print_symbolic
>
> Pulled the series.
Thanks Eduardo.
>
> Thanks.
>
>>
>> Punit Agrawal (3):
>> thermal: trace: Trace temperature changes
>> thermal: trace: Trace when a cooling device's state is updated
>> thermal: trace: Trace when temperature is above a trip point
>>
>> drivers/thermal/fair_share.c | 12 ++++++
>> drivers/thermal/step_wise.c | 5 ++-
>> drivers/thermal/thermal_core.c | 7 ++++
>> include/trace/events/thermal.h | 87 ++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 110 insertions(+), 1 deletion(-)
>> create mode 100644 include/trace/events/thermal.h
>>
>> --
>> 1.7.10.4
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html