linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL||RFC 00/11] perf library and regression testing improvements
@ 2011-01-04  3:48 Arnaldo Carvalho de Melo
  2011-01-04  3:48 ` [PATCH 01/11] perf tools: Introduce event selectors Arnaldo Carvalho de Melo
                   ` (11 more replies)
  0 siblings, 12 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04  3:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Han Pingtian, Ingo Molnar, Mike Galbraith, Paul Mackerras,
	Peter Zijlstra, Stephane Eranian, Tom Zanussi, Thomas Gleixner,
	Arnaldo Carvalho de Melo

Hi Ingo, Peter et al,

	Trying to simplify using the API for tools and more specifically for
'perf test' regression tests, please take a look, perhaps starting from the last
changeset, that implements a regression test using the resulting library API.

	It also reduces the 'perf' tool footprint by not using hard coded array
sizes, more need to be done, but should be a good start, one changeset shows a
good reduction in BSS use.

	Suggestions for naming most welcome, I thought about using "event__",
but that is taken, "perf_event__", but thought it would clash with "event_t",
so used the jargon used for the '-e' parameters: "Event Selector", but don't
like it that much, what do you think?

	Writing the first regression test I think there are more ways to simplify,
on top of these, like not requiring a thread_map, i.e. passing NULL for that
parameter would mean: self-monitor, etc.

        If you are pleased as-is, please pull from:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/test

Regards,

- Arnaldo

Arnaldo Carvalho de Melo (11):
  perf tools: Introduce event selectors
  perf evsel: Adopt MATCH_EVENT macro from 'stat'
  perf util: Move do_read from session to util
  perf evsel: Delete the event selectors at exit
  perf evsel: Steal the counter reading routines from stat
  perf evsel: Introduce per cpu and per thread open helpers
  perf tools: Refactor cpumap to hold nr and the map
  perf tools: Refactor all_tids to hold nr and the map
  perf evsel: Use {cpu,thread}_map to shorten list of parameters
  perf evsel: Auto allocate resources needed for some methods
  perf test: Add test for counting open syscalls

 tools/perf/Makefile                |    4 +
 tools/perf/builtin-record.c        |  152 +++++++--------
 tools/perf/builtin-stat.c          |  368 +++++++++++++++---------------------
 tools/perf/builtin-test.c          |   83 ++++++++
 tools/perf/builtin-top.c           |  221 ++++++++++++----------
 tools/perf/perf.c                  |    2 +
 tools/perf/util/cpumap.c           |  123 +++++++++---
 tools/perf/util/cpumap.h           |   10 +-
 tools/perf/util/evsel.c            |  186 ++++++++++++++++++
 tools/perf/util/evsel.h            |  115 +++++++++++
 tools/perf/util/header.c           |   15 +-
 tools/perf/util/header.h           |    3 +-
 tools/perf/util/parse-events.c     |   58 ++++--
 tools/perf/util/parse-events.h     |   18 ++-
 tools/perf/util/session.c          |   22 +--
 tools/perf/util/session.h          |    1 -
 tools/perf/util/thread.c           |   43 +++--
 tools/perf/util/thread.h           |   15 ++-
 tools/perf/util/trace-event-info.c |   30 ++--
 tools/perf/util/trace-event.h      |    5 +-
 tools/perf/util/util.c             |   17 ++
 tools/perf/util/util.h             |    1 +
 tools/perf/util/xyarray.c          |   20 ++
 tools/perf/util/xyarray.h          |   20 ++
 24 files changed, 1013 insertions(+), 519 deletions(-)
 create mode 100644 tools/perf/util/evsel.c
 create mode 100644 tools/perf/util/evsel.h
 create mode 100644 tools/perf/util/xyarray.c
 create mode 100644 tools/perf/util/xyarray.h


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

* [PATCH 01/11] perf tools: Introduce event selectors
  2011-01-04  3:48 [GIT PULL||RFC 00/11] perf library and regression testing improvements Arnaldo Carvalho de Melo
@ 2011-01-04  3:48 ` Arnaldo Carvalho de Melo
  2011-01-04  3:48 ` [PATCH 02/11] perf evsel: Adopt MATCH_EVENT macro from 'stat' Arnaldo Carvalho de Melo
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04  3:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Tom Zanussi, Thomas Gleixner

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Out of ad-hoc code and global arrays with hard coded sizes.

This is the first step on having a library that will be first
used on regression tests in the 'perf test' tool.

[acme@felicio linux]$ size /tmp/perf.before
   text	   data	    bss	    dec	    hex	filename
1273776	  97384	5104416	6475576	 62cf38	/tmp/perf.before
[acme@felicio linux]$ size /tmp/perf.new
   text	   data	    bss	    dec	    hex	filename
1275422	  97416	1392416	2765254	 2a31c6	/tmp/perf.new

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile                |    4 +
 tools/perf/builtin-record.c        |  113 +++++++++++------------
 tools/perf/builtin-stat.c          |  175 ++++++++++++++++++++++--------------
 tools/perf/builtin-top.c           |  176 +++++++++++++++++++++---------------
 tools/perf/util/evsel.c            |   35 +++++++
 tools/perf/util/evsel.h            |   24 +++++
 tools/perf/util/header.c           |    9 +-
 tools/perf/util/header.h           |    3 +-
 tools/perf/util/parse-events.c     |   47 +++++++----
 tools/perf/util/parse-events.h     |   17 +++--
 tools/perf/util/trace-event-info.c |   30 ++++---
 tools/perf/util/trace-event.h      |    5 +-
 tools/perf/util/xyarray.c          |   20 ++++
 tools/perf/util/xyarray.h          |   20 ++++
 14 files changed, 433 insertions(+), 245 deletions(-)
 create mode 100644 tools/perf/util/evsel.c
 create mode 100644 tools/perf/util/evsel.h
 create mode 100644 tools/perf/util/xyarray.c
 create mode 100644 tools/perf/util/xyarray.h

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index ac6692c..1b9b13e 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -396,6 +396,7 @@ LIB_H += util/build-id.h
 LIB_H += util/debug.h
 LIB_H += util/debugfs.h
 LIB_H += util/event.h
+LIB_H += util/evsel.h
 LIB_H += util/exec_cmd.h
 LIB_H += util/types.h
 LIB_H += util/levenshtein.h
@@ -404,6 +405,7 @@ LIB_H += util/parse-options.h
 LIB_H += util/parse-events.h
 LIB_H += util/quote.h
 LIB_H += util/util.h
+LIB_H += util/xyarray.h
 LIB_H += util/header.h
 LIB_H += util/help.h
 LIB_H += util/session.h
@@ -433,6 +435,7 @@ LIB_OBJS += $(OUTPUT)util/ctype.o
 LIB_OBJS += $(OUTPUT)util/debugfs.o
 LIB_OBJS += $(OUTPUT)util/environment.o
 LIB_OBJS += $(OUTPUT)util/event.o
+LIB_OBJS += $(OUTPUT)util/evsel.o
 LIB_OBJS += $(OUTPUT)util/exec_cmd.o
 LIB_OBJS += $(OUTPUT)util/help.o
 LIB_OBJS += $(OUTPUT)util/levenshtein.o
@@ -470,6 +473,7 @@ LIB_OBJS += $(OUTPUT)util/sort.o
 LIB_OBJS += $(OUTPUT)util/hist.o
 LIB_OBJS += $(OUTPUT)util/probe-event.o
 LIB_OBJS += $(OUTPUT)util/util.o
+LIB_OBJS += $(OUTPUT)util/xyarray.o
 LIB_OBJS += $(OUTPUT)util/cpumap.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 50efbd5..e68aee3 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -18,6 +18,7 @@
 
 #include "util/header.h"
 #include "util/event.h"
+#include "util/evsel.h"
 #include "util/debug.h"
 #include "util/session.h"
 #include "util/symbol.h"
@@ -27,13 +28,13 @@
 #include <sched.h>
 #include <sys/mman.h>
 
+#define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
+
 enum write_mode_t {
 	WRITE_FORCE,
 	WRITE_APPEND
 };
 
-static int			*fd[MAX_NR_CPUS][MAX_COUNTERS];
-
 static u64			user_interval			= ULLONG_MAX;
 static u64			default_interval		=      0;
 static u64			sample_type;
