All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] perf tools updates
@ 2011-05-22  1:45 Frederic Weisbecker
  2011-05-22  1:45 ` [PATCH 1/6] perf tools: Check we are able to read the event size on mmap Frederic Weisbecker
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Frederic Weisbecker @ 2011-05-22  1:45 UTC (permalink / raw)
  To: Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: LKML, Frederic Weisbecker, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Stephane Eranian, Linus Torvalds

Ingo, Arnaldo,

Please pull the perf/core branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
	perf/core

Thanks,
	Frederic
---

Frederic Weisbecker (6):
      perf tools: Check we are able to read the event size on mmap
      perf tools: Remove junk code in mmap size handling
      perf tools: Move evlist sample helpers to evlist area
      perf tools: Pre-check sample size before parsing
      perf tools: Robustify dynamic sample content fetch
      perf tools: Propagate event parse error handling


 tools/perf/builtin-test.c |    9 ++++++++-
 tools/perf/builtin-top.c  |    7 ++++++-
 tools/perf/util/event.c   |   16 ++++++++++++++++
 tools/perf/util/event.h   |   12 +++++++++++-
 tools/perf/util/evlist.c  |   31 +++++++++++++++++++++++++++++++
 tools/perf/util/evlist.h  |    3 +++
 tools/perf/util/evsel.c   |   32 +++++++++++++++++++++++++++++++-
 tools/perf/util/header.c  |   31 -------------------------------
 tools/perf/util/header.h  |    2 --
 tools/perf/util/python.c  |   13 ++++++++++---
 tools/perf/util/session.c |   25 ++++++++++++++++++-------
 tools/perf/util/session.h |    2 ++
 12 files changed, 136 insertions(+), 47 deletions(-)

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

* [PATCH 1/6] perf tools: Check we are able to read the event size on mmap
  2011-05-22  1:45 [GIT PULL] perf tools updates Frederic Weisbecker
@ 2011-05-22  1:45 ` Frederic Weisbecker
  2011-05-22  1:45 ` [PATCH 2/6] perf tools: Remove junk code in mmap size handling Frederic Weisbecker
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Frederic Weisbecker @ 2011-05-22  1:45 UTC (permalink / raw)
  To: Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: LKML, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Stephane Eranian

Check we have enough mmaped space to read the current event
size from its headers, otherwise we may dereference some
hell there.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/util/session.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index fff6674..61746b5 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1007,6 +1007,13 @@ remap:
 	file_pos = file_offset + head;
 
 more:
+	/*
+	 * Ensure we have enough space remaining to read
+	 * the size of the event in the headers.
+	 */
+	if (head + sizeof(event->header) > mmap_size)
+		goto remap;
+
 	event = (union perf_event *)(buf + head);
 
 	if (session->header.needs_swap)
-- 
1.7.3.2


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

* [PATCH 2/6] perf tools: Remove junk code in mmap size handling
  2011-05-22  1:45 [GIT PULL] perf tools updates Frederic Weisbecker
  2011-05-22  1:45 ` [PATCH 1/6] perf tools: Check we are able to read the event size on mmap Frederic Weisbecker
@ 2011-05-22  1:45 ` Frederic Weisbecker
  2011-05-22  1:45 ` [PATCH 4/6] perf tools: Pre-check sample size before parsing Frederic Weisbecker
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Frederic Weisbecker @ 2011-05-22  1:45 UTC (permalink / raw)
  To: Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: LKML, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Stephane Eranian

size is overriden later and used only then. Those
lines are only junk, probably a leftover.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/util/session.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 61746b5..db65206 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1018,9 +1018,6 @@ more:
 
 	if (session->header.needs_swap)
 		perf_event_header__bswap(&event->header);
