linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL 0/8] perf/core improvements and fixes
@ 2012-09-06 19:31 Arnaldo Carvalho de Melo
  2012-09-06 19:31 ` [PATCH 1/8] perf tools: Clean target should do clean for lib/traceevent too Arnaldo Carvalho de Melo
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-06 19:31 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Corey Ashford,
	David Ahern, Frederic Weisbecker, Ingo Molnar, Jiri Olsa,
	Joel Uckelman, Mike Galbraith, Namhyung Kim, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian, Steven Rostedt,
	Arnaldo Carvalho de Melo

Hi Ingo,

	Please consider pulling,

- Arnaldo

The following changes since commit 7a4ec938857cf534270b23545495300fbac7f5de:

  perf tools: Allow user to indicate path to objdump in command line (2012-09-05 19:41:55 -0300)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo

for you to fetch changes up to 275ef3878f698941353780440fec6926107a320b:

  perf tools: Fix cache event name generation (2012-09-06 15:01:08 -0300)

----------------------------------------------------------------
perf/core improvements and fixes

. Fix hardware cache event name generation, fix from Jiri Olsa

. Add round trip test for sw, hw and cache event names, catching the
  problem Jiri fixed, after Jiri's patch, the test passes successfully.

. Clean target should do clean for lib/traceevent too, fix from David Ahern

. Check the right variable for allocation failure, fix from Namhyung Kim

. Set up evsel->tp_format regardless of evsel->name being set already,
  fix from Namhyung Kim

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

----------------------------------------------------------------
Arnaldo Carvalho de Melo (4):
      perf test: Add round trip test for sw and hw event names
      perf tools: Remove extraneous newline when parsing hardware cache events
      perf evlist: Add fprintf method
      perf test: Add roundtrip test for hardware cache events

David Ahern (1):
      perf tools: Clean target should do clean for lib/traceevent too

Jiri Olsa (1):
      perf tools: Fix cache event name generation

Namhyung Kim (2):
      perf header: Fix a typo on evsel
      perf header: Prepare tracepoint events regardless of name

 tools/perf/Makefile            |    5 +-
 tools/perf/builtin-test.c      |  114 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/evlist.c       |   13 +++++
 tools/perf/util/evlist.h       |    2 +
 tools/perf/util/evsel.c        |    6 +--
 tools/perf/util/evsel.h        |    6 ++-
 tools/perf/util/header.c       |   36 ++++++++-----
 tools/perf/util/parse-events.c |    2 +-
 8 files changed, 163 insertions(+), 21 deletions(-)

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

* [PATCH 1/8] perf tools: Clean target should do clean for lib/traceevent too
  2012-09-06 19:31 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
@ 2012-09-06 19:31 ` Arnaldo Carvalho de Melo
  2012-09-06 19:31 ` [PATCH 2/8] perf header: Fix a typo on evsel Arnaldo Carvalho de Melo
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-06 19:31 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, David Ahern, Namhyung Kim, Steven Rostedt,
	Arnaldo Carvalho de Melo

From: David Ahern <dsahern@gmail.com>

It's built as part of perf, so it should be cleaned too.

Tested-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/1346892816-61779-1-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 939cf6d..afd5075 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -917,6 +917,9 @@ $(LIB_FILE): $(LIB_OBJS)
 $(LIBTRACEEVENT):
 	$(QUIET_SUBDIR0)$(TRACE_EVENT_DIR) $(QUIET_SUBDIR1) O=$(OUTPUT) libtraceevent.a
 
+$(LIBTRACEEVENT)-clean:
+	$(QUIET_SUBDIR0)$(TRACE_EVENT_DIR) $(QUIET_SUBDIR1) O=$(OUTPUT) clean
+
 help:
 	@echo 'Perf make targets:'
 	@echo '  doc		- make *all* documentation (see below)'
@@ -1056,7 +1059,7 @@ quick-install-html:
 
 ### Cleaning rules
 
-clean:
+clean: $(LIBTRACEEVENT)-clean
 	$(RM) $(LIB_OBJS) $(BUILTIN_OBJS) $(LIB_FILE) $(OUTPUT)perf-archive $(OUTPUT)perf.o $(LANG_BINDINGS)
 	$(RM) $(ALL_PROGRAMS) perf
 	$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo $(OUTPUT)common-cmds.h TAGS tags cscope*
-- 
1.7.9.2.358.g22243


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

* [PATCH 2/8] perf header: Fix a typo on evsel
  2012-09-06 19:31 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
  2012-09-06 19:31 ` [PATCH 1/8] perf tools: Clean target should do clean for lib/traceevent too Arnaldo Carvalho de Melo
