diff mbox

[powertop2.0] Modification to fix the removal of lock_depth field

Message ID 1308805920-18203-1-git-send-email-amit.kachhap@linaro.org
State New
Headers show

Commit Message

Amit Daniel Kachhap June 23, 2011, 5:12 a.m. UTC 
lock_depth field is removed from the power frequency events in the
new linux kernel(2.6.38 onwards). So this creates issue to retrieve
the lower members of the trace data. To fix this problem 2 separate
structures are created and their use depends upon the format of the
power_frequency events. These changes have been tested to work fine
with both old and new kernels as well as on x86 and ARM platform.

Signed-off-by:  Amit Daniel Kachhap <amit.kachhap@linaro.org>
---
This patch is for powertop2.0 and fixes the issue of frequency
reported as always 0 in kernel from 2.6.38 onwards.

 cpu/cpu.cpp          |    7 +---
 perf/perf_bundle.cpp |   98 ++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 82 insertions(+), 23 deletions(-)

Comments

Arjan van de Ven June 23, 2011, 5:11 a.m. UTC | #1 
On 6/22/2011 10:12 PM, Amit Daniel Kachhap wrote:
> lock_depth field is removed from the power frequency events in the
> new linux kernel(2.6.38 onwards). So this creates issue to retrieve
> the lower members of the trace data. To fix this problem 2 separate
> structures are created and their use depends upon the format of the
> power_frequency events. These changes have been tested to work fine
> with both old and new kernels as well as on x86 and ARM platform.
>


eh no

while it was temporarily removed in an -rc, it was added back (as 
padding) after it was
found to break powertop (and abi).

or am I missing something???
Amit Daniel Kachhap June 23, 2011, 5:35 a.m. UTC | #2 
On 23 June 2011 10:41, Arjan van de Ven <arjan@linux.intel.com> wrote:
> On 6/22/2011 10:12 PM, Amit Daniel Kachhap wrote:
>>
>> lock_depth field is removed from the power frequency events in the
>> new linux kernel(2.6.38 onwards). So this creates issue to retrieve
>> the lower members of the trace data. To fix this problem 2 separate
>> structures are created and their use depends upon the format of the
>> power_frequency events. These changes have been tested to work fine
>> with both old and new kernels as well as on x86 and ARM platform.
>>
>
>
> eh no
>
> while it was temporarily removed in an -rc, it was added back (as padding)
> after it was
> found to break powertop (and abi).
>
> or am I missing something???
>
>
Ohh the padding field fixes this issue.
Then this patch may not be needed.
Thanks Arjan for quick review.
diff mbox

Patch

diff --git a/cpu/cpu.cpp b/cpu/cpu.cpp
index b901b87..1193d7b 100644
--- a/cpu/cpu.cpp
+++ b/cpu/cpu.cpp
@@ -742,11 +742,8 @@  void w_display_cpu_pstates(void)
 
 
 struct power_entry {
-#ifndef __i386__
-	int dummy;
-#endif
-	int64_t	type;
-	int64_t	value;
+	uint64_t	type;
+	uint64_t	value;
 } __attribute__((packed));
 
 
diff --git a/perf/perf_bundle.cpp b/perf/perf_bundle.cpp
index 2276c3a..f4c8912 100644
--- a/perf/perf_bundle.cpp
+++ b/perf/perf_bundle.cpp
@@ -27,6 +27,7 @@ 
 #include <algorithm>
 #include <string.h>
 #include <stdint.h>
+#include <dirent.h>
 
 #include "perf_bundle.h"
 #include "perf_event.h"
@@ -110,11 +111,30 @@  void perf_bundle::add_event(const char *event_name)
 		}
 	}
 }