-	size = event->header.size;
-	if (size == 0)
-		size = 8;
 
 	if (head + event->header.size > mmap_size) {
 		if (mmaps[map_idx]) {
-- 
1.7.3.2


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

* [PATCH 4/6] perf tools: Pre-check sample size before parsing
  2011-05-22  1:45 [GIT PULL] perf tools updates Frederic Weisbecker
  2011-05-22  1:45 ` [PATCH 1/6] perf tools: Check we are able to read the event size on mmap Frederic Weisbecker
  2011-05-22  1:45 ` [PATCH 2/6] perf tools: Remove junk code in mmap size handling Frederic Weisbecker
@ 2011-05-22  1:45 ` Frederic Weisbecker
  2011-05-22  1:45 ` [PATCH 5/6] perf tools: Robustify dynamic sample content fetch Frederic Weisbecker
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Frederic Weisbecker @ 2011-05-22  1:45 UTC (permalink / raw)
  To: Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: LKML, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Stephane Eranian

Check that the total size of the sample fields having a fixed
size do not exceed the one of the whole event. This robustifies
the sample parsing.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/builtin-test.c |    4 +++-
 tools/perf/util/event.c   |   16 ++++++++++++++++
 tools/perf/util/event.h   |   12 +++++++++++-
 tools/perf/util/evsel.c   |    6 +++++-
 tools/perf/util/python.c  |    5 +++--
 tools/perf/util/session.c |    1 +
 tools/perf/util/session.h |    2 ++
 7 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 11e3c84..44d7df2 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -474,6 +474,7 @@ static int test__basic_mmap(void)
 	unsigned int nr_events[nsyscalls],
 		     expected_nr_events[nsyscalls], i, j;
 	struct perf_evsel *evsels[nsyscalls], *evsel;
+	int sample_size = perf_sample_size(attr.sample_type);
 
 	for (i = 0; i < nsyscalls; ++i) {
 		char name[64];
@@ -558,7 +559,8 @@ static int test__basic_mmap(void)
 			goto out_munmap;
 		}
 
-		perf_event__parse_sample(event, attr.sample_type, false, &sample);
+		perf_event__parse_sample(event, attr.sample_type, sample_size,
+					 false, &sample);
 		evsel = perf_evlist__id2evsel(evlist, sample.id);
 		if (evsel == NULL) {
 			pr_debug("event with id %" PRIu64
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 1023f67..17c1c3c 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -35,6 +35,22 @@ const char *perf_event__name(unsigned int id)
 	return perf_event__names[id];
 }
 
+int perf_sample_size(u64 sample_type)
+{
+	u64 mask = sample_type & PERF_SAMPLE_MASK;
+	int size = 0;
+	int i;
+
+	for (i = 0; i < 64; i++) {
+		if ((mask << i) & 1)
+			size++;
+	}
+
+	size *= sizeof(u64);
+
+	return size;
+}
+
 static struct perf_sample synth_sample = {
 	.pid	   = -1,
 	.tid	   = -1,
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 9c35170..c083328 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -56,6 +56,13 @@ struct read_event {
 	u64 id;
 };
 
+
+#define PERF_SAMPLE_MASK				\
+	(PERF_SAMPLE_IP | PERF_SAMPLE_TID |		\
+	 PERF_SAMPLE_TIME | PERF_SAMPLE_ADDR |		\
+	PERF_SAMPLE_ID | PERF_SAMPLE_STREAM_ID |	\
+	 PERF_SAMPLE_CPU | PERF_SAMPLE_PERIOD)
+
 struct sample_event {
 	struct perf_event_header        header;
 	u64 array[];
@@ -75,6 +82,8 @@ struct perf_sample {
 	struct ip_callchain *callchain;
 };
 
+int perf_sample_size(u64 sample_type);
+
 #define BUILD_ID_SIZE 20
 
 struct build_id_event {
@@ -178,6 +187,7 @@ int perf_event__preprocess_sample(const union perf_event *self,
 const char *perf_event__name(unsigned int id);
 
 int perf_event__parse_sample(const union perf_event *event, u64 type,
-			     bool sample_id_all, struct perf_sample *sample);
+			     int sample_size, bool sample_id_all,
+			     struct perf_sample *sample);
 
 #endif /* __PERF_RECORD_H */
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d6fd59b..bfce8bf 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -304,7 +304,8 @@ static int perf_event__parse_id_sample(const union perf_event *event, u64 type,
 }
 
 int perf_event__parse_sample(const union perf_event *event, u64 type,
-			     bool sample_id_all, struct perf_sample *data)
+			     int sample_size, bool sample_id_all,
+			     struct perf_sample *data)
 {
 	const u64 *array;
 
@@ -319,6 +320,9 @@ int perf_event__parse_sample(const union perf_event *event, u64 type,
 
 	array = event->sample.array;
 
+	if (sample_size + sizeof(event->header) > event->header.size)
+		return -EFAULT;
+
 	if (type & PERF_SAMPLE_IP) {
 		data->ip = event->ip.ip;
 		array++;
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 8b0eff8..4174c09 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -690,8 +690,9 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist,
 			return PyErr_NoMemory();
 
 		first = list_entry(evlist->entries.next, struct perf_evsel, node);
-		perf_event__parse_sample(event, first->attr.sample_type, sample_id_all,
-					 &pevent->sample);
+		perf_event__parse_sample(event, first->attr.sample_type,
+					 perf_sample_size(first->attr.sample_type),
+					 sample_id_all, &pevent->sample);
 		return pyevent;
 	}
 
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index db65206..8940fd8 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -97,6 +97,7 @@ out:
 void perf_session__update_sample_type(struct perf_session *self)
 {
 	self->sample_type = perf_evlist__sample_type(self->evlist);
+	self->sample_size = perf_sample_size(self->sample_type);
 	self->sample_id_all = perf_evlist__sample_id_all(self->evlist);
 	perf_session__id_header_size(self);
 }
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 8daaa2d..66d4e14 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -43,6 +43,7 @@ struct perf_session {
 	 */
 	struct hists		hists;
 	u64			sample_type;
+	int			sample_size;
 	int			fd;
 	bool			fd_pipe;
 	bool			repipe;
@@ -159,6 +160,7 @@ static inline int perf_session__parse_sample(struct perf_session *session,
 					     struct perf_sample *sample)
 {
 	return perf_event__parse_sample(event, session->sample_type,
+					session->sample_size,
 					session->sample_id_all, sample);
 }
 
-- 
1.7.3.2


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

* [PATCH 5/6] perf tools: Robustify dynamic sample content fetch
  2011-05-22  1:45 [GIT PULL] perf tools updates Frederic Weisbecker
                   ` (2 preceding siblings ...)
  2011-05-22  1:45 ` [PATCH 4/6] perf tools: Pre-check sample size before parsing Frederic Weisbecker
@ 2011-05-22  1:45 ` Frederic Weisbecker
  2011-05-22  1:45 ` [PATCH 6/6] perf tools: Propagate event parse error handling Frederic Weisbecker
  2011-05-22  8:49 ` [GIT PULL] perf tools updates Ingo Molnar
  5 siblings, 0 replies; 13+ messages in thread
From: Frederic Weisbecker @ 2011-05-22  1:45 UTC (permalink / raw)
  To: Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: LKML, Frederic Weisbecker, Linus Torvalds, Ingo Molnar,
	Peter Zijlstra, Arnaldo Carvalho de Melo, Stephane Eranian

Ensure the size of the dynamic fields such as callchains
or raw events don't overlap the whole event boundaries.

This prevents from dereferencing junk if the given size of
the callchain goes too eager.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/util/evsel.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index bfce8bf..ee0fe0d 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -303,6 +303,17 @@ static int perf_event__parse_id_sample(const union perf_event *event, u64 type,
 	return 0;
 }
 
+static bool sample_overlap(const union perf_event *event,
+			   const void *offset, u64 size)
+{
+	const void *base = event;
+
+	if (offset + size > base + event->header.size)
+		return true;
+
+	return false;
+}
+
 int perf_event__parse_sample(const union perf_event *event, u64 type,
 			     int sample_size, bool sample_id_all,
 			     struct perf_sample *data)
@@ -373,14 +384,29 @@ int perf_event__parse_sample(const union perf_event *event, u64 type,
 	}
 
 	if (type & PERF_SAMPLE_CALLCHAIN) {
+		if (sample_overlap(event, array, sizeof(data->callchain->nr)))
+			return -EFAULT;
+
 		data->callchain = (struct ip_callchain *)array;
+
+		if (sample_overlap(event, array, data->callchain->nr))
+			return -EFAULT;
+
 		array += 1 + data->callchain->nr;
 	}
 
 	if (type & PERF_SAMPLE_RAW) {
 		u32 *p = (u32 *)array;
+
+		if (sample_overlap(event, array, sizeof(u32)))
+			return -EFAULT;
+
 		data->raw_size = *p;
 		p++;
+
+		if (sample_overlap(event, p, data->raw_size))
+			return -EFAULT;
+
 		data->raw_data = p;
 	}
 
-- 
1.7.3.2


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

* [PATCH 6/6] perf tools: Propagate event parse error handling
  2011-05-22  1:45 [GIT PULL] perf tools updates Frederic Weisbecker
                   ` (3 preceding siblings ...)
  2011-05-22  1:45 ` [PATCH 5/6] perf tools: Robustify dynamic sample content fetch Frederic Weisbecker
@ 2011-05-22  1:45 ` Frederic Weisbecker
  2011-05-22  8:49 ` [GIT PULL] perf tools updates Ingo Molnar
  5 siblings, 0 replies; 13+ messages in thread
From: Frederic Weisbecker @ 2011-05-22  1:45 UTC (permalink / raw)
  To: Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: LKML, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Stephane Eranian

Better handle event parsing error by propagating the details
in upper layers or by dumping some failure message. So that
the user knows he has some crazy events in the batch.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/builtin-test.c |    9 +++++++--
 tools/perf/builtin-top.c  |    7 ++++++-
 tools/perf/util/python.c  |   14 ++++++++++----
 tools/perf/util/session.c |   14 ++++++++++----
 4 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 44d7df2..1fa9f58 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -559,8 +559,13 @@ static int test__basic_mmap(void)
 			goto out_munmap;
 		}
 
-		perf_event__parse_sample(event, attr.sample_type, sample_size,
-					 false, &sample);
+		err = perf_event__parse_sample(event, attr.sample_type, sample_size,
+					       false, &sample);
+		if (err) {
+			pr_err("Can't parse sample, err = %d\n", err);
+			goto out_munmap;
+		}
+
 		evsel = perf_evlist__id2evsel(evlist, sample.id);
 		if (evsel == NULL) {
 			pr_debug("event with id %" PRIu64
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 7e3d6e3..74f533c 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -805,9 +805,14 @@ static void perf_session__mmap_read_cpu(struct perf_session *self, int cpu)
 {
 	struct perf_sample sample;
 	union perf_event *event;
+	int ret;
 
 	while ((event = perf_evlist__read_on_cpu(top.evlist, cpu)) != NULL) {
-		perf_session__parse_sample(self, event, &sample);
+		ret = perf_session__parse_sample(self, event, &sample);
+		if (ret) {
+			pr_err("Can't parse sample, err = %d\n", ret);
+			continue;
+		}
 
 		if (event->header.type == PERF_RECORD_SAMPLE)
 			perf_event__process_sample(event, &sample, self);
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 4174c09..3344d6e 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -675,6 +675,7 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist,
 	union perf_event *event;
 	int sample_id_all = 1, cpu;
 	static char *kwlist[] = {"sample_id_all", NULL, NULL};
+	int err;
 
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "i|i", kwlist,
 					 &cpu, &sample_id_all))
@@ -690,12 +691,17 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist,
 			return PyErr_NoMemory();
 
 		first = list_entry(evlist->entries.next, struct perf_evsel, node);
-		perf_event__parse_sample(event, first->attr.sample_type,
-					 perf_sample_size(first->attr.sample_type),
-					 sample_id_all, &pevent->sample);
+		err = perf_event__parse_sample(event, first->attr.sample_type,
+					       perf_sample_size(first->attr.sample_type),
+					       sample_id_all, &pevent->sample);
+		if (err) {
+			pr_err("Can't parse sample, err = %d\n", err);
+			goto end;
+		}
+
 		return pyevent;
 	}
-
+end:
 	Py_INCREF(Py_None);
 	return Py_None;
 }
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 8940fd8..948327d 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -480,6 +480,7 @@ static void flush_sample_queue(struct perf_session *s,
 	struct perf_sample sample;
 	u64 limit = os->next_flush;
 	u64 last_ts = os->last_sample ? os->last_sample->timestamp : 0ULL;
+	int ret;
 
 	if (!ops->ordered_samples || !limit)
 		return;
@@ -488,9 +489,12 @@ static void flush_sample_queue(struct perf_session *s,
 		if (iter->timestamp > limit)
 			break;
 
-		perf_session__parse_sample(s, iter->event, &sample);
-		perf_session_deliver_event(s, iter->event, &sample, ops,
-					   iter->file_offset);
+		ret = perf_session__parse_sample(s, iter->event, &sample);
+		if (ret)
+			pr_err("Can't parse sample, err = %d\n", ret);
+		else
+			perf_session_deliver_event(s, iter->event, &sample, ops,
+						   iter->file_offset);
 
 		os->last_flush = iter->timestamp;
 		list_del(&iter->list);
@@ -806,7 +810,9 @@ static int perf_session__process_event(struct perf_session *session,
 	/*
 	 * For all kernel events we get the sample data
 	 */
-	perf_session__parse_sample(session, event, &sample);
+	ret = perf_session__parse_sample(session, event, &sample);
+	if (ret)
+		return ret;
 
 	/* Preprocess sample records - precheck callchains */
 	if (perf_session__preprocess_sample(session, event, &sample))
-- 
1.7.3.2


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

* Re: [GIT PULL] perf tools updates
  2011-05-22  1:45 [GIT PULL] perf tools updates Frederic Weisbecker
                   ` (4 preceding siblings ...)
  2011-05-22  1:45 ` [PATCH 6/6] perf tools: Propagate event parse error handling Frederic Weisbecker
@ 2011-05-22  8:49 ` Ingo Molnar
  2011-05-22 12:07   ` Frederic Weisbecker
  5 siblings, 1 reply; 13+ messages in thread
From: Ingo Molnar @ 2011-05-22  8:49 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Arnaldo Carvalho de Melo, LKML, Peter Zijlstra, Stephane Eranian,
	Linus Torvalds


* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> Ingo, Arnaldo,
> 
> Please pull the perf/core branch that can be found at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> 	perf/core
> 
> Thanks,
> 	Frederic
> ---
> 
> Frederic Weisbecker (6):
>       perf tools: Check we are able to read the event size on mmap
>       perf tools: Remove junk code in mmap size handling
>       perf tools: Move evlist sample helpers to evlist area
>       perf tools: Pre-check sample size before parsing
>       perf tools: Robustify dynamic sample content fetch
>       perf tools: Propagate event parse error handling
> 
> 
>  tools/perf/builtin-test.c |    9 ++++++++-
>  tools/perf/builtin-top.c  |    7 ++++++-
>  tools/perf/util/event.c   |   16 ++++++++++++++++
>  tools/perf/util/event.h   |   12 +++++++++++-
>  tools/perf/util/evlist.c  |   31 +++++++++++++++++++++++++++++++
>  tools/perf/util/evlist.h  |    3 +++
>  tools/perf/util/evsel.c   |   32 +++++++++++++++++++++++++++++++-
>  tools/perf/util/header.c  |   31 -------------------------------
>  tools/perf/util/header.h  |    2 --
>  tools/perf/util/python.c  |   13 ++++++++++---
>  tools/perf/util/session.c |   25 ++++++++++++++++++-------
>  tools/perf/util/session.h |    2 ++
>  12 files changed, 136 insertions(+), 47 deletions(-)

To get this upstream ASAP i pulled them into perf/urgent and resolved two 
conflicts there (a contextual and a semantic one) - please double check my 
resolutions.

Thanks,

	Ingo

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

* Re: [GIT PULL] perf tools updates
  2011-05-22  8:49 ` [GIT PULL] perf tools updates Ingo Molnar
@ 2011-05-22 12:07   ` Frederic Weisbecker
  0 siblings, 0 replies; 13+ messages in thread
From: Frederic Weisbecker @ 2011-05-22 12:07 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Arnaldo Carvalho de Melo, LKML, Peter Zijlstra, Stephane Eranian,
	Linus Torvalds

On Sun, May 22, 2011 at 10:49:34AM +0200, Ingo Molnar wrote:
> 
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> 
> > Ingo, Arnaldo,
> > 
> > Please pull the perf/core branch that can be found at:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> > 	perf/core
> > 
> > Thanks,
> > 	Frederic
> > ---
> > 
> > Frederic Weisbecker (6):
> >       perf tools: Check we are able to read the event size on mmap
> >       perf tools: Remove junk code in mmap size handling
> >       perf tools: Move evlist sample helpers to evlist area
> >       perf tools: Pre-check sample size before parsing
> >       perf tools: Robustify dynamic sample content fetch
> >       perf tools: Propagate event parse error handling
> > 
> > 
> >  tools/perf/builtin-test.c |    9 ++++++++-
> >  tools/perf/builtin-top.c  |    7 ++++++-
> >  tools/perf/util/event.c   |   16 ++++++++++++++++
> >  tools/perf/util/event.h   |   12 +++++++++++-
> >  tools/perf/util/evlist.c  |   31 +++++++++++++++++++++++++++++++
> >  tools/perf/util/evlist.h  |    3 +++
> >  tools/perf/util/evsel.c   |   32 +++++++++++++++++++++++++++++++-
> >  tools/perf/util/header.c  |   31 -------------------------------
> >  tools/perf/util/header.h  |    2 --
> >  tools/perf/util/python.c  |   13 ++++++++++---
> >  tools/perf/util/session.c |   25 ++++++++++++++++++-------
> >  tools/perf/util/session.h |    2 ++
> >  12 files changed, 136 insertions(+), 47 deletions(-)
> 
> To get this upstream ASAP i pulled them into perf/urgent and resolved two 
> conflicts there (a contextual and a semantic one) - please double check my 
> resolutions.

Yeah looks good.

Thanks!

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

* Re: [GIT PULL] perf tools updates
  2011-06-29 23:34 Frederic Weisbecker
@ 2011-07-01 10:01 ` Ingo Molnar
  0 siblings, 0 replies; 13+ messages in thread
From: Ingo Molnar @ 2011-07-01 10:01 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: LKML, Peter Zijlstra, Arnaldo Carvalho de Melo, Stephane Eranian,
	David Ahern, Sam Liao


* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> Ingo,
> 
> Please pull the perf/core branch that can be found at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> 	perf/core
> 
> It adds the inverted callchains support and let one use
> parent filtering with parent sorting at the same time, because
> it appears to me that inverted callchains sorted by filtered
> parents is pretty useful, and extendable to more cool things.
> 
> Anyway inverted callchains used with some different sorting combination
> in general can provide some interesting analysis flavours.
> 
> Having played with it a bit. It seems to me the callee point
> of view (traditional -g callchains) is better suited to
> find the precise zoomed-in places where cpu time is most
> spent. Spot contention places, etc...
> 
> OTOH, caller point of view (-G, inverted callchain), is
> for zoomed out observation, of course. It's more suited for
> global profiling. To get a big overview of where the hot bulk
> of a program is executing for example.
> 
> Examples:
> 
> - look at the hottest tree of call of a program.
> 
> 	./perf report -G -s pid --stdio
> 	
>      5.73%               perf:11933
>             |
>             --- __libc_start_main
>                |          
>                |--99.18%-- main
>                |          run_builtin
>                |          cmd_bench
>                |          |          
>                |          |--89.68%-- bench_sched_messaging
>                |          |          |          
>                |          |          |--96.11%-- create_worker
>                |          |          |          |          
>                |          |          |          |--95.10%-- __libc_fork
>                |          |          |          |          |          
>                |          |          |          |          |--93.99%-- stub_clone
>                |          |          |          |          |          sys_clone
>                |          |          |          |          |          do_fork
>                |          |          |          |          |          |          
>                |          |          |          |          |          |--99.09%-- copy_process
>                |          |          |          |          |          |          |          
>                |          |          |          |          |          |          |--91.62%-- dup_mm
> 
> - look at where kernel threads spend their time
> 
> 	perf report -G -p kernel_thread -s parent --stdio
> 	
> # Overhead  Parent symbol
> # ........  .............
> #
>      0.07%  kernel_thread_helper
>             |
>             --- kernel_thread_helper
>                 kthread
>                |          
>                |--50.00%-- kjournald2
>                |          jbd2_journal_commit_transaction
>                |          journal_submit_commit_record
>                |          submit_bh
>                |          submit_bio
>                |          generic_make_request
>                |          __make_request
>                |          __blk_run_queue
>                |          scsi_request_fn
>                |          scsi_dispatch_cmd
>                |          ata_scsi_queuecmd
>                |          ata_scsi_translate
>                |          ata_qc_issue
>                |          ata_bmdma_qc_issue
>                |          ata_sff_qc_issue
>                |          ata_sff_tf_load
>                |          ata_sff_check_status
>                |          ioread8
>                |          
>                 --50.00%-- rcu_kthread
>                           rcu_process_callbacks
>                           delayed_put_task_struct
>                           __put_task_struct
>                           free_task
>                           free_thread_info
>                           free_thread_xstate
>                           kmem_cache_free
>                           __slab_free
>                           add_partial
>                           _raw_spin_lock
>                           lock_acquire
>                           
> etc...
> 
> We could extend that by applying some cut in the callchains.
> For example stop a callchain on a given dso and you can profile
> which exported function is most called in it.
> 
> Anyway, this has some nice potential.
> 
> 
> Thanks,
> 	Frederic
> ---
> 
> Frederic Weisbecker (5):
>       perf tools: Make sort operations static
>       perf tools: Remove sort print helpers declarations
>       perf tools: Don't display ignored entries on stdio ui
>       perf tools: Allow sort dimensions to be registered more than once
>       perf tools: Only display parent field if explictly sorted
> 
> Sam Liao (1):
>       perf tools: Add inverted call graph report support.
> 
> 
>  tools/perf/Documentation/perf-report.txt |   15 ++-
>  tools/perf/builtin-report.c              |   42 +++++-
>  tools/perf/util/callchain.h              |    6 +
>  tools/perf/util/hist.c                   |    6 +-
>  tools/perf/util/session.c                |    7 +-
>  tools/perf/util/sort.c                   |  223 ++++++++++++++----------------
>  tools/perf/util/sort.h                   |   14 --
>  7 files changed, 169 insertions(+), 144 deletions(-)

Pulled, thanks a lot Frederic and Sam Liao!

This feature looks really useful.

One thing that occured to me: could we perhaps make -G the default 
for -g -A profiles and keep -g the default for task-hierarchy (and 
per PID) profiling? [a hint could be added to the comment section of 
the output to show that there's a -g/-G distinction.]

The reason is that -G is arguably more suited for global, system-wide 
profiling - and this is also the mode of display that sysprof uses 
and which people got used to in general.

There is some small confusion potential from switching the view like 
this but i think if we point it out in the output it should be fine:

#
# Bottom-up (-g) call-graph, use -G to view the top-down call-graph
#

#
# Top-down (-G) call-graph, use -g to view the bottom-up call-graph
#

Another thing: could we perhaps make inverted call-graphs the default 
view for perf top --tui as well? That is a common 'global view' 
profiling tool as well.

Finally, we should perhaps refer to them as bottom-up versus top-down 
call-graphs, 'inverted' and 'normal' does not really reflect the true 
nature of the call-graph, and to many people top-down is the natural 
call-graph view mode ...

Thanks,

	Ingo

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

* [GIT PULL] perf tools updates
@ 2011-06-29 23:34 Frederic Weisbecker
  2011-07-01 10:01 ` Ingo Molnar
  0 siblings, 1 reply; 13+ messages in thread
From: Frederic Weisbecker @ 2011-06-29 23:34 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Stephane Eranian, David Ahern,
	Sam Liao

Ingo,

Please pull the perf/core branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
	perf/core

It adds the inverted callchains support and let one use
parent filtering with parent sorting at the same time, because
it appears to me that inverted callchains sorted by filtered
parents is pretty useful, and extendable to more cool things.

Anyway inverted callchains used with some different sorting combination
in general can provide some interesting analysis flavours.

Having played with it a bit. It seems to me the callee point
of view (traditional -g callchains) is better suited to
find the precise zoomed-in places where cpu time is most
spent. Spot contention places, etc...

OTOH, caller point of view (-G, inverted callchain), is
for zoomed out observation, of course. It's more suited for
global profiling. To get a big overview of where the hot bulk
of a program is executing for example.

Examples:

- look at the hottest tree of call of a program.

	./perf report -G -s pid --stdio
	
     5.73%               perf:11933
            |
            --- __libc_start_main
               |          
               |--99.18%-- main
               |          run_builtin
               |          cmd_bench
               |          |          
               |          |--89.68%-- bench_sched_messaging
               |          |          |          
               |          |          |--96.11%-- create_worker
               |          |          |          |          
               |          |          |          |--95.10%-- __libc_fork
               |          |          |          |          |          
               |          |          |          |          |--93.99%-- stub_clone
               |          |          |          |          |          sys_clone
               |          |          |          |          |          do_fork
               |          |          |          |          |          |          
               |          |          |          |          |          |--99.09%-- copy_process
               |          |          |          |          |          |          |          
               |          |          |          |          |          |          |--91.62%-- dup_mm

- look at where kernel threads spend their time

	perf report -G -p kernel_thread -s parent --stdio
	
# Overhead  Parent symbol
# ........  .............
#
     0.07%  kernel_thread_helper
            |
            --- kernel_thread_helper
                kthread
               |          
               |--50.00%-- kjournald2
               |          jbd2_journal_commit_transaction
               |          journal_submit_commit_record
               |          submit_bh
               |          submit_bio
               |          generic_make_request
               |          __make_request
               |          __blk_run_queue
               |          scsi_request_fn
               |          scsi_dispatch_cmd
               |          ata_scsi_queuecmd
               |          ata_scsi_translate
               |          ata_qc_issue
               |          ata_bmdma_qc_issue
               |          ata_sff_qc_issue
               |          ata_sff_tf_load
               |          ata_sff_check_status
               |          ioread8
               |          
                --50.00%-- rcu_kthread
                          rcu_process_callbacks
                          delayed_put_task_struct
                          __put_task_struct
                          free_task
                          free_thread_info
                          free_thread_xstate
                          kmem_cache_free
                          __slab_free
                          add_partial
                          _raw_spin_lock
                          lock_acquire
                          
etc...

We could extend that by applying some cut in the callchains.
For example stop a callchain on a given dso and you can profile
which exported function is most called in it.

Anyway, this has some nice potential.


Thanks,
	Frederic
---

Frederic Weisbecker (5):
      perf tools: Make sort operations static
      perf tools: Remove sort print helpers declarations
      perf tools: Don't display ignored entries on stdio ui
      perf tools: Allow sort dimensions to be registered more than once
      perf tools: Only display parent field if explictly sorted

Sam Liao (1):
      perf tools: Add inverted call graph report support.


 tools/perf/Documentation/perf-report.txt |   15 ++-
 tools/perf/builtin-report.c              |   42 +++++-
 tools/perf/util/callchain.h              |    6 +
 tools/perf/util/hist.c                   |    6 +-
 tools/perf/util/session.c                |    7 +-
 tools/perf/util/sort.c                   |  223 ++++++++++++++----------------
 tools/perf/util/sort.h                   |   14 --
 7 files changed, 169 insertions(+), 144 deletions(-)

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

* Re: [GIT PULL] perf tools updates
  2010-04-24  2:05 Frederic Weisbecker
  2010-04-24  2:27 ` Frederic Weisbecker
@ 2010-04-27  9:15 ` Ingo Molnar
  1 sibling, 0 replies; 13+ messages in thread
From: Ingo Molnar @ 2010-04-27  9:15 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: LKML, William Cohen, Peter Zijlstra, Arnaldo Carvalho de Melo,
	Paul Mackerras, Hitoshi Mitake, Masami Hiramatsu, Tom Zanussi,
	Arjan van de Ven, Pekka Enberg, Li Zefan, Stephane Eranian,
	Jens Axboe, Jason Baron, Xiao Guangrong


* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> Ingo,
> 
> Please pull the perf/core branch that can be found at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> 	perf/core
> 
> Thanks,
> 	Frederic
> ---
> 
> Frederic Weisbecker (6):
>       perf: Generalize perf lock's sample event reordering to the session layer
>       perf: Use generic sample reordering in perf sched
>       perf: Use generic sample reordering in perf kmem
>       perf: Use generic sample reordering in perf trace
>       perf: Use generic sample reordering in perf timechart
>       perf: Add a perf trace option to check samples ordering reliability
> 
> Hitoshi Mitake (1):
>       perf lock: Fix state machine to recognize lock sequence
> 
> Stephane Eranian (1):
>       perf: Fix initialization bug in parse_single_tracepoint_event()
> 
> William Cohen (1):
>       perf: Some perf-kvm documentation edits
> 
> 
>  tools/perf/Documentation/perf-kvm.txt |    9 +-
>  tools/perf/builtin-kmem.c             |    6 +-
>  tools/perf/builtin-lock.c             |  595 ++++++++++++++++++++-------------
>  tools/perf/builtin-sched.c            |    8 +-
>  tools/perf/builtin-timechart.c        |  112 +------
>  tools/perf/builtin-trace.c            |   13 +
>  tools/perf/util/parse-events.c        |   13 +-
>  tools/perf/util/session.c             |  179 ++++++++++-
>  tools/perf/util/session.h             |   10 +
>  9 files changed, 583 insertions(+), 362 deletions(-)

Pulled, thanks a lot Frederic!

	Ingo

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

* Re: [GIT PULL] perf tools updates
  2010-04-24  2:05 Frederic Weisbecker
@ 2010-04-24  2:27 ` Frederic Weisbecker
  2010-04-27  9:15 ` Ingo Molnar
  1 sibling, 0 replies; 13+ messages in thread
From: Frederic Weisbecker @ 2010-04-24  2:27 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, William Cohen, Peter Zijlstra, Arnaldo Carvalho de Melo,
	Paul Mackerras, Hitoshi Mitake, Masami Hiramatsu, Tom Zanussi,
	Arjan van de Ven, Pekka Enberg, Li Zefan, Stephane Eranian,
	Jens Axboe, Jason Baron, Xiao Guangrong

On Sat, Apr 24, 2010 at 04:05:33AM +0200, Frederic Weisbecker wrote:
> Ingo,
> 
> Please pull the perf/core branch that can be found at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> 	perf/core
> 
> Thanks,
> 	Frederic



I forgot to highlight some things here.

- The -M option is not used anymore. Well actually I just checked and it's
  used by the record perl/python scripts. But it's not needed there anymore, so
  I'll drop it in another pass. But globally it's over with the buffers
  multiplexing needs.

- But I haven't plugged the reordering thing to the live mode, because I'm
  not sure exactly if that would be welcome. With live mode we want the
  events as they arrive, using the reordering there would make it get the
  events per bunches of 2 seconds slices. I guess we'll figure out a solution
  for that.

- Perf lock gets into a better shape. There is still some work to make
  it truly usable though. I need to unearth the event injection thing
  to lower the size of the events, profile by lock classes, etc...

- Various important fixes

Thanks.


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

* [GIT PULL] perf tools updates
@ 2010-04-24  2:05 Frederic Weisbecker
  2010-04-24  2:27 ` Frederic Weisbecker
  2010-04-27  9:15 ` Ingo Molnar
  0 siblings, 2 replies; 13+ messages in thread
From: Frederic Weisbecker @ 2010-04-24  2:05 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, William Cohen, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Paul Mackerras, Hitoshi Mitake,
	Masami Hiramatsu, Tom Zanussi, Arjan van de Ven, Pekka Enberg,
	Li Zefan, Stephane Eranian, Jens Axboe, Jason Baron,
	Xiao Guangrong

Ingo,

Please pull the perf/core branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
	perf/core

Thanks,
	Frederic
---

Frederic Weisbecker (6):
      perf: Generalize perf lock's sample event reordering to the session layer
      perf: Use generic sample reordering in perf sched
      perf: Use generic sample reordering in perf kmem
      perf: Use generic sample reordering in perf trace
      perf: Use generic sample reordering in perf timechart
      perf: Add a perf trace option to check samples ordering reliability

Hitoshi Mitake (1):
      perf lock: Fix state machine to recognize lock sequence

Stephane Eranian (1):
      perf: Fix initialization bug in parse_single_tracepoint_event()

William Cohen (1):
      perf: Some perf-kvm documentation edits


 tools/perf/Documentation/perf-kvm.txt |    9 +-
 tools/perf/builtin-kmem.c             |    6 +-
 tools/perf/builtin-lock.c             |  595 ++++++++++++++++++++-------------
 tools/perf/builtin-sched.c            |    8 +-
 tools/perf/builtin-timechart.c        |  112 +------
 tools/perf/builtin-trace.c            |   13 +
 tools/perf/util/parse-events.c        |   13 +-
 tools/perf/util/session.c             |  179 ++++++++++-
 tools/perf/util/session.h             |   10 +
 9 files changed, 583 insertions(+), 362 deletions(-)

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

end of thread, other threads:[~2011-07-01 10:01 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-22  1:45 [GIT PULL] perf tools updates Frederic Weisbecker
2011-05-22  1:45 ` [PATCH 1/6] perf tools: Check we are able to read the event size on mmap Frederic Weisbecker
2011-05-22  1:45 ` [PATCH 2/6] perf tools: Remove junk code in mmap size handling Frederic Weisbecker
2011-05-22  1:45 ` [PATCH 4/6] perf tools: Pre-check sample size before parsing Frederic Weisbecker
2011-05-22  1:45 ` [PATCH 5/6] perf tools: Robustify dynamic sample content fetch Frederic Weisbecker
2011-05-22  1:45 ` [PATCH 6/6] perf tools: Propagate event parse error handling Frederic Weisbecker
2011-05-22  8:49 ` [GIT PULL] perf tools updates Ingo Molnar
2011-05-22 12:07   ` Frederic Weisbecker
  -- strict thread matches above, loose matches on Subject: below --
2011-06-29 23:34 Frederic Weisbecker
2011-07-01 10:01 ` Ingo Molnar
2010-04-24  2:05 Frederic Weisbecker
2010-04-24  2:27 ` Frederic Weisbecker
2010-04-27  9:15 ` Ingo Molnar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.