linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] Add trace to thermal framework
@ 2014-06-11 11:31 Punit Agrawal
  2014-06-11 11:31 ` [RFC PATCH 1/3] thermal: trace: Trace temperature changes Punit Agrawal
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Punit Agrawal @ 2014-06-11 11:31 UTC (permalink / raw)
  To: linux-pm, linux-kernel
  Cc: javi.merino, Punit Agrawal, Zhang Rui, Eduardo Valentin, Steven Rostedt

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [RFC PATCH 1/3] thermal: trace: Trace temperature changes
  2014-06-11 11:31 [RFC PATCH 0/3] Add trace to thermal framework Punit Agrawal
@ 2014-06-11 11:31 ` Punit Agrawal
  2014-06-11 11:31 ` [RFC PATCH 2/3] thermal: trace: Trace when a cooling device's state is updated Punit Agrawal
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 20+ messages in thread
From: Punit Agrawal @ 2014-06-11 11:31 UTC (permalink / raw)
  To: linux-pm, linux-kernel
  Cc: javi.merino, Punit Agrawal, Zhang Rui, Eduardo Valentin,
	Steven Rostedt, Frederic Weisbecker, Ingo Molnar

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 <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 2/3] thermal: trace: Trace when a cooling device's state is updated
  2014-06-11 11:31 [RFC PATCH 0/3] Add trace to thermal framework Punit Agrawal
  2014-06-11 11:31 ` [RFC PATCH 1/3] thermal: trace: Trace temperature changes Punit Agrawal