-
+static int lock_depth = 0;
 void perf_bundle::start(void)
 {
 	unsigned int i;
 	class perf_event *ev;
+	char line[1023];
+	FILE *file;
+        char filename[PATH_MAX];
+
+	/*check the format of power_frequency event*/
+	strcpy(filename,
+	 "/sys/kernel/debug/tracing/events/power/power_frequency/format");
+	if (!lock_depth) {
+		file = fopen(filename, "r");
+		if (file) {
+			while (fgets(line, 1023,file) != NULL) {
+				if (strstr(line, "lock_depth")) {
+					lock_depth = 1;
+					break;
+				}
+			}
+			fclose(file);
+		}
+	}
 
 	for (i = 0; i < events.size(); i++) {
 		ev = events[i];
@@ -154,34 +174,57 @@  void perf_bundle::clear(void)
 	records.resize(0);
 }
 
-
-struct trace_entry {
+/*Trace entry for the common fields of perf events*/
+struct trace_entry_common {
 	uint64_t		time;
 	uint32_t		cpu;
 	uint32_t		res;
 	__u32			size;
+} __attribute__((packed));
+
+/*Trace entry for the fields specific to each perf events.
+* lock_depth field to support old kernels*/
+struct trace_data_lockdepth {
 	unsigned short		type;
 	unsigned char		flags;
 	unsigned char		preempt_count;
 	int			pid;
 	int			lock_depth;
+#ifndef __i386__
+	int			filler;
+#endif
 } __attribute__((packed));;
 
+/*Trace entry for the fields specific to each perf events*/
+struct trace_data_normal {
+	unsigned short		type;
+	unsigned char		flags;
+	unsigned char		preempt_count;
+	int			pid;
+} __attribute__((packed));;
 
-struct perf_sample {
+struct perf_sample_normal {
 	struct perf_event_header        header;
-	struct trace_entry		trace;
+	struct trace_entry_common	trace_common;
+	struct trace_data_normal	trace_data;
 	unsigned char			data[0];
 } __attribute__((packed));
 
+struct perf_sample_lockdepth {
+	struct perf_event_header        header;
+	struct trace_entry_common	trace_common;
+	struct trace_data_lockdepth	trace_data;
+	unsigned char			data[0];
+} __attribute__((packed));;
+
 static uint64_t timestamp(perf_event_header *event)
 {
-	struct perf_sample *sample;
+	struct perf_sample_lockdepth *sample;
 
 	if (event->type != PERF_RECORD_SAMPLE)
 		return 0;
 
-	sample = (struct perf_sample *)event;
+	sample = (struct perf_sample_lockdepth *)event;
 
 #if 0
 	int i;
@@ -207,7 +250,7 @@  static uint64_t timestamp(perf_event_header *event)
 		printf("%02x ", *(x+i));
 	printf("\n");
 #endif
-	return sample->trace.time;
+	return sample->trace_common.time;
 	
 }
 
@@ -235,16 +278,35 @@  void perf_bundle::process(void)
 	sort(records.begin(), records.end(), event_sort_function);
 
 	for (i = 0; i < records.size(); i++) {
-		struct perf_sample *sample;
-
-		sample = (struct perf_sample *)records[i];
-		if (!sample)
-			continue;
-
-		if (sample->header.type != PERF_RECORD_SAMPLE)
-			continue;
-
-		handle_trace_point(sample->trace.type, &sample->data, sample->trace.cpu, sample->trace.time, sample->trace.flags);
+		/*Check for lock_depth */
+		if (lock_depth) {
+			struct perf_sample_lockdepth *sample =
+				(struct perf_sample_lockdepth *)records[i];
+			if (!sample)
+				continue;
+
+			if (sample->header.type != PERF_RECORD_SAMPLE)
+				continue;
+
+			handle_trace_point(sample->trace_data.type,
+				&sample->data, sample->trace_common.cpu,
+				sample->trace_common.time,
+				sample->trace_data.flags);
+		}
+		else {
+			struct perf_sample_normal *sample =
+					(struct perf_sample_normal *)records[i];
+			if (!sample)
+				continue;
+
+			if (sample->header.type != PERF_RECORD_SAMPLE)
+				continue;
+
+			handle_trace_point(sample->trace_data.type,
+				&sample->data, sample->trace_common.cpu,
+				sample->trace_common.time,
+				sample->trace_data.flags);
+		}
 		
 	}
 }