@ 2012-09-06 19:31 ` Arnaldo Carvalho de Melo
  2012-09-06 19:31 ` [PATCH 3/8] perf header: Prepare tracepoint events regardless of name Arnaldo Carvalho de Melo
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-06 19:31 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Namhyung Kim, David Ahern,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung.kim@lge.com>

For checking return value of the strdup, 'event' should be 'evsel'.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1346897446-16569-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 43425b7..8b0b873 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2307,7 +2307,7 @@ static int perf_evsel__set_tracepoint_name(struct perf_evsel *evsel,
 
 	snprintf(bf, sizeof(bf), "%s:%s", event->system, event->name);
 	evsel->name = strdup(bf);
-	if (event->name == NULL)
+	if (evsel->name == NULL)
 		return -1;
 
 	evsel->tp_format = event;
-- 
1.7.9.2.358.g22243


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

* [PATCH 3/8] perf header: Prepare tracepoint events regardless of name
  2012-09-06 19:31 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
  2012-09-06 19:31 ` [PATCH 1/8] perf tools: Clean target should do clean for lib/traceevent too Arnaldo Carvalho de Melo
  2012-09-06 19:31 ` [PATCH 2/8] perf header: Fix a typo on evsel Arnaldo Carvalho de Melo
@ 2012-09-06 19:31 ` Arnaldo Carvalho de Melo
  2012-09-06 19:31 ` [PATCH 4/8] perf test: Add round trip test for sw and hw event names Arnaldo Carvalho de Melo
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-06 19:31 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Namhyung Kim, David Ahern,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung.kim@lge.com>

Current perf_evlist__set_tracepoint_names is a misnomer because it finds
and sets correspoding event_format in addition to the name.  So skipping
it when a event has set name already caused a trouble.

Rename it and set name only a event doesn't have one.

Reported-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1346897446-16569-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c |   36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 8b0b873..d07bc13 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2295,33 +2295,39 @@ static int read_attr(int fd, struct perf_header *ph,
 	return ret <= 0 ? -1 : 0;
 }
 
-static int perf_evsel__set_tracepoint_name(struct perf_evsel *evsel,
-					   struct pevent *pevent)
+static int perf_evsel__prepare_tracepoint_event(struct perf_evsel *evsel,
+						struct pevent *pevent)
 {
-	struct event_format *event = pevent_find_event(pevent,
-						       evsel->attr.config);
+	struct event_format *event;
 	char bf[128];
 
+	/* already prepared */
+	if (evsel->tp_format)
+		return 0;
+
+	event = pevent_find_event(pevent, evsel->attr.config);
 	if (event == NULL)
 		return -1;
 
-	snprintf(bf, sizeof(bf), "%s:%s", event->system, event->name);
-	evsel->name = strdup(bf);
-	if (evsel->name == NULL)
-		return -1;
+	if (!evsel->name) {
+		snprintf(bf, sizeof(bf), "%s:%s", event->system, event->name);
+		evsel->name = strdup(bf);
+		if (evsel->name == NULL)
+			return -1;
+	}
 
 	evsel->tp_format = event;
 	return 0;
 }
 
-static int perf_evlist__set_tracepoint_names(struct perf_evlist *evlist,
-					     struct pevent *pevent)
+static int perf_evlist__prepare_tracepoint_events(struct perf_evlist *evlist,
+						  struct pevent *pevent)
 {
 	struct perf_evsel *pos;
 
 	list_for_each_entry(pos, &evlist->entries, node) {
-		if (pos->attr.type == PERF_TYPE_TRACEPOINT && !pos->name &&
-		    perf_evsel__set_tracepoint_name(pos, pevent))
+		if (pos->attr.type == PERF_TYPE_TRACEPOINT &&
+		    perf_evsel__prepare_tracepoint_event(pos, pevent))
 			return -1;
 	}
 
@@ -2409,7 +2415,8 @@ int perf_session__read_header(struct perf_session *session, int fd)
 
 	lseek(fd, header->data_offset, SEEK_SET);
 
-	if (perf_evlist__set_tracepoint_names(session->evlist, session->pevent))
+	if (perf_evlist__prepare_tracepoint_events(session->evlist,
+						   session->pevent))
 		goto out_delete_evlist;
 
 	header->frozen = 1;
@@ -2643,7 +2650,8 @@ int perf_event__process_tracing_data(union perf_event *event,
 	if (size_read + padding != size)
 		die("tracing data size mismatch");
 
-	perf_evlist__set_tracepoint_names(session->evlist, session->pevent);
+	perf_evlist__prepare_tracepoint_events(session->evlist,
+					       session->pevent);
 
 	return size_read + padding;
 }
-- 
1.7.9.2.358.g22243


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

* [PATCH 4/8] perf test: Add round trip test for sw and hw event names
  2012-09-06 19:31 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2012-09-06 19:31 ` [PATCH 3/8] perf header: Prepare tracepoint events regardless of name Arnaldo Carvalho de Melo
@ 2012-09-06 19:31 ` Arnaldo Carvalho de Melo
  2012-09-06 19:31 ` [PATCH 5/8] perf tools: Remove extraneous newline when parsing hardware cache events Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-06 19:31 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian

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

It basically traverses the hardware and software event name arrays
creating an evlist with all events, then it uses perf_evsel__name to
check that the name is the expected one.

With it I noticed this problem:

[root@sandy ~]# perf test 10
10: roundtrip evsel->name check:invalid or unsupported event: 'CPU-migrations'
Run 'perf list' for a list of valid events
 FAILED!

Changed it to "cpu-migrations" in the software event arrays and it
worked.

This is to catch problems like the one reported by Joel Uckelman in
http://permalink.gmane.org/gmane.linux.kernel.perf.user/1016

Hardware cache events will be checked in the following patch.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-5jskfkuqvf2fi257zmni0ftz@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-test.c |   53 +++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/evsel.c   |    6 ++---
 tools/perf/util/evsel.h   |    6 +++--
 3 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 381d5ab..ba94fbe 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -1092,6 +1092,55 @@ static int test__perf_pmu(void)
 	return perf_pmu__test();
 }
 
+static int __perf_evsel__name_array_test(const char *names[], int nr_names)
+{
+	int i, err;
+	struct perf_evsel *evsel;
+        struct perf_evlist *evlist = perf_evlist__new(NULL, NULL);
+
+        if (evlist == NULL)
+                return -ENOMEM;
+
+	for (i = 0; i < nr_names; ++i) {
+		err = parse_events(evlist, names[i], 0);
+		if (err) {
+			pr_debug("failed to parse event '%s', err %d\n",
+				 names[i], err);
+			goto out_delete_evlist;
+		}
+	}
+
+	err = 0;
+	list_for_each_entry(evsel, &evlist->entries, node) {
+		if (strcmp(perf_evsel__name(evsel), names[evsel->idx])) {
+			--err;
+			pr_debug("%s != %s\n", perf_evsel__name(evsel), names[evsel->idx]);
+		}
+	}
+
+out_delete_evlist:
+	perf_evlist__delete(evlist);
+	return err;
+}
+
+#define perf_evsel__name_array_test(names) \
+	__perf_evsel__name_array_test(names, ARRAY_SIZE(names))
+
+static int perf_evsel__roundtrip_name_test(void)
+{
+	int err = 0, ret = 0;
+
+	err = perf_evsel__name_array_test(perf_evsel__hw_names);
+	if (err)
+		ret = err;
+
+	err = perf_evsel__name_array_test(perf_evsel__sw_names);
+	if (err)
+		ret = err;
+
+	return ret;
+}
+
 static struct test {
 	const char *desc;
 	int (*func)(void);
@@ -1135,6 +1184,10 @@ static struct test {
 		.func = dso__test_data,
 	},
 	{
+		.desc = "roundtrip evsel->name check",
+		.func = perf_evsel__roundtrip_name_test,
+	},
+	{
 		.func = NULL,
 	},
 };
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 7ff3c8f..06f7644 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -68,7 +68,7 @@ struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr, int idx)
 	return evsel;
 }
 
-static const char *perf_evsel__hw_names[PERF_COUNT_HW_MAX] = {
+const char *perf_evsel__hw_names[PERF_COUNT_HW_MAX] = {
 	"cycles",
 	"instructions",
 	"cache-references",
@@ -131,12 +131,12 @@ static int perf_evsel__hw_name(struct perf_evsel *evsel, char *bf, size_t size)
 	return r + perf_evsel__add_modifiers(evsel, bf + r, size - r);
 }
 
-static const char *perf_evsel__sw_names[PERF_COUNT_SW_MAX] = {
+const char *perf_evsel__sw_names[PERF_COUNT_SW_MAX] = {
 	"cpu-clock",
 	"task-clock",
 	"page-faults",
 	"context-switches",
-	"CPU-migrations",
+	"cpu-migrations",
 	"minor-faults",
 	"major-faults",
 	"alignment-faults",
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 94f6ba1..a3f562c 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -97,8 +97,10 @@ extern const char *perf_evsel__hw_cache[PERF_COUNT_HW_CACHE_MAX]
 				       [PERF_EVSEL__MAX_ALIASES];
 extern const char *perf_evsel__hw_cache_op[PERF_COUNT_HW_CACHE_OP_MAX]
 					  [PERF_EVSEL__MAX_ALIASES];
-const char *perf_evsel__hw_cache_result[PERF_COUNT_HW_CACHE_RESULT_MAX]
-				       [PERF_EVSEL__MAX_ALIASES];
+extern const char *perf_evsel__hw_cache_result[PERF_COUNT_HW_CACHE_RESULT_MAX]
+					      [PERF_EVSEL__MAX_ALIASES];
+extern const char *perf_evsel__hw_names[PERF_COUNT_HW_MAX];
+extern const char *perf_evsel__sw_names[PERF_COUNT_SW_MAX];
 int __perf_evsel__hw_cache_type_op_res_name(u8 type, u8 op, u8 result,
 					    char *bf, size_t size);
 const char *perf_evsel__name(struct perf_evsel *evsel);
-- 
1.7.9.2.358.g22243


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

* [PATCH 5/8] perf tools: Remove extraneous newline when parsing hardware cache events
  2012-09-06 19:31 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2012-09-06 19:31 ` [PATCH 4/8] perf test: Add round trip test for sw and hw event names Arnaldo Carvalho de Melo
@ 2012-09-06 19:31 ` Arnaldo Carvalho de Melo
  2012-09-06 19:31 ` [PATCH 6/8] perf evlist: Add fprintf method Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-06 19:31 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian

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

Noticed while developing a 'perf test' entry to verify that
perf_evsel__name works.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-xz6zgh38mp3cjnd2udh38z8f@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index b246303..66d235e 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -308,7 +308,7 @@ int parse_events_add_cache(struct list_head **list, int *idx,
 	for (i = 0; (i < 2) && (op_result[i]); i++) {
 		char *str = op_result[i];
 
-		snprintf(name + n, MAX_NAME_LEN - n, "-%s\n", str);
+		snprintf(name + n, MAX_NAME_LEN - n, "-%s", str);
 
 		if (cache_op == -1) {
 			cache_op = parse_aliases(str, perf_evsel__hw_cache_op,
-- 
1.7.9.2.358.g22243


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

* [PATCH 6/8] perf evlist: Add fprintf method
  2012-09-06 19:31 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2012-09-06 19:31 ` [PATCH 5/8] perf tools: Remove extraneous newline when parsing hardware cache events Arnaldo Carvalho de Melo
@ 2012-09-06 19:31 ` Arnaldo Carvalho de Melo
  2012-09-06 19:31 ` [PATCH 7/8] perf test: Add roundtrip test for hardware cache events Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-06 19:31 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian

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

For debugging, etc.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-fjimge1ovgh976qlt8dtmlp0@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evlist.c |   13 +++++++++++++
 tools/perf/util/evlist.h |    2 ++
 2 files changed, 15 insertions(+)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 4774ac1..8923537 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -889,3 +889,16 @@ int perf_evlist__parse_sample(struct perf_evlist *evlist, union perf_event *even
 	struct perf_evsel *evsel = perf_evlist__first(evlist);
 	return perf_evsel__parse_sample(evsel, event, sample, swapped);
 }
+
+size_t perf_evlist__fprintf(struct perf_evlist *evlist, FILE *fp)
+{
+	struct perf_evsel *evsel;
+	size_t printed = 0;
+
+	list_for_each_entry(evsel, &evlist->entries, node) {
+		printed += fprintf(fp, "%s%s", evsel->idx ? ", " : "",
+				   perf_evsel__name(evsel));
+	}
+
+	return printed + fprintf(fp, "\n");;
+}
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 2ed2557..3f2e1e4 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -143,4 +143,6 @@ static inline struct perf_evsel *perf_evlist__last(struct perf_evlist *evlist)
 {
 	return list_entry(evlist->entries.prev, struct perf_evsel, node);
 }
+
+size_t perf_evlist__fprintf(struct perf_evlist *evlist, FILE *fp);
 #endif /* __PERF_EVLIST_H */
-- 
1.7.9.2.358.g22243


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

* [PATCH 7/8] perf test: Add roundtrip test for hardware cache events
  2012-09-06 19:31 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (5 preceding siblings ...)
  2012-09-06 19:31 ` [PATCH 6/8] perf evlist: Add fprintf method Arnaldo Carvalho de Melo
@ 2012-09-06 19:31 ` Arnaldo Carvalho de Melo
  2012-09-06 19:31 ` [PATCH 8/8] perf tools: Fix cache event name generation Arnaldo Carvalho de Melo
  2012-09-07  5:39 ` [GIT PULL 0/8] perf/core improvements and fixes Ingo Molnar
  8 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-06 19:31 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern,
	Frederic Weisbecker, Joel Uckelman, Jiri Olsa, Mike Galbraith,
	Namhyung Kim, Paul Mackerras, Peter Zijlstra, Stephane Eranian

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

That nicely catches the problem reported by Joel Uckelman in
http://permalink.gmane.org/gmane.linux.kernel.perf.user/1016 :

  [root@sandy ~]# perf test
   1: vmlinux symtab matches kallsyms: Ok
   2: detect open syscall event: Ok
   3: detect open syscall event on all cpus: Ok
   4: read samples using the mmap interface: Ok
   5: parse events tests: Ok
   6: x86 rdpmc test: Ok
   7: Validate PERF_RECORD_* events & perf_sample fields: Ok
   8: Test perf pmu format parsing: Ok
   9: Test dso data interface: Ok
  10: roundtrip evsel->name check: FAILED!

  [root@sandy ~]# perf test -v 10
  10: roundtrip evsel->name check:
  --- start ---
  L1-dcache-misses != L1-dcache-load-misses
  L1-dcache-misses != L1-dcache-store-misses
  L1-dcache-misses != L1-dcache-prefetch-misses
  L1-icache-misses != L1-icache-load-misses
  L1-icache-misses != L1-icache-prefetch-misses
  LLC-misses != LLC-load-misses
  LLC-misses != LLC-store-misses
  LLC-misses != LLC-prefetch-misses
  dTLB-misses != dTLB-load-misses
  dTLB-misses != dTLB-store-misses
  dTLB-misses != dTLB-prefetch-misses
  iTLB-misses != iTLB-load-misses
  branch-misses != branch-load-misses
  node-misses != node-load-misses
  node-misses != node-store-misses
  node-misses != node-prefetch-misses
  ---- end ----
  roundtrip evsel->name check: FAILED!

  [root@sandy ~]#

Now lemme apply Jiri's fix and try it again...

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Joel Uckelman <joel@lightboxtechnologies.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-bbewtxw0rfipp5qy1j3jtg5d@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-test.c |   61 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index ba94fbe..cf33e50 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -1092,6 +1092,63 @@ static int test__perf_pmu(void)
 	return perf_pmu__test();
 }
 
+static int perf_evsel__roundtrip_cache_name_test(void)
+{
+	char name[128];
+	int type, op, err = 0, ret = 0, i, idx;
+	struct perf_evsel *evsel;
+        struct perf_evlist *evlist = perf_evlist__new(NULL, NULL);
+
+        if (evlist == NULL)
+                return -ENOMEM;
+
+	for (type = 0; type < PERF_COUNT_HW_CACHE_MAX; type++) {
+		for (op = 0; op < PERF_COUNT_HW_CACHE_OP_MAX; op++) {
+			/* skip invalid cache type */
+			if (!perf_evsel__is_cache_op_valid(type, op))
+				continue;
+
+			for (i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
+				__perf_evsel__hw_cache_type_op_res_name(type, op, i,
+									name, sizeof(name));
+				err = parse_events(evlist, name, 0);
+				if (err)
+					ret = err;
+			}
+		}
+	}
+
+	idx = 0;
+	evsel = perf_evlist__first(evlist);
+
+	for (type = 0; type < PERF_COUNT_HW_CACHE_MAX; type++) {
+		for (op = 0; op < PERF_COUNT_HW_CACHE_OP_MAX; op++) {
+			/* skip invalid cache type */
+			if (!perf_evsel__is_cache_op_valid(type, op))
+				continue;
+
+			for (i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
+				__perf_evsel__hw_cache_type_op_res_name(type, op, i,
+									name, sizeof(name));
+				if (evsel->idx != idx)
+					continue;
+
+				++idx;
+
+				if (strcmp(perf_evsel__name(evsel), name)) {
+					pr_debug("%s != %s\n", perf_evsel__name(evsel), name);
+					ret = -1;
+				}
+
+				evsel = perf_evsel__next(evsel);
+			}
+		}
+	}
+
+	perf_evlist__delete(evlist);
+	return ret;
+}
+
 static int __perf_evsel__name_array_test(const char *names[], int nr_names)
 {
 	int i, err;
@@ -1138,6 +1195,10 @@ static int perf_evsel__roundtrip_name_test(void)
 	if (err)
 		ret = err;
 
+	err = perf_evsel__roundtrip_cache_name_test();
+	if (err)
+		ret = err;
+
 	return ret;
 }
 
-- 
1.7.9.2.358.g22243


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

* [PATCH 8/8] perf tools: Fix cache event name generation
  2012-09-06 19:31 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (6 preceding siblings ...)
  2012-09-06 19:31 ` [PATCH 7/8] perf test: Add roundtrip test for hardware cache events Arnaldo Carvalho de Melo
@ 2012-09-06 19:31 ` Arnaldo Carvalho de Melo
  2012-09-07  5:39 ` [GIT PULL 0/8] perf/core improvements and fixes Ingo Molnar
  8 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-06 19:31 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Jiri Olsa, Corey Ashford, Frederic Weisbecker,
	Ingo Molnar, Joel Uckelman, Paul Mackerras, Peter Zijlstra,
	Arnaldo Carvalho de Melo

From: Jiri Olsa <jolsa@redhat.com>

If the event name is specified with all 3 components, the last one
overwrites the previous one during the name composing within the
parse_events_add_cache function.

Fixing this by properly adjusting the string index.

Reported-by: Joel Uckelman <joel@lightboxtechnologies.com>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Joel Uckelman <joel@lightboxtechnologies.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
LPU-Reference: 20120905175133.GA18352@krava.brq.redhat.com
[ committer note: Remove the newline fix, done already in 42e1fb7 ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 66d235e..a031ee1 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -308,7 +308,7 @@ int parse_events_add_cache(struct list_head **list, int *idx,
 	for (i = 0; (i < 2) && (op_result[i]); i++) {
 		char *str = op_result[i];
 
-		snprintf(name + n, MAX_NAME_LEN - n, "-%s", str);
+		n += snprintf(name + n, MAX_NAME_LEN - n, "-%s", str);
 
 		if (cache_op == -1) {
 			cache_op = parse_aliases(str, perf_evsel__hw_cache_op,
-- 
1.7.9.2.358.g22243


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

* Re: [GIT PULL 0/8] perf/core improvements and fixes
  2012-09-06 19:31 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (7 preceding siblings ...)
  2012-09-06 19:31 ` [PATCH 8/8] perf tools: Fix cache event name generation Arnaldo Carvalho de Melo
@ 2012-09-07  5:39 ` Ingo Molnar
  8 siblings, 0 replies; 10+ messages in thread
From: Ingo Molnar @ 2012-09-07  5:39 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Corey Ashford, David Ahern, Frederic Weisbecker,
	Ingo Molnar, Jiri Olsa, Joel Uckelman, Mike Galbraith,
	Namhyung Kim, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Steven Rostedt, Arnaldo Carvalho de Melo


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

> Hi Ingo,
> 
> 	Please consider pulling,
> 
> - Arnaldo
> 
> The following changes since commit 7a4ec938857cf534270b23545495300fbac7f5de:
> 
>   perf tools: Allow user to indicate path to objdump in command line (2012-09-05 19:41:55 -0300)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo
> 
> for you to fetch changes up to 275ef3878f698941353780440fec6926107a320b:
> 
>   perf tools: Fix cache event name generation (2012-09-06 15:01:08 -0300)
> 
> ----------------------------------------------------------------
> perf/core improvements and fixes
> 
> . Fix hardware cache event name generation, fix from Jiri Olsa
> 
> . Add round trip test for sw, hw and cache event names, catching the
>   problem Jiri fixed, after Jiri's patch, the test passes successfully.
> 
> . Clean target should do clean for lib/traceevent too, fix from David Ahern
> 
> . Check the right variable for allocation failure, fix from Namhyung Kim
> 
> . Set up evsel->tp_format regardless of evsel->name being set already,
>   fix from Namhyung Kim
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (4):
>       perf test: Add round trip test for sw and hw event names
>       perf tools: Remove extraneous newline when parsing hardware cache events
>       perf evlist: Add fprintf method
>       perf test: Add roundtrip test for hardware cache events
> 
> David Ahern (1):
>       perf tools: Clean target should do clean for lib/traceevent too
> 
> Jiri Olsa (1):
>       perf tools: Fix cache event name generation
> 
> Namhyung Kim (2):
>       perf header: Fix a typo on evsel
>       perf header: Prepare tracepoint events regardless of name
> 
>  tools/perf/Makefile            |    5 +-
>  tools/perf/builtin-test.c      |  114 ++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/evlist.c       |   13 +++++
>  tools/perf/util/evlist.h       |    2 +
>  tools/perf/util/evsel.c        |    6 +--
>  tools/perf/util/evsel.h        |    6 ++-
>  tools/perf/util/header.c       |   36 ++++++++-----
>  tools/perf/util/parse-events.c |    2 +-
>  8 files changed, 163 insertions(+), 21 deletions(-)

Pulled, thanks a lot Arnaldo!

	Ingo

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

end of thread, other threads:[~2012-09-07  5:39 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-06 19:31 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
2012-09-06 19:31 ` [PATCH 1/8] perf tools: Clean target should do clean for lib/traceevent too Arnaldo Carvalho de Melo
2012-09-06 19:31 ` [PATCH 2/8] perf header: Fix a typo on evsel Arnaldo Carvalho de Melo
2012-09-06 19:31 ` [PATCH 3/8] perf header: Prepare tracepoint events regardless of name Arnaldo Carvalho de Melo
2012-09-06 19:31 ` [PATCH 4/8] perf test: Add round trip test for sw and hw event names Arnaldo Carvalho de Melo
2012-09-06 19:31 ` [PATCH 5/8] perf tools: Remove extraneous newline when parsing hardware cache events Arnaldo Carvalho de Melo
2012-09-06 19:31 ` [PATCH 6/8] perf evlist: Add fprintf method Arnaldo Carvalho de Melo
2012-09-06 19:31 ` [PATCH 7/8] perf test: Add roundtrip test for hardware cache events Arnaldo Carvalho de Melo
2012-09-06 19:31 ` [PATCH 8/8] perf tools: Fix cache event name generation Arnaldo Carvalho de Melo
2012-09-07  5:39 ` [GIT PULL 0/8] perf/core improvements and fixes Ingo Molnar

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