@@ -81,7 +82,6 @@ static struct perf_session	*session;
 static const char		*cpu_list;
 
 struct mmap_data {
-	int			counter;
 	void			*base;
 	unsigned int		mask;
 	unsigned int		prev;
@@ -229,12 +229,12 @@ static struct perf_header_attr *get_header_attr(struct perf_event_attr *a, int n
 	return h_attr;
 }
 
-static void create_counter(int counter, int cpu)
+static void create_counter(struct perf_evsel *evsel, int cpu)
 {
-	char *filter = filters[counter];
-	struct perf_event_attr *attr = attrs + counter;
+	char *filter = evsel->filter;
+	struct perf_event_attr *attr = &evsel->attr;
 	struct perf_header_attr *h_attr;
-	int track = !counter; /* only the first counter needs these */
+	int track = !evsel->idx; /* only the first counter needs these */
 	int thread_index;
 	int ret;
 	struct {
@@ -320,10 +320,9 @@ retry_sample_id:
 
 	for (thread_index = 0; thread_index < thread_num; thread_index++) {
 try_again:
-		fd[nr_cpu][counter][thread_index] = sys_perf_event_open(attr,
-				all_tids[thread_index], cpu, group_fd, 0);
+		FD(evsel, nr_cpu, thread_index) = sys_perf_event_open(attr, all_tids[thread_index], cpu, group_fd, 0);
 
-		if (fd[nr_cpu][counter][thread_index] < 0) {
+		if (FD(evsel, nr_cpu, thread_index) < 0) {
 			int err = errno;
 
 			if (err == EPERM || err == EACCES)
@@ -360,7 +359,7 @@ try_again:
 			}
 			printf("\n");
 			error("sys_perf_event_open() syscall returned with %d (%s).  /bin/dmesg may provide additional information.\n",
-					fd[nr_cpu][counter][thread_index], strerror(err));
+			      FD(evsel, nr_cpu, thread_index), strerror(err));
 
 #if defined(__i386__) || defined(__x86_64__)
 			if (attr->type == PERF_TYPE_HARDWARE && err == EOPNOTSUPP)
@@ -374,7 +373,7 @@ try_again:
 			exit(-1);
 		}
 
-		h_attr = get_header_attr(attr, counter);
+		h_attr = get_header_attr(attr, evsel->idx);
 		if (h_attr == NULL)
 			die("nomem\n");
 
@@ -385,7 +384,7 @@ try_again:
 			}
 		}
 
-		if (read(fd[nr_cpu][counter][thread_index], &read_data, sizeof(read_data)) == -1) {
+		if (read(FD(evsel, nr_cpu, thread_index), &read_data, sizeof(read_data)) == -1) {
 			perror("Unable to read perf file descriptor");
 			exit(-1);
 		}
@@ -395,43 +394,44 @@ try_again:
 			exit(-1);
 		}
 
-		assert(fd[nr_cpu][counter][thread_index] >= 0);
-		fcntl(fd[nr_cpu][counter][thread_index], F_SETFL, O_NONBLOCK);
+		assert(FD(evsel, nr_cpu, thread_index) >= 0);
+		fcntl(FD(evsel, nr_cpu, thread_index), F_SETFL, O_NONBLOCK);
 
 		/*
 		 * First counter acts as the group leader:
 		 */
 		if (group && group_fd == -1)
-			group_fd = fd[nr_cpu][counter][thread_index];
-
-		if (counter || thread_index) {
-			ret = ioctl(fd[nr_cpu][counter][thread_index],
-					PERF_EVENT_IOC_SET_OUTPUT,
-					fd[nr_cpu][0][0]);
+			group_fd = FD(evsel, nr_cpu, thread_index);
+
+		if (evsel->idx || thread_index) {
+			struct perf_evsel *first;
+			first = list_entry(evsel_list.next, struct perf_evsel, node);
+			ret = ioctl(FD(evsel, nr_cpu, thread_index),
+				    PERF_EVENT_IOC_SET_OUTPUT,
+				    FD(first, nr_cpu, 0));
 			if (ret) {
 				error("failed to set output: %d (%s)\n", errno,
 						strerror(errno));
 				exit(-1);
 			}
 		} else {
-			mmap_array[nr_cpu].counter = counter;
 			mmap_array[nr_cpu].prev = 0;
 			mmap_array[nr_cpu].mask = mmap_pages*page_size - 1;
 			mmap_array[nr_cpu].base = mmap(NULL, (mmap_pages+1)*page_size,
-				PROT_READ|PROT_WRITE, MAP_SHARED, fd[nr_cpu][counter][thread_index], 0);
+				PROT_READ | PROT_WRITE, MAP_SHARED, FD(evsel, nr_cpu, thread_index), 0);
 			if (mmap_array[nr_cpu].base == MAP_FAILED) {
 				error("failed to mmap with %d (%s)\n", errno, strerror(errno));
 				exit(-1);
 			}
 
-			event_array[nr_poll].fd = fd[nr_cpu][counter][thread_index];
+			event_array[nr_poll].fd = FD(evsel, nr_cpu, thread_index);
 			event_array[nr_poll].events = POLLIN;
 			nr_poll++;
 		}
 
 		if (filter != NULL) {
-			ret = ioctl(fd[nr_cpu][counter][thread_index],
-					PERF_EVENT_IOC_SET_FILTER, filter);
+			ret = ioctl(FD(evsel, nr_cpu, thread_index),
+				    PERF_EVENT_IOC_SET_FILTER, filter);
 			if (ret) {
 				error("failed to set filter with %d (%s)\n", errno,
 						strerror(errno));
@@ -446,11 +446,12 @@ try_again:
 
 static void open_counters(int cpu)
 {
-	int counter;
+	struct perf_evsel *pos;
 
 	group_fd = -1;
-	for (counter = 0; counter < nr_counters; counter++)
-		create_counter(counter, cpu);
+
+	list_for_each_entry(pos, &evsel_list, node)
+		create_counter(pos, cpu);
 
 	nr_cpu++;
 }
@@ -537,7 +538,7 @@ static void mmap_read_all(void)
 
 static int __cmd_record(int argc, const char **argv)
 {
-	int i, counter;
+	int i;
 	struct stat st;
 	int flags;
 	int err;
@@ -604,7 +605,7 @@ static int __cmd_record(int argc, const char **argv)
 			goto out_delete_session;
 	}
 
-	if (have_tracepoints(attrs, nr_counters))
+	if (have_tracepoints(&evsel_list))
 		perf_header__set_feat(&session->header, HEADER_TRACE_INFO);
 
 	/*
@@ -666,12 +667,6 @@ static int __cmd_record(int argc, const char **argv)
 		close(child_ready_pipe[0]);
 	}
 
-	nr_cpus = read_cpu_map(cpu_list);
-	if (nr_cpus < 1) {
-		perror("failed to collect number of CPUs");
-		return -1;
-	}
-
 	if (!system_wide && no_inherit && !cpu_list) {
 		open_counters(-1);
 	} else {
@@ -711,7 +706,7 @@ static int __cmd_record(int argc, const char **argv)
 			return err;
 		}
 
-		if (have_tracepoints(attrs, nr_counters)) {
+		if (have_tracepoints(&evsel_list)) {
 			/*
 			 * FIXME err <= 0 here actually means that
 			 * there were no tracepoints so its not really
@@ -720,8 +715,7 @@ static int __cmd_record(int argc, const char **argv)
 			 * return this more properly and also
 			 * propagate errors that now are calling die()
 			 */
-			err = event__synthesize_tracing_data(output, attrs,
-							     nr_counters,
+			err = event__synthesize_tracing_data(output, &evsel_list,
 							     process_synthesized_event,
 							     session);
 			if (err <= 0) {
@@ -795,13 +789,13 @@ static int __cmd_record(int argc, const char **argv)
 
 		if (done) {
 			for (i = 0; i < nr_cpu; i++) {
-				for (counter = 0;
-					counter < nr_counters;
-					counter++) {
+				struct perf_evsel *pos;
+
+				list_for_each_entry(pos, &evsel_list, node) {
 					for (thread = 0;
 						thread < thread_num;
 						thread++)
-						ioctl(fd[i][counter][thread],
+						ioctl(FD(pos, i, thread),
 							PERF_EVENT_IOC_DISABLE);
 				}
 			}
@@ -887,7 +881,8 @@ const struct option record_options[] = {
 
 int cmd_record(int argc, const char **argv, const char *prefix __used)
 {
-	int i, j, err = -ENOMEM;
+	int err = -ENOMEM;
+	struct perf_evsel *pos;
 
 	argc = parse_options(argc, argv, record_options, record_usage,
 			    PARSE_OPT_STOP_AT_NON_OPTION);
@@ -910,10 +905,9 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 	if (no_buildid_cache || no_buildid)
 		disable_buildid_cache();
 
-	if (!nr_counters) {
-		nr_counters	= 1;
-		attrs[0].type	= PERF_TYPE_HARDWARE;
-		attrs[0].config = PERF_COUNT_HW_CPU_CYCLES;
+	if (list_empty(&evsel_list) && perf_evsel_list__create_default() < 0) {
+		pr_err("Not enough memory for event selector list\n");
+		goto out_symbol_exit;
 	}
 
 	if (target_pid != -1) {
@@ -933,12 +927,15 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		thread_num = 1;
 	}
 
-	for (i = 0; i < MAX_NR_CPUS; i++) {
-		for (j = 0; j < MAX_COUNTERS; j++) {
-			fd[i][j] = malloc(sizeof(int)*thread_num);
-			if (!fd[i][j])
-				goto out_free_fd;
-		}
+	nr_cpus = read_cpu_map(cpu_list);
+	if (nr_cpus < 1) {
+		perror("failed to collect number of CPUs");
+		return -1;
+	}
+
+	list_for_each_entry(pos, &evsel_list, node) {
+		if (perf_evsel__alloc_fd(pos, nr_cpus, thread_num) < 0)
+			goto out_free_fd;
 	}
 	event_array = malloc(
 		sizeof(struct pollfd)*MAX_NR_CPUS*MAX_COUNTERS*thread_num);
@@ -968,10 +965,8 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 out_free_event_array:
 	free(event_array);
 out_free_fd:
-	for (i = 0; i < MAX_NR_CPUS; i++) {
-		for (j = 0; j < MAX_COUNTERS; j++)
-			free(fd[i][j]);
-	}
+	list_for_each_entry(pos, &evsel_list, node)
+		perf_evsel__free_fd(pos);
 	free(all_tids);
 	all_tids = NULL;
 out_symbol_exit:
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 7ff746d..511ebaf 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -43,6 +43,7 @@
 #include "util/parse-options.h"
 #include "util/parse-events.h"
 #include "util/event.h"
+#include "util/evsel.h"
 #include "util/debug.h"
 #include "util/header.h"
 #include "util/cpumap.h"
@@ -52,6 +53,8 @@
 #include <math.h>
 #include <locale.h>
 
+#define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
+
 #define DEFAULT_SEPARATOR	" "
 
 static struct perf_event_attr default_attrs[] = {
@@ -90,16 +93,11 @@ static const char		*cpu_list;
 static const char		*csv_sep			= NULL;
 static bool			csv_output			= false;
 
-
-static int			*fd[MAX_NR_CPUS][MAX_COUNTERS];
-
-static int			event_scaled[MAX_COUNTERS];
-
-static struct {
+struct cpu_counts {
 	u64 val;
 	u64 ena;
 	u64 run;
-} cpu_counts[MAX_NR_CPUS][MAX_COUNTERS];
+};
 
 static volatile int done = 0;
 
@@ -108,6 +106,26 @@ struct stats
 	double n, mean, M2;
 };
 
+struct perf_stat {
+	struct stats	  res_stats[3];
+	int		  scaled;
+	struct cpu_counts cpu_counts[];
+};
+
+static int perf_evsel__alloc_stat_priv(struct perf_evsel *evsel, int ncpus)
+{
+	size_t priv_size = (sizeof(struct perf_stat) +
+			    (ncpus * sizeof(struct cpu_counts)));
+	evsel->priv = zalloc(priv_size);
+	return evsel->priv == NULL ? -ENOMEM : 0;
+}
+
+static void perf_evsel__free_stat_priv(struct perf_evsel *evsel)
+{
+	free(evsel->priv);
+	evsel->priv = NULL;
+}
+
 static void update_stats(struct stats *stats, u64 val)
 {
 	double delta;
@@ -147,22 +165,21 @@ static double stddev_stats(struct stats *stats)
 	return sqrt(variance_mean);
 }
 
-struct stats			event_res_stats[MAX_COUNTERS][3];
 struct stats			runtime_nsecs_stats[MAX_NR_CPUS];
 struct stats			runtime_cycles_stats[MAX_NR_CPUS];
 struct stats			runtime_branches_stats[MAX_NR_CPUS];
 struct stats			walltime_nsecs_stats;
 
-#define MATCH_EVENT(t, c, counter)			\
-	(attrs[counter].type == PERF_TYPE_##t &&	\
-	 attrs[counter].config == PERF_COUNT_##c)
+#define MATCH_EVENT(t, c, evsel)			\
+	(evsel->attr.type == PERF_TYPE_##t &&	\
+	 evsel->attr.config == PERF_COUNT_##c)
 
 #define ERR_PERF_OPEN \
 "counter %d, sys_perf_event_open() syscall returned with %d (%s).  /bin/dmesg may provide additional information."
 
-static int create_perf_stat_counter(int counter, bool *perm_err)
+static int create_perf_stat_counter(struct perf_evsel *evsel, bool *perm_err)
 {
-	struct perf_event_attr *attr = attrs + counter;
+	struct perf_event_attr *attr = &evsel->attr;
 	int thread;
 	int ncreated = 0;
 
@@ -174,13 +191,13 @@ static int create_perf_stat_counter(int counter, bool *perm_err)
 		int cpu;
 
 		for (cpu = 0; cpu < nr_cpus; cpu++) {
-			fd[cpu][counter][0] = sys_perf_event_open(attr,
+			FD(evsel, cpu, 0) = sys_perf_event_open(attr,
 					-1, cpumap[cpu], -1, 0);
-			if (fd[cpu][counter][0] < 0) {
+			if (FD(evsel, cpu, 0) < 0) {
 				if (errno == EPERM || errno == EACCES)
 					*perm_err = true;
-				error(ERR_PERF_OPEN, counter,
-					 fd[cpu][counter][0], strerror(errno));
+				error(ERR_PERF_OPEN, evsel->idx,
+					FD(evsel, cpu, 0), strerror(errno));
 			} else {
 				++ncreated;
 			}
@@ -192,13 +209,13 @@ static int create_perf_stat_counter(int counter, bool *perm_err)
 			attr->enable_on_exec = 1;
 		}
 		for (thread = 0; thread < thread_num; thread++) {
-			fd[0][counter][thread] = sys_perf_event_open(attr,
+			FD(evsel, 0, thread) = sys_perf_event_open(attr,
 				all_tids[thread], -1, -1, 0);
-			if (fd[0][counter][thread] < 0) {
+			if (FD(evsel, 0, thread) < 0) {
 				if (errno == EPERM || errno == EACCES)
 					*perm_err = true;
-				error(ERR_PERF_OPEN, counter,
-					 fd[0][counter][thread],
+				error(ERR_PERF_OPEN, evsel->idx,
+					FD(evsel, 0, thread),
 					 strerror(errno));
 			} else {
 				++ncreated;
@@ -212,7 +229,7 @@ static int create_perf_stat_counter(int counter, bool *perm_err)
 /*
  * Does the counter have nsecs as a unit?
  */
-static inline int nsec_counter(int counter)
+static inline int nsec_counter(struct perf_evsel *counter)
 {
 	if (MATCH_EVENT(SOFTWARE, SW_CPU_CLOCK, counter) ||
 	    MATCH_EVENT(SOFTWARE, SW_TASK_CLOCK, counter))
@@ -225,8 +242,9 @@ static inline int nsec_counter(int counter)
  * Read out the results of a single counter:
  * aggregate counts across CPUs in system-wide mode
  */
-static void read_counter_aggr(int counter)
+static void read_counter_aggr(struct perf_evsel *counter)
 {
+	struct perf_stat *ps = counter->priv;
 	u64 count[3], single_count[3];
 	int cpu;
 	size_t res, nv;
@@ -238,15 +256,15 @@ static void read_counter_aggr(int counter)
 	nv = scale ? 3 : 1;
 	for (cpu = 0; cpu < nr_cpus; cpu++) {
 		for (thread = 0; thread < thread_num; thread++) {
-			if (fd[cpu][counter][thread] < 0)
+			if (FD(counter, cpu, thread) < 0)
 				continue;
 
-			res = read(fd[cpu][counter][thread],
+			res = read(FD(counter, cpu, thread),
 					single_count, nv * sizeof(u64));
 			assert(res == nv * sizeof(u64));
 
-			close(fd[cpu][counter][thread]);
-			fd[cpu][counter][thread] = -1;
+			close(FD(counter, cpu, thread));
+			FD(counter, cpu, thread) = -1;
 
 			count[0] += single_count[0];
 			if (scale) {
@@ -259,20 +277,20 @@ static void read_counter_aggr(int counter)
 	scaled = 0;
 	if (scale) {
 		if (count[2] == 0) {
-			event_scaled[counter] = -1;
+			ps->scaled = -1;
 			count[0] = 0;
 			return;
 		}
 
 		if (count[2] < count[1]) {
-			event_scaled[counter] = 1;
+			ps->scaled = 1;
 			count[0] = (unsigned long long)
 				((double)count[0] * count[1] / count[2] + 0.5);
 		}
 	}
 
 	for (i = 0; i < 3; i++)
-		update_stats(&event_res_stats[counter][i], count[i]);
+		update_stats(&ps->res_stats[i], count[i]);
 
 	if (verbose) {
 		fprintf(stderr, "%s: %Ld %Ld %Ld\n", event_name(counter),
@@ -294,8 +312,9 @@ static void read_counter_aggr(int counter)
  * Read out the results of a single counter:
  * do not aggregate counts across CPUs in system-wide mode
  */
-static void read_counter(int counter)
+static void read_counter(struct perf_evsel *counter)
 {
+	struct cpu_counts *cpu_counts = counter->priv;
 	u64 count[3];
 	int cpu;
 	size_t res, nv;
@@ -306,15 +325,15 @@ static void read_counter(int counter)
 
 	for (cpu = 0; cpu < nr_cpus; cpu++) {
 
-		if (fd[cpu][counter][0] < 0)
+		if (FD(counter, cpu, 0) < 0)
 			continue;
 
-		res = read(fd[cpu][counter][0], count, nv * sizeof(u64));
+		res = read(FD(counter, cpu, 0), count, nv * sizeof(u64));
 
 		assert(res == nv * sizeof(u64));
 
-		close(fd[cpu][counter][0]);
-		fd[cpu][counter][0] = -1;
+		close(FD(counter, cpu, 0));
+		FD(counter, cpu, 0) = -1;
 
 		if (scale) {
 			if (count[2] == 0) {
@@ -324,9 +343,9 @@ static void read_counter(int counter)
 				((double)count[0] * count[1] / count[2] + 0.5);
 			}
 		}
-		cpu_counts[cpu][counter].val = count[0]; /* scaled count */
-		cpu_counts[cpu][counter].ena = count[1];
-		cpu_counts[cpu][counter].run = count[2];
+		cpu_counts[cpu].val = count[0]; /* scaled count */
+		cpu_counts[cpu].ena = count[1];
+		cpu_counts[cpu].run = count[2];
 
 		if (MATCH_EVENT(SOFTWARE, SW_TASK_CLOCK, counter))
 			update_stats(&runtime_nsecs_stats[cpu], count[0]);
@@ -340,8 +359,9 @@ static void read_counter(int counter)
 static int run_perf_stat(int argc __used, const char **argv)
 {
 	unsigned long long t0, t1;
+	struct perf_evsel *counter;
 	int status = 0;
-	int counter, ncreated = 0;
+	int ncreated = 0;
 	int child_ready_pipe[2], go_pipe[2];
 	bool perm_err = false;
 	const bool forks = (argc > 0);
@@ -401,7 +421,7 @@ static int run_perf_stat(int argc __used, const char **argv)
 		close(child_ready_pipe[0]);
 	}
 
-	for (counter = 0; counter < nr_counters; counter++)
+	list_for_each_entry(counter, &evsel_list, node)
 		ncreated += create_perf_stat_counter(counter, &perm_err);
 
 	if (ncreated < nr_counters) {
@@ -433,25 +453,28 @@ static int run_perf_stat(int argc __used, const char **argv)
 	update_stats(&walltime_nsecs_stats, t1 - t0);
 
 	if (no_aggr) {
-		for (counter = 0; counter < nr_counters; counter++)
+		list_for_each_entry(counter, &evsel_list, node)
 			read_counter(counter);
 	} else {
-		for (counter = 0; counter < nr_counters; counter++)
+		list_for_each_entry(counter, &evsel_list, node)
 			read_counter_aggr(counter);
 	}
 	return WEXITSTATUS(status);
 }
 
-static void print_noise(int counter, double avg)
+static void print_noise(struct perf_evsel *evsel, double avg)
 {
+	struct perf_stat *ps;
+
 	if (run_count == 1)
 		return;
 
+	ps = evsel->priv;
 	fprintf(stderr, "   ( +- %7.3f%% )",
-			100 * stddev_stats(&event_res_stats[counter][0]) / avg);
+			100 * stddev_stats(&ps->res_stats[0]) / avg);
 }
 
-static void nsec_printout(int cpu, int counter, double avg)
+static void nsec_printout(int cpu, struct perf_evsel *counter, double avg)
 {
 	double msecs = avg / 1e6;
 	char cpustr[16] = { '\0', };
@@ -473,7 +496,7 @@ static void nsec_printout(int cpu, int counter, double avg)
 	}
 }
 
-static void abs_printout(int cpu, int counter, double avg)
+static void abs_printout(int cpu, struct perf_evsel *counter, double avg)
 {
 	double total, ratio = 0.0;
 	char cpustr[16] = { '\0', };
@@ -528,10 +551,11 @@ static void abs_printout(int cpu, int counter, double avg)
  * Print out the results of a single counter:
  * aggregated counts in system-wide mode
  */
-static void print_counter_aggr(int counter)
+static void print_counter_aggr(struct perf_evsel *counter)
 {
-	double avg = avg_stats(&event_res_stats[counter][0]);
-	int scaled = event_scaled[counter];
+	struct perf_stat *ps = counter->priv;
+	double avg = avg_stats(&ps->res_stats[0]);
+	int scaled = ps->scaled;
 
 	if (scaled == -1) {
 		fprintf(stderr, "%*s%s%-24s\n",
@@ -555,8 +579,8 @@ static void print_counter_aggr(int counter)
 	if (scaled) {
 		double avg_enabled, avg_running;
 
-		avg_enabled = avg_stats(&event_res_stats[counter][1]);
-		avg_running = avg_stats(&event_res_stats[counter][2]);
+		avg_enabled = avg_stats(&ps->res_stats[1]);
+		avg_running = avg_stats(&ps->res_stats[2]);
 
 		fprintf(stderr, "  (scaled from %.2f%%)",
 				100 * avg_running / avg_enabled);
@@ -569,15 +593,16 @@ static void print_counter_aggr(int counter)
  * Print out the results of a single counter:
  * does not use aggregated count in system-wide
  */
-static void print_counter(int counter)
+static void print_counter(struct perf_evsel *counter)
 {
+	struct perf_stat *ps = counter->priv;
 	u64 ena, run, val;
 	int cpu;
 
 	for (cpu = 0; cpu < nr_cpus; cpu++) {
-		val = cpu_counts[cpu][counter].val;
-		ena = cpu_counts[cpu][counter].ena;
-		run = cpu_counts[cpu][counter].run;
+		val = ps->cpu_counts[cpu].val;
+		ena = ps->cpu_counts[cpu].ena;
+		run = ps->cpu_counts[cpu].run;
 		if (run == 0 || ena == 0) {
 			fprintf(stderr, "CPU%*d%s%*s%s%-24s",
 				csv_output ? 0 : -4,
@@ -609,7 +634,8 @@ static void print_counter(int counter)
 
 static void print_stat(int argc, const char **argv)
 {
-	int i, counter;
+	struct perf_evsel *counter;
+	int i;
 
 	fflush(stdout);
 
@@ -632,10 +658,10 @@ static void print_stat(int argc, const char **argv)
 	}
 
 	if (no_aggr) {
-		for (counter = 0; counter < nr_counters; counter++)
+		list_for_each_entry(counter, &evsel_list, node)
 			print_counter(counter);
 	} else {
-		for (counter = 0; counter < nr_counters; counter++)
+		list_for_each_entry(counter, &evsel_list, node)
 			print_counter_aggr(counter);
 	}
 
@@ -720,8 +746,8 @@ static const struct option options[] = {
 
 int cmd_stat(int argc, const char **argv, const char *prefix __used)
 {
-	int status;
-	int i,j;
+	struct perf_evsel *pos;
+	int status = -ENOMEM;
 
 	setlocale(LC_ALL, "");
 
@@ -757,8 +783,18 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 
 	/* Set attrs and nr_counters if no event is selected and !null_run */
 	if (!null_run && !nr_counters) {
-		memcpy(attrs, default_attrs, sizeof(default_attrs));
+		size_t c;
+
 		nr_counters = ARRAY_SIZE(default_attrs);
+
+		for (c = 0; c < ARRAY_SIZE(default_attrs); ++c) {
+			pos = perf_evsel__new(default_attrs[c].type,
+					      default_attrs[c].config,
+					      nr_counters);
+			if (pos == NULL)
+				goto out;
+			list_add(&pos->node, &evsel_list);
+		}
 	}
 
 	if (system_wide)
@@ -786,12 +822,10 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 		thread_num = 1;
 	}
 
-	for (i = 0; i < MAX_NR_CPUS; i++) {
-		for (j = 0; j < MAX_COUNTERS; j++) {
-			fd[i][j] = malloc(sizeof(int)*thread_num);
-			if (!fd[i][j])
-				return -ENOMEM;
-		}
+	list_for_each_entry(pos, &evsel_list, node) {
+		if (perf_evsel__alloc_stat_priv(pos, nr_cpus) < 0 ||
+		    perf_evsel__alloc_fd(pos, nr_cpus, thread_num) < 0)
+			goto out_free_fd;
 	}
 
 	/*
@@ -814,6 +848,11 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 
 	if (status != -1)
 		print_stat(argc, argv);
-
+out_free_fd:
+	list_for_each_entry(pos, &evsel_list, node) {
+		perf_evsel__free_fd(pos);
+		perf_evsel__free_stat_priv(pos);
+	}
+out:
 	return status;
 }
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index ae15f04..13a836e 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -21,6 +21,7 @@
 #include "perf.h"
 
 #include "util/color.h"
+#include "util/evsel.h"
 #include "util/session.h"
 #include "util/symbol.h"
 #include "util/thread.h"
@@ -29,6 +30,7 @@
 #include "util/parse-options.h"
 #include "util/parse-events.h"
 #include "util/cpumap.h"
+#include "util/xyarray.h"
 
 #include "util/debug.h"
 
@@ -55,7 +57,7 @@
 #include <linux/unistd.h>
 #include <linux/types.h>
 
-static int			*fd[MAX_NR_CPUS][MAX_COUNTERS];
+#define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
 
 static bool			system_wide			=  false;
 
@@ -100,6 +102,7 @@ struct sym_entry		*sym_filter_entry		=   NULL;
 struct sym_entry		*sym_filter_entry_sched		=   NULL;
 static int			sym_pcnt_filter			=      5;
 static int			sym_counter			=      0;
+static struct perf_evsel	*sym_evsel			=   NULL;
 static int			display_weighted		=     -1;
 static const char		*cpu_list;
 
@@ -353,7 +356,7 @@ static void show_details(struct sym_entry *syme)
 		return;
 
 	symbol = sym_entry__symbol(syme);
-	printf("Showing %s for %s\n", event_name(sym_counter), symbol->name);
+	printf("Showing %s for %s\n", event_name(sym_evsel), symbol->name);
 	printf("  Events  Pcnt (>=%d%%)\n", sym_pcnt_filter);
 
 	pthread_mutex_lock(&syme->src->lock);
@@ -460,7 +463,8 @@ static void rb_insert_active_sym(struct rb_root *tree, struct sym_entry *se)
 static void print_sym_table(void)
 {
 	int printed = 0, j;
-	int counter, snap = !display_weighted ? sym_counter : 0;
+	struct perf_evsel *counter;
+	int snap = !display_weighted ? sym_counter : 0;
 	float samples_per_sec = samples/delay_secs;
 	float ksamples_per_sec = kernel_samples/delay_secs;
 	float us_samples_per_sec = (us_samples)/delay_secs;
@@ -532,7 +536,9 @@ static void print_sym_table(void)
 	}
 
 	if (nr_counters == 1 || !display_weighted) {
-		printf("%Ld", (u64)attrs[0].sample_period);
+		struct perf_evsel *first;
+		first = list_entry(evsel_list.next, struct perf_evsel, node);
+		printf("%Ld", first->attr.sample_period);
 		if (freq)
 			printf("Hz ");
 		else
@@ -540,9 +546,9 @@ static void print_sym_table(void)
 	}
 
 	if (!display_weighted)
-		printf("%s", event_name(sym_counter));
-	else for (counter = 0; counter < nr_counters; counter++) {
-		if (counter)
+		printf("%s", event_name(sym_evsel));
+	else list_for_each_entry(counter, &evsel_list, node) {
+		if (counter->idx)
 			printf("/");
 
 		printf("%s", event_name(counter));
@@ -739,7 +745,7 @@ static void print_mapped_keys(void)
 	fprintf(stdout, "\t[e]     display entries (lines).           \t(%d)\n", print_entries);
 
 	if (nr_counters > 1)
-		fprintf(stdout, "\t[E]     active event counter.              \t(%s)\n", event_name(sym_counter));
+		fprintf(stdout, "\t[E]     active event counter.              \t(%s)\n", event_name(sym_evsel));
 
 	fprintf(stdout, "\t[f]     profile display filter (count).    \t(%d)\n", count_filter);
 
@@ -826,19 +832,23 @@ static void handle_keypress(struct perf_session *session, int c)
 			break;
 		case 'E':
 			if (nr_counters > 1) {
-				int i;
-
 				fprintf(stderr, "\nAvailable events:");
-				for (i = 0; i < nr_counters; i++)
-					fprintf(stderr, "\n\t%d %s", i, event_name(i));
+
+				list_for_each_entry(sym_evsel, &evsel_list, node)
+					fprintf(stderr, "\n\t%d %s", sym_evsel->idx, event_name(sym_evsel));
 
 				prompt_integer(&sym_counter, "Enter details event counter");
 
 				if (sym_counter >= nr_counters) {
-					fprintf(stderr, "Sorry, no such event, using %s.\n", event_name(0));
+					sym_evsel = list_entry(evsel_list.next, struct perf_evsel, node);
 					sym_counter = 0;
+					fprintf(stderr, "Sorry, no such event, using %s.\n", event_name(sym_evsel));
 					sleep(1);
+					break;
 				}
+				list_for_each_entry(sym_evsel, &evsel_list, node)
+					if (sym_evsel->idx == sym_counter)
+						break;
 			} else sym_counter = 0;
 			break;
 		case 'f':
@@ -978,7 +988,8 @@ static int symbol_filter(struct map *map, struct symbol *sym)
 
 static void event__process_sample(const event_t *self,
 				  struct sample_data *sample,
-				  struct perf_session *session, int counter)
+				  struct perf_session *session,
+				  struct perf_evsel *evsel)
 {
 	u64 ip = self->ip.ip;
 	struct sym_entry *syme;
@@ -1071,9 +1082,9 @@ static void event__process_sample(const event_t *self,
 
 	syme = symbol__priv(al.sym);
 	if (!syme->skip) {
-		syme->count[counter]++;
+		syme->count[evsel->idx]++;
 		syme->origin = origin;
-		record_precise_ip(syme, counter, ip);
+		record_precise_ip(syme, evsel->idx, ip);
 		pthread_mutex_lock(&active_symbols_lock);
 		if (list_empty(&syme->node) || !syme->node.next)
 			__list_insert_active_sym(syme);
@@ -1082,12 +1093,24 @@ static void event__process_sample(const event_t *self,
 }
 
 struct mmap_data {
-	int			counter;
 	void			*base;
 	int			mask;
 	unsigned int		prev;
 };
 
+static int perf_evsel__alloc_mmap_per_thread(struct perf_evsel *evsel,
+					     int ncpus, int nthreads)
+{
+	evsel->priv = xyarray__new(ncpus, nthreads, sizeof(struct mmap_data));
+	return evsel->priv != NULL ? 0 : -ENOMEM;
+}
+
+static void perf_evsel__free_mmap(struct perf_evsel *evsel)
+{
+	xyarray__delete(evsel->priv);
+	evsel->priv = NULL;
+}
+
 static unsigned int mmap_read_head(struct mmap_data *md)
 {
 	struct perf_event_mmap_page *pc = md->base;
@@ -1100,8 +1123,11 @@ static unsigned int mmap_read_head(struct mmap_data *md)
 }
 
 static void perf_session__mmap_read_counter(struct perf_session *self,
-					    struct mmap_data *md)
+					    struct perf_evsel *evsel,
+					    int cpu, int thread_idx)
 {
+	struct xyarray *mmap_array = evsel->priv;
+	struct mmap_data *md = xyarray__entry(mmap_array, cpu, thread_idx);
 	unsigned int head = mmap_read_head(md);
 	unsigned int old = md->prev;
 	unsigned char *data = md->base + page_size;
@@ -1155,7 +1181,7 @@ static void perf_session__mmap_read_counter(struct perf_session *self,
 
 		event__parse_sample(event, self, &sample);
 		if (event->header.type == PERF_RECORD_SAMPLE)
-			event__process_sample(event, &sample, self, md->counter);
+			event__process_sample(event, &sample, self, evsel);
 		else
 			event__process(event, &sample, self);
 		old += size;
@@ -1165,28 +1191,31 @@ static void perf_session__mmap_read_counter(struct perf_session *self,
 }
 
 static struct pollfd *event_array;
-static struct mmap_data *mmap_array[MAX_NR_CPUS][MAX_COUNTERS];
 
 static void perf_session__mmap_read(struct perf_session *self)
 {
-	int i, counter, thread_index;
+	struct perf_evsel *counter;
+	int i, thread_index;
 
 	for (i = 0; i < nr_cpus; i++) {
-		for (counter = 0; counter < nr_counters; counter++)
+		list_for_each_entry(counter, &evsel_list, node) {
 			for (thread_index = 0;
 				thread_index < thread_num;
 				thread_index++) {
 				perf_session__mmap_read_counter(self,
-					&mmap_array[i][counter][thread_index]);
+					counter, i, thread_index);
 			}
+		}
 	}
 }
 
 int nr_poll;
 int group_fd;
 
-static void start_counter(int i, int counter)
+static void start_counter(int i, struct perf_evsel *evsel)
 {
+	struct xyarray *mmap_array = evsel->priv;
+	struct mmap_data *mm;
 	struct perf_event_attr *attr;
 	int cpu = -1;
 	int thread_index;
@@ -1194,7 +1223,7 @@ static void start_counter(int i, int counter)
 	if (target_tid == -1)
 		cpu = cpumap[i];
 
-	attr = attrs + counter;
+	attr = &evsel->attr;
 
 	attr->sample_type	= PERF_SAMPLE_IP | PERF_SAMPLE_TID;
 
@@ -1209,10 +1238,10 @@ static void start_counter(int i, int counter)
 
 	for (thread_index = 0; thread_index < thread_num; thread_index++) {
 try_again:
-		fd[i][counter][thread_index] = sys_perf_event_open(attr,
+		FD(evsel, i, thread_index) = sys_perf_event_open(attr,
 				all_tids[thread_index], cpu, group_fd, 0);
 
-		if (fd[i][counter][thread_index] < 0) {
+		if (FD(evsel, i, thread_index) < 0) {
 			int err = errno;
 
 			if (err == EPERM || err == EACCES)
@@ -1236,29 +1265,29 @@ try_again:
 			}
 			printf("\n");
 			error("sys_perf_event_open() syscall returned with %d (%s).  /bin/dmesg may provide additional information.\n",
-					fd[i][counter][thread_index], strerror(err));
+					FD(evsel, i, thread_index), strerror(err));
 			die("No CONFIG_PERF_EVENTS=y kernel support configured?\n");
 			exit(-1);
 		}
-		assert(fd[i][counter][thread_index] >= 0);
-		fcntl(fd[i][counter][thread_index], F_SETFL, O_NONBLOCK);
+		assert(FD(evsel, i, thread_index) >= 0);
+		fcntl(FD(evsel, i, thread_index), F_SETFL, O_NONBLOCK);
 
 		/*
 		 * First counter acts as the group leader:
 		 */
 		if (group && group_fd == -1)
-			group_fd = fd[i][counter][thread_index];
+			group_fd = FD(evsel, i, thread_index);
 
-		event_array[nr_poll].fd = fd[i][counter][thread_index];
+		event_array[nr_poll].fd = FD(evsel, i, thread_index);
 		event_array[nr_poll].events = POLLIN;
 		nr_poll++;
 
-		mmap_array[i][counter][thread_index].counter = counter;
-		mmap_array[i][counter][thread_index].prev = 0;
-		mmap_array[i][counter][thread_index].mask = mmap_pages*page_size - 1;
-		mmap_array[i][counter][thread_index].base = mmap(NULL, (mmap_pages+1)*page_size,
-				PROT_READ, MAP_SHARED, fd[i][counter][thread_index], 0);
-		if (mmap_array[i][counter][thread_index].base == MAP_FAILED)
+		mm = xyarray__entry(mmap_array, i, thread_index);
+		mm->prev = 0;
+		mm->mask = mmap_pages*page_size - 1;
+		mm->base = mmap(NULL, (mmap_pages+1)*page_size,
+				PROT_READ, MAP_SHARED, FD(evsel, i, thread_index), 0);
+		if (mm->base == MAP_FAILED)
 			die("failed to mmap with %d (%s)\n", errno, strerror(errno));
 	}
 }
@@ -1266,8 +1295,8 @@ try_again:
 static int __cmd_top(void)
 {
 	pthread_t thread;
-	int i, counter;
-	int ret;
+	struct perf_evsel *counter;
+	int i, ret;
 	/*
 	 * FIXME: perf_session__new should allow passing a O_MMAP, so that all this
 	 * mmap reading, etc is encapsulated in it. Use O_WRONLY for now.
@@ -1283,7 +1312,7 @@ static int __cmd_top(void)
 
 	for (i = 0; i < nr_cpus; i++) {
 		group_fd = -1;
-		for (counter = 0; counter < nr_counters; counter++)
+		list_for_each_entry(counter, &evsel_list, node)
 			start_counter(i, counter);
 	}
 
@@ -1372,8 +1401,8 @@ static const struct option options[] = {
 
 int cmd_top(int argc, const char **argv, const char *prefix __used)
 {
-	int counter;
-	int i,j;
+	struct perf_evsel *pos;
+	int status = -ENOMEM;
 
 	page_size = sysconf(_SC_PAGE_SIZE);
 
@@ -1398,15 +1427,6 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 		thread_num = 1;
 	}
 
-	for (i = 0; i < MAX_NR_CPUS; i++) {
-		for (j = 0; j < MAX_COUNTERS; j++) {
-			fd[i][j] = malloc(sizeof(int)*thread_num);
-			mmap_array[i][j] = zalloc(
-				sizeof(struct mmap_data)*thread_num);
-			if (!fd[i][j] || !mmap_array[i][j])
-				return -ENOMEM;
-		}
-	}
 	event_array = malloc(
 		sizeof(struct pollfd)*MAX_NR_CPUS*MAX_COUNTERS*thread_num);
 	if (!event_array)
@@ -1419,15 +1439,10 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 		cpu_list = NULL;
 	}
 
-	if (!nr_counters)
-		nr_counters = 1;
-
-	symbol_conf.priv_size = (sizeof(struct sym_entry) +
-				 (nr_counters + 1) * sizeof(unsigned long));
-
-	symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
-	if (symbol__init() < 0)
-		return -1;
+	if (!nr_counters && perf_evsel_list__create_default() < 0) {
+		pr_err("Not enough memory for event selector list\n");
+		return -ENOMEM;
+	}
 
 	if (delay_secs < 1)
 		delay_secs = 1;
@@ -1444,16 +1459,6 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 		exit(EXIT_FAILURE);
 	}
 
-	/*
-	 * Fill in the ones not specifically initialized via -c:
-	 */
-	for (counter = 0; counter < nr_counters; counter++) {
-		if (attrs[counter].sample_period)
-			continue;
-
-		attrs[counter].sample_period = default_interval;
-	}
-
 	if (target_tid != -1)
 		nr_cpus = 1;
 	else
@@ -1462,11 +1467,38 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 	if (nr_cpus < 1)
 		usage_with_options(top_usage, options);
 
+	list_for_each_entry(pos, &evsel_list, node) {
+		if (perf_evsel__alloc_mmap_per_thread(pos, nr_cpus, thread_num) < 0 ||
+		    perf_evsel__alloc_fd(pos, nr_cpus, thread_num) < 0)
+			goto out_free_fd;
+		/*
+		 * Fill in the ones not specifically initialized via -c:
+		 */
+		if (pos->attr.sample_period)
+			continue;
+
+		pos->attr.sample_period = default_interval;
+	}
+
+	symbol_conf.priv_size = (sizeof(struct sym_entry) +
+				 (nr_counters + 1) * sizeof(unsigned long));
+
+	symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
+	if (symbol__init() < 0)
+		return -1;
+
 	get_term_dimensions(&winsize);
 	if (print_entries == 0) {
 		update_print_entries(&winsize);
 		signal(SIGWINCH, sig_winch_handler);
 	}
 
-	return __cmd_top();
+	status = __cmd_top();
+out_free_fd:
+	list_for_each_entry(pos, &evsel_list, node) {
+		perf_evsel__free_fd(pos);
+		perf_evsel__free_mmap(pos);
+	}
+
+	return status;
 }
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
new file mode 100644
index 0000000..6539ec9
--- /dev/null
+++ b/tools/perf/util/evsel.c
@@ -0,0 +1,35 @@
+#include "evsel.h"
+#include "util.h"
+
+struct perf_evsel *perf_evsel__new(u32 type, u64 config, int idx)
+{
+	struct perf_evsel *evsel = zalloc(sizeof(*evsel));
+
+	if (evsel != NULL) {
+		evsel->idx	   = idx;
+		evsel->attr.type   = type;
+		evsel->attr.config = config;
+		INIT_LIST_HEAD(&evsel->node);
+	}
+
+	return evsel;
+}
+
+int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
+{
+	evsel->fd = xyarray__new(ncpus, nthreads, sizeof(int));
+	return evsel->fd != NULL ? 0 : -ENOMEM;
+}
+
+void perf_evsel__free_fd(struct perf_evsel *evsel)
+{
+	xyarray__delete(evsel->fd);
+	evsel->fd = NULL;
+}
+
+void perf_evsel__delete(struct perf_evsel *evsel)
+{
+	assert(list_empty(&evsel->node));
+	xyarray__delete(evsel->fd);
+	free(evsel);
+}
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
new file mode 100644
index 0000000..3eb3989
--- /dev/null
+++ b/tools/perf/util/evsel.h
@@ -0,0 +1,24 @@
+#ifndef __PERF_EVSEL_H
+#define __PERF_EVSEL_H 1
+
+#include <linux/list.h>
+#include <linux/perf_event.h>
+#include "types.h"
+#include "xyarray.h"
+
+struct perf_evsel {
+	struct list_head	node;
+	struct perf_event_attr	attr;
+	char			*filter;
+	struct xyarray		*fd;
+	int			idx;
+	void			*priv;
+};
+
+struct perf_evsel *perf_evsel__new(u32 type, u64 config, int idx);
+void perf_evsel__delete(struct perf_evsel *evsel);
+
+int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
+void perf_evsel__free_fd(struct perf_evsel *evsel);
+
+#endif /* __PERF_EVSEL_H */
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 16a1602..ecb5a84 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -461,7 +461,7 @@ static int perf_header__adds_write(struct perf_header *self, int fd)
 
 		/* Write trace info */
 		trace_sec->offset = lseek(fd, 0, SEEK_CUR);
-		read_tracing_data(fd, attrs, nr_counters);
+		read_tracing_data(fd, &evsel_list);
 		trace_sec->size = lseek(fd, 0, SEEK_CUR) - trace_sec->offset;
 	}
 
@@ -1131,8 +1131,7 @@ int event__process_event_type(event_t *self,
 	return 0;
 }
 
-int event__synthesize_tracing_data(int fd, struct perf_event_attr *pattrs,
-				   int nb_events,
+int event__synthesize_tracing_data(int fd, struct list_head *pattrs,
 				   event__handler_t process,
 				   struct perf_session *session __unused)
 {
@@ -1143,7 +1142,7 @@ int event__synthesize_tracing_data(int fd, struct perf_event_attr *pattrs,
 	memset(&ev, 0, sizeof(ev));
 
 	ev.tracing_data.header.type = PERF_RECORD_HEADER_TRACING_DATA;
-	size = read_tracing_data_size(fd, pattrs, nb_events);
+	size = read_tracing_data_size(fd, pattrs);
 	if (size <= 0)
 		return size;
 	aligned_size = ALIGN(size, sizeof(u64));
@@ -1153,7 +1152,7 @@ int event__synthesize_tracing_data(int fd, struct perf_event_attr *pattrs,
 
 	process(&ev, NULL, session);
 
-	err = read_tracing_data(fd, pattrs, nb_events);
+	err = read_tracing_data(fd, pattrs);
 	write_padded(fd, NULL, 0, padding);
 
 	return aligned_size;
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 6335965..33f16be 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -113,8 +113,7 @@ int event__synthesize_event_types(event__handler_t process,
 int event__process_event_type(event_t *self,
 			      struct perf_session *session);
 
-int event__synthesize_tracing_data(int fd, struct perf_event_attr *pattrs,
-				   int nb_events,
+int event__synthesize_tracing_data(int fd, struct list_head *pattrs,
 				   event__handler_t process,
 				   struct perf_session *session);
 int event__process_tracing_data(event_t *self,
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index c305305..2d948ad 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1,6 +1,7 @@
 #include "../../../include/linux/hw_breakpoint.h"
 #include "util.h"
 #include "../perf.h"
+#include "evsel.h"
 #include "parse-options.h"
 #include "parse-events.h"
 #include "exec_cmd.h"
@@ -12,8 +13,7 @@
 
 int				nr_counters;
 
-struct perf_event_attr		attrs[MAX_COUNTERS];
-char				*filters[MAX_COUNTERS];
+LIST_HEAD(evsel_list);
 
 struct event_symbol {
 	u8		type;
@@ -266,10 +266,10 @@ static char *event_cache_name(u8 cache_type, u8 cache_op, u8 cache_result)
 	return name;
 }
 
-const char *event_name(int counter)
+const char *event_name(struct perf_evsel *evsel)
 {
-	u64 config = attrs[counter].config;
-	int type = attrs[counter].type;
+	u64 config = evsel->attr.config;
+	int type = evsel->attr.type;
 
 	return __event_name(type, config);
 }
@@ -814,9 +814,6 @@ int parse_events(const struct option *opt __used, const char *str, int unset __u
 			return -1;
 
 	for (;;) {
-		if (nr_counters == MAX_COUNTERS)
-			return -1;
-
 		memset(&attr, 0, sizeof(attr));
 		ret = parse_event_symbols(&str, &attr);
 		if (ret == EVT_FAILED)
@@ -826,8 +823,13 @@ int parse_events(const struct option *opt __used, const char *str, int unset __u
 			return -1;
 
 		if (ret != EVT_HANDLED_ALL) {
-			attrs[nr_counters] = attr;
-			nr_counters++;
+			struct perf_evsel *evsel;
+			evsel = perf_evsel__new(attr.type, attr.config,
+						nr_counters);
+			if (evsel == NULL)
+				return -1;
+			list_add_tail(&evsel->node, &evsel_list);
+			++nr_counters;
 		}
 
 		if (*str == 0)
@@ -844,21 +846,22 @@ int parse_events(const struct option *opt __used, const char *str, int unset __u
 int parse_filter(const struct option *opt __used, const char *str,
 		 int unset __used)
 {
-	int i = nr_counters - 1;
-	int len = strlen(str);
+	struct perf_evsel *last = NULL;
 
-	if (i < 0 || attrs[i].type != PERF_TYPE_TRACEPOINT) {
+	if (!list_empty(&evsel_list))
+		last = list_entry(evsel_list.prev, struct perf_evsel, node);
+
+	if (last == NULL || last->attr.type != PERF_TYPE_TRACEPOINT) {
 		fprintf(stderr,
 			"-F option should follow a -e tracepoint option\n");
 		return -1;
 	}
 
-	filters[i] = malloc(len + 1);
-	if (!filters[i]) {
+	last->filter = strdup(str);
+	if (last->filter == NULL) {
 		fprintf(stderr, "not enough memory to hold filter string\n");
 		return -1;
 	}
-	strcpy(filters[i], str);
 
 	return 0;
 }
@@ -967,3 +970,15 @@ void print_events(void)
 
 	exit(129);
 }
+
+int perf_evsel_list__create_default(void)
+{
+	struct perf_evsel *evsel = perf_evsel__new(PERF_TYPE_HARDWARE,
+						   PERF_COUNT_HW_CPU_CYCLES, 0);
+	if (evsel == NULL)
+		return -ENOMEM;
+
+	list_add(&evsel->node, &evsel_list);
+	++nr_counters;
+	return 0;
+}
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index fc4ab3f..0f915a0 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -4,6 +4,15 @@
  * Parse symbolic events/counts passed in as options:
  */
 
+#include <linux/perf_event.h>
+
+struct list_head;
+struct perf_evsel;
+
+extern struct list_head evsel_list;
+
+int perf_evsel_list__create_default(void);
+
 struct option;
 
 struct tracepoint_path {
@@ -13,14 +22,11 @@ struct tracepoint_path {
 };
 
 extern struct tracepoint_path *tracepoint_id_to_path(u64 config);
-extern bool have_tracepoints(struct perf_event_attr *pattrs, int nb_events);
+extern bool have_tracepoints(struct list_head *evsel_list);
 
 extern int			nr_counters;
 
-extern struct perf_event_attr attrs[MAX_COUNTERS];
-extern char *filters[MAX_COUNTERS];
-
-extern const char *event_name(int ctr);
+const char *event_name(struct perf_evsel *event);
 extern const char *__event_name(int type, u64 config);
 
 extern int parse_events(const struct option *opt, const char *str, int unset);
@@ -33,5 +39,4 @@ extern void print_events(void);
 extern char debugfs_path[];
 extern int valid_debugfs_mount(const char *debugfs);
 
-
 #endif /* __PERF_PARSE_EVENTS_H */
diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c
index b157260..35729f4 100644
--- a/tools/perf/util/trace-event-info.c
+++ b/tools/perf/util/trace-event-info.c
@@ -34,11 +34,13 @@
 #include <ctype.h>
 #include <errno.h>
 #include <stdbool.h>
+#include <linux/list.h>
 #include <linux/kernel.h>
 
 #include "../perf.h"
 #include "trace-event.h"
 #include "debugfs.h"
+#include "evsel.h"
 
 #define VERSION "0.5"
 
@@ -469,16 +471,17 @@ out:
 }
 
 static struct tracepoint_path *
-get_tracepoints_path(struct perf_event_attr *pattrs, int nb_events)
+get_tracepoints_path(struct list_head *pattrs)
 {
 	struct tracepoint_path path, *ppath = &path;
-	int i, nr_tracepoints = 0;
+	struct perf_evsel *pos;
+	int nr_tracepoints = 0;
 
-	for (i = 0; i < nb_events; i++) {
-		if (pattrs[i].type != PERF_TYPE_TRACEPOINT)
+	list_for_each_entry(pos, pattrs, node) {
+		if (pos->attr.type != PERF_TYPE_TRACEPOINT)
 			continue;
 		++nr_tracepoints;
-		ppath->next = tracepoint_id_to_path(pattrs[i].config);
+		ppath->next = tracepoint_id_to_path(pos->attr.config);
 		if (!ppath->next)
 			die("%s\n", "No memory to alloc tracepoints list");
 		ppath = ppath->next;
@@ -487,21 +490,21 @@ get_tracepoints_path(struct perf_event_attr *pattrs, int nb_events)
 	return nr_tracepoints > 0 ? path.next : NULL;
 }
 
-bool have_tracepoints(struct perf_event_attr *pattrs, int nb_events)
+bool have_tracepoints(struct list_head *pattrs)
 {
-	int i;
+	struct perf_evsel *pos;
 
-	for (i = 0; i < nb_events; i++)
-		if (pattrs[i].type == PERF_TYPE_TRACEPOINT)
+	list_for_each_entry(pos, pattrs, node)
+		if (pos->attr.type == PERF_TYPE_TRACEPOINT)
 			return true;
 
 	return false;
 }
 
-int read_tracing_data(int fd, struct perf_event_attr *pattrs, int nb_events)
+int read_tracing_data(int fd, struct list_head *pattrs)
 {
 	char buf[BUFSIZ];
-	struct tracepoint_path *tps = get_tracepoints_path(pattrs, nb_events);
+	struct tracepoint_path *tps = get_tracepoints_path(pattrs);
 
 	/*
 	 * What? No tracepoints? No sense writing anything here, bail out.
@@ -545,14 +548,13 @@ int read_tracing_data(int fd, struct perf_event_attr *pattrs, int nb_events)
 	return 0;
 }
 
-ssize_t read_tracing_data_size(int fd, struct perf_event_attr *pattrs,
-			       int nb_events)
+ssize_t read_tracing_data_size(int fd, struct list_head *pattrs)
 {
 	ssize_t size;
 	int err = 0;
 
 	calc_data_size = 1;
-	err = read_tracing_data(fd, pattrs, nb_events);
+	err = read_tracing_data(fd, pattrs);
 	size = calc_data_size - 1;
 	calc_data_size = 0;
 
diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
index b3e86b1..b5f12ca 100644
--- a/tools/perf/util/trace-event.h
+++ b/tools/perf/util/trace-event.h
@@ -262,9 +262,8 @@ raw_field_value(struct event *event, const char *name, void *data);
 void *raw_field_ptr(struct event *event, const char *name, void *data);
 unsigned long long eval_flag(const char *flag);
 
-int read_tracing_data(int fd, struct perf_event_attr *pattrs, int nb_events);
-ssize_t read_tracing_data_size(int fd, struct perf_event_attr *pattrs,
-			       int nb_events);
+int read_tracing_data(int fd, struct list_head *pattrs);
+ssize_t read_tracing_data_size(int fd, struct list_head *pattrs);
 
 /* taken from kernel/trace/trace.h */
 enum trace_flag_type {
diff --git a/tools/perf/util/xyarray.c b/tools/perf/util/xyarray.c
new file mode 100644
index 0000000..22afbf6
--- /dev/null
+++ b/tools/perf/util/xyarray.c
@@ -0,0 +1,20 @@
+#include "xyarray.h"
+#include "util.h"
+
+struct xyarray *xyarray__new(int xlen, int ylen, size_t entry_size)
+{
+	size_t row_size = ylen * entry_size;
+	struct xyarray *xy = zalloc(sizeof(*xy) + xlen * row_size);
+
+	if (xy != NULL) {
+		xy->entry_size = entry_size;
+		xy->row_size   = row_size;
+	}
+
+	return xy;
+}
+
+void xyarray__delete(struct xyarray *xy)
+{
+	free(xy);
+}
diff --git a/tools/perf/util/xyarray.h b/tools/perf/util/xyarray.h
new file mode 100644
index 0000000..c488a07
--- /dev/null
+++ b/tools/perf/util/xyarray.h
@@ -0,0 +1,20 @@
+#ifndef _PERF_XYARRAY_H_
+#define _PERF_XYARRAY_H_ 1
+
+#include <sys/types.h>
+
+struct xyarray {
+	size_t row_size;
+	size_t entry_size;
+	char contents[];
+};
+
+struct xyarray *xyarray__new(int xlen, int ylen, size_t entry_size);
+void xyarray__delete(struct xyarray *xy);
+
+static inline void *xyarray__entry(struct xyarray *xy, int x, int y)
+{
+	return &xy->contents[x * xy->row_size + y * xy->entry_size];
+}
+
+#endif /* _PERF_XYARRAY_H_ */
-- 
1.6.2.5


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

* [PATCH 02/11] perf evsel: Adopt MATCH_EVENT macro from 'stat'
  2011-01-04  3:48 [GIT PULL||RFC 00/11] perf library and regression testing improvements Arnaldo Carvalho de Melo
  2011-01-04  3:48 ` [PATCH 01/11] perf tools: Introduce event selectors Arnaldo Carvalho de Melo
@ 2011-01-04  3:48 ` Arnaldo Carvalho de Melo
  2011-01-04  3:48 ` [PATCH 03/11] perf util: Move do_read from session to util Arnaldo Carvalho de Melo
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04  3:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Tom Zanussi, Thomas Gleixner

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c |   37 ++++++++++++++++---------------------
 tools/perf/util/evsel.h   |    4 ++++
 2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 511ebaf..3e5f356 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -170,10 +170,6 @@ struct stats			runtime_cycles_stats[MAX_NR_CPUS];
 struct stats			runtime_branches_stats[MAX_NR_CPUS];
 struct stats			walltime_nsecs_stats;
 
-#define MATCH_EVENT(t, c, evsel)			\
-	(evsel->attr.type == PERF_TYPE_##t &&	\
-	 evsel->attr.config == PERF_COUNT_##c)
-
 #define ERR_PERF_OPEN \
 "counter %d, sys_perf_event_open() syscall returned with %d (%s).  /bin/dmesg may provide additional information."
 
@@ -229,10 +225,10 @@ static int create_perf_stat_counter(struct perf_evsel *evsel, bool *perm_err)
 /*
  * Does the counter have nsecs as a unit?
  */
-static inline int nsec_counter(struct perf_evsel *counter)
+static inline int nsec_counter(struct perf_evsel *evsel)
 {
-	if (MATCH_EVENT(SOFTWARE, SW_CPU_CLOCK, counter) ||
-	    MATCH_EVENT(SOFTWARE, SW_TASK_CLOCK, counter))
+	if (perf_evsel__match(evsel, SOFTWARE, SW_CPU_CLOCK) ||
+	    perf_evsel__match(evsel, SOFTWARE, SW_TASK_CLOCK))
 		return 1;
 
 	return 0;
@@ -300,11 +296,11 @@ static void read_counter_aggr(struct perf_evsel *counter)
 	/*
 	 * Save the full runtime - to allow normalization during printout:
 	 */
-	if (MATCH_EVENT(SOFTWARE, SW_TASK_CLOCK, counter))
+	if (perf_evsel__match(counter, SOFTWARE, SW_TASK_CLOCK))
 		update_stats(&runtime_nsecs_stats[0], count[0]);
-	if (MATCH_EVENT(HARDWARE, HW_CPU_CYCLES, counter))
+	if (perf_evsel__match(counter, HARDWARE, HW_CPU_CYCLES))
 		update_stats(&runtime_cycles_stats[0], count[0]);
-	if (MATCH_EVENT(HARDWARE, HW_BRANCH_INSTRUCTIONS, counter))
+	if (perf_evsel__match(counter, HARDWARE, HW_BRANCH_INSTRUCTIONS))
 		update_stats(&runtime_branches_stats[0], count[0]);
 }
 
@@ -347,11 +343,11 @@ static void read_counter(struct perf_evsel *counter)
 		cpu_counts[cpu].ena = count[1];
 		cpu_counts[cpu].run = count[2];
 
-		if (MATCH_EVENT(SOFTWARE, SW_TASK_CLOCK, counter))
+		if (perf_evsel__match(counter, SOFTWARE, SW_TASK_CLOCK))
 			update_stats(&runtime_nsecs_stats[cpu], count[0]);
-		if (MATCH_EVENT(HARDWARE, HW_CPU_CYCLES, counter))
+		if (perf_evsel__match(counter, HARDWARE, HW_CPU_CYCLES))
 			update_stats(&runtime_cycles_stats[cpu], count[0]);
-		if (MATCH_EVENT(HARDWARE, HW_BRANCH_INSTRUCTIONS, counter))
+		if (perf_evsel__match(counter, HARDWARE, HW_BRANCH_INSTRUCTIONS))
 			update_stats(&runtime_branches_stats[cpu], count[0]);
 	}
 }
@@ -474,7 +470,7 @@ static void print_noise(struct perf_evsel *evsel, double avg)
 			100 * stddev_stats(&ps->res_stats[0]) / avg);
 }
 
-static void nsec_printout(int cpu, struct perf_evsel *counter, double avg)
+static void nsec_printout(int cpu, struct perf_evsel *evsel, double avg)
 {
 	double msecs = avg / 1e6;
 	char cpustr[16] = { '\0', };
@@ -485,18 +481,17 @@ static void nsec_printout(int cpu, struct perf_evsel *counter, double avg)
 			csv_output ? 0 : -4,
 			cpumap[cpu], csv_sep);
 
-	fprintf(stderr, fmt, cpustr, msecs, csv_sep, event_name(counter));
+	fprintf(stderr, fmt, cpustr, msecs, csv_sep, event_name(evsel));
 
 	if (csv_output)
 		return;
 
-	if (MATCH_EVENT(SOFTWARE, SW_TASK_CLOCK, counter)) {
+	if (perf_evsel__match(evsel, SOFTWARE, SW_TASK_CLOCK))
 		fprintf(stderr, " # %10.3f CPUs ",
 				avg / avg_stats(&walltime_nsecs_stats));
-	}
 }
 
-static void abs_printout(int cpu, struct perf_evsel *counter, double avg)
+static void abs_printout(int cpu, struct perf_evsel *evsel, double avg)
 {
 	double total, ratio = 0.0;
 	char cpustr[16] = { '\0', };
@@ -516,19 +511,19 @@ static void abs_printout(int cpu, struct perf_evsel *counter, double avg)
 	else
 		cpu = 0;
 
-	fprintf(stderr, fmt, cpustr, avg, csv_sep, event_name(counter));
+	fprintf(stderr, fmt, cpustr, avg, csv_sep, event_name(evsel));
 
 	if (csv_output)
 		return;
 
-	if (MATCH_EVENT(HARDWARE, HW_INSTRUCTIONS, counter)) {
+	if (perf_evsel__match(evsel, HARDWARE, HW_INSTRUCTIONS)) {
 		total = avg_stats(&runtime_cycles_stats[cpu]);
 
 		if (total)
 			ratio = avg / total;
 
 		fprintf(stderr, " # %10.3f IPC  ", ratio);
-	} else if (MATCH_EVENT(HARDWARE, HW_BRANCH_MISSES, counter) &&
+	} else if (perf_evsel__match(evsel, HARDWARE, HW_BRANCH_MISSES) &&
 			runtime_branches_stats[cpu].n != 0) {
 		total = avg_stats(&runtime_branches_stats[cpu]);
 
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 3eb3989..8a5cfb6 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -21,4 +21,8 @@ void perf_evsel__delete(struct perf_evsel *evsel);
 int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
 void perf_evsel__free_fd(struct perf_evsel *evsel);
 
+#define perf_evsel__match(evsel, t, c)		\
+	(evsel->attr.type == PERF_TYPE_##t &&	\
+	 evsel->attr.config == PERF_COUNT_##c)
+
 #endif /* __PERF_EVSEL_H */
-- 
1.6.2.5


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

* [PATCH 03/11] perf util: Move do_read from session to util
  2011-01-04  3:48 [GIT PULL||RFC 00/11] perf library and regression testing improvements Arnaldo Carvalho de Melo
  2011-01-04  3:48 ` [PATCH 01/11] perf tools: Introduce event selectors Arnaldo Carvalho de Melo
  2011-01-04  3:48 ` [PATCH 02/11] perf evsel: Adopt MATCH_EVENT macro from 'stat' Arnaldo Carvalho de Melo
@ 2011-01-04  3:48 ` Arnaldo Carvalho de Melo
  2011-01-04  3:48 ` [PATCH 04/11] perf evsel: Delete the event selectors at exit Arnaldo Carvalho de Melo
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04  3:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Tom Zanussi, Thomas Gleixner

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Not really something to be exported from session.c. Rename it to
'readn' as others did in the past.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c  |    6 +++---
 tools/perf/util/session.c |   22 ++--------------------
 tools/perf/util/session.h |    1 -
 tools/perf/util/util.c    |   17 +++++++++++++++++
 tools/perf/util/util.h    |    1 +
 5 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index ecb5a84..05dec98 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -604,7 +604,7 @@ int perf_header__write(struct perf_header *self, int fd, bool at_exit)
 static int perf_header__getbuffer64(struct perf_header *self,
 				    int fd, void *buf, size_t size)
 {
-	if (do_read(fd, buf, size) <= 0)
+	if (readn(fd, buf, size) <= 0)
 		return -1;
 
 	if (self->needs_swap)
@@ -660,7 +660,7 @@ int perf_file_header__read(struct perf_file_header *self,
 {
 	lseek(fd, 0, SEEK_SET);
 
-	if (do_read(fd, self, sizeof(*self)) <= 0 ||
+	if (readn(fd, self, sizeof(*self)) <= 0 ||
 	    memcmp(&self->magic, __perf_magic, sizeof(self->magic)))
 		return -1;
 
@@ -821,7 +821,7 @@ static int perf_file_header__read_pipe(struct perf_pipe_file_header *self,
 				       struct perf_header *ph, int fd,
 				       bool repipe)
 {
-	if (do_read(fd, self, sizeof(*self)) <= 0 ||
+	if (readn(fd, self, sizeof(*self)) <= 0 ||
 	    memcmp(&self->magic, __perf_magic, sizeof(self->magic)))
 		return -1;
 
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 0f7e544..b163dfd 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -838,23 +838,6 @@ static struct thread *perf_session__register_idle_thread(struct perf_session *se
 	return thread;
 }
 
-int do_read(int fd, void *buf, size_t size)
-{
-	void *buf_start = buf;
-
-	while (size) {
-		int ret = read(fd, buf, size);
-
-		if (ret <= 0)
-			return ret;
-
-		size -= ret;
-		buf += ret;
-	}
-
-	return buf - buf_start;
-}
-
 #define session_done()	(*(volatile int *)(&session_done))
 volatile int session_done;
 
@@ -872,7 +855,7 @@ static int __perf_session__process_pipe_events(struct perf_session *self,
 
 	head = 0;
 more:
-	err = do_read(self->fd, &event, sizeof(struct perf_event_header));
+	err = readn(self->fd, &event, sizeof(struct perf_event_header));
 	if (err <= 0) {
 		if (err == 0)
 			goto done;
@@ -892,8 +875,7 @@ more:
 	p += sizeof(struct perf_event_header);
 
 	if (size - sizeof(struct perf_event_header)) {
-		err = do_read(self->fd, p,
-			      size - sizeof(struct perf_event_header));
+		err = readn(self->fd, p, size - sizeof(struct perf_event_header));
 		if (err <= 0) {
 			if (err == 0) {
 				pr_err("unexpected end of event stream\n");
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index ffe4b98..decd83f 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -109,7 +109,6 @@ void mem_bswap_64(void *src, int byte_size);
 
 int perf_session__create_kernel_maps(struct perf_session *self);
 
-int do_read(int fd, void *buf, size_t size);
 void perf_session__update_sample_type(struct perf_session *self);
 void perf_session__set_sample_id_all(struct perf_session *session, bool value);
 void perf_session__set_sample_type(struct perf_session *session, u64 type);
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 2142656..5b3ea49 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -114,3 +114,20 @@ unsigned long convert_unit(unsigned long value, char *unit)
 
 	return value;
 }
+
+int readn(int fd, void *buf, size_t n)
+{
+	void *buf_start = buf;
+
+	while (n) {
+		int ret = read(fd, buf, n);
+
+		if (ret <= 0)
+			return ret;
+
+		n -= ret;
+		buf += ret;
+	}
+
+	return buf - buf_start;
+}
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 7562707..e833f26 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -265,6 +265,7 @@ void argv_free(char **argv);
 bool strglobmatch(const char *str, const char *pat);
 bool strlazymatch(const char *str, const char *pat);
 unsigned long convert_unit(unsigned long value, char *unit);
+int readn(int fd, void *buf, size_t size);
 
 #define _STR(x) #x
 #define STR(x) _STR(x)
-- 
1.6.2.5


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

* [PATCH 04/11] perf evsel: Delete the event selectors at exit
  2011-01-04  3:48 [GIT PULL||RFC 00/11] perf library and regression testing improvements Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2011-01-04  3:48 ` [PATCH 03/11] perf util: Move do_read from session to util Arnaldo Carvalho de Melo
@ 2011-01-04  3:48 ` Arnaldo Carvalho de Melo
  2011-01-04  3:48 ` [PATCH 05/11] perf evsel: Steal the counter reading routines from stat Arnaldo Carvalho de Melo
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04  3:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Tom Zanussi, Thomas Gleixner

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Freeing all the possibly allocated resources, reducing complexity
on each tool exit path.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c    |    2 --
 tools/perf/builtin-stat.c      |    4 +---
 tools/perf/builtin-top.c       |    4 +---
 tools/perf/perf.c              |    2 ++
 tools/perf/util/parse-events.c |   11 +++++++++++
 tools/perf/util/parse-events.h |    1 +
 6 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e68aee3..052de17 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -965,8 +965,6 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 out_free_event_array:
 	free(event_array);
 out_free_fd:
-	list_for_each_entry(pos, &evsel_list, node)
-		perf_evsel__free_fd(pos);
 	free(all_tids);
 	all_tids = NULL;
 out_symbol_exit:
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 3e5f356..589ba3a 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -844,10 +844,8 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 	if (status != -1)
 		print_stat(argc, argv);
 out_free_fd:
-	list_for_each_entry(pos, &evsel_list, node) {
-		perf_evsel__free_fd(pos);
+	list_for_each_entry(pos, &evsel_list, node)
 		perf_evsel__free_stat_priv(pos);
-	}
 out:
 	return status;
 }
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 13a836e..27b9c14 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1495,10 +1495,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 
 	status = __cmd_top();
 out_free_fd:
-	list_for_each_entry(pos, &evsel_list, node) {
-		perf_evsel__free_fd(pos);
+	list_for_each_entry(pos, &evsel_list, node)
 		perf_evsel__free_mmap(pos);
-	}
 
 	return status;
 }
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 595d0f4..5b1ecd6 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -286,6 +286,8 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 	status = p->fn(argc, argv, prefix);
 	exit_browser(status);
 
+	perf_evsel_list__delete();
+
 	if (status)
 		return status & 0xff;
 
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 2d948ad..3a142e9 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -982,3 +982,14 @@ int perf_evsel_list__create_default(void)
 	++nr_counters;
 	return 0;
 }
+
+void perf_evsel_list__delete(void)
+{
+	struct perf_evsel *pos, *n;
+
+	list_for_each_entry_safe(pos, n, &evsel_list, node) {
+		list_del_init(&pos->node);
+		perf_evsel__delete(pos);
+	}
+	nr_counters = 0;
+}
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 0f915a0..0a0abc1 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -12,6 +12,7 @@ struct perf_evsel;
 extern struct list_head evsel_list;
 
 int perf_evsel_list__create_default(void);
+void perf_evsel_list__delete(void);
 
 struct option;
 
-- 
1.6.2.5


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

* [PATCH 05/11] perf evsel: Steal the counter reading routines from stat
  2011-01-04  3:48 [GIT PULL||RFC 00/11] perf library and regression testing improvements Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2011-01-04  3:48 ` [PATCH 04/11] perf evsel: Delete the event selectors at exit Arnaldo Carvalho de Melo
@ 2011-01-04  3:48 ` Arnaldo Carvalho de Melo
  2011-01-04  3:48 ` [PATCH 06/11] perf evsel: Introduce per cpu and per thread open helpers Arnaldo Carvalho de Melo
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04  3:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Tom Zanussi, Thomas Gleixner

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Making them hopefully generic enough to be used in 'perf test',
well see.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c |  121 +++++++++++----------------------------------
 tools/perf/util/evsel.c   |   88 ++++++++++++++++++++++++++++++++
 tools/perf/util/evsel.h   |   79 +++++++++++++++++++++++++++++
 3 files changed, 196 insertions(+), 92 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 589ba3a..a8b00b4 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -93,12 +93,6 @@ static const char		*cpu_list;
 static const char		*csv_sep			= NULL;
 static bool			csv_output			= false;
 
-struct cpu_counts {
-	u64 val;
-	u64 ena;
-	u64 run;
-};
-
 static volatile int done = 0;
 
 struct stats
@@ -108,15 +102,11 @@ struct stats
 
 struct perf_stat {
 	struct stats	  res_stats[3];
-	int		  scaled;
-	struct cpu_counts cpu_counts[];
 };
 
-static int perf_evsel__alloc_stat_priv(struct perf_evsel *evsel, int ncpus)
+static int perf_evsel__alloc_stat_priv(struct perf_evsel *evsel)
 {
-	size_t priv_size = (sizeof(struct perf_stat) +
-			    (ncpus * sizeof(struct cpu_counts)));
-	evsel->priv = zalloc(priv_size);
+	evsel->priv = zalloc(sizeof(struct perf_stat));
 	return evsel->priv == NULL ? -ENOMEM : 0;
 }
 
@@ -238,52 +228,14 @@ static inline int nsec_counter(struct perf_evsel *evsel)
  * Read out the results of a single counter:
  * aggregate counts across CPUs in system-wide mode
  */
-static void read_counter_aggr(struct perf_evsel *counter)
+static int read_counter_aggr(struct perf_evsel *counter)
 {
 	struct perf_stat *ps = counter->priv;
-	u64 count[3], single_count[3];
-	int cpu;
-	size_t res, nv;
-	int scaled;
-	int i, thread;
-
-	count[0] = count[1] = count[2] = 0;
-
-	nv = scale ? 3 : 1;
-	for (cpu = 0; cpu < nr_cpus; cpu++) {
-		for (thread = 0; thread < thread_num; thread++) {
-			if (FD(counter, cpu, thread) < 0)
-				continue;
-
-			res = read(FD(counter, cpu, thread),
-					single_count, nv * sizeof(u64));
-			assert(res == nv * sizeof(u64));
-
-			close(FD(counter, cpu, thread));
-			FD(counter, cpu, thread) = -1;
-
-			count[0] += single_count[0];
-			if (scale) {
-				count[1] += single_count[1];
-				count[2] += single_count[2];
-			}
-		}
-	}
-
-	scaled = 0;
-	if (scale) {
-		if (count[2] == 0) {
-			ps->scaled = -1;
-			count[0] = 0;
-			return;
-		}
+	u64 *count = counter->counts->aggr.values;
+	int i;
 
-		if (count[2] < count[1]) {
-			ps->scaled = 1;
-			count[0] = (unsigned long long)
-				((double)count[0] * count[1] / count[2] + 0.5);
-		}
-	}
+	if (__perf_evsel__read(counter, nr_cpus, thread_num, scale) < 0)
+		return -1;
 
 	for (i = 0; i < 3; i++)
 		update_stats(&ps->res_stats[i], count[i]);
@@ -302,46 +254,24 @@ static void read_counter_aggr(struct perf_evsel *counter)
 		update_stats(&runtime_cycles_stats[0], count[0]);
 	if (perf_evsel__match(counter, HARDWARE, HW_BRANCH_INSTRUCTIONS))
 		update_stats(&runtime_branches_stats[0], count[0]);
+
+	return 0;
 }
 
 /*
  * Read out the results of a single counter:
  * do not aggregate counts across CPUs in system-wide mode
  */
-static void read_counter(struct perf_evsel *counter)
+static int read_counter(struct perf_evsel *counter)
 {
-	struct cpu_counts *cpu_counts = counter->priv;
-	u64 count[3];
+	u64 *count;
 	int cpu;
-	size_t res, nv;
-
-	count[0] = count[1] = count[2] = 0;
-
-	nv = scale ? 3 : 1;
 
 	for (cpu = 0; cpu < nr_cpus; cpu++) {
+		if (__perf_evsel__read_on_cpu(counter, cpu, 0, scale) < 0)
+			return -1;
 
-		if (FD(counter, cpu, 0) < 0)
-			continue;
-
-		res = read(FD(counter, cpu, 0), count, nv * sizeof(u64));
-
-		assert(res == nv * sizeof(u64));
-
-		close(FD(counter, cpu, 0));
-		FD(counter, cpu, 0) = -1;
-
-		if (scale) {
-			if (count[2] == 0) {
-				count[0] = 0;
-			} else if (count[2] < count[1]) {
-				count[0] = (unsigned long long)
-				((double)count[0] * count[1] / count[2] + 0.5);
-			}
-		}
-		cpu_counts[cpu].val = count[0]; /* scaled count */
-		cpu_counts[cpu].ena = count[1];
-		cpu_counts[cpu].run = count[2];
+		count = counter->counts->cpu[cpu].values;
 
 		if (perf_evsel__match(counter, SOFTWARE, SW_TASK_CLOCK))
 			update_stats(&runtime_nsecs_stats[cpu], count[0]);
@@ -350,6 +280,8 @@ static void read_counter(struct perf_evsel *counter)
 		if (perf_evsel__match(counter, HARDWARE, HW_BRANCH_INSTRUCTIONS))
 			update_stats(&runtime_branches_stats[cpu], count[0]);
 	}
+
+	return 0;
 }
 
 static int run_perf_stat(int argc __used, const char **argv)
@@ -449,12 +381,17 @@ static int run_perf_stat(int argc __used, const char **argv)
 	update_stats(&walltime_nsecs_stats, t1 - t0);
 
 	if (no_aggr) {
-		list_for_each_entry(counter, &evsel_list, node)
+		list_for_each_entry(counter, &evsel_list, node) {
 			read_counter(counter);
+			perf_evsel__close_fd(counter, nr_cpus, 1);
+		}
 	} else {
-		list_for_each_entry(counter, &evsel_list, node)
+		list_for_each_entry(counter, &evsel_list, node) {
 			read_counter_aggr(counter);
+			perf_evsel__close_fd(counter, nr_cpus, thread_num);
+		}
 	}
+
 	return WEXITSTATUS(status);
 }
 
@@ -550,7 +487,7 @@ static void print_counter_aggr(struct perf_evsel *counter)
 {
 	struct perf_stat *ps = counter->priv;
 	double avg = avg_stats(&ps->res_stats[0]);
-	int scaled = ps->scaled;
+	int scaled = counter->counts->scaled;
 
 	if (scaled == -1) {
 		fprintf(stderr, "%*s%s%-24s\n",
@@ -590,14 +527,13 @@ static void print_counter_aggr(struct perf_evsel *counter)
  */
 static void print_counter(struct perf_evsel *counter)
 {
-	struct perf_stat *ps = counter->priv;
 	u64 ena, run, val;
 	int cpu;
 
 	for (cpu = 0; cpu < nr_cpus; cpu++) {
-		val = ps->cpu_counts[cpu].val;
-		ena = ps->cpu_counts[cpu].ena;
-		run = ps->cpu_counts[cpu].run;
+		val = counter->counts->cpu[cpu].val;
+		ena = counter->counts->cpu[cpu].ena;
+		run = counter->counts->cpu[cpu].run;
 		if (run == 0 || ena == 0) {
 			fprintf(stderr, "CPU%*d%s%*s%s%-24s",
 				csv_output ? 0 : -4,
@@ -818,7 +754,8 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 	}
 
 	list_for_each_entry(pos, &evsel_list, node) {
-		if (perf_evsel__alloc_stat_priv(pos, nr_cpus) < 0 ||
+		if (perf_evsel__alloc_stat_priv(pos) < 0 ||
+		    perf_evsel__alloc_counts(pos, nr_cpus) < 0 ||
 		    perf_evsel__alloc_fd(pos, nr_cpus, thread_num) < 0)
 			goto out_free_fd;
 	}
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 6539ec9..3f5de51 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1,6 +1,8 @@
 #include "evsel.h"
 #include "util.h"
 
+#define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
+
 struct perf_evsel *perf_evsel__new(u32 type, u64 config, int idx)
 {
 	struct perf_evsel *evsel = zalloc(sizeof(*evsel));
@@ -21,15 +23,101 @@ int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
 	return evsel->fd != NULL ? 0 : -ENOMEM;
 }
 
+int perf_evsel__alloc_counts(struct perf_evsel *evsel, int ncpus)
+{
+	evsel->counts = zalloc((sizeof(*evsel->counts) +
+				(ncpus * sizeof(struct perf_counts_values))));
+	return evsel->counts != NULL ? 0 : -ENOMEM;
+}
+
 void perf_evsel__free_fd(struct perf_evsel *evsel)
 {
 	xyarray__delete(evsel->fd);
 	evsel->fd = NULL;
 }
 
+void perf_evsel__close_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
+{
+	int cpu, thread;
+
+	for (cpu = 0; cpu < ncpus; cpu++)
+		for (thread = 0; thread < nthreads; ++thread) {
+			close(FD(evsel, cpu, thread));
+			FD(evsel, cpu, thread) = -1;
+		}
+}
+
 void perf_evsel__delete(struct perf_evsel *evsel)
 {
 	assert(list_empty(&evsel->node));
 	xyarray__delete(evsel->fd);
 	free(evsel);
 }
+
+int __perf_evsel__read_on_cpu(struct perf_evsel *evsel,
+			      int cpu, int thread, bool scale)
+{
+	struct perf_counts_values count;
+	size_t nv = scale ? 3 : 1;
+
+	if (FD(evsel, cpu, thread) < 0)
+		return -EINVAL;
+
+	if (readn(FD(evsel, cpu, thread), &count, nv * sizeof(u64)) < 0)
+		return -errno;
+
+	if (scale) {
+		if (count.run == 0)
+			count.val = 0;
+		else if (count.run < count.ena)
+			count.val = (u64)((double)count.val * count.ena / count.run + 0.5);
+	} else
+		count.ena = count.run = 0;
+
+	evsel->counts->cpu[cpu] = count;
+	return 0;
+}
+
+int __perf_evsel__read(struct perf_evsel *evsel,
+		       int ncpus, int nthreads, bool scale)
+{
+	size_t nv = scale ? 3 : 1;
+	int cpu, thread;
+	struct perf_counts_values *aggr = &evsel->counts->aggr, count;
+
+	aggr->val = 0;
+
+	for (cpu = 0; cpu < ncpus; cpu++) {
+		for (thread = 0; thread < nthreads; thread++) {
+			if (FD(evsel, cpu, thread) < 0)
+				continue;
+
+			if (readn(FD(evsel, cpu, thread),
+				  &count, nv * sizeof(u64)) < 0)
+				return -errno;
+
+			aggr->val += count.val;
+			if (scale) {
+				aggr->ena += count.ena;
+				aggr->run += count.run;
+			}
+		}
+	}
+
+	evsel->counts->scaled = 0;
+	if (scale) {
+		if (aggr->run == 0) {
+			evsel->counts->scaled = -1;
+			aggr->val = 0;
+			return 0;
+		}
+
+		if (aggr->run < aggr->ena) {
+			evsel->counts->scaled = 1;
+			aggr->val = (u64)((double)aggr->val * aggr->ena / aggr->run + 0.5);
+		}
+	} else
+		aggr->ena = aggr->run = 0;
+
+	return 0;
+}
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 8a5cfb6..8b48ef1 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -2,15 +2,34 @@
 #define __PERF_EVSEL_H 1
 
 #include <linux/list.h>
+#include <stdbool.h>
 #include <linux/perf_event.h>
 #include "types.h"
 #include "xyarray.h"
+ 
+struct perf_counts_values {
+	union {
+		struct {
+			u64 val;
+			u64 ena;
+			u64 run;
+		};
+		u64 values[3];
+	};
+};
+
+struct perf_counts {
+	s8		   	  scaled;
+	struct perf_counts_values aggr;
+	struct perf_counts_values cpu[];
+};
 
 struct perf_evsel {
 	struct list_head	node;
 	struct perf_event_attr	attr;
 	char			*filter;
 	struct xyarray		*fd;
+	struct perf_counts	*counts;
 	int			idx;
 	void			*priv;
 };
@@ -19,10 +38,70 @@ struct perf_evsel *perf_evsel__new(u32 type, u64 config, int idx);
 void perf_evsel__delete(struct perf_evsel *evsel);
 
 int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
+int perf_evsel__alloc_counts(struct perf_evsel *evsel, int ncpus);
 void perf_evsel__free_fd(struct perf_evsel *evsel);
+void perf_evsel__close_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
 
 #define perf_evsel__match(evsel, t, c)		\
 	(evsel->attr.type == PERF_TYPE_##t &&	\
 	 evsel->attr.config == PERF_COUNT_##c)
 
+int __perf_evsel__read_on_cpu(struct perf_evsel *evsel,
+			      int cpu, int thread, bool scale);
+
+/**
+ * perf_evsel__read_on_cpu - Read out the results on a CPU and thread
+ *
+ * @evsel - event selector to read value
+ * @cpu - CPU of interest
+ * @thread - thread of interest
+ */
+static inline int perf_evsel__read_on_cpu(struct perf_evsel *evsel,
+					  int cpu, int thread)
+{
+	return __perf_evsel__read_on_cpu(evsel, cpu, thread, false);
+}
+
+/**
+ * perf_evsel__read_on_cpu_scaled - Read out the results on a CPU and thread, scaled
+ *
+ * @evsel - event selector to read value
+ * @cpu - CPU of interest
+ * @thread - thread of interest
+ */
+static inline int perf_evsel__read_on_cpu_scaled(struct perf_evsel *evsel,
+						 int cpu, int thread)
+{
+	return __perf_evsel__read_on_cpu(evsel, cpu, thread, true);
+}
+
+int __perf_evsel__read(struct perf_evsel *evsel, int ncpus, int nthreads,
+		       bool scale);
+
+/**
+ * perf_evsel__read - Read the aggregate results on all CPUs
+ *
+ * @evsel - event selector to read value
+ * @ncpus - Number of cpus affected, from zero
+ * @nthreads - Number of threads affected, from zero
+ */
+static inline int perf_evsel__read(struct perf_evsel *evsel,
+				    int ncpus, int nthreads)
+{
+	return __perf_evsel__read(evsel, ncpus, nthreads, false);
+}
+
+/**
+ * perf_evsel__read_scaled - Read the aggregate results on all CPUs, scaled
+ *
+ * @evsel - event selector to read value
+ * @ncpus - Number of cpus affected, from zero
+ * @nthreads - Number of threads affected, from zero
+ */
+static inline int perf_evsel__read_scaled(struct perf_evsel *evsel,
+					  int ncpus, int nthreads)
+{
+	return __perf_evsel__read(evsel, ncpus, nthreads, true);
+}
+
 #endif /* __PERF_EVSEL_H */
-- 
1.6.2.5


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

* [PATCH 06/11] perf evsel: Introduce per cpu and per thread open helpers
  2011-01-04  3:48 [GIT PULL||RFC 00/11] perf library and regression testing improvements Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2011-01-04  3:48 ` [PATCH 05/11] perf evsel: Steal the counter reading routines from stat Arnaldo Carvalho de Melo
@ 2011-01-04  3:48 ` Arnaldo Carvalho de Melo
  2011-01-04  3:48 ` [PATCH 07/11] perf tools: Refactor cpumap to hold nr and the map Arnaldo Carvalho de Melo
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04  3:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Tom Zanussi, Thomas Gleixner

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Abstracting away the loops needed to create the various event fd handlers.

The users have to pass a confiruged perf->evsel.attr field, which is already
usable after perf_evsel__new (constructor) time, using defaults.

Comes out of the ad-hoc routines in builtin-stat, that now uses it.

Fixed a small silly bug where we were die()ing before killing our
children, dysfunctional family this one 8-)

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c |   84 ++++++++++++++-------------------------------
 tools/perf/util/evsel.c   |   52 ++++++++++++++++++++++++++++
 tools/perf/util/evsel.h   |    5 +++
 3 files changed, 83 insertions(+), 58 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index a8b00b4..065e79e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -53,8 +53,6 @@
 #include <math.h>
 #include <locale.h>
 
-#define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
-
 #define DEFAULT_SEPARATOR	" "
 
 static struct perf_event_attr default_attrs[] = {
@@ -160,56 +158,24 @@ struct stats			runtime_cycles_stats[MAX_NR_CPUS];
 struct stats			runtime_branches_stats[MAX_NR_CPUS];
 struct stats			walltime_nsecs_stats;
 
-#define ERR_PERF_OPEN \
-"counter %d, sys_perf_event_open() syscall returned with %d (%s).  /bin/dmesg may provide additional information."
-
-static int create_perf_stat_counter(struct perf_evsel *evsel, bool *perm_err)
+static int create_perf_stat_counter(struct perf_evsel *evsel)
 {
 	struct perf_event_attr *attr = &evsel->attr;
-	int thread;
-	int ncreated = 0;
 
 	if (scale)
 		attr->read_format = PERF_FORMAT_TOTAL_TIME_ENABLED |
 				    PERF_FORMAT_TOTAL_TIME_RUNNING;
 
-	if (system_wide) {
-		int cpu;
-
-		for (cpu = 0; cpu < nr_cpus; cpu++) {
-			FD(evsel, cpu, 0) = sys_perf_event_open(attr,
-					-1, cpumap[cpu], -1, 0);
-			if (FD(evsel, cpu, 0) < 0) {
-				if (errno == EPERM || errno == EACCES)
-					*perm_err = true;
-				error(ERR_PERF_OPEN, evsel->idx,
-					FD(evsel, cpu, 0), strerror(errno));
-			} else {
-				++ncreated;
-			}
-		}
-	} else {
-		attr->inherit = !no_inherit;
-		if (target_pid == -1 && target_tid == -1) {
-			attr->disabled = 1;
-			attr->enable_on_exec = 1;
-		}
-		for (thread = 0; thread < thread_num; thread++) {
-			FD(evsel, 0, thread) = sys_perf_event_open(attr,
-				all_tids[thread], -1, -1, 0);
-			if (FD(evsel, 0, thread) < 0) {
-				if (errno == EPERM || errno == EACCES)
-					*perm_err = true;
-				error(ERR_PERF_OPEN, evsel->idx,
-					FD(evsel, 0, thread),
-					 strerror(errno));
-			} else {
-				++ncreated;
-			}
-		}
+	if (system_wide)
+		return perf_evsel__open_per_cpu(evsel, nr_cpus, cpumap);
+
+	attr->inherit = !no_inherit;
+	if (target_pid == -1 && target_tid == -1) {
+		attr->disabled = 1;
+		attr->enable_on_exec = 1;
 	}
 
-	return ncreated;
+	return perf_evsel__open_per_thread(evsel, thread_num, all_tids);
 }
 
 /*
@@ -289,9 +255,7 @@ static int run_perf_stat(int argc __used, const char **argv)
 	unsigned long long t0, t1;
 	struct perf_evsel *counter;
 	int status = 0;
-	int ncreated = 0;
 	int child_ready_pipe[2], go_pipe[2];
-	bool perm_err = false;
 	const bool forks = (argc > 0);
 	char buf;
 
@@ -349,19 +313,23 @@ static int run_perf_stat(int argc __used, const char **argv)
 		close(child_ready_pipe[0]);
 	}
 
-	list_for_each_entry(counter, &evsel_list, node)
-		ncreated += create_perf_stat_counter(counter, &perm_err);
-
-	if (ncreated < nr_counters) {
-		if (perm_err)
-			error("You may not have permission to collect %sstats.\n"
-			      "\t Consider tweaking"
-			      " /proc/sys/kernel/perf_event_paranoid or running as root.",
-			      system_wide ? "system-wide " : "");
-		die("Not all events could be opened.\n");
-		if (child_pid != -1)
-			kill(child_pid, SIGTERM);
-		return -1;
+	list_for_each_entry(counter, &evsel_list, node) {
+		if (create_perf_stat_counter(counter) < 0) {
+			if (errno == -EPERM || errno == -EACCES) {
+				error("You may not have permission to collect %sstats.\n"
+				      "\t Consider tweaking"
+				      " /proc/sys/kernel/perf_event_paranoid or running as root.",
+				      system_wide ? "system-wide " : "");
+			} else {
+				error("open_counter returned with %d (%s). "
+				      "/bin/dmesg may provide additional information.\n",
+				       errno, strerror(errno));
+			}
+			if (child_pid != -1)
+				kill(child_pid, SIGTERM);
+			die("Not all events could be opened.\n");
+			return -1;
+		}
 	}
 
 	/*
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 3f5de51..e62cc5e 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1,4 +1,5 @@
 #include "evsel.h"
+#include "../perf.h"
 #include "util.h"
 
 #define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
@@ -121,3 +122,54 @@ int __perf_evsel__read(struct perf_evsel *evsel,
 
 	return 0;
 }
+
+int perf_evsel__open_per_cpu(struct perf_evsel *evsel, int ncpus, int *cpu_map)
+{
+	int cpu;
+
+	for (cpu = 0; cpu < ncpus; cpu++) {
+		FD(evsel, cpu, 0) = sys_perf_event_open(&evsel->attr, -1,
+							cpu_map[cpu], -1, 0);
+		if (FD(evsel, cpu, 0) < 0)
+			goto out_close;
+	}
+
+	return 0;
+
+out_close:
+	while (--cpu >= 0) {
+		close(FD(evsel, cpu, 0));
+		FD(evsel, cpu, 0) = -1;
+	}
+	return -1;
+}
+
+int perf_evsel__open_per_thread(struct perf_evsel *evsel, int nthreads, int *thread_map)
+{
+	int thread;
+
+	for (thread = 0; thread < nthreads; thread++) {
+		FD(evsel, 0, thread) = sys_perf_event_open(&evsel->attr,
+							   thread_map[thread], -1, -1, 0);
+		if (FD(evsel, 0, thread) < 0)
+			goto out_close;
+	}
+
+	return 0;
+
+out_close:
+	while (--thread >= 0) {
+		close(FD(evsel, 0, thread));
+		FD(evsel, 0, thread) = -1;
+	}
+	return -1;
+}
+
+int perf_evsel__open(struct perf_evsel *evsel, int ncpus, int nthreads,
+		     int *cpu_map, int *thread_map)
+{
+	if (nthreads < 0)
+		return perf_evsel__open_per_cpu(evsel, ncpus, cpu_map);
+
+	return perf_evsel__open_per_thread(evsel, nthreads, thread_map);
+}
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 8b48ef1..a62fb55 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -42,6 +42,11 @@ int perf_evsel__alloc_counts(struct perf_evsel *evsel, int ncpus);
 void perf_evsel__free_fd(struct perf_evsel *evsel);
 void perf_evsel__close_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
 
+int perf_evsel__open_per_cpu(struct perf_evsel *evsel, int ncpus, int *cpu_map);
+int perf_evsel__open_per_thread(struct perf_evsel *evsel, int nthreads, int *thread_map);
+int perf_evsel__open(struct perf_evsel *evsel, int ncpus, int nthreads,
+		     int *cpu_map, int *thread_map);
+
 #define perf_evsel__match(evsel, t, c)		\
 	(evsel->attr.type == PERF_TYPE_##t &&	\
 	 evsel->attr.config == PERF_COUNT_##c)
-- 
1.6.2.5


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

* [PATCH 07/11] perf tools: Refactor cpumap to hold nr and the map
  2011-01-04  3:48 [GIT PULL||RFC 00/11] perf library and regression testing improvements Arnaldo Carvalho de Melo
                   ` (5 preceding siblings ...)
  2011-01-04  3:48 ` [PATCH 06/11] perf evsel: Introduce per cpu and per thread open helpers Arnaldo Carvalho de Melo
@ 2011-01-04  3:48 ` Arnaldo Carvalho de Melo
  2011-01-04  3:48 ` [PATCH 08/11] perf tools: Refactor all_tids " Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04  3:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Tom Zanussi, Thomas Gleixner

From: Arnaldo Carvalho de Melo <acme@redhat.com>

So that later, we can pass the cpu_map instance instead of (nr_cpus, cpu_map)
for things like perf_evsel__open and friends.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c |   14 +++---
 tools/perf/builtin-stat.c   |   36 ++++++------
 tools/perf/builtin-top.c    |   22 ++++----
 tools/perf/util/cpumap.c    |  123 +++++++++++++++++++++++++++++++++----------
 tools/perf/util/cpumap.h    |   10 +++-
 5 files changed, 138 insertions(+), 67 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 052de17..220e6e7 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -39,7 +39,7 @@ static u64			user_interval			= ULLONG_MAX;
 static u64			default_interval		=      0;
 static u64			sample_type;
 
-static int			nr_cpus				=      0;
+static struct cpu_map		*cpus;
 static unsigned int		page_size;
 static unsigned int		mmap_pages			=    128;
 static unsigned int		user_freq 			= UINT_MAX;
@@ -670,8 +670,8 @@ static int __cmd_record(int argc, const char **argv)
 	if (!system_wide && no_inherit && !cpu_list) {
 		open_counters(-1);
 	} else {
-		for (i = 0; i < nr_cpus; i++)
-			open_counters(cpumap[i]);
+		for (i = 0; i < cpus->nr; i++)
+			open_counters(cpus->map[i]);
 	}
 
 	perf_session__set_sample_type(session, sample_type);
@@ -927,14 +927,14 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		thread_num = 1;
 	}
 
-	nr_cpus = read_cpu_map(cpu_list);
-	if (nr_cpus < 1) {
-		perror("failed to collect number of CPUs");
+	cpus = cpu_map__new(cpu_list);
+	if (cpus == NULL) {
+		perror("failed to parse CPUs map");
 		return -1;
 	}
 
 	list_for_each_entry(pos, &evsel_list, node) {
-		if (perf_evsel__alloc_fd(pos, nr_cpus, thread_num) < 0)
+		if (perf_evsel__alloc_fd(pos, cpus->nr, thread_num) < 0)
 			goto out_free_fd;
 	}
 	event_array = malloc(
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 065e79e..3f4a431 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -72,7 +72,7 @@ static struct perf_event_attr default_attrs[] = {
 };
 
 static bool			system_wide			=  false;
-static int			nr_cpus				=  0;
+static struct cpu_map		*cpus;
 static int			run_idx				=  0;
 
 static int			run_count			=  1;
@@ -167,7 +167,7 @@ static int create_perf_stat_counter(struct perf_evsel *evsel)
 				    PERF_FORMAT_TOTAL_TIME_RUNNING;
 
 	if (system_wide)
-		return perf_evsel__open_per_cpu(evsel, nr_cpus, cpumap);
+		return perf_evsel__open_per_cpu(evsel, cpus->nr, cpus->map);
 
 	attr->inherit = !no_inherit;
 	if (target_pid == -1 && target_tid == -1) {
@@ -200,7 +200,7 @@ static int read_counter_aggr(struct perf_evsel *counter)
 	u64 *count = counter->counts->aggr.values;
 	int i;
 
-	if (__perf_evsel__read(counter, nr_cpus, thread_num, scale) < 0)
+	if (__perf_evsel__read(counter, cpus->nr, thread_num, scale) < 0)
 		return -1;
 
 	for (i = 0; i < 3; i++)
@@ -233,7 +233,7 @@ static int read_counter(struct perf_evsel *counter)
 	u64 *count;
 	int cpu;
 
-	for (cpu = 0; cpu < nr_cpus; cpu++) {
+	for (cpu = 0; cpu < cpus->nr; cpu++) {
 		if (__perf_evsel__read_on_cpu(counter, cpu, 0, scale) < 0)
 			return -1;
 
@@ -259,9 +259,6 @@ static int run_perf_stat(int argc __used, const char **argv)
 	const bool forks = (argc > 0);
 	char buf;
 
-	if (!system_wide)
-		nr_cpus = 1;
-
 	if (forks && (pipe(child_ready_pipe) < 0 || pipe(go_pipe) < 0)) {
 		perror("failed to create pipes");
 		exit(1);
@@ -351,12 +348,12 @@ static int run_perf_stat(int argc __used, const char **argv)
 	if (no_aggr) {
 		list_for_each_entry(counter, &evsel_list, node) {
 			read_counter(counter);
-			perf_evsel__close_fd(counter, nr_cpus, 1);
+			perf_evsel__close_fd(counter, cpus->nr, 1);
 		}
 	} else {
 		list_for_each_entry(counter, &evsel_list, node) {
 			read_counter_aggr(counter);
-			perf_evsel__close_fd(counter, nr_cpus, thread_num);
+			perf_evsel__close_fd(counter, cpus->nr, thread_num);
 		}
 	}
 
@@ -384,7 +381,7 @@ static void nsec_printout(int cpu, struct perf_evsel *evsel, double avg)
 	if (no_aggr)
 		sprintf(cpustr, "CPU%*d%s",
 			csv_output ? 0 : -4,
-			cpumap[cpu], csv_sep);
+			cpus->map[cpu], csv_sep);
 
 	fprintf(stderr, fmt, cpustr, msecs, csv_sep, event_name(evsel));
 
@@ -412,7 +409,7 @@ static void abs_printout(int cpu, struct perf_evsel *evsel, double avg)
 	if (no_aggr)
 		sprintf(cpustr, "CPU%*d%s",
 			csv_output ? 0 : -4,
-			cpumap[cpu], csv_sep);
+			cpus->map[cpu], csv_sep);
 	else
 		cpu = 0;
 
@@ -498,14 +495,14 @@ static void print_counter(struct perf_evsel *counter)
 	u64 ena, run, val;
 	int cpu;
 
-	for (cpu = 0; cpu < nr_cpus; cpu++) {
+	for (cpu = 0; cpu < cpus->nr; cpu++) {
 		val = counter->counts->cpu[cpu].val;
 		ena = counter->counts->cpu[cpu].ena;
 		run = counter->counts->cpu[cpu].run;
 		if (run == 0 || ena == 0) {
 			fprintf(stderr, "CPU%*d%s%*s%s%-24s",
 				csv_output ? 0 : -4,
-				cpumap[cpu], csv_sep,
+				cpus->map[cpu], csv_sep,
 				csv_output ? 0 : 18,
 				"<not counted>", csv_sep,
 				event_name(counter));
@@ -697,12 +694,15 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 	}
 
 	if (system_wide)
-		nr_cpus = read_cpu_map(cpu_list);
+		cpus = cpu_map__new(cpu_list);
 	else
-		nr_cpus = 1;
+		cpus = cpu_map__dummy_new();
 
-	if (nr_cpus < 1)
+	if (cpus == NULL) {
+		perror("failed to parse CPUs map");
 		usage_with_options(stat_usage, options);
+		return -1;
+	}
 
 	if (target_pid != -1) {
 		target_tid = target_pid;
@@ -723,8 +723,8 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 
 	list_for_each_entry(pos, &evsel_list, node) {
 		if (perf_evsel__alloc_stat_priv(pos) < 0 ||
-		    perf_evsel__alloc_counts(pos, nr_cpus) < 0 ||
-		    perf_evsel__alloc_fd(pos, nr_cpus, thread_num) < 0)
+		    perf_evsel__alloc_counts(pos, cpus->nr) < 0 ||
+		    perf_evsel__alloc_fd(pos, cpus->nr, thread_num) < 0)
 			goto out_free_fd;
 	}
 
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 27b9c14..0e42666 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -71,7 +71,7 @@ static int			target_tid			=     -1;
 static pid_t			*all_tids			=      NULL;
 static int			thread_num			=      0;
 static bool			inherit				=  false;
-static int			nr_cpus				=      0;
+static struct cpu_map		*cpus;
 static int			realtime_prio			=      0;
 static bool			group				=  false;
 static unsigned int		page_size;
@@ -564,12 +564,12 @@ static void print_sym_table(void)
 		printf(" (all");
 
 	if (cpu_list)
-		printf(", CPU%s: %s)\n", nr_cpus > 1 ? "s" : "", cpu_list);
+		printf(", CPU%s: %s)\n", cpus->nr > 1 ? "s" : "", cpu_list);
 	else {
 		if (target_tid != -1)
 			printf(")\n");
 		else
-			printf(", %d CPU%s)\n", nr_cpus, nr_cpus > 1 ? "s" : "");
+			printf(", %d CPU%s)\n", cpus->nr, cpus->nr > 1 ? "s" : "");
 	}
 
 	printf("%-*.*s\n", win_width, win_width, graph_dotted_line);
@@ -1197,7 +1197,7 @@ static void perf_session__mmap_read(struct perf_session *self)
 	struct perf_evsel *counter;
 	int i, thread_index;
 
-	for (i = 0; i < nr_cpus; i++) {
+	for (i = 0; i < cpus->nr; i++) {
 		list_for_each_entry(counter, &evsel_list, node) {
 			for (thread_index = 0;
 				thread_index < thread_num;
@@ -1221,7 +1221,7 @@ static void start_counter(int i, struct perf_evsel *evsel)
 	int thread_index;
 
 	if (target_tid == -1)
-		cpu = cpumap[i];
+		cpu = cpus->map[i];
 
 	attr = &evsel->attr;
 
@@ -1310,7 +1310,7 @@ static int __cmd_top(void)
 	else
 		event__synthesize_threads(event__process, session);
 
-	for (i = 0; i < nr_cpus; i++) {
+	for (i = 0; i < cpus->nr; i++) {
 		group_fd = -1;
 		list_for_each_entry(counter, &evsel_list, node)
 			start_counter(i, counter);
@@ -1460,16 +1460,16 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 	}
 
 	if (target_tid != -1)
-		nr_cpus = 1;
+		cpus = cpu_map__dummy_new();
 	else
-		nr_cpus = read_cpu_map(cpu_list);
+		cpus = cpu_map__new(cpu_list);
 
-	if (nr_cpus < 1)
+	if (cpus == NULL)
 		usage_with_options(top_usage, options);
 
 	list_for_each_entry(pos, &evsel_list, node) {
-		if (perf_evsel__alloc_mmap_per_thread(pos, nr_cpus, thread_num) < 0 ||
-		    perf_evsel__alloc_fd(pos, nr_cpus, thread_num) < 0)
+		if (perf_evsel__alloc_mmap_per_thread(pos, cpus->nr, thread_num) < 0 ||
+		    perf_evsel__alloc_fd(pos, cpus->nr, thread_num) < 0)
 			goto out_free_fd;
 		/*
 		 * Fill in the ones not specifically initialized via -c:
diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 0f9b8d7..3ccaa10 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -4,32 +4,53 @@
 #include <assert.h>
 #include <stdio.h>
 
-int cpumap[MAX_NR_CPUS];
-
-static int default_cpu_map(void)
+static struct cpu_map *cpu_map__default_new(void)
 {
-	int nr_cpus, i;
+	struct cpu_map *cpus;
+	int nr_cpus;
 
 	nr_cpus = sysconf(_SC_NPROCESSORS_ONLN);
-	assert(nr_cpus <= MAX_NR_CPUS);
-	assert((int)nr_cpus >= 0);
+	if (nr_cpus < 0)
+		return NULL;
+
+	cpus = malloc(sizeof(*cpus) + nr_cpus * sizeof(int));
+	if (cpus != NULL) {
+		int i;
+		for (i = 0; i < nr_cpus; ++i)
+			cpus->map[i] = i;
 
-	for (i = 0; i < nr_cpus; ++i)
-		cpumap[i] = i;
+		cpus->nr = nr_cpus;
+	}
 
-	return nr_cpus;
+	return cpus;
 }
 
-static int read_all_cpu_map(void)
+static struct cpu_map *cpu_map__trim_new(int nr_cpus, int *tmp_cpus)
 {
+	size_t payload_size = nr_cpus * sizeof(int);
+	struct cpu_map *cpus = malloc(sizeof(*cpus) + payload_size);
+
+	if (cpus != NULL) {
+		cpus->nr = nr_cpus;
+		memcpy(cpus->map, tmp_cpus, payload_size);
+	}
+
+	return cpus;
+}
+
+static struct cpu_map *cpu_map__read_all_cpu_map(void)
+{
+	struct cpu_map *cpus = NULL;
 	FILE *onlnf;
 	int nr_cpus = 0;
+	int *tmp_cpus = NULL, *tmp;
+	int max_entries = 0;
 	int n, cpu, prev;
 	char sep;
 
 	onlnf = fopen("/sys/devices/system/cpu/online", "r");
 	if (!onlnf)
-		return default_cpu_map();
+		return cpu_map__default_new();
 
 	sep = 0;
 	prev = -1;
@@ -38,12 +59,28 @@ static int read_all_cpu_map(void)
 		if (n <= 0)
 			break;
 		if (prev >= 0) {
-			assert(nr_cpus + cpu - prev - 1 < MAX_NR_CPUS);
+			int new_max = nr_cpus + cpu - prev - 1;
+
+			if (new_max >= max_entries) {
+				max_entries = new_max + MAX_NR_CPUS / 2;
+				tmp = realloc(tmp_cpus, max_entries * sizeof(int));
+				if (tmp == NULL)
+					goto out_free_tmp;
+				tmp_cpus = tmp;
+			}
+
 			while (++prev < cpu)
-				cpumap[nr_cpus++] = prev;
+				tmp_cpus[nr_cpus++] = prev;
+		}
+		if (nr_cpus == max_entries) {
+			max_entries += MAX_NR_CPUS;
+			tmp = realloc(tmp_cpus, max_entries * sizeof(int));
+			if (tmp == NULL)
+				goto out_free_tmp;
+			tmp_cpus = tmp;
 		}
-		assert (nr_cpus < MAX_NR_CPUS);
-		cpumap[nr_cpus++] = cpu;
+
+		tmp_cpus[nr_cpus++] = cpu;
 		if (n == 2 && sep == '-')
 			prev = cpu;
 		else
@@ -51,24 +88,31 @@ static int read_all_cpu_map(void)
 		if (n == 1 || sep == '\n')
 			break;
 	}
-	fclose(onlnf);
-	if (nr_cpus > 0)
-		return nr_cpus;
 
-	return default_cpu_map();
+	if (nr_cpus > 0)
+		cpus = cpu_map__trim_new(nr_cpus, tmp_cpus);
+	else
+		cpus = cpu_map__default_new();
+out_free_tmp:
+	free(tmp_cpus);
+	fclose(onlnf);
+	return cpus;
 }
 
-int read_cpu_map(const char *cpu_list)
+struct cpu_map *cpu_map__new(const char *cpu_list)
 {
+	struct cpu_map *cpus = NULL;
 	unsigned long start_cpu, end_cpu = 0;
 	char *p = NULL;
 	int i, nr_cpus = 0;
+	int *tmp_cpus = NULL, *tmp;
+	int max_entries = 0;
 
 	if (!cpu_list)
-		return read_all_cpu_map();
+		return cpu_map__read_all_cpu_map();
 
 	if (!isdigit(*cpu_list))
-		goto invalid;
+		goto out;
 
 	while (isdigit(*cpu_list)) {
 		p = NULL;
@@ -94,21 +138,42 @@ int read_cpu_map(const char *cpu_list)
 		for (; start_cpu <= end_cpu; start_cpu++) {
 			/* check for duplicates */
 			for (i = 0; i < nr_cpus; i++)
-				if (cpumap[i] == (int)start_cpu)
+				if (tmp_cpus[i] == (int)start_cpu)
 					goto invalid;
 
-			assert(nr_cpus < MAX_NR_CPUS);
-			cpumap[nr_cpus++] = (int)start_cpu;
+			if (nr_cpus == max_entries) {
+				max_entries += MAX_NR_CPUS;
+				tmp = realloc(tmp_cpus, max_entries * sizeof(int));
+				if (tmp == NULL)
+					goto invalid;
+				tmp_cpus = tmp;
+			}
+			tmp_cpus[nr_cpus++] = (int)start_cpu;
 		}
 		if (*p)
 			++p;
 
 		cpu_list = p;
 	}
-	if (nr_cpus > 0)
-		return nr_cpus;
 
-	return default_cpu_map();
+	if (nr_cpus > 0)
+		cpus = cpu_map__trim_new(nr_cpus, tmp_cpus);
+	else
+		cpus = cpu_map__default_new();
 invalid:
-	return -1;
+	free(tmp_cpus);
+out:
+	return cpus;
+}
+
+struct cpu_map *cpu_map__dummy_new(void)
+{
+	struct cpu_map *cpus = malloc(sizeof(*cpus) + sizeof(int));
+
+	if (cpus != NULL) {
+		cpus->nr = 1;
+		cpus->map[0] = -1;
+	}
+
+	return cpus;
 }
diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h
index 3e60f56..f7a4f42 100644
--- a/tools/perf/util/cpumap.h
+++ b/tools/perf/util/cpumap.h
@@ -1,7 +1,13 @@
 #ifndef __PERF_CPUMAP_H
 #define __PERF_CPUMAP_H
 
-extern int read_cpu_map(const char *cpu_list);
-extern int cpumap[];
+struct cpu_map {
+	int nr;
+	int map[];
+};
+
+struct cpu_map *cpu_map__new(const char *cpu_list);
+struct cpu_map *cpu_map__dummy_new(void);
+void *cpu_map__delete(struct cpu_map *map);
 
 #endif /* __PERF_CPUMAP_H */
-- 
1.6.2.5


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

* [PATCH 08/11] perf tools: Refactor all_tids to hold nr and the map
  2011-01-04  3:48 [GIT PULL||RFC 00/11] perf library and regression testing improvements Arnaldo Carvalho de Melo
                   ` (6 preceding siblings ...)
  2011-01-04  3:48 ` [PATCH 07/11] perf tools: Refactor cpumap to hold nr and the map Arnaldo Carvalho de Melo
@ 2011-01-04  3:48 ` Arnaldo Carvalho de Melo
  2011-01-04  3:48 ` [PATCH 09/11] perf evsel: Use {cpu,thread}_map to shorten list of parameters Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04  3:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Tom Zanussi, Thomas Gleixner

From: Arnaldo Carvalho de Melo <acme@redhat.com>

So that later, we can pass the thread_map instance instead of
(thread_num, thread_map) for things like perf_evsel__open and friends,
just like was done with cpu_map.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c |   39 +++++++++++++++------------------------
 tools/perf/builtin-stat.c   |   41 +++++++++++++++++------------------------
 tools/perf/builtin-top.c    |   35 +++++++++++++----------------------
 tools/perf/util/thread.c    |   43 +++++++++++++++++++++++++++++--------------
 tools/perf/util/thread.h    |   15 ++++++++++++++-
 5 files changed, 88 insertions(+), 85 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 220e6e7..7bc0490 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -54,8 +54,7 @@ static bool			sample_id_all_avail		=   true;
 static bool			system_wide			=  false;
 static pid_t			target_pid			=     -1;
 static pid_t			target_tid			=     -1;
-static pid_t			*all_tids			=      NULL;
-static int			thread_num			=      0;
+static struct thread_map	*threads;
 static pid_t			child_pid			=     -1;
 static bool			no_inherit			=  false;
 static enum write_mode_t	write_mode			= WRITE_FORCE;
@@ -318,9 +317,9 @@ static void create_counter(struct perf_evsel *evsel, int cpu)
 retry_sample_id:
 	attr->sample_id_all = sample_id_all_avail ? 1 : 0;
 
-	for (thread_index = 0; thread_index < thread_num; thread_index++) {
+	for (thread_index = 0; thread_index < threads->nr; thread_index++) {
 try_again:
-		FD(evsel, nr_cpu, thread_index) = sys_perf_event_open(attr, all_tids[thread_index], cpu, group_fd, 0);
+		FD(evsel, nr_cpu, thread_index) = sys_perf_event_open(attr, threads->map[thread_index], cpu, group_fd, 0);
 
 		if (FD(evsel, nr_cpu, thread_index) < 0) {
 			int err = errno;
@@ -653,7 +652,7 @@ static int __cmd_record(int argc, const char **argv)
 		}
 
 		if (!system_wide && target_tid == -1 && target_pid == -1)
-			all_tids[0] = child_pid;
+			threads->map[0] = child_pid;
 
 		close(child_ready_pipe[1]);
 		close(go_pipe[0]);
@@ -793,7 +792,7 @@ static int __cmd_record(int argc, const char **argv)
 
 				list_for_each_entry(pos, &evsel_list, node) {
 					for (thread = 0;
-						thread < thread_num;
+						thread < threads->nr;
 						thread++)
 						ioctl(FD(pos, i, thread),
 							PERF_EVENT_IOC_DISABLE);
@@ -910,21 +909,13 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		goto out_symbol_exit;
 	}
 
-	if (target_pid != -1) {
+	if (target_pid != -1)
 		target_tid = target_pid;
-		thread_num = find_all_tid(target_pid, &all_tids);
-		if (thread_num <= 0) {
-			fprintf(stderr, "Can't find all threads of pid %d\n",
-					target_pid);
-			usage_with_options(record_usage, record_options);
-		}
-	} else {
-		all_tids=malloc(sizeof(pid_t));
-		if (!all_tids)
-			goto out_symbol_exit;
 
-		all_tids[0] = target_tid;
-		thread_num = 1;
+	threads = thread_map__new(target_pid, target_tid);
+	if (threads == NULL) {
+		pr_err("Problems finding threads of monitor\n");
+		usage_with_options(record_usage, record_options);
 	}
 
 	cpus = cpu_map__new(cpu_list);
@@ -934,11 +925,11 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 	}
 
 	list_for_each_entry(pos, &evsel_list, node) {
-		if (perf_evsel__alloc_fd(pos, cpus->nr, thread_num) < 0)
+		if (perf_evsel__alloc_fd(pos, cpus->nr, threads->nr) < 0)
 			goto out_free_fd;
 	}
-	event_array = malloc(
-		sizeof(struct pollfd)*MAX_NR_CPUS*MAX_COUNTERS*thread_num);
+	event_array = malloc((sizeof(struct pollfd) * MAX_NR_CPUS *
+			      MAX_COUNTERS * threads->nr));
 	if (!event_array)
 		goto out_free_fd;
 
@@ -965,8 +956,8 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 out_free_event_array:
 	free(event_array);
 out_free_fd:
-	free(all_tids);
-	all_tids = NULL;
+	thread_map__delete(threads);
+	threads = NULL;
 out_symbol_exit:
 	symbol__exit();
 	return err;
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 3f4a431..6b9146c 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -81,8 +81,7 @@ static bool			scale				=  true;
 static bool			no_aggr				= false;
 static pid_t			target_pid			= -1;
 static pid_t			target_tid			= -1;
-static pid_t			*all_tids			=  NULL;
-static int			thread_num			=  0;
+static struct thread_map	*threads;
 static pid_t			child_pid			= -1;
 static bool			null_run			=  false;
 static bool			big_num				=  true;
@@ -175,7 +174,7 @@ static int create_perf_stat_counter(struct perf_evsel *evsel)
 		attr->enable_on_exec = 1;
 	}
 
-	return perf_evsel__open_per_thread(evsel, thread_num, all_tids);
+	return perf_evsel__open_per_thread(evsel, threads->nr, threads->map);
 }
 
 /*
@@ -200,7 +199,7 @@ static int read_counter_aggr(struct perf_evsel *counter)
 	u64 *count = counter->counts->aggr.values;
 	int i;
 
-	if (__perf_evsel__read(counter, cpus->nr, thread_num, scale) < 0)
+	if (__perf_evsel__read(counter, cpus->nr, threads->nr, scale) < 0)
 		return -1;
 
 	for (i = 0; i < 3; i++)
@@ -298,7 +297,7 @@ static int run_perf_stat(int argc __used, const char **argv)
 		}
 
 		if (target_tid == -1 && target_pid == -1 && !system_wide)
-			all_tids[0] = child_pid;
+			threads->map[0] = child_pid;
 
 		/*
 		 * Wait for the child to be ready to exec.
@@ -353,7 +352,7 @@ static int run_perf_stat(int argc __used, const char **argv)
 	} else {
 		list_for_each_entry(counter, &evsel_list, node) {
 			read_counter_aggr(counter);
-			perf_evsel__close_fd(counter, cpus->nr, thread_num);
+			perf_evsel__close_fd(counter, cpus->nr, threads->nr);
 		}
 	}
 
@@ -693,6 +692,15 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 		}
 	}
 
+	if (target_pid != -1)
+		target_tid = target_pid;
+
+	threads = thread_map__new(target_pid, target_tid);
+	if (threads == NULL) {
+		pr_err("Problems finding threads of monitor\n");
+		usage_with_options(stat_usage, options);
+	}
+
 	if (system_wide)
 		cpus = cpu_map__new(cpu_list);
 	else
@@ -704,27 +712,10 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 		return -1;
 	}
 
-	if (target_pid != -1) {
-		target_tid = target_pid;
-		thread_num = find_all_tid(target_pid, &all_tids);
-		if (thread_num <= 0) {
-			fprintf(stderr, "Can't find all threads of pid %d\n",
-					target_pid);
-			usage_with_options(stat_usage, options);
-		}
-	} else {
-		all_tids=malloc(sizeof(pid_t));
-		if (!all_tids)
-			return -ENOMEM;
-
-		all_tids[0] = target_tid;
-		thread_num = 1;
-	}
-
 	list_for_each_entry(pos, &evsel_list, node) {
 		if (perf_evsel__alloc_stat_priv(pos) < 0 ||
 		    perf_evsel__alloc_counts(pos, cpus->nr) < 0 ||
-		    perf_evsel__alloc_fd(pos, cpus->nr, thread_num) < 0)
+		    perf_evsel__alloc_fd(pos, cpus->nr, threads->nr) < 0)
 			goto out_free_fd;
 	}
 
@@ -752,5 +743,7 @@ out_free_fd:
 	list_for_each_entry(pos, &evsel_list, node)
 		perf_evsel__free_stat_priv(pos);
 out:
+	thread_map__delete(threads);
+	threads = NULL;
 	return status;
 }
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 0e42666..1e67ab9 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -68,8 +68,7 @@ static int			print_entries;
 
 static int			target_pid			=     -1;
 static int			target_tid			=     -1;
-static pid_t			*all_tids			=      NULL;
-static int			thread_num			=      0;
+static struct thread_map	*threads;
 static bool			inherit				=  false;
 static struct cpu_map		*cpus;
 static int			realtime_prio			=      0;
@@ -1200,7 +1199,7 @@ static void perf_session__mmap_read(struct perf_session *self)
 	for (i = 0; i < cpus->nr; i++) {
 		list_for_each_entry(counter, &evsel_list, node) {
 			for (thread_index = 0;
-				thread_index < thread_num;
+				thread_index < threads->nr;
 				thread_index++) {
 				perf_session__mmap_read_counter(self,
 					counter, i, thread_index);
@@ -1236,10 +1235,10 @@ static void start_counter(int i, struct perf_evsel *evsel)
 	attr->inherit		= (cpu < 0) && inherit;
 	attr->mmap		= 1;
 
-	for (thread_index = 0; thread_index < thread_num; thread_index++) {
+	for (thread_index = 0; thread_index < threads->nr; thread_index++) {
 try_again:
 		FD(evsel, i, thread_index) = sys_perf_event_open(attr,
-				all_tids[thread_index], cpu, group_fd, 0);
+				threads->map[thread_index], cpu, group_fd, 0);
 
 		if (FD(evsel, i, thread_index) < 0) {
 			int err = errno;
@@ -1410,25 +1409,17 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 	if (argc)
 		usage_with_options(top_usage, options);
 
-	if (target_pid != -1) {
+	if (target_pid != -1)
 		target_tid = target_pid;
-		thread_num = find_all_tid(target_pid, &all_tids);
-		if (thread_num <= 0) {
-			fprintf(stderr, "Can't find all threads of pid %d\n",
-				target_pid);
-			usage_with_options(top_usage, options);
-		}
-	} else {
-		all_tids=malloc(sizeof(pid_t));
-		if (!all_tids)
-			return -ENOMEM;
 
-		all_tids[0] = target_tid;
-		thread_num = 1;
+	threads = thread_map__new(target_pid, target_tid);
+	if (threads == NULL) {
+		pr_err("Problems finding threads of monitor\n");
+		usage_with_options(top_usage, options);
 	}
 
-	event_array = malloc(
-		sizeof(struct pollfd)*MAX_NR_CPUS*MAX_COUNTERS*thread_num);
+	event_array = malloc((sizeof(struct pollfd) *
+			      MAX_NR_CPUS * MAX_COUNTERS * threads->nr));
 	if (!event_array)
 		return -ENOMEM;
 
@@ -1468,8 +1459,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 		usage_with_options(top_usage, options);
 
 	list_for_each_entry(pos, &evsel_list, node) {
-		if (perf_evsel__alloc_mmap_per_thread(pos, cpus->nr, thread_num) < 0 ||
-		    perf_evsel__alloc_fd(pos, cpus->nr, thread_num) < 0)
+		if (perf_evsel__alloc_mmap_per_thread(pos, cpus->nr, threads->nr) < 0 ||
+		    perf_evsel__alloc_fd(pos, cpus->nr, threads->nr) < 0)
 			goto out_free_fd;
 		/*
 		 * Fill in the ones not specifically initialized via -c:
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 8c72d88..00f4ead 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -16,35 +16,50 @@ static int filter(const struct dirent *dir)
 		return 1;
 }
 
-int find_all_tid(int pid, pid_t ** all_tid)
+struct thread_map *thread_map__new_by_pid(pid_t pid)
 {
+	struct thread_map *threads;
 	char name[256];
 	int items;
 	struct dirent **namelist = NULL;
-	int ret = 0;
 	int i;
 
 	sprintf(name, "/proc/%d/task", pid);
 	items = scandir(name, &namelist, filter, NULL);
 	if (items <= 0)
-                return -ENOENT;
-	*all_tid = malloc(sizeof(pid_t) * items);
-	if (!*all_tid) {
-		ret = -ENOMEM;
-		goto failure;
-	}
-
-	for (i = 0; i < items; i++)
-		(*all_tid)[i] = atoi(namelist[i]->d_name);
+                return NULL;
 
-	ret = items;
+	threads = malloc(sizeof(*threads) + sizeof(pid_t) * items);
+	if (threads != NULL) {
+		for (i = 0; i < items; i++)
+			threads->map[i] = atoi(namelist[i]->d_name);
+		threads->nr = items;
+	}
 
-failure:
 	for (i=0; i<items; i++)
 		free(namelist[i]);
 	free(namelist);
 
-	return ret;
+	return threads;
+}
+
+struct thread_map *thread_map__new_by_tid(pid_t tid)
+{
+	struct thread_map *threads = malloc(sizeof(*threads) + sizeof(pid_t));
+
+	if (threads != NULL) {
+		threads->map[0] = tid;
+		threads->nr	= 1;
+	}
+
+	return threads;
+}
+
+struct thread_map *thread_map__new(pid_t pid, pid_t tid)
+{
+	if (pid != -1)
+		return thread_map__new_by_pid(pid);
+	return thread_map__new_by_tid(tid);
 }
 
 static struct thread *thread__new(pid_t pid)
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 688500f..d757410 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -18,11 +18,24 @@ struct thread {
 	int			comm_len;
 };
 
+struct thread_map {
+	int nr;
+	int map[];
+};
+
 struct perf_session;
 
 void thread__delete(struct thread *self);
 
-int find_all_tid(int pid, pid_t ** all_tid);
+struct thread_map *thread_map__new_by_pid(pid_t pid);
+struct thread_map *thread_map__new_by_tid(pid_t tid);
+struct thread_map *thread_map__new(pid_t pid, pid_t tid);
+
+static inline void thread_map__delete(struct thread_map *threads)
+{
+	free(threads);
+}
+
 int thread__set_comm(struct thread *self, const char *comm);
 int thread__comm_len(struct thread *self);
 struct thread *perf_session__findnew(struct perf_session *self, pid_t pid);
-- 
1.6.2.5


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

* [PATCH 09/11] perf evsel: Use {cpu,thread}_map to shorten list of parameters
  2011-01-04  3:48 [GIT PULL||RFC 00/11] perf library and regression testing improvements Arnaldo Carvalho de Melo
                   ` (7 preceding siblings ...)
  2011-01-04  3:48 ` [PATCH 08/11] perf tools: Refactor all_tids " Arnaldo Carvalho de Melo
@ 2011-01-04  3:48 ` Arnaldo Carvalho de Melo
  2011-01-04  3:48 ` [PATCH 10/11] perf evsel: Auto allocate resources needed for some methods Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04  3:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Tom Zanussi, Thomas Gleixner

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c |    4 ++--
 tools/perf/util/evsel.c   |   24 +++++++++++++-----------
 tools/perf/util/evsel.h   |   11 +++++++----
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 6b9146c..02b2d80 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -166,7 +166,7 @@ static int create_perf_stat_counter(struct perf_evsel *evsel)
 				    PERF_FORMAT_TOTAL_TIME_RUNNING;
 
 	if (system_wide)
-		return perf_evsel__open_per_cpu(evsel, cpus->nr, cpus->map);
+		return perf_evsel__open_per_cpu(evsel, cpus);
 
 	attr->inherit = !no_inherit;
 	if (target_pid == -1 && target_tid == -1) {
@@ -174,7 +174,7 @@ static int create_perf_stat_counter(struct perf_evsel *evsel)
 		attr->enable_on_exec = 1;
 	}
 
-	return perf_evsel__open_per_thread(evsel, threads->nr, threads->map);
+	return perf_evsel__open_per_thread(evsel, threads);
 }
 
 /*
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index e62cc5e..e44be52 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1,6 +1,8 @@
 #include "evsel.h"
 #include "../perf.h"
 #include "util.h"
+#include "cpumap.h"
+#include "thread.h"
 
 #define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
 
@@ -123,13 +125,13 @@ int __perf_evsel__read(struct perf_evsel *evsel,
 	return 0;
 }
 
-int perf_evsel__open_per_cpu(struct perf_evsel *evsel, int ncpus, int *cpu_map)
+int perf_evsel__open_per_cpu(struct perf_evsel *evsel, struct cpu_map *cpus)
 {
 	int cpu;
 
-	for (cpu = 0; cpu < ncpus; cpu++) {
+	for (cpu = 0; cpu < cpus->nr; cpu++) {
 		FD(evsel, cpu, 0) = sys_perf_event_open(&evsel->attr, -1,
-							cpu_map[cpu], -1, 0);
+							cpus->map[cpu], -1, 0);
 		if (FD(evsel, cpu, 0) < 0)
 			goto out_close;
 	}
@@ -144,13 +146,13 @@ out_close:
 	return -1;
 }
 
-int perf_evsel__open_per_thread(struct perf_evsel *evsel, int nthreads, int *thread_map)
+int perf_evsel__open_per_thread(struct perf_evsel *evsel, struct thread_map *threads)
 {
 	int thread;
 
-	for (thread = 0; thread < nthreads; thread++) {
+	for (thread = 0; thread < threads->nr; thread++) {
 		FD(evsel, 0, thread) = sys_perf_event_open(&evsel->attr,
-							   thread_map[thread], -1, -1, 0);
+							   threads->map[thread], -1, -1, 0);
 		if (FD(evsel, 0, thread) < 0)
 			goto out_close;
 	}
@@ -165,11 +167,11 @@ out_close:
 	return -1;
 }
 
-int perf_evsel__open(struct perf_evsel *evsel, int ncpus, int nthreads,
-		     int *cpu_map, int *thread_map)
+int perf_evsel__open(struct perf_evsel *evsel, 
+		     struct cpu_map *cpus, struct thread_map *threads)
 {
-	if (nthreads < 0)
-		return perf_evsel__open_per_cpu(evsel, ncpus, cpu_map);
+	if (threads == NULL)
+		return perf_evsel__open_per_cpu(evsel, cpus);
 
-	return perf_evsel__open_per_thread(evsel, nthreads, thread_map);
+	return perf_evsel__open_per_thread(evsel, threads);
 }
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index a62fb55..863d78d 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -34,6 +34,9 @@ struct perf_evsel {
 	void			*priv;
 };
 
+struct cpu_map;
+struct thread_map;
+
 struct perf_evsel *perf_evsel__new(u32 type, u64 config, int idx);
 void perf_evsel__delete(struct perf_evsel *evsel);
 
@@ -42,10 +45,10 @@ int perf_evsel__alloc_counts(struct perf_evsel *evsel, int ncpus);
 void perf_evsel__free_fd(struct perf_evsel *evsel);
 void perf_evsel__close_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
 
-int perf_evsel__open_per_cpu(struct perf_evsel *evsel, int ncpus, int *cpu_map);
-int perf_evsel__open_per_thread(struct perf_evsel *evsel, int nthreads, int *thread_map);
-int perf_evsel__open(struct perf_evsel *evsel, int ncpus, int nthreads,
-		     int *cpu_map, int *thread_map);
+int perf_evsel__open_per_cpu(struct perf_evsel *evsel, struct cpu_map *cpus);
+int perf_evsel__open_per_thread(struct perf_evsel *evsel, struct thread_map *threads);
+int perf_evsel__open(struct perf_evsel *evsel, 
+		     struct cpu_map *cpus, struct thread_map *threads);
 
 #define perf_evsel__match(evsel, t, c)		\
 	(evsel->attr.type == PERF_TYPE_##t &&	\
-- 
1.6.2.5


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

* [PATCH 10/11] perf evsel: Auto allocate resources needed for some methods
  2011-01-04  3:48 [GIT PULL||RFC 00/11] perf library and regression testing improvements Arnaldo Carvalho de Melo
                   ` (8 preceding siblings ...)
  2011-01-04  3:48 ` [PATCH 09/11] perf evsel: Use {cpu,thread}_map to shorten list of parameters Arnaldo Carvalho de Melo
@ 2011-01-04  3:48 ` Arnaldo Carvalho de Melo
  2011-01-04  3:48 ` [PATCH 11/11] perf test: Add test for counting open syscalls Arnaldo Carvalho de Melo
  2011-01-04  7:16 ` [GIT PULL||RFC 00/11] perf library and regression testing improvements Ingo Molnar
  11 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04  3:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Tom Zanussi, Thomas Gleixner

From: Arnaldo Carvalho de Melo <acme@redhat.com>

While writing the first user of the routines created from the ad-hoc
routines in the existing builtins I noticed that the resulting set of
calls was too long, reduce it by doing some best effort allocations.

Tools that need to operate on multiple threads and cpus should pre-allocate
enough resources by explicitely calling the perf_evsel__alloc_{fd,counters}
methods.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index e44be52..c95267e 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -66,6 +66,9 @@ int __perf_evsel__read_on_cpu(struct perf_evsel *evsel,
 	if (FD(evsel, cpu, thread) < 0)
 		return -EINVAL;
 
+	if (evsel->counts == NULL && perf_evsel__alloc_counts(evsel, cpu + 1) < 0)
+		return -ENOMEM;
+
 	if (readn(FD(evsel, cpu, thread), &count, nv * sizeof(u64)) < 0)
 		return -errno;
 
@@ -129,6 +132,9 @@ int perf_evsel__open_per_cpu(struct perf_evsel *evsel, struct cpu_map *cpus)
 {
 	int cpu;
 
+	if (evsel->fd == NULL && perf_evsel__alloc_fd(evsel, cpus->nr, 1) < 0)
+		return -1;
+
 	for (cpu = 0; cpu < cpus->nr; cpu++) {
 		FD(evsel, cpu, 0) = sys_perf_event_open(&evsel->attr, -1,
 							cpus->map[cpu], -1, 0);
@@ -150,6 +156,9 @@ int perf_evsel__open_per_thread(struct perf_evsel *evsel, struct thread_map *thr
 {
 	int thread;
 
+	if (evsel->fd == NULL && perf_evsel__alloc_fd(evsel, 1, threads->nr))
+		return -1;
+
 	for (thread = 0; thread < threads->nr; thread++) {
 		FD(evsel, 0, thread) = sys_perf_event_open(&evsel->attr,
 							   threads->map[thread], -1, -1, 0);
-- 
1.6.2.5


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

* [PATCH 11/11] perf test: Add test for counting open syscalls
  2011-01-04  3:48 [GIT PULL||RFC 00/11] perf library and regression testing improvements Arnaldo Carvalho de Melo
                   ` (9 preceding siblings ...)
  2011-01-04  3:48 ` [PATCH 10/11] perf evsel: Auto allocate resources needed for some methods Arnaldo Carvalho de Melo
@ 2011-01-04  3:48 ` Arnaldo Carvalho de Melo
  2011-01-04  7:16 ` [GIT PULL||RFC 00/11] perf library and regression testing improvements Ingo Molnar
  11 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04  3:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
	Han Pingtian, Ingo Molnar, Mike Galbraith, Paul Mackerras,
	Peter Zijlstra, Stephane Eranian, Tom Zanussi, Thomas Gleixner

From: Arnaldo Carvalho de Melo <acme@redhat.com>

To test the use of the perf_evsel class on something other than
the tools from where we refactored code to create it.

It calls open() N times and then checks if the event created to
monitor it returns N events.

[acme@felicio linux]$ perf test
 1: vmlinux symtab matches kallsyms: Ok
 2: detect open syscall event: Ok
[acme@felicio linux]$

It does.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Han Pingtian <phan@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-test.c |   83 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 83 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index e0c3f47..6c99152 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -234,6 +234,85 @@ out:
 	return err;
 }
 
+#include "util/evsel.h"
+#include <sys/types.h>
+
+static int trace_event__id(const char *event_name)
+{
+	char *filename;
+	int err = -1, fd;
+
+	if (asprintf(&filename,
+		     "/sys/kernel/debug/tracing/events/syscalls/%s/id",
+		     event_name) < 0)
+		return -1;
+
+	fd = open(filename, O_RDONLY);
+	if (fd >= 0) {
+		char id[16];
+		if (read(fd, id, sizeof(id)) > 0)
+			err = atoi(id);
+		close(fd);
+	}
+
+	free(filename);
+	return err;
+}
+
+static int test__open_syscall_event(void)
+{
+	int err = -1, fd;
+	struct thread_map *threads;
+	struct perf_evsel *evsel;
+	unsigned int nr_open_calls = 111, i;
+	int id = trace_event__id("sys_enter_open");
+
+	if (id < 0) {
+		pr_debug("trace_event__id(\"sys_enter_open\") ");
+		return -1;
+	}
+
+	threads = thread_map__new(-1, getpid());
+	if (threads == NULL) {
+		pr_debug("thread_map__new ");
+		return -1;
+	}
+
+	evsel = perf_evsel__new(PERF_TYPE_TRACEPOINT, id, 0);
+	if (evsel == NULL) {
+		pr_debug("perf_evsel__new ");
+		goto out_thread_map_delete;
+	}
+
+	if (perf_evsel__open_per_thread(evsel, threads) < 0) {
+		pr_debug("perf_evsel__open_per_thread ");
+		goto out_evsel_delete;
+	}
+
+	for (i = 0; i < nr_open_calls; ++i) {
+		fd = open("/etc/passwd", O_RDONLY);
+		close(fd);
+	}
+
+	if (perf_evsel__read_on_cpu(evsel, 0, 0) < 0) {
+		pr_debug("perf_evsel__open_read_on_cpu ");
+		goto out_close_fd;
+	}
+
+	if (evsel->counts->cpu[0].val != nr_open_calls)
+		pr_debug("perf_evsel__read_on_cpu: expected to intercept %d calls, got %Ld ",
+			 nr_open_calls, evsel->counts->cpu[0].val);
+	
+	err = 0;
+out_close_fd:
+	perf_evsel__close_fd(evsel, 1, threads->nr);
+out_evsel_delete:
+	perf_evsel__delete(evsel);
+out_thread_map_delete:
+	thread_map__delete(threads);
+	return err;
+}
+
 static struct test {
 	const char *desc;
 	int (*func)(void);
@@ -243,6 +322,10 @@ static struct test {
 		.func = test__vmlinux_matches_kallsyms,
 	},
 	{
+		.desc = "detect open syscall event",
+		.func = test__open_syscall_event,
+	},
+	{
 		.func = NULL,
 	},
 };
-- 
1.6.2.5


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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04  3:48 [GIT PULL||RFC 00/11] perf library and regression testing improvements Arnaldo Carvalho de Melo
                   ` (10 preceding siblings ...)
  2011-01-04  3:48 ` [PATCH 11/11] perf test: Add test for counting open syscalls Arnaldo Carvalho de Melo
@ 2011-01-04  7:16 ` Ingo Molnar
  2011-01-04 13:59   ` Stephane Eranian
  11 siblings, 1 reply; 30+ messages in thread
From: Ingo Molnar @ 2011-01-04  7:16 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian, Tom Zanussi,
	Thomas Gleixner, Arnaldo Carvalho de Melo


* Arnaldo Carvalho de Melo <acme@infradead.org> wrote:

> Hi Ingo, Peter et al,
> 
> 	Trying to simplify using the API for tools and more specifically for
> 'perf test' regression tests, please take a look, perhaps starting from the last
> changeset, that implements a regression test using the resulting library API.
> 
> 	It also reduces the 'perf' tool footprint by not using hard coded array
> sizes, more need to be done, but should be a good start, one changeset shows a
> good reduction in BSS use.
> 
> 	Suggestions for naming most welcome, I thought about using "event__",
> but that is taken, "perf_event__", but thought it would clash with "event_t",
> so used the jargon used for the '-e' parameters: "Event Selector", but don't
> like it that much, what do you think?
> 
> 	Writing the first regression test I think there are more ways to simplify,
> on top of these, like not requiring a thread_map, i.e. passing NULL for that
> parameter would mean: self-monitor, etc.
> 
>         If you are pleased as-is, please pull from:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/test
> 
> Regards,
> 
> - Arnaldo
> 
> Arnaldo Carvalho de Melo (11):
>   perf tools: Introduce event selectors
>   perf evsel: Adopt MATCH_EVENT macro from 'stat'
>   perf util: Move do_read from session to util
>   perf evsel: Delete the event selectors at exit
>   perf evsel: Steal the counter reading routines from stat
>   perf evsel: Introduce per cpu and per thread open helpers
>   perf tools: Refactor cpumap to hold nr and the map
>   perf tools: Refactor all_tids to hold nr and the map
>   perf evsel: Use {cpu,thread}_map to shorten list of parameters
>   perf evsel: Auto allocate resources needed for some methods
>   perf test: Add test for counting open syscalls
> 
>  tools/perf/Makefile                |    4 +
>  tools/perf/builtin-record.c        |  152 +++++++--------
>  tools/perf/builtin-stat.c          |  368 +++++++++++++++---------------------
>  tools/perf/builtin-test.c          |   83 ++++++++
>  tools/perf/builtin-top.c           |  221 ++++++++++++----------
>  tools/perf/perf.c                  |    2 +
>  tools/perf/util/cpumap.c           |  123 +++++++++---
>  tools/perf/util/cpumap.h           |   10 +-
>  tools/perf/util/evsel.c            |  186 ++++++++++++++++++
>  tools/perf/util/evsel.h            |  115 +++++++++++
>  tools/perf/util/header.c           |   15 +-
>  tools/perf/util/header.h           |    3 +-
>  tools/perf/util/parse-events.c     |   58 ++++--
>  tools/perf/util/parse-events.h     |   18 ++-
>  tools/perf/util/session.c          |   22 +--
>  tools/perf/util/session.h          |    1 -
>  tools/perf/util/thread.c           |   43 +++--
>  tools/perf/util/thread.h           |   15 ++-
>  tools/perf/util/trace-event-info.c |   30 ++--
>  tools/perf/util/trace-event.h      |    5 +-
>  tools/perf/util/util.c             |   17 ++
>  tools/perf/util/util.h             |    1 +
>  tools/perf/util/xyarray.c          |   20 ++
>  tools/perf/util/xyarray.h          |   20 ++
>  24 files changed, 1013 insertions(+), 519 deletions(-)
>  create mode 100644 tools/perf/util/evsel.c
>  create mode 100644 tools/perf/util/evsel.h
>  create mode 100644 tools/perf/util/xyarray.c
>  create mode 100644 tools/perf/util/xyarray.h

Pulled, thanks a lot Arnaldo!

	Ingo

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04  7:16 ` [GIT PULL||RFC 00/11] perf library and regression testing improvements Ingo Molnar
@ 2011-01-04 13:59   ` Stephane Eranian
  2011-01-04 14:03     ` Arnaldo Carvalho de Melo
  2011-01-04 14:12     ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 30+ messages in thread
From: Stephane Eranian @ 2011-01-04 13:59 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	Arnaldo Carvalho de Melo, mingo

Arnaldo,

The version of perf at tip-x86 commit 9274b36, segfaults for me:

$ gdb perf
(gdb) r stat date

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fdc602eb6e0 (LWP 4590)]
cmd_stat (argc=1, argv=0x7fff6fa2a9f0, prefix=<value optimized out>)
at builtin-stat.c:206
206			update_stats(&ps->res_stats[i], count[i]);
(gdb) bt
#0  cmd_stat (argc=1, argv=0x7fff6fa2a9f0, prefix=<value optimized
out>) at builtin-stat.c:206
#1  0x0000000000405c8b in handle_internal_command (argc=2,
argv=0x7fff6fa2a9f0) at perf.c:286
#2  0x00000000004060b0 in main (argc=2, argv=0x7fff6fa2a680) at perf.c:403

Most like ps is NULL.


On Tue, Jan 4, 2011 at 8:16 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
>
>> Hi Ingo, Peter et al,
>>
>>       Trying to simplify using the API for tools and more specifically for
>> 'perf test' regression tests, please take a look, perhaps starting from the last
>> changeset, that implements a regression test using the resulting library API.
>>
>>       It also reduces the 'perf' tool footprint by not using hard coded array
>> sizes, more need to be done, but should be a good start, one changeset shows a
>> good reduction in BSS use.
>>
>>       Suggestions for naming most welcome, I thought about using "event__",
>> but that is taken, "perf_event__", but thought it would clash with "event_t",
>> so used the jargon used for the '-e' parameters: "Event Selector", but don't
>> like it that much, what do you think?
>>
>>       Writing the first regression test I think there are more ways to simplify,
>> on top of these, like not requiring a thread_map, i.e. passing NULL for that
>> parameter would mean: self-monitor, etc.
>>
>>         If you are pleased as-is, please pull from:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/test
>>
>> Regards,
>>
>> - Arnaldo
>>
>> Arnaldo Carvalho de Melo (11):
>>   perf tools: Introduce event selectors
>>   perf evsel: Adopt MATCH_EVENT macro from 'stat'
>>   perf util: Move do_read from session to util
>>   perf evsel: Delete the event selectors at exit
>>   perf evsel: Steal the counter reading routines from stat
>>   perf evsel: Introduce per cpu and per thread open helpers
>>   perf tools: Refactor cpumap to hold nr and the map
>>   perf tools: Refactor all_tids to hold nr and the map
>>   perf evsel: Use {cpu,thread}_map to shorten list of parameters
>>   perf evsel: Auto allocate resources needed for some methods
>>   perf test: Add test for counting open syscalls
>>
>>  tools/perf/Makefile                |    4 +
>>  tools/perf/builtin-record.c        |  152 +++++++--------
>>  tools/perf/builtin-stat.c          |  368 +++++++++++++++---------------------
>>  tools/perf/builtin-test.c          |   83 ++++++++
>>  tools/perf/builtin-top.c           |  221 ++++++++++++----------
>>  tools/perf/perf.c                  |    2 +
>>  tools/perf/util/cpumap.c           |  123 +++++++++---
>>  tools/perf/util/cpumap.h           |   10 +-
>>  tools/perf/util/evsel.c            |  186 ++++++++++++++++++
>>  tools/perf/util/evsel.h            |  115 +++++++++++
>>  tools/perf/util/header.c           |   15 +-
>>  tools/perf/util/header.h           |    3 +-
>>  tools/perf/util/parse-events.c     |   58 ++++--
>>  tools/perf/util/parse-events.h     |   18 ++-
>>  tools/perf/util/session.c          |   22 +--
>>  tools/perf/util/session.h          |    1 -
>>  tools/perf/util/thread.c           |   43 +++--
>>  tools/perf/util/thread.h           |   15 ++-
>>  tools/perf/util/trace-event-info.c |   30 ++--
>>  tools/perf/util/trace-event.h      |    5 +-
>>  tools/perf/util/util.c             |   17 ++
>>  tools/perf/util/util.h             |    1 +
>>  tools/perf/util/xyarray.c          |   20 ++
>>  tools/perf/util/xyarray.h          |   20 ++
>>  24 files changed, 1013 insertions(+), 519 deletions(-)
>>  create mode 100644 tools/perf/util/evsel.c
>>  create mode 100644 tools/perf/util/evsel.h
>>  create mode 100644 tools/perf/util/xyarray.c
>>  create mode 100644 tools/perf/util/xyarray.h
>
> Pulled, thanks a lot Arnaldo!
>
>        Ingo
>

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 13:59   ` Stephane Eranian
@ 2011-01-04 14:03     ` Arnaldo Carvalho de Melo
  2011-01-04 14:09       ` Stephane Eranian
  2011-01-04 14:12     ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04 14:03 UTC (permalink / raw)
  To: Stephane Eranian
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

Em Tue, Jan 04, 2011 at 02:59:00PM +0100, Stephane Eranian escreveu:
> Arnaldo,
> 
> The version of perf at tip-x86 commit 9274b36, segfaults for me:
> 
> $ gdb perf
> (gdb) r stat date
> 
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7fdc602eb6e0 (LWP 4590)]
> cmd_stat (argc=1, argv=0x7fff6fa2a9f0, prefix=<value optimized out>)
> at builtin-stat.c:206
> 206			update_stats(&ps->res_stats[i], count[i]);
> (gdb) bt
> #0  cmd_stat (argc=1, argv=0x7fff6fa2a9f0, prefix=<value optimized
> out>) at builtin-stat.c:206
> #1  0x0000000000405c8b in handle_internal_command (argc=2,
> argv=0x7fff6fa2a9f0) at perf.c:286
> #2  0x00000000004060b0 in main (argc=2, argv=0x7fff6fa2a680) at perf.c:403
> 
> Most like ps is NULL.

[acme@felicio linux]$ perf stat date | head -5
Tue Jan  4 12:03:05 BRST 2011

 Performance counter stats for 'date':

             4,525 cache-misses             #      7.640 M/sec
           343,171 cache-references         #    579.405 M/sec
            14,853 branch-misses            #      8.214 %    
           180,833 branches                 #    305.316 M/sec
           897,837 instructions             #      0.000 IPC    (scaled from 67.67%)
     <not counted> cycles                  
               201 page-faults              #      0.339 M/sec
                 1 CPU-migrations           #      0.002 M/sec
                 1 context-switches         #      0.002 M/sec
          0.592282 task-clock-msecs         #      0.583 CPUs 

        0.001015291  seconds time elapsed

[acme@felicio linux]$

Re-reading the code now, thanks!

- Arnaldo
 
> On Tue, Jan 4, 2011 at 8:16 AM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> >
> >> Hi Ingo, Peter et al,
> >>
> >>       Trying to simplify using the API for tools and more specifically for
> >> 'perf test' regression tests, please take a look, perhaps starting from the last
> >> changeset, that implements a regression test using the resulting library API.
> >>
> >>       It also reduces the 'perf' tool footprint by not using hard coded array
> >> sizes, more need to be done, but should be a good start, one changeset shows a
> >> good reduction in BSS use.
> >>
> >>       Suggestions for naming most welcome, I thought about using "event__",
> >> but that is taken, "perf_event__", but thought it would clash with "event_t",
> >> so used the jargon used for the '-e' parameters: "Event Selector", but don't
> >> like it that much, what do you think?
> >>
> >>       Writing the first regression test I think there are more ways to simplify,
> >> on top of these, like not requiring a thread_map, i.e. passing NULL for that
> >> parameter would mean: self-monitor, etc.
> >>
> >>         If you are pleased as-is, please pull from:
> >>
> >> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/test
> >>
> >> Regards,
> >>
> >> - Arnaldo
> >>
> >> Arnaldo Carvalho de Melo (11):
> >>   perf tools: Introduce event selectors
> >>   perf evsel: Adopt MATCH_EVENT macro from 'stat'
> >>   perf util: Move do_read from session to util
> >>   perf evsel: Delete the event selectors at exit
> >>   perf evsel: Steal the counter reading routines from stat
> >>   perf evsel: Introduce per cpu and per thread open helpers
> >>   perf tools: Refactor cpumap to hold nr and the map
> >>   perf tools: Refactor all_tids to hold nr and the map
> >>   perf evsel: Use {cpu,thread}_map to shorten list of parameters
> >>   perf evsel: Auto allocate resources needed for some methods
> >>   perf test: Add test for counting open syscalls
> >>
> >>  tools/perf/Makefile                |    4 +
> >>  tools/perf/builtin-record.c        |  152 +++++++--------
> >>  tools/perf/builtin-stat.c          |  368 +++++++++++++++---------------------
> >>  tools/perf/builtin-test.c          |   83 ++++++++
> >>  tools/perf/builtin-top.c           |  221 ++++++++++++----------
> >>  tools/perf/perf.c                  |    2 +
> >>  tools/perf/util/cpumap.c           |  123 +++++++++---
> >>  tools/perf/util/cpumap.h           |   10 +-
> >>  tools/perf/util/evsel.c            |  186 ++++++++++++++++++
> >>  tools/perf/util/evsel.h            |  115 +++++++++++
> >>  tools/perf/util/header.c           |   15 +-
> >>  tools/perf/util/header.h           |    3 +-
> >>  tools/perf/util/parse-events.c     |   58 ++++--
> >>  tools/perf/util/parse-events.h     |   18 ++-
> >>  tools/perf/util/session.c          |   22 +--
> >>  tools/perf/util/session.h          |    1 -
> >>  tools/perf/util/thread.c           |   43 +++--
> >>  tools/perf/util/thread.h           |   15 ++-
> >>  tools/perf/util/trace-event-info.c |   30 ++--
> >>  tools/perf/util/trace-event.h      |    5 +-
> >>  tools/perf/util/util.c             |   17 ++
> >>  tools/perf/util/util.h             |    1 +
> >>  tools/perf/util/xyarray.c          |   20 ++
> >>  tools/perf/util/xyarray.h          |   20 ++
> >>  24 files changed, 1013 insertions(+), 519 deletions(-)
> >>  create mode 100644 tools/perf/util/evsel.c
> >>  create mode 100644 tools/perf/util/evsel.h
> >>  create mode 100644 tools/perf/util/xyarray.c
> >>  create mode 100644 tools/perf/util/xyarray.h
> >
> > Pulled, thanks a lot Arnaldo!
> >
> >        Ingo
> >

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 14:03     ` Arnaldo Carvalho de Melo
@ 2011-01-04 14:09       ` Stephane Eranian
  2011-01-04 14:19         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 30+ messages in thread
From: Stephane Eranian @ 2011-01-04 14:09 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

Arnaldo,
Looks like what's wrong is not ps but count:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f96967fd6e0 (LWP 5156)]
0x0000000000412b58 in read_counter_aggr (counter=0x7d4820) at builtin-stat.c:206
206			update_stats(&ps->res_stats[i], count[i]);
(gdb) print ps
$1 = (struct perf_stat *) 0x7d48a0
(gdb) print *ps
$2 = {res_stats = {{n = 0, mean = 0, M2 = 0}, {n = 0, mean = 0, M2 =
0}, {n = 0, mean = 0, M2 = 0}}}
(gdb) print count
$3 = (u64 *) 0x12
(gdb) print *count
Cannot access memory at address 0x12
(gdb) print count
$4 = (u64 *) 0x12


On Tue, Jan 4, 2011 at 3:03 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
> Em Tue, Jan 04, 2011 at 02:59:00PM +0100, Stephane Eranian escreveu:
>> Arnaldo,
>>
>> The version of perf at tip-x86 commit 9274b36, segfaults for me:
>>
>> $ gdb perf
>> (gdb) r stat date
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> [Switching to Thread 0x7fdc602eb6e0 (LWP 4590)]
>> cmd_stat (argc=1, argv=0x7fff6fa2a9f0, prefix=<value optimized out>)
>> at builtin-stat.c:206
>> 206                   update_stats(&ps->res_stats[i], count[i]);
>> (gdb) bt
>> #0  cmd_stat (argc=1, argv=0x7fff6fa2a9f0, prefix=<value optimized
>> out>) at builtin-stat.c:206
>> #1  0x0000000000405c8b in handle_internal_command (argc=2,
>> argv=0x7fff6fa2a9f0) at perf.c:286
>> #2  0x00000000004060b0 in main (argc=2, argv=0x7fff6fa2a680) at perf.c:403
>>
>> Most like ps is NULL.
>
> [acme@felicio linux]$ perf stat date | head -5
> Tue Jan  4 12:03:05 BRST 2011
>
>  Performance counter stats for 'date':
>
>             4,525 cache-misses             #      7.640 M/sec
>           343,171 cache-references         #    579.405 M/sec
>            14,853 branch-misses            #      8.214 %
>           180,833 branches                 #    305.316 M/sec
>           897,837 instructions             #      0.000 IPC    (scaled from 67.67%)
>     <not counted> cycles
>               201 page-faults              #      0.339 M/sec
>                 1 CPU-migrations           #      0.002 M/sec
>                 1 context-switches         #      0.002 M/sec
>          0.592282 task-clock-msecs         #      0.583 CPUs
>
>        0.001015291  seconds time elapsed
>
> [acme@felicio linux]$
>
> Re-reading the code now, thanks!
>
> - Arnaldo
>
>> On Tue, Jan 4, 2011 at 8:16 AM, Ingo Molnar <mingo@elte.hu> wrote:
>> >
>> > * Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
>> >
>> >> Hi Ingo, Peter et al,
>> >>
>> >>       Trying to simplify using the API for tools and more specifically for
>> >> 'perf test' regression tests, please take a look, perhaps starting from the last
>> >> changeset, that implements a regression test using the resulting library API.
>> >>
>> >>       It also reduces the 'perf' tool footprint by not using hard coded array
>> >> sizes, more need to be done, but should be a good start, one changeset shows a
>> >> good reduction in BSS use.
>> >>
>> >>       Suggestions for naming most welcome, I thought about using "event__",
>> >> but that is taken, "perf_event__", but thought it would clash with "event_t",
>> >> so used the jargon used for the '-e' parameters: "Event Selector", but don't
>> >> like it that much, what do you think?
>> >>
>> >>       Writing the first regression test I think there are more ways to simplify,
>> >> on top of these, like not requiring a thread_map, i.e. passing NULL for that
>> >> parameter would mean: self-monitor, etc.
>> >>
>> >>         If you are pleased as-is, please pull from:
>> >>
>> >> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/test
>> >>
>> >> Regards,
>> >>
>> >> - Arnaldo
>> >>
>> >> Arnaldo Carvalho de Melo (11):
>> >>   perf tools: Introduce event selectors
>> >>   perf evsel: Adopt MATCH_EVENT macro from 'stat'
>> >>   perf util: Move do_read from session to util
>> >>   perf evsel: Delete the event selectors at exit
>> >>   perf evsel: Steal the counter reading routines from stat
>> >>   perf evsel: Introduce per cpu and per thread open helpers
>> >>   perf tools: Refactor cpumap to hold nr and the map
>> >>   perf tools: Refactor all_tids to hold nr and the map
>> >>   perf evsel: Use {cpu,thread}_map to shorten list of parameters
>> >>   perf evsel: Auto allocate resources needed for some methods
>> >>   perf test: Add test for counting open syscalls
>> >>
>> >>  tools/perf/Makefile                |    4 +
>> >>  tools/perf/builtin-record.c        |  152 +++++++--------
>> >>  tools/perf/builtin-stat.c          |  368 +++++++++++++++---------------------
>> >>  tools/perf/builtin-test.c          |   83 ++++++++
>> >>  tools/perf/builtin-top.c           |  221 ++++++++++++----------
>> >>  tools/perf/perf.c                  |    2 +
>> >>  tools/perf/util/cpumap.c           |  123 +++++++++---
>> >>  tools/perf/util/cpumap.h           |   10 +-
>> >>  tools/perf/util/evsel.c            |  186 ++++++++++++++++++
>> >>  tools/perf/util/evsel.h            |  115 +++++++++++
>> >>  tools/perf/util/header.c           |   15 +-
>> >>  tools/perf/util/header.h           |    3 +-
>> >>  tools/perf/util/parse-events.c     |   58 ++++--
>> >>  tools/perf/util/parse-events.h     |   18 ++-
>> >>  tools/perf/util/session.c          |   22 +--
>> >>  tools/perf/util/session.h          |    1 -
>> >>  tools/perf/util/thread.c           |   43 +++--
>> >>  tools/perf/util/thread.h           |   15 ++-
>> >>  tools/perf/util/trace-event-info.c |   30 ++--
>> >>  tools/perf/util/trace-event.h      |    5 +-
>> >>  tools/perf/util/util.c             |   17 ++
>> >>  tools/perf/util/util.h             |    1 +
>> >>  tools/perf/util/xyarray.c          |   20 ++
>> >>  tools/perf/util/xyarray.h          |   20 ++
>> >>  24 files changed, 1013 insertions(+), 519 deletions(-)
>> >>  create mode 100644 tools/perf/util/evsel.c
>> >>  create mode 100644 tools/perf/util/evsel.h
>> >>  create mode 100644 tools/perf/util/xyarray.c
>> >>  create mode 100644 tools/perf/util/xyarray.h
>> >
>> > Pulled, thanks a lot Arnaldo!
>> >
>> >        Ingo
>> >
>

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 13:59   ` Stephane Eranian
  2011-01-04 14:03     ` Arnaldo Carvalho de Melo
@ 2011-01-04 14:12     ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04 14:12 UTC (permalink / raw)
  To: Stephane Eranian
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

Em Tue, Jan 04, 2011 at 02:59:00PM +0100, Stephane Eranian escreveu:
> Arnaldo,
> 
> The version of perf at tip-x86 commit 9274b36, segfaults for me:
> 
> $ gdb perf
> (gdb) r stat date
> 
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7fdc602eb6e0 (LWP 4590)]
> cmd_stat (argc=1, argv=0x7fff6fa2a9f0, prefix=<value optimized out>)
> at builtin-stat.c:206
> 206			update_stats(&ps->res_stats[i], count[i]);
> (gdb) bt
> #0  cmd_stat (argc=1, argv=0x7fff6fa2a9f0, prefix=<value optimized
> out>) at builtin-stat.c:206
> #1  0x0000000000405c8b in handle_internal_command (argc=2,
> argv=0x7fff6fa2a9f0) at perf.c:286
> #2  0x00000000004060b0 in main (argc=2, argv=0x7fff6fa2a680) at perf.c:403
> 
> Most like ps is NULL.

Well, it shoudldn't, as we do, before calling that function:

        list_for_each_entry(pos, &evsel_list, node) {
                if (perf_evsel__alloc_stat_priv(pos) < 0 ||
                    perf_evsel__alloc_counts(pos, cpus->nr) < 0 ||
                    perf_evsel__alloc_fd(pos, cpus->nr, threads->nr) < 0)
                        goto out_free_fd;
        }

And then at line 206 it does:

	update_stats(&ps->res_stats[i], count[i]);

ps was set to be:

	struct perf_stat *ps = counter->priv;

that was set by perf_evsel__alloc_stat_priv

and count is

u64 *count = counter->counts->aggr.values;

That was allocated by perf_evsel__alloc_counts

/me scratches head :-\

- Arnaldo

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 14:09       ` Stephane Eranian
@ 2011-01-04 14:19         ` Arnaldo Carvalho de Melo
  2011-01-04 14:27           ` Arnaldo Carvalho de Melo
  2011-01-04 14:29           ` Stephane Eranian
  0 siblings, 2 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04 14:19 UTC (permalink / raw)
  To: Stephane Eranian
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

Em Tue, Jan 04, 2011 at 03:09:08PM +0100, Stephane Eranian escreveu:
> Arnaldo,
> Looks like what's wrong is not ps but count:
> 
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7f96967fd6e0 (LWP 5156)]
> 0x0000000000412b58 in read_counter_aggr (counter=0x7d4820) at builtin-stat.c:206
> 206			update_stats(&ps->res_stats[i], count[i]);
> (gdb) print ps
> $1 = (struct perf_stat *) 0x7d48a0
> (gdb) print *ps
> $2 = {res_stats = {{n = 0, mean = 0, M2 = 0}, {n = 0, mean = 0, M2 =
> 0}, {n = 0, mean = 0, M2 = 0}}}
> (gdb) print count
> $3 = (u64 *) 0x12
> (gdb) print *count
> Cannot access memory at address 0x12
> (gdb) print count
> $4 = (u64 *) 0x12

Count is:

u64 *count = counter->counts->aggr.values;

And counter->counts type is:

struct perf_counts_values {
        union {
                struct {
                        u64 val;
                        u64 ena;
                        u64 run;
                };
                u64 values[3];
        };
};

struct perf_counts {
        s8                        scaled;
        struct perf_counts_values aggr;
        struct perf_counts_values cpu[];
};

So for counter->counts->aggr.values to be 0x12 counter->counts must be
NULL...
 
> 
> On Tue, Jan 4, 2011 at 3:03 PM, Arnaldo Carvalho de Melo
> <acme@infradead.org> wrote:
> > Em Tue, Jan 04, 2011 at 02:59:00PM +0100, Stephane Eranian escreveu:
> >> Arnaldo,
> >>
> >> The version of perf at tip-x86 commit 9274b36, segfaults for me:
> >>
> >> $ gdb perf
> >> (gdb) r stat date
> >>
> >> Program received signal SIGSEGV, Segmentation fault.
> >> [Switching to Thread 0x7fdc602eb6e0 (LWP 4590)]
> >> cmd_stat (argc=1, argv=0x7fff6fa2a9f0, prefix=<value optimized out>)
> >> at builtin-stat.c:206
> >> 206                   update_stats(&ps->res_stats[i], count[i]);
> >> (gdb) bt
> >> #0  cmd_stat (argc=1, argv=0x7fff6fa2a9f0, prefix=<value optimized
> >> out>) at builtin-stat.c:206
> >> #1  0x0000000000405c8b in handle_internal_command (argc=2,
> >> argv=0x7fff6fa2a9f0) at perf.c:286
> >> #2  0x00000000004060b0 in main (argc=2, argv=0x7fff6fa2a680) at perf.c:403
> >>
> >> Most like ps is NULL.
> >
> > [acme@felicio linux]$ perf stat date | head -5
> > Tue Jan  4 12:03:05 BRST 2011
> >
> >  Performance counter stats for 'date':
> >
> >             4,525 cache-misses             #      7.640 M/sec
> >           343,171 cache-references         #    579.405 M/sec
> >            14,853 branch-misses            #      8.214 %
> >           180,833 branches                 #    305.316 M/sec
> >           897,837 instructions             #      0.000 IPC    (scaled from 67.67%)
> >     <not counted> cycles
> >               201 page-faults              #      0.339 M/sec
> >                 1 CPU-migrations           #      0.002 M/sec
> >                 1 context-switches         #      0.002 M/sec
> >          0.592282 task-clock-msecs         #      0.583 CPUs
> >
> >        0.001015291  seconds time elapsed
> >
> > [acme@felicio linux]$
> >
> > Re-reading the code now, thanks!
> >
> > - Arnaldo
> >
> >> On Tue, Jan 4, 2011 at 8:16 AM, Ingo Molnar <mingo@elte.hu> wrote:
> >> >
> >> > * Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> >> >
> >> >> Hi Ingo, Peter et al,
> >> >>
> >> >>       Trying to simplify using the API for tools and more specifically for
> >> >> 'perf test' regression tests, please take a look, perhaps starting from the last
> >> >> changeset, that implements a regression test using the resulting library API.
> >> >>
> >> >>       It also reduces the 'perf' tool footprint by not using hard coded array
> >> >> sizes, more need to be done, but should be a good start, one changeset shows a
> >> >> good reduction in BSS use.
> >> >>
> >> >>       Suggestions for naming most welcome, I thought about using "event__",
> >> >> but that is taken, "perf_event__", but thought it would clash with "event_t",
> >> >> so used the jargon used for the '-e' parameters: "Event Selector", but don't
> >> >> like it that much, what do you think?
> >> >>
> >> >>       Writing the first regression test I think there are more ways to simplify,
> >> >> on top of these, like not requiring a thread_map, i.e. passing NULL for that
> >> >> parameter would mean: self-monitor, etc.
> >> >>
> >> >>         If you are pleased as-is, please pull from:
> >> >>
> >> >> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/test
> >> >>
> >> >> Regards,
> >> >>
> >> >> - Arnaldo
> >> >>
> >> >> Arnaldo Carvalho de Melo (11):
> >> >>   perf tools: Introduce event selectors
> >> >>   perf evsel: Adopt MATCH_EVENT macro from 'stat'
> >> >>   perf util: Move do_read from session to util
> >> >>   perf evsel: Delete the event selectors at exit
> >> >>   perf evsel: Steal the counter reading routines from stat
> >> >>   perf evsel: Introduce per cpu and per thread open helpers
> >> >>   perf tools: Refactor cpumap to hold nr and the map
> >> >>   perf tools: Refactor all_tids to hold nr and the map
> >> >>   perf evsel: Use {cpu,thread}_map to shorten list of parameters
> >> >>   perf evsel: Auto allocate resources needed for some methods
> >> >>   perf test: Add test for counting open syscalls
> >> >>
> >> >>  tools/perf/Makefile                |    4 +
> >> >>  tools/perf/builtin-record.c        |  152 +++++++--------
> >> >>  tools/perf/builtin-stat.c          |  368 +++++++++++++++---------------------
> >> >>  tools/perf/builtin-test.c          |   83 ++++++++
> >> >>  tools/perf/builtin-top.c           |  221 ++++++++++++----------
> >> >>  tools/perf/perf.c                  |    2 +
> >> >>  tools/perf/util/cpumap.c           |  123 +++++++++---
> >> >>  tools/perf/util/cpumap.h           |   10 +-
> >> >>  tools/perf/util/evsel.c            |  186 ++++++++++++++++++
> >> >>  tools/perf/util/evsel.h            |  115 +++++++++++
> >> >>  tools/perf/util/header.c           |   15 +-
> >> >>  tools/perf/util/header.h           |    3 +-
> >> >>  tools/perf/util/parse-events.c     |   58 ++++--
> >> >>  tools/perf/util/parse-events.h     |   18 ++-
> >> >>  tools/perf/util/session.c          |   22 +--
> >> >>  tools/perf/util/session.h          |    1 -
> >> >>  tools/perf/util/thread.c           |   43 +++--
> >> >>  tools/perf/util/thread.h           |   15 ++-
> >> >>  tools/perf/util/trace-event-info.c |   30 ++--
> >> >>  tools/perf/util/trace-event.h      |    5 +-
> >> >>  tools/perf/util/util.c             |   17 ++
> >> >>  tools/perf/util/util.h             |    1 +
> >> >>  tools/perf/util/xyarray.c          |   20 ++
> >> >>  tools/perf/util/xyarray.h          |   20 ++
> >> >>  24 files changed, 1013 insertions(+), 519 deletions(-)
> >> >>  create mode 100644 tools/perf/util/evsel.c
> >> >>  create mode 100644 tools/perf/util/evsel.h
> >> >>  create mode 100644 tools/perf/util/xyarray.c
> >> >>  create mode 100644 tools/perf/util/xyarray.h
> >> >
> >> > Pulled, thanks a lot Arnaldo!
> >> >
> >> >        Ingo
> >> >
> >

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 14:19         ` Arnaldo Carvalho de Melo
@ 2011-01-04 14:27           ` Arnaldo Carvalho de Melo
  2011-01-04 14:33             ` Stephane Eranian
  2011-01-04 14:29           ` Stephane Eranian
  1 sibling, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04 14:27 UTC (permalink / raw)
  To: Stephane Eranian
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

Em Tue, Jan 04, 2011 at 12:19:58PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Jan 04, 2011 at 03:09:08PM +0100, Stephane Eranian escreveu:
> > Arnaldo,
> > Looks like what's wrong is not ps but count:
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 0x7f96967fd6e0 (LWP 5156)]
> > 0x0000000000412b58 in read_counter_aggr (counter=0x7d4820) at builtin-stat.c:206
> > 206			update_stats(&ps->res_stats[i], count[i]);
> > (gdb) print ps
> > $1 = (struct perf_stat *) 0x7d48a0
> > (gdb) print *ps
> > $2 = {res_stats = {{n = 0, mean = 0, M2 = 0}, {n = 0, mean = 0, M2 =
> > 0}, {n = 0, mean = 0, M2 = 0}}}
> > (gdb) print count
> > $3 = (u64 *) 0x12
> > (gdb) print *count
> > Cannot access memory at address 0x12
> > (gdb) print count
> > $4 = (u64 *) 0x12
> 
> Count is:
> 
> u64 *count = counter->counts->aggr.values;

Can you try with this patch?

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 02b2d80..7876b11 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -196,12 +196,14 @@ static inline int nsec_counter(struct perf_evsel *evsel)
 static int read_counter_aggr(struct perf_evsel *counter)
 {
 	struct perf_stat *ps = counter->priv;
-	u64 *count = counter->counts->aggr.values;
+	u64 *count;
 	int i;
 
 	if (__perf_evsel__read(counter, cpus->nr, threads->nr, scale) < 0)
 		return -1;
 
+	count = counter->counts->aggr.values;
+
 	for (i = 0; i < 3; i++)
 		update_stats(&ps->res_stats[i], count[i]);
 
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d337761..26962ea 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -89,8 +89,12 @@ int __perf_evsel__read(struct perf_evsel *evsel,
 {
 	size_t nv = scale ? 3 : 1;
 	int cpu, thread;
-	struct perf_counts_values *aggr = &evsel->counts->aggr, count;
+	struct perf_counts_values *aggr, count;
 
+	if (evsel->counts == NULL && perf_evsel__alloc_counts(evsel, ncpus) < 0)
+		return -ENOMEM;
+
+	aggr = &evsel->counts->aggr;
 	aggr->val = 0;
 
 	for (cpu = 0; cpu < ncpus; cpu++) {

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 14:19         ` Arnaldo Carvalho de Melo
  2011-01-04 14:27           ` Arnaldo Carvalho de Melo
@ 2011-01-04 14:29           ` Stephane Eranian
  2011-01-04 14:34             ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 30+ messages in thread
From: Stephane Eranian @ 2011-01-04 14:29 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

Indeed counts is bogus:

./perf stat -i -e cycles date

evt:0x7d4250 counts=0x7d4450 <-- from perf_evsel__alloc_counts()

Tue Jan  4 15:28:36 CET 2011

counter=0x7d4250 counts=(nil) <-- from read_counter_aggr()

Segmentation fault


On Tue, Jan 4, 2011 at 3:19 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
> Em Tue, Jan 04, 2011 at 03:09:08PM +0100, Stephane Eranian escreveu:
>> Arnaldo,
>> Looks like what's wrong is not ps but count:
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> [Switching to Thread 0x7f96967fd6e0 (LWP 5156)]
>> 0x0000000000412b58 in read_counter_aggr (counter=0x7d4820) at builtin-stat.c:206
>> 206                   update_stats(&ps->res_stats[i], count[i]);
>> (gdb) print ps
>> $1 = (struct perf_stat *) 0x7d48a0
>> (gdb) print *ps
>> $2 = {res_stats = {{n = 0, mean = 0, M2 = 0}, {n = 0, mean = 0, M2 =
>> 0}, {n = 0, mean = 0, M2 = 0}}}
>> (gdb) print count
>> $3 = (u64 *) 0x12
>> (gdb) print *count
>> Cannot access memory at address 0x12
>> (gdb) print count
>> $4 = (u64 *) 0x12
>
> Count is:
>
> u64 *count = counter->counts->aggr.values;
>
> And counter->counts type is:
>
> struct perf_counts_values {
>        union {
>                struct {
>                        u64 val;
>                        u64 ena;
>                        u64 run;
>                };
>                u64 values[3];
>        };
> };
>
> struct perf_counts {
>        s8                        scaled;
>        struct perf_counts_values aggr;
>        struct perf_counts_values cpu[];
> };
>
> So for counter->counts->aggr.values to be 0x12 counter->counts must be
> NULL...
>
>>
>> On Tue, Jan 4, 2011 at 3:03 PM, Arnaldo Carvalho de Melo
>> <acme@infradead.org> wrote:
>> > Em Tue, Jan 04, 2011 at 02:59:00PM +0100, Stephane Eranian escreveu:
>> >> Arnaldo,
>> >>
>> >> The version of perf at tip-x86 commit 9274b36, segfaults for me:
>> >>
>> >> $ gdb perf
>> >> (gdb) r stat date
>> >>
>> >> Program received signal SIGSEGV, Segmentation fault.
>> >> [Switching to Thread 0x7fdc602eb6e0 (LWP 4590)]
>> >> cmd_stat (argc=1, argv=0x7fff6fa2a9f0, prefix=<value optimized out>)
>> >> at builtin-stat.c:206
>> >> 206                   update_stats(&ps->res_stats[i], count[i]);
>> >> (gdb) bt
>> >> #0  cmd_stat (argc=1, argv=0x7fff6fa2a9f0, prefix=<value optimized
>> >> out>) at builtin-stat.c:206
>> >> #1  0x0000000000405c8b in handle_internal_command (argc=2,
>> >> argv=0x7fff6fa2a9f0) at perf.c:286
>> >> #2  0x00000000004060b0 in main (argc=2, argv=0x7fff6fa2a680) at perf.c:403
>> >>
>> >> Most like ps is NULL.
>> >
>> > [acme@felicio linux]$ perf stat date | head -5
>> > Tue Jan  4 12:03:05 BRST 2011
>> >
>> >  Performance counter stats for 'date':
>> >
>> >             4,525 cache-misses             #      7.640 M/sec
>> >           343,171 cache-references         #    579.405 M/sec
>> >            14,853 branch-misses            #      8.214 %
>> >           180,833 branches                 #    305.316 M/sec
>> >           897,837 instructions             #      0.000 IPC    (scaled from 67.67%)
>> >     <not counted> cycles
>> >               201 page-faults              #      0.339 M/sec
>> >                 1 CPU-migrations           #      0.002 M/sec
>> >                 1 context-switches         #      0.002 M/sec
>> >          0.592282 task-clock-msecs         #      0.583 CPUs
>> >
>> >        0.001015291  seconds time elapsed
>> >
>> > [acme@felicio linux]$
>> >
>> > Re-reading the code now, thanks!
>> >
>> > - Arnaldo
>> >
>> >> On Tue, Jan 4, 2011 at 8:16 AM, Ingo Molnar <mingo@elte.hu> wrote:
>> >> >
>> >> > * Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
>> >> >
>> >> >> Hi Ingo, Peter et al,
>> >> >>
>> >> >>       Trying to simplify using the API for tools and more specifically for
>> >> >> 'perf test' regression tests, please take a look, perhaps starting from the last
>> >> >> changeset, that implements a regression test using the resulting library API.
>> >> >>
>> >> >>       It also reduces the 'perf' tool footprint by not using hard coded array
>> >> >> sizes, more need to be done, but should be a good start, one changeset shows a
>> >> >> good reduction in BSS use.
>> >> >>
>> >> >>       Suggestions for naming most welcome, I thought about using "event__",
>> >> >> but that is taken, "perf_event__", but thought it would clash with "event_t",
>> >> >> so used the jargon used for the '-e' parameters: "Event Selector", but don't
>> >> >> like it that much, what do you think?
>> >> >>
>> >> >>       Writing the first regression test I think there are more ways to simplify,
>> >> >> on top of these, like not requiring a thread_map, i.e. passing NULL for that
>> >> >> parameter would mean: self-monitor, etc.
>> >> >>
>> >> >>         If you are pleased as-is, please pull from:
>> >> >>
>> >> >> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/test
>> >> >>
>> >> >> Regards,
>> >> >>
>> >> >> - Arnaldo
>> >> >>
>> >> >> Arnaldo Carvalho de Melo (11):
>> >> >>   perf tools: Introduce event selectors
>> >> >>   perf evsel: Adopt MATCH_EVENT macro from 'stat'
>> >> >>   perf util: Move do_read from session to util
>> >> >>   perf evsel: Delete the event selectors at exit
>> >> >>   perf evsel: Steal the counter reading routines from stat
>> >> >>   perf evsel: Introduce per cpu and per thread open helpers
>> >> >>   perf tools: Refactor cpumap to hold nr and the map
>> >> >>   perf tools: Refactor all_tids to hold nr and the map
>> >> >>   perf evsel: Use {cpu,thread}_map to shorten list of parameters
>> >> >>   perf evsel: Auto allocate resources needed for some methods
>> >> >>   perf test: Add test for counting open syscalls
>> >> >>
>> >> >>  tools/perf/Makefile                |    4 +
>> >> >>  tools/perf/builtin-record.c        |  152 +++++++--------
>> >> >>  tools/perf/builtin-stat.c          |  368 +++++++++++++++---------------------
>> >> >>  tools/perf/builtin-test.c          |   83 ++++++++
>> >> >>  tools/perf/builtin-top.c           |  221 ++++++++++++----------
>> >> >>  tools/perf/perf.c                  |    2 +
>> >> >>  tools/perf/util/cpumap.c           |  123 +++++++++---
>> >> >>  tools/perf/util/cpumap.h           |   10 +-
>> >> >>  tools/perf/util/evsel.c            |  186 ++++++++++++++++++
>> >> >>  tools/perf/util/evsel.h            |  115 +++++++++++
>> >> >>  tools/perf/util/header.c           |   15 +-
>> >> >>  tools/perf/util/header.h           |    3 +-
>> >> >>  tools/perf/util/parse-events.c     |   58 ++++--
>> >> >>  tools/perf/util/parse-events.h     |   18 ++-
>> >> >>  tools/perf/util/session.c          |   22 +--
>> >> >>  tools/perf/util/session.h          |    1 -
>> >> >>  tools/perf/util/thread.c           |   43 +++--
>> >> >>  tools/perf/util/thread.h           |   15 ++-
>> >> >>  tools/perf/util/trace-event-info.c |   30 ++--
>> >> >>  tools/perf/util/trace-event.h      |    5 +-
>> >> >>  tools/perf/util/util.c             |   17 ++
>> >> >>  tools/perf/util/util.h             |    1 +
>> >> >>  tools/perf/util/xyarray.c          |   20 ++
>> >> >>  tools/perf/util/xyarray.h          |   20 ++
>> >> >>  24 files changed, 1013 insertions(+), 519 deletions(-)
>> >> >>  create mode 100644 tools/perf/util/evsel.c
>> >> >>  create mode 100644 tools/perf/util/evsel.h
>> >> >>  create mode 100644 tools/perf/util/xyarray.c
>> >> >>  create mode 100644 tools/perf/util/xyarray.h
>> >> >
>> >> > Pulled, thanks a lot Arnaldo!
>> >> >
>> >> >        Ingo
>> >> >
>> >
>

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 14:27           ` Arnaldo Carvalho de Melo
@ 2011-01-04 14:33             ` Stephane Eranian
  2011-01-04 14:36               ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 30+ messages in thread
From: Stephane Eranian @ 2011-01-04 14:33 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

No improvement with this patch.

On Tue, Jan 4, 2011 at 3:27 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
> Em Tue, Jan 04, 2011 at 12:19:58PM -0200, Arnaldo Carvalho de Melo escreveu:
>> Em Tue, Jan 04, 2011 at 03:09:08PM +0100, Stephane Eranian escreveu:
>> > Arnaldo,
>> > Looks like what's wrong is not ps but count:
>> >
>> > Program received signal SIGSEGV, Segmentation fault.
>> > [Switching to Thread 0x7f96967fd6e0 (LWP 5156)]
>> > 0x0000000000412b58 in read_counter_aggr (counter=0x7d4820) at builtin-stat.c:206
>> > 206                 update_stats(&ps->res_stats[i], count[i]);
>> > (gdb) print ps
>> > $1 = (struct perf_stat *) 0x7d48a0
>> > (gdb) print *ps
>> > $2 = {res_stats = {{n = 0, mean = 0, M2 = 0}, {n = 0, mean = 0, M2 =
>> > 0}, {n = 0, mean = 0, M2 = 0}}}
>> > (gdb) print count
>> > $3 = (u64 *) 0x12
>> > (gdb) print *count
>> > Cannot access memory at address 0x12
>> > (gdb) print count
>> > $4 = (u64 *) 0x12
>>
>> Count is:
>>
>> u64 *count = counter->counts->aggr.values;
>
> Can you try with this patch?
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 02b2d80..7876b11 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -196,12 +196,14 @@ static inline int nsec_counter(struct perf_evsel *evsel)
>  static int read_counter_aggr(struct perf_evsel *counter)
>  {
>        struct perf_stat *ps = counter->priv;
> -       u64 *count = counter->counts->aggr.values;
> +       u64 *count;
>        int i;
>
>        if (__perf_evsel__read(counter, cpus->nr, threads->nr, scale) < 0)
>                return -1;
>
> +       count = counter->counts->aggr.values;
> +
>        for (i = 0; i < 3; i++)
>                update_stats(&ps->res_stats[i], count[i]);
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index d337761..26962ea 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -89,8 +89,12 @@ int __perf_evsel__read(struct perf_evsel *evsel,
>  {
>        size_t nv = scale ? 3 : 1;
>        int cpu, thread;
> -       struct perf_counts_values *aggr = &evsel->counts->aggr, count;
> +       struct perf_counts_values *aggr, count;
>
> +       if (evsel->counts == NULL && perf_evsel__alloc_counts(evsel, ncpus) < 0)
> +               return -ENOMEM;
> +
> +       aggr = &evsel->counts->aggr;
>        aggr->val = 0;
>
>        for (cpu = 0; cpu < ncpus; cpu++) {
>

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 14:29           ` Stephane Eranian
@ 2011-01-04 14:34             ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04 14:34 UTC (permalink / raw)
  To: Stephane Eranian
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

Em Tue, Jan 04, 2011 at 03:29:59PM +0100, Stephane Eranian escreveu:
> Indeed counts is bogus:
> 
> ./perf stat -i -e cycles date
> 
> evt:0x7d4250 counts=0x7d4450 <-- from perf_evsel__alloc_counts()
> 
> Tue Jan  4 15:28:36 CET 2011
> 
> counter=0x7d4250 counts=(nil) <-- from read_counter_aggr()
> 
> Segmentation fault

Strange I'm not reproducing it here (printf just on read_counter_aggr):

[acme@felicio linux]$ perf stat date
Tue Jan  4 12:32:15 BRST 2011
0x2586820: 0x2586948
0x2586790: 0x25869f8
0x2586700: 0x2586aa8
0x2586670: 0x2586b58
0x25865e0: 0x2586c08
0x2586550: 0x2586cb8
0x25864c0: 0x2586d68
0x2586430: 0x2586e18
0x25863a0: 0x2586ec8
0x2586310: 0x2586f78

 Performance counter stats for 'date':

             7,512 cache-misses             #     13.010 M/sec
           342,645 cache-references         #    593.445 M/sec
            14,884 branch-misses            #      8.241 %    
           180,611 branches                 #    312.810 M/sec
           925,986 instructions             #      0.000 IPC    (scaled from 49.88%)
     <not counted> cycles                  
               201 page-faults              #      0.348 M/sec
                 0 CPU-migrations           #      0.000 M/sec
                 0 context-switches         #      0.000 M/sec
          0.577383 task-clock-msecs         #      0.588 CPUs 

        0.000982448  seconds time elapsed

[acme@felicio linux]$

[acme@felicio linux]$ gcc -v
Using built-in specs.
COLLECT_GCC=/usr/bin/gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/4.5.1/lto-wrapper
Target: x86_64-redhat-linux
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap --enable-shared --enable-threads=posix --enable-checking=release --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,lto --enable-plugin --enable-java-awt=gtk --disable-dssi --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre --enable-libgcj-multifile --enable-java-maintainer-mode --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --disable-libjava-multilib --with-ppl --with-cloog --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
Thread model: posix
gcc version 4.5.1 20100924 (Red Hat 4.5.1-4) (GCC) 
[acme@felicio linux]$

[acme@felicio linux]$ uname -a
Linux felicio.ghostprotocols.net 2.6.35.10-72.fc14.x86_64 #1 SMP Mon Dec 20 21:14:22 UTC 2010 x86_64 x86_64 x86_64 GNU/Linux
[acme@felicio linux]$

[acme@felicio linux]$ cat /etc/fedora-release 
Fedora release 14 (Laughlin)
[acme@felicio linux]$

- Arnaldo

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 14:33             ` Stephane Eranian
@ 2011-01-04 14:36               ` Arnaldo Carvalho de Melo
  2011-01-04 14:46                 ` Stephane Eranian
  0 siblings, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04 14:36 UTC (permalink / raw)
  To: Stephane Eranian
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

Em Tue, Jan 04, 2011 at 03:33:38PM +0100, Stephane Eranian escreveu:
> No improvement with this patch.

I was guessing you were using it somehow without calling
perf_evsel__alloc_counts, now I guess you're just using what is in
tip/perf/core, no local changes, right?

Next step: there are anonymous structs inside anonymous unions, what
compiler/distro?

- Arnaldo
 
> On Tue, Jan 4, 2011 at 3:27 PM, Arnaldo Carvalho de Melo
> <acme@infradead.org> wrote:
> > Em Tue, Jan 04, 2011 at 12:19:58PM -0200, Arnaldo Carvalho de Melo escreveu:
> >> Em Tue, Jan 04, 2011 at 03:09:08PM +0100, Stephane Eranian escreveu:
> >> > Arnaldo,
> >> > Looks like what's wrong is not ps but count:
> >> >
> >> > Program received signal SIGSEGV, Segmentation fault.
> >> > [Switching to Thread 0x7f96967fd6e0 (LWP 5156)]
> >> > 0x0000000000412b58 in read_counter_aggr (counter=0x7d4820) at builtin-stat.c:206
> >> > 206                 update_stats(&ps->res_stats[i], count[i]);
> >> > (gdb) print ps
> >> > $1 = (struct perf_stat *) 0x7d48a0
> >> > (gdb) print *ps
> >> > $2 = {res_stats = {{n = 0, mean = 0, M2 = 0}, {n = 0, mean = 0, M2 =
> >> > 0}, {n = 0, mean = 0, M2 = 0}}}
> >> > (gdb) print count
> >> > $3 = (u64 *) 0x12
> >> > (gdb) print *count
> >> > Cannot access memory at address 0x12
> >> > (gdb) print count
> >> > $4 = (u64 *) 0x12
> >>
> >> Count is:
> >>
> >> u64 *count = counter->counts->aggr.values;
> >
> > Can you try with this patch?
> >
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index 02b2d80..7876b11 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -196,12 +196,14 @@ static inline int nsec_counter(struct perf_evsel *evsel)
> >  static int read_counter_aggr(struct perf_evsel *counter)
> >  {
> >        struct perf_stat *ps = counter->priv;
> > -       u64 *count = counter->counts->aggr.values;
> > +       u64 *count;
> >        int i;
> >
> >        if (__perf_evsel__read(counter, cpus->nr, threads->nr, scale) < 0)
> >                return -1;
> >
> > +       count = counter->counts->aggr.values;
> > +
> >        for (i = 0; i < 3; i++)
> >                update_stats(&ps->res_stats[i], count[i]);
> >
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index d337761..26962ea 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -89,8 +89,12 @@ int __perf_evsel__read(struct perf_evsel *evsel,
> >  {
> >        size_t nv = scale ? 3 : 1;
> >        int cpu, thread;
> > -       struct perf_counts_values *aggr = &evsel->counts->aggr, count;
> > +       struct perf_counts_values *aggr, count;
> >
> > +       if (evsel->counts == NULL && perf_evsel__alloc_counts(evsel, ncpus) < 0)
> > +               return -ENOMEM;
> > +
> > +       aggr = &evsel->counts->aggr;
> >        aggr->val = 0;
> >
> >        for (cpu = 0; cpu < ncpus; cpu++) {
> >

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 14:36               ` Arnaldo Carvalho de Melo
@ 2011-01-04 14:46                 ` Stephane Eranian
  2011-01-04 14:59                   ` Stephane Eranian
  0 siblings, 1 reply; 30+ messages in thread
From: Stephane Eranian @ 2011-01-04 14:46 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

On Tue, Jan 4, 2011 at 3:36 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
> Em Tue, Jan 04, 2011 at 03:33:38PM +0100, Stephane Eranian escreveu:
>> No improvement with this patch.
>
> I was guessing you were using it somehow without calling
> perf_evsel__alloc_counts, now I guess you're just using what is in
> tip/perf/core, no local changes, right?
>
I am using:
git://git.eu.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-tip.git

> Next step: there are anonymous structs inside anonymous unions, what
> compiler/distro?
Yes, I am looking at those right now.
>
gcc 4.4.3 or gcc 4.2.4 (ubuntu hardy or 10.04).

> - Arnaldo
>
>> On Tue, Jan 4, 2011 at 3:27 PM, Arnaldo Carvalho de Melo
>> <acme@infradead.org> wrote:
>> > Em Tue, Jan 04, 2011 at 12:19:58PM -0200, Arnaldo Carvalho de Melo escreveu:
>> >> Em Tue, Jan 04, 2011 at 03:09:08PM +0100, Stephane Eranian escreveu:
>> >> > Arnaldo,
>> >> > Looks like what's wrong is not ps but count:
>> >> >
>> >> > Program received signal SIGSEGV, Segmentation fault.
>> >> > [Switching to Thread 0x7f96967fd6e0 (LWP 5156)]
>> >> > 0x0000000000412b58 in read_counter_aggr (counter=0x7d4820) at builtin-stat.c:206
>> >> > 206                 update_stats(&ps->res_stats[i], count[i]);
>> >> > (gdb) print ps
>> >> > $1 = (struct perf_stat *) 0x7d48a0
>> >> > (gdb) print *ps
>> >> > $2 = {res_stats = {{n = 0, mean = 0, M2 = 0}, {n = 0, mean = 0, M2 =
>> >> > 0}, {n = 0, mean = 0, M2 = 0}}}
>> >> > (gdb) print count
>> >> > $3 = (u64 *) 0x12
>> >> > (gdb) print *count
>> >> > Cannot access memory at address 0x12
>> >> > (gdb) print count
>> >> > $4 = (u64 *) 0x12
>> >>
>> >> Count is:
>> >>
>> >> u64 *count = counter->counts->aggr.values;
>> >
>> > Can you try with this patch?
>> >
>> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> > index 02b2d80..7876b11 100644
>> > --- a/tools/perf/builtin-stat.c
>> > +++ b/tools/perf/builtin-stat.c
>> > @@ -196,12 +196,14 @@ static inline int nsec_counter(struct perf_evsel *evsel)
>> >  static int read_counter_aggr(struct perf_evsel *counter)
>> >  {
>> >        struct perf_stat *ps = counter->priv;
>> > -       u64 *count = counter->counts->aggr.values;
>> > +       u64 *count;
>> >        int i;
>> >
>> >        if (__perf_evsel__read(counter, cpus->nr, threads->nr, scale) < 0)
>> >                return -1;
>> >
>> > +       count = counter->counts->aggr.values;
>> > +
>> >        for (i = 0; i < 3; i++)
>> >                update_stats(&ps->res_stats[i], count[i]);
>> >
>> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>> > index d337761..26962ea 100644
>> > --- a/tools/perf/util/evsel.c
>> > +++ b/tools/perf/util/evsel.c
>> > @@ -89,8 +89,12 @@ int __perf_evsel__read(struct perf_evsel *evsel,
>> >  {
>> >        size_t nv = scale ? 3 : 1;
>> >        int cpu, thread;
>> > -       struct perf_counts_values *aggr = &evsel->counts->aggr, count;
>> > +       struct perf_counts_values *aggr, count;
>> >
>> > +       if (evsel->counts == NULL && perf_evsel__alloc_counts(evsel, ncpus) < 0)
>> > +               return -ENOMEM;
>> > +
>> > +       aggr = &evsel->counts->aggr;
>> >        aggr->val = 0;
>> >
>> >        for (cpu = 0; cpu < ncpus; cpu++) {
>> >
>

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 14:46                 ` Stephane Eranian
@ 2011-01-04 14:59                   ` Stephane Eranian
  2011-01-04 15:10                     ` Stephane Eranian
  0 siblings, 1 reply; 30+ messages in thread
From: Stephane Eranian @ 2011-01-04 14:59 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

The issue is related to a mismatch in the data structure layout
between builtin-stat.c and util/evsel.c. The offset of the counts fields
is different between the two files. Getting closer.....

On Tue, Jan 4, 2011 at 3:46 PM, Stephane Eranian <eranian@google.com> wrote:
> On Tue, Jan 4, 2011 at 3:36 PM, Arnaldo Carvalho de Melo
> <acme@infradead.org> wrote:
>> Em Tue, Jan 04, 2011 at 03:33:38PM +0100, Stephane Eranian escreveu:
>>> No improvement with this patch.
>>
>> I was guessing you were using it somehow without calling
>> perf_evsel__alloc_counts, now I guess you're just using what is in
>> tip/perf/core, no local changes, right?
>>
> I am using:
> git://git.eu.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-tip.git
>
>> Next step: there are anonymous structs inside anonymous unions, what
>> compiler/distro?
> Yes, I am looking at those right now.
>>
> gcc 4.4.3 or gcc 4.2.4 (ubuntu hardy or 10.04).
>
>> - Arnaldo
>>
>>> On Tue, Jan 4, 2011 at 3:27 PM, Arnaldo Carvalho de Melo
>>> <acme@infradead.org> wrote:
>>> > Em Tue, Jan 04, 2011 at 12:19:58PM -0200, Arnaldo Carvalho de Melo escreveu:
>>> >> Em Tue, Jan 04, 2011 at 03:09:08PM +0100, Stephane Eranian escreveu:
>>> >> > Arnaldo,
>>> >> > Looks like what's wrong is not ps but count:
>>> >> >
>>> >> > Program received signal SIGSEGV, Segmentation fault.
>>> >> > [Switching to Thread 0x7f96967fd6e0 (LWP 5156)]
>>> >> > 0x0000000000412b58 in read_counter_aggr (counter=0x7d4820) at builtin-stat.c:206
>>> >> > 206                 update_stats(&ps->res_stats[i], count[i]);
>>> >> > (gdb) print ps
>>> >> > $1 = (struct perf_stat *) 0x7d48a0
>>> >> > (gdb) print *ps
>>> >> > $2 = {res_stats = {{n = 0, mean = 0, M2 = 0}, {n = 0, mean = 0, M2 =
>>> >> > 0}, {n = 0, mean = 0, M2 = 0}}}
>>> >> > (gdb) print count
>>> >> > $3 = (u64 *) 0x12
>>> >> > (gdb) print *count
>>> >> > Cannot access memory at address 0x12
>>> >> > (gdb) print count
>>> >> > $4 = (u64 *) 0x12
>>> >>
>>> >> Count is:
>>> >>
>>> >> u64 *count = counter->counts->aggr.values;
>>> >
>>> > Can you try with this patch?
>>> >
>>> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>>> > index 02b2d80..7876b11 100644
>>> > --- a/tools/perf/builtin-stat.c
>>> > +++ b/tools/perf/builtin-stat.c
>>> > @@ -196,12 +196,14 @@ static inline int nsec_counter(struct perf_evsel *evsel)
>>> >  static int read_counter_aggr(struct perf_evsel *counter)
>>> >  {
>>> >        struct perf_stat *ps = counter->priv;
>>> > -       u64 *count = counter->counts->aggr.values;
>>> > +       u64 *count;
>>> >        int i;
>>> >
>>> >        if (__perf_evsel__read(counter, cpus->nr, threads->nr, scale) < 0)
>>> >                return -1;
>>> >
>>> > +       count = counter->counts->aggr.values;
>>> > +
>>> >        for (i = 0; i < 3; i++)
>>> >                update_stats(&ps->res_stats[i], count[i]);
>>> >
>>> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>>> > index d337761..26962ea 100644
>>> > --- a/tools/perf/util/evsel.c
>>> > +++ b/tools/perf/util/evsel.c
>>> > @@ -89,8 +89,12 @@ int __perf_evsel__read(struct perf_evsel *evsel,
>>> >  {
>>> >        size_t nv = scale ? 3 : 1;
>>> >        int cpu, thread;
>>> > -       struct perf_counts_values *aggr = &evsel->counts->aggr, count;
>>> > +       struct perf_counts_values *aggr, count;
>>> >
>>> > +       if (evsel->counts == NULL && perf_evsel__alloc_counts(evsel, ncpus) < 0)
>>> > +               return -ENOMEM;
>>> > +
>>> > +       aggr = &evsel->counts->aggr;
>>> >        aggr->val = 0;
>>> >
>>> >        for (cpu = 0; cpu < ncpus; cpu++) {
>>> >
>>
>

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 14:59                   ` Stephane Eranian
@ 2011-01-04 15:10                     ` Stephane Eranian
  2011-01-04 15:24                       ` Arnaldo Carvalho de Melo
  2011-01-04 15:30                       ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 30+ messages in thread
From: Stephane Eranian @ 2011-01-04 15:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

Ok, The problem comes both files picking up a different verison for
perf_event.h.

In my case, util/evsel.c was using /usr/include/linux/perf_event.h whereas
builtin-stat.c was using ../../include/linux/perf_event.h. Both have a different
struct perf_event_attr.

When I remove the /usr/include/linux/perf_event.h file, then I cannot compile
perf anymore:
In file included from perf.c:15:
util/parse-events.h:7:30: error: linux/perf_event.h: No such file or directory

Looks like something changed in the Makefile and the util modules don't know
where to pickup perf_event.h


On Tue, Jan 4, 2011 at 3:59 PM, Stephane Eranian <eranian@google.com> wrote:
> The issue is related to a mismatch in the data structure layout
> between builtin-stat.c and util/evsel.c. The offset of the counts fields
> is different between the two files. Getting closer.....
>
> On Tue, Jan 4, 2011 at 3:46 PM, Stephane Eranian <eranian@google.com> wrote:
>> On Tue, Jan 4, 2011 at 3:36 PM, Arnaldo Carvalho de Melo
>> <acme@infradead.org> wrote:
>>> Em Tue, Jan 04, 2011 at 03:33:38PM +0100, Stephane Eranian escreveu:
>>>> No improvement with this patch.
>>>
>>> I was guessing you were using it somehow without calling
>>> perf_evsel__alloc_counts, now I guess you're just using what is in
>>> tip/perf/core, no local changes, right?
>>>
>> I am using:
>> git://git.eu.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-tip.git
>>
>>> Next step: there are anonymous structs inside anonymous unions, what
>>> compiler/distro?
>> Yes, I am looking at those right now.
>>>
>> gcc 4.4.3 or gcc 4.2.4 (ubuntu hardy or 10.04).
>>
>>> - Arnaldo
>>>
>>>> On Tue, Jan 4, 2011 at 3:27 PM, Arnaldo Carvalho de Melo
>>>> <acme@infradead.org> wrote:
>>>> > Em Tue, Jan 04, 2011 at 12:19:58PM -0200, Arnaldo Carvalho de Melo escreveu:
>>>> >> Em Tue, Jan 04, 2011 at 03:09:08PM +0100, Stephane Eranian escreveu:
>>>> >> > Arnaldo,
>>>> >> > Looks like what's wrong is not ps but count:
>>>> >> >
>>>> >> > Program received signal SIGSEGV, Segmentation fault.
>>>> >> > [Switching to Thread 0x7f96967fd6e0 (LWP 5156)]
>>>> >> > 0x0000000000412b58 in read_counter_aggr (counter=0x7d4820) at builtin-stat.c:206
>>>> >> > 206                 update_stats(&ps->res_stats[i], count[i]);
>>>> >> > (gdb) print ps
>>>> >> > $1 = (struct perf_stat *) 0x7d48a0
>>>> >> > (gdb) print *ps
>>>> >> > $2 = {res_stats = {{n = 0, mean = 0, M2 = 0}, {n = 0, mean = 0, M2 =
>>>> >> > 0}, {n = 0, mean = 0, M2 = 0}}}
>>>> >> > (gdb) print count
>>>> >> > $3 = (u64 *) 0x12
>>>> >> > (gdb) print *count
>>>> >> > Cannot access memory at address 0x12
>>>> >> > (gdb) print count
>>>> >> > $4 = (u64 *) 0x12
>>>> >>
>>>> >> Count is:
>>>> >>
>>>> >> u64 *count = counter->counts->aggr.values;
>>>> >
>>>> > Can you try with this patch?
>>>> >
>>>> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>>>> > index 02b2d80..7876b11 100644
>>>> > --- a/tools/perf/builtin-stat.c
>>>> > +++ b/tools/perf/builtin-stat.c
>>>> > @@ -196,12 +196,14 @@ static inline int nsec_counter(struct perf_evsel *evsel)
>>>> >  static int read_counter_aggr(struct perf_evsel *counter)
>>>> >  {
>>>> >        struct perf_stat *ps = counter->priv;
>>>> > -       u64 *count = counter->counts->aggr.values;
>>>> > +       u64 *count;
>>>> >        int i;
>>>> >
>>>> >        if (__perf_evsel__read(counter, cpus->nr, threads->nr, scale) < 0)
>>>> >                return -1;
>>>> >
>>>> > +       count = counter->counts->aggr.values;
>>>> > +
>>>> >        for (i = 0; i < 3; i++)
>>>> >                update_stats(&ps->res_stats[i], count[i]);
>>>> >
>>>> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>>>> > index d337761..26962ea 100644
>>>> > --- a/tools/perf/util/evsel.c
>>>> > +++ b/tools/perf/util/evsel.c
>>>> > @@ -89,8 +89,12 @@ int __perf_evsel__read(struct perf_evsel *evsel,
>>>> >  {
>>>> >        size_t nv = scale ? 3 : 1;
>>>> >        int cpu, thread;
>>>> > -       struct perf_counts_values *aggr = &evsel->counts->aggr, count;
>>>> > +       struct perf_counts_values *aggr, count;
>>>> >
>>>> > +       if (evsel->counts == NULL && perf_evsel__alloc_counts(evsel, ncpus) < 0)
>>>> > +               return -ENOMEM;
>>>> > +
>>>> > +       aggr = &evsel->counts->aggr;
>>>> >        aggr->val = 0;
>>>> >
>>>> >        for (cpu = 0; cpu < ncpus; cpu++) {
>>>> >
>>>
>>
>

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 15:10                     ` Stephane Eranian
@ 2011-01-04 15:24                       ` Arnaldo Carvalho de Melo
  2011-01-04 15:30                         ` Stephane Eranian
  2011-01-04 15:30                       ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04 15:24 UTC (permalink / raw)
  To: Stephane Eranian
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

Em Tue, Jan 04, 2011 at 04:10:17PM +0100, Stephane Eranian escreveu:
> Ok, The problem comes both files picking up a different verison for
> perf_event.h.
> 
> In my case, util/evsel.c was using /usr/include/linux/perf_event.h whereas
> builtin-stat.c was using ../../include/linux/perf_event.h. Both have a different
> struct perf_event_attr.
> 
> When I remove the /usr/include/linux/perf_event.h file, then I cannot compile
> perf anymore:
> In file included from perf.c:15:
> util/parse-events.h:7:30: error: linux/perf_event.h: No such file or directory
> 
> Looks like something changed in the Makefile and the util modules don't know
> where to pickup perf_event.h

Ouch, all need to use ../../include/linux/perf_event.h, I think, my bad,
checking that.

Thanks for figuring out the issue!

- Arnaldo

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 15:10                     ` Stephane Eranian
  2011-01-04 15:24                       ` Arnaldo Carvalho de Melo
@ 2011-01-04 15:30                       ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-01-04 15:30 UTC (permalink / raw)
  To: Stephane Eranian
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

Em Tue, Jan 04, 2011 at 04:10:17PM +0100, Stephane Eranian escreveu:
> Ok, The problem comes both files picking up a different verison for
> perf_event.h.
> 
> In my case, util/evsel.c was using /usr/include/linux/perf_event.h whereas
> builtin-stat.c was using ../../include/linux/perf_event.h. Both have a different
> struct perf_event_attr.
> 
> When I remove the /usr/include/linux/perf_event.h file, then I cannot compile
> perf anymore:
> In file included from perf.c:15:
> util/parse-events.h:7:30: error: linux/perf_event.h: No such file or directory
> 
> Looks like something changed in the Makefile and the util modules don't know
> where to pickup perf_event.h

With the patch below now we have all as:

[acme@felicio linux]$ find tools/ -name "*.[ch]" | xargs grep 'include.\+perf_event\.h'
tools/perf/util/session.h:#include "../../../include/linux/perf_event.h"
tools/perf/util/header.h:#include "../../../include/linux/perf_event.h"
tools/perf/util/evsel.h:#include "../../../include/linux/perf_event.h"
tools/perf/perf.h:#include "../../include/linux/perf_event.h"
[acme@felicio linux]$

Can you try it so that I can get your Acked-by and Tested-by?

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 863d78d..a0ccd69 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -3,7 +3,7 @@
 
 #include <linux/list.h>
 #include <stdbool.h>
-#include <linux/perf_event.h>
+#include "../../../include/linux/perf_event.h"
 #include "types.h"
 #include "xyarray.h"
  
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 1c9043c..cefef9a 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -4,9 +4,9 @@
  * Parse symbolic events/counts passed in as options:
  */
 
-#include <linux/perf_event.h>
+#include <linux/list.h>
+#include "types.h"
 
-struct list_head;
 struct perf_evsel;
 
 extern struct list_head evsel_list;

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 15:24                       ` Arnaldo Carvalho de Melo
@ 2011-01-04 15:30                         ` Stephane Eranian
  2011-01-04 15:30                           ` Stephane Eranian
  0 siblings, 1 reply; 30+ messages in thread
From: Stephane Eranian @ 2011-01-04 15:30 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

Just submitted a patch to fix this.


On Tue, Jan 4, 2011 at 4:24 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
> Em Tue, Jan 04, 2011 at 04:10:17PM +0100, Stephane Eranian escreveu:
>> Ok, The problem comes both files picking up a different verison for
>> perf_event.h.
>>
>> In my case, util/evsel.c was using /usr/include/linux/perf_event.h whereas
>> builtin-stat.c was using ../../include/linux/perf_event.h. Both have a different
>> struct perf_event_attr.
>>
>> When I remove the /usr/include/linux/perf_event.h file, then I cannot compile
>> perf anymore:
>> In file included from perf.c:15:
>> util/parse-events.h:7:30: error: linux/perf_event.h: No such file or directory
>>
>> Looks like something changed in the Makefile and the util modules don't know
>> where to pickup perf_event.h
>
> Ouch, all need to use ../../include/linux/perf_event.h, I think, my bad,
> checking that.
>
> Thanks for figuring out the issue!
>
> - Arnaldo
>

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

* Re: [GIT PULL||RFC 00/11] perf library and regression testing improvements
  2011-01-04 15:30                         ` Stephane Eranian
@ 2011-01-04 15:30                           ` Stephane Eranian
  0 siblings, 0 replies; 30+ messages in thread
From: Stephane Eranian @ 2011-01-04 15:30 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Frederic Weisbecker, Han Pingtian, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Tom Zanussi, Thomas Gleixner,
	mingo

On Tue, Jan 4, 2011 at 4:30 PM, Stephane Eranian <eranian@google.com> wrote:
> Just submitted a patch to fix this.
>
That also means that on your build machine, you have a matching perf_event.h.
You cannot expect this from most distros.

>
> On Tue, Jan 4, 2011 at 4:24 PM, Arnaldo Carvalho de Melo
> <acme@infradead.org> wrote:
>> Em Tue, Jan 04, 2011 at 04:10:17PM +0100, Stephane Eranian escreveu:
>>> Ok, The problem comes both files picking up a different verison for
>>> perf_event.h.
>>>
>>> In my case, util/evsel.c was using /usr/include/linux/perf_event.h whereas
>>> builtin-stat.c was using ../../include/linux/perf_event.h. Both have a different
>>> struct perf_event_attr.
>>>
>>> When I remove the /usr/include/linux/perf_event.h file, then I cannot compile
>>> perf anymore:
>>> In file included from perf.c:15:
>>> util/parse-events.h:7:30: error: linux/perf_event.h: No such file or directory
>>>
>>> Looks like something changed in the Makefile and the util modules don't know
>>> where to pickup perf_event.h
>>
>> Ouch, all need to use ../../include/linux/perf_event.h, I think, my bad,
>> checking that.
>>
>> Thanks for figuring out the issue!
>>
>> - Arnaldo
>>
>

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

end of thread, other threads:[~2011-01-04 15:30 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-04  3:48 [GIT PULL||RFC 00/11] perf library and regression testing improvements Arnaldo Carvalho de Melo
2011-01-04  3:48 ` [PATCH 01/11] perf tools: Introduce event selectors Arnaldo Carvalho de Melo
2011-01-04  3:48 ` [PATCH 02/11] perf evsel: Adopt MATCH_EVENT macro from 'stat' Arnaldo Carvalho de Melo
2011-01-04  3:48 ` [PATCH 03/11] perf util: Move do_read from session to util Arnaldo Carvalho de Melo
2011-01-04  3:48 ` [PATCH 04/11] perf evsel: Delete the event selectors at exit Arnaldo Carvalho de Melo
2011-01-04  3:48 ` [PATCH 05/11] perf evsel: Steal the counter reading routines from stat Arnaldo Carvalho de Melo
2011-01-04  3:48 ` [PATCH 06/11] perf evsel: Introduce per cpu and per thread open helpers Arnaldo Carvalho de Melo
2011-01-04  3:48 ` [PATCH 07/11] perf tools: Refactor cpumap to hold nr and the map Arnaldo Carvalho de Melo
2011-01-04  3:48 ` [PATCH 08/11] perf tools: Refactor all_tids " Arnaldo Carvalho de Melo
2011-01-04  3:48 ` [PATCH 09/11] perf evsel: Use {cpu,thread}_map to shorten list of parameters Arnaldo Carvalho de Melo
2011-01-04  3:48 ` [PATCH 10/11] perf evsel: Auto allocate resources needed for some methods Arnaldo Carvalho de Melo
2011-01-04  3:48 ` [PATCH 11/11] perf test: Add test for counting open syscalls Arnaldo Carvalho de Melo
2011-01-04  7:16 ` [GIT PULL||RFC 00/11] perf library and regression testing improvements Ingo Molnar
2011-01-04 13:59   ` Stephane Eranian
2011-01-04 14:03     ` Arnaldo Carvalho de Melo
2011-01-04 14:09       ` Stephane Eranian
2011-01-04 14:19         ` Arnaldo Carvalho de Melo
2011-01-04 14:27           ` Arnaldo Carvalho de Melo
2011-01-04 14:33             ` Stephane Eranian
2011-01-04 14:36               ` Arnaldo Carvalho de Melo
2011-01-04 14:46                 ` Stephane Eranian
2011-01-04 14:59                   ` Stephane Eranian
2011-01-04 15:10                     ` Stephane Eranian
2011-01-04 15:24                       ` Arnaldo Carvalho de Melo
2011-01-04 15:30                         ` Stephane Eranian
2011-01-04 15:30                           ` Stephane Eranian
2011-01-04 15:30                       ` Arnaldo Carvalho de Melo
2011-01-04 14:29           ` Stephane Eranian
2011-01-04 14:34             ` Arnaldo Carvalho de Melo
2011-01-04 14:12     ` Arnaldo Carvalho de Melo

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).