@ 2014-06-11 11:31 ` Punit Agrawal
  2014-06-11 11:31 ` [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point Punit Agrawal
  2014-07-29 10:50 ` [PATCH 0/3] Add trace to thermal framework Punit Agrawal
  3 siblings, 0 replies; 20+ messages in thread
From: Punit Agrawal @ 2014-06-11 11:31 UTC (permalink / raw)
  To: linux-pm, linux-kernel
  Cc: javi.merino, Punit Agrawal, Zhang Rui, Eduardo Valentin,
	Steven Rostedt, Frederic Weisbecker, Ingo Molnar

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 <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point
  2014-06-11 11:31 [RFC PATCH 0/3] Add trace to thermal framework Punit Agrawal
  2014-06-11 11:31 ` [RFC PATCH 1/3] thermal: trace: Trace temperature changes Punit Agrawal
  2014-06-11 11:31 ` [RFC PATCH 2/3] thermal: trace: Trace when a cooling device's state is updated Punit Agrawal
@ 2014-06-11 11:31 ` Punit Agrawal
  2014-06-11 12:49   ` Steven Rostedt
  2014-06-20 17:24   ` Javi Merino
  2014-07-29 10:50 ` [PATCH 0/3] Add trace to thermal framework Punit Agrawal
  3 siblings, 2 replies; 20+ messages in thread
From: Punit Agrawal @ 2014-06-11 11:31 UTC (permalink / raw)
  To: linux-pm, linux-kernel
  Cc: javi.merino, Punit Agrawal, Zhang Rui, Eduardo Valentin,
	Steven Rostedt, Frederic Weisbecker, Ingo Molnar

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 <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point
  2014-06-11 11:31 ` [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point Punit Agrawal
@ 2014-06-11 12:49   ` Steven Rostedt
  2014-06-11 14:11     ` Punit Agrawal
  2014-06-20 17:24   ` Javi Merino
  1 sibling, 1 reply; 20+ messages in thread
From: Steven Rostedt @ 2014-06-11 12:49 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: linux-pm, linux-kernel, javi.merino, Zhang Rui, Eduardo Valentin,
	Frederic Weisbecker, Ingo Molnar

On Wed, 11 Jun 2014 12:31:44 +0100
Punit Agrawal <punit.agrawal@arm.com> 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 <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> ---
> 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 */


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point
  2014-06-11 12:49   ` Steven Rostedt
@ 2014-06-11 14:11     ` Punit Agrawal
  2014-06-11 14:20       ` Steven Rostedt
  0 siblings, 1 reply; 20+ messages in thread
From: Punit Agrawal @ 2014-06-11 14:11 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-pm, linux-kernel, Javi Merino, Zhang Rui, Eduardo Valentin,
	Frederic Weisbecker, Ingo Molnar

Thanks for the quick response.

Steven Rostedt <rostedt@goodmis.org> writes:

> On Wed, 11 Jun 2014 12:31:44 +0100
> Punit Agrawal <punit.agrawal@arm.com> 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 <rui.zhang@intel.com>
>> Cc: Eduardo Valentin <edubezval@gmail.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> ---
>> 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 */

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point
  2014-06-11 14:11     ` Punit Agrawal
@ 2014-06-11 14:20       ` Steven Rostedt
  2014-06-11 14:53         ` Punit Agrawal
  0 siblings, 1 reply; 20+ messages in thread
From: Steven Rostedt @ 2014-06-11 14:20 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: linux-pm, linux-kernel, Javi Merino, Zhang Rui, Eduardo Valentin,
	Frederic Weisbecker, Ingo Molnar

On Wed, 11 Jun 2014 15:11:02 +0100
Punit Agrawal <punit.agrawal@arm.com> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point
  2014-06-11 14:20       ` Steven Rostedt
@ 2014-06-11 14:53         ` Punit Agrawal
  2014-06-11 15:08           ` Steven Rostedt
  0 siblings, 1 reply; 20+ messages in thread
From: Punit Agrawal @ 2014-06-11 14:53 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-pm, linux-kernel, Javi Merino, Zhang Rui, Eduardo Valentin,
	Frederic Weisbecker, Ingo Molnar

Steven Rostedt <rostedt@goodmis.org> writes:

> On Wed, 11 Jun 2014 15:11:02 +0100
> Punit Agrawal <punit.agrawal@arm.com> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point
  2014-06-11 14:53         ` Punit Agrawal
@ 2014-06-11 15:08           ` Steven Rostedt
  2014-06-12 16:16             ` Punit Agrawal
  0 siblings, 1 reply; 20+ messages in thread
From: Steven Rostedt @ 2014-06-11 15:08 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: linux-pm, linux-kernel, Javi Merino, Zhang Rui, Eduardo Valentin,
	Frederic Weisbecker, Ingo Molnar

On Wed, 11 Jun 2014 15:53:42 +0100
Punit Agrawal <punit.agrawal@arm.com> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point
  2014-06-11 15:08           ` Steven Rostedt
@ 2014-06-12 16:16             ` Punit Agrawal
  0 siblings, 0 replies; 20+ messages in thread
From: Punit Agrawal @ 2014-06-12 16:16 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-pm, linux-kernel, Javi Merino, Zhang Rui, Eduardo Valentin,
	Frederic Weisbecker, Ingo Molnar

Steven Rostedt <rostedt@goodmis.org> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point
  2014-06-11 11:31 ` [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point Punit Agrawal
  2014-06-11 12:49   ` Steven Rostedt
@ 2014-06-20 17:24   ` Javi Merino
  2014-06-24 10:41     ` Punit Agrawal
  1 sibling, 1 reply; 20+ messages in thread
From: Javi Merino @ 2014-06-20 17:24 UTC (permalink / raw)
  To: Punit Agrawal; +Cc: linux-pm, linux-kernel, Zhang Rui, Eduardo Valentin

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 <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> ---
> 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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point
  2014-06-20 17:24   ` Javi Merino
@ 2014-06-24 10:41     ` Punit Agrawal
  2014-06-25 13:26       ` Javi Merino
  2014-07-25 14:11       ` edubezval
  0 siblings, 2 replies; 20+ messages in thread
From: Punit Agrawal @ 2014-06-24 10:41 UTC (permalink / raw)
  To: Javi Merino; +Cc: linux-pm, linux-kernel, Zhang Rui, Eduardo Valentin

"Javi Merino" <javi.merino@arm.com> 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 <rui.zhang@intel.com>
>> Cc: Eduardo Valentin <edubezval@gmail.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> ---
>> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point
  2014-06-24 10:41     ` Punit Agrawal
@ 2014-06-25 13:26       ` Javi Merino
  2014-07-25 14:11       ` edubezval
  1 sibling, 0 replies; 20+ messages in thread
From: Javi Merino @ 2014-06-25 13:26 UTC (permalink / raw)
  To: Punit Agrawal; +Cc: linux-pm, linux-kernel, Zhang Rui, Eduardo Valentin

On Tue, Jun 24, 2014 at 11:41:38AM +0100, Punit Agrawal wrote:
> "Javi Merino" <javi.merino@arm.com> 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 <rui.zhang@intel.com>
> >> Cc: Eduardo Valentin <edubezval@gmail.com>
> >> Cc: Steven Rostedt <rostedt@goodmis.org>
> >> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> >> Cc: Ingo Molnar <mingo@redhat.com>
> >> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> >> ---
> >> 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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point
  2014-06-24 10:41     ` Punit Agrawal
  2014-06-25 13:26       ` Javi Merino
@ 2014-07-25 14:11       ` edubezval
  1 sibling, 0 replies; 20+ messages in thread
From: edubezval @ 2014-07-25 14:11 UTC (permalink / raw)
  To: Punit Agrawal; +Cc: Javi Merino, linux-pm, linux-kernel, Zhang Rui

Punit,

On Tue, Jun 24, 2014 at 6:41 AM, Punit Agrawal <punit.agrawal@arm.com> wrote:
> "Javi Merino" <javi.merino@arm.com> 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 <rui.zhang@intel.com>
>>> Cc: Eduardo Valentin <edubezval@gmail.com>
>>> Cc: Steven Rostedt <rostedt@goodmis.org>
>>> Cc: Frederic Weisbecker <fweisbec@gmail.com>
>>> Cc: Ingo Molnar <mingo@redhat.com>
>>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>>> ---
>>> 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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Eduardo Bezerra Valentin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3] Add trace to thermal framework
  2014-06-11 11:31 [RFC PATCH 0/3] Add trace to thermal framework Punit Agrawal
                   ` (2 preceding siblings ...)
  2014-06-11 11:31 ` [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point Punit Agrawal
@ 2014-07-29 10:50 ` Punit Agrawal
  2014-07-29 10:50   ` [PATCH 1/3] thermal: trace: Trace temperature changes Punit Agrawal
                     ` (3 more replies)
  3 siblings, 4 replies; 20+ messages in thread
From: Punit Agrawal @ 2014-07-29 10:50 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-kernel, Punit Agrawal, javi.merino, rui.zhang, edubezval, rostedt

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/3] thermal: trace: Trace temperature changes
  2014-07-29 10:50 ` [PATCH 0/3] Add trace to thermal framework Punit Agrawal
@ 2014-07-29 10:50   ` Punit Agrawal
  2014-07-29 10:50   ` [PATCH 2/3] thermal: trace: Trace when a cooling device's state is updated Punit Agrawal
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 20+ messages in thread
From: Punit Agrawal @ 2014-07-29 10:50 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-kernel, Punit Agrawal, Zhang Rui, Eduardo Valentin,
	Steven Rostedt, Frederic Weisbecker, Ingo Molnar

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 <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/3] thermal: trace: Trace when a cooling device's state is updated
  2014-07-29 10:50 ` [PATCH 0/3] Add trace to thermal framework Punit Agrawal
  2014-07-29 10:50   ` [PATCH 1/3] thermal: trace: Trace temperature changes Punit Agrawal
@ 2014-07-29 10:50   ` Punit Agrawal
  2014-07-29 10:50   ` [PATCH 3/3] thermal: trace: Trace when temperature is above a trip point Punit Agrawal
  2014-07-29 13:33   ` [PATCH 0/3] Add trace to thermal framework Eduardo Valentin
  3 siblings, 0 replies; 20+ messages in thread
From: Punit Agrawal @ 2014-07-29 10:50 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-kernel, Punit Agrawal, Zhang Rui, Eduardo Valentin,
	Steven Rostedt, Frederic Weisbecker, Ingo Molnar

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 <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/3] thermal: trace: Trace when temperature is above a trip point
  2014-07-29 10:50 ` [PATCH 0/3] Add trace to thermal framework Punit Agrawal
  2014-07-29 10:50   ` [PATCH 1/3] thermal: trace: Trace temperature changes Punit Agrawal
  2014-07-29 10:50   ` [PATCH 2/3] thermal: trace: Trace when a cooling device's state is updated Punit Agrawal
@ 2014-07-29 10:50   ` Punit Agrawal
  2014-07-29 13:33   ` [PATCH 0/3] Add trace to thermal framework Eduardo Valentin
  3 siblings, 0 replies; 20+ messages in thread
From: Punit Agrawal @ 2014-07-29 10:50 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-kernel, Punit Agrawal, Zhang Rui, Eduardo Valentin,
	Steven Rostedt, Frederic Weisbecker, Ingo Molnar

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 <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] Add trace to thermal framework
  2014-07-29 10:50 ` [PATCH 0/3] Add trace to thermal framework Punit Agrawal
                     ` (2 preceding siblings ...)
  2014-07-29 10:50   ` [PATCH 3/3] thermal: trace: Trace when temperature is above a trip point Punit Agrawal
@ 2014-07-29 13:33   ` Eduardo Valentin
  2014-07-30 11:40     ` Punit Agrawal
  3 siblings, 1 reply; 20+ messages in thread
From: Eduardo Valentin @ 2014-07-29 13:33 UTC (permalink / raw)
  To: Punit Agrawal; +Cc: linux-pm, linux-kernel, javi.merino, rui.zhang, rostedt

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
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] Add trace to thermal framework
  2014-07-29 13:33   ` [PATCH 0/3] Add trace to thermal framework Eduardo Valentin
@ 2014-07-30 11:40     ` Punit Agrawal
  0 siblings, 0 replies; 20+ messages in thread
From: Punit Agrawal @ 2014-07-30 11:40 UTC (permalink / raw)
  To: Eduardo Valentin; +Cc: linux-pm, linux-kernel, javi.merino, rui.zhang, rostedt

Eduardo Valentin <edubezval@gmail.com> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2014-07-30 11:40 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-11 11:31 [RFC PATCH 0/3] Add trace to thermal framework Punit Agrawal
2014-06-11 11:31 ` [RFC PATCH 1/3] thermal: trace: Trace temperature changes Punit Agrawal
2014-06-11 11:31 ` [RFC PATCH 2/3] thermal: trace: Trace when a cooling device's state is updated Punit Agrawal
2014-06-11 11:31 ` [RFC PATCH 3/3] thermal: trace: Trace when temperature is above a trip point Punit Agrawal
2014-06-11 12:49   ` Steven Rostedt
2014-06-11 14:11     ` Punit Agrawal
2014-06-11 14:20       ` Steven Rostedt
2014-06-11 14:53         ` Punit Agrawal
2014-06-11 15:08           ` Steven Rostedt
2014-06-12 16:16             ` Punit Agrawal
2014-06-20 17:24   ` Javi Merino
2014-06-24 10:41     ` Punit Agrawal
2014-06-25 13:26       ` Javi Merino
2014-07-25 14:11       ` edubezval
2014-07-29 10:50 ` [PATCH 0/3] Add trace to thermal framework Punit Agrawal
2014-07-29 10:50   ` [PATCH 1/3] thermal: trace: Trace temperature changes Punit Agrawal
2014-07-29 10:50   ` [PATCH 2/3] thermal: trace: Trace when a cooling device's state is updated Punit Agrawal
2014-07-29 10:50   ` [PATCH 3/3] thermal: trace: Trace when temperature is above a trip point Punit Agrawal
2014-07-29 13:33   ` [PATCH 0/3] Add trace to thermal framework Eduardo Valentin
2014-07-30 11:40     ` Punit Agrawal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).