From: Arnaldo Carvalho de Melo <[email protected]>
Hi Ingo,
Please consider pulling,
- Arnaldo
The following changes since commit 816434ec4a674fcdb3c2221a6dffdc8f34020550:
Merge branch 'x86-spinlocks-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (2013-09-04 11:55:10 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-urgent-for-mingo
for you to fetch changes up to 526fd8d4f770d18e99680ff87965e16bb8f1d806:
perf session: Separate progress bar update when processing events (2013-09-05 16:19:02 -0300)
----------------------------------------------------------------
perf/urgent fixes:
. Fix parsing with no sample_id_all bit set, this regression prevents perf
from reading old perf.data files generated in systems where
perf_event_attr.sample_id_all isn't available, from Adrian Hunter.
. Add signal checking to the inner 'perf trace' event processing loop, allowing
faster response to control+C.
. Fix formatting of long symbol names removing the hardcoding of a buffer
size used to format histogram entries, which was truncating the lines.
. Separate progress bar update when processing events, reducing potentially big
overhead in not needed TUI progress bar screen updates, from Jiri Olsa.
. Fix 'perf trace' build in architectures where MAP_32BIT is not defined, from
Kyle McMartin.
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
----------------------------------------------------------------
Adrian Hunter (2):
perf tools: Add test for parsing with no sample_id_all bit
perf evlist: Fix parsing with no sample_id_all bit set
Arnaldo Carvalho de Melo (2):
perf trace: Check control+C more often
perf hists: Fix formatting of long symbol names
Jiri Olsa (1):
perf session: Separate progress bar update when processing events
Kyle McMartin (1):
perf trace: Check if MAP_32BIT is defined
tools/perf/Makefile | 3 +-
tools/perf/builtin-trace.c | 5 ++
tools/perf/tests/builtin-test.c | 4 ++
tools/perf/tests/parse-no-sample-id-all.c | 108 ++++++++++++++++++++++++++++++
tools/perf/tests/tests.h | 1 +
tools/perf/ui/stdio/hist.c | 23 +++++--
tools/perf/util/evlist.c | 9 ++-
tools/perf/util/session.c | 3 +-
8 files changed, 146 insertions(+), 10 deletions(-)
create mode 100644 tools/perf/tests/parse-no-sample-id-all.c
From: Kyle McMartin <[email protected]>
MAP_32BIT is defined only on x86... this means perf fails to build on
all other platforms.
Signed-off-by: Kyle McMartin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Linus Torvalds <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-trace.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 6016112..f5aa637 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -100,7 +100,9 @@ static size_t syscall_arg__scnprintf_mmap_flags(char *bf, size_t size,
P_MMAP_FLAG(SHARED);
P_MMAP_FLAG(PRIVATE);
+#ifdef MAP_32BIT
P_MMAP_FLAG(32BIT);
+#endif
P_MMAP_FLAG(ANONYMOUS);
P_MMAP_FLAG(DENYWRITE);
P_MMAP_FLAG(EXECUTABLE);
--
1.8.1.4
From: Arnaldo Carvalho de Melo <[email protected]>
We were checking for it only after processing all events in the buffer,
delaying processing the termination request for long periods.
Cc: Adrian Hunter <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-trace.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index b6f0725..6016112 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -994,6 +994,9 @@ again:
handler = evsel->handler.func;
handler(trace, evsel, &sample);
+
+ if (done)
+ goto out_unmap_evlist;
}
}
--
1.8.1.4
From: Adrian Hunter <[email protected]>
Add a test for parsing a non-sample event when there is more than one
selected event but no sample_id_all bit set.
The test fails because of a bug in the evlist logic. That is fixed in a
separate patch.
Signed-off-by: Adrian Hunter <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Makefile | 3 +-
tools/perf/tests/builtin-test.c | 4 ++
tools/perf/tests/parse-no-sample-id-all.c | 108 ++++++++++++++++++++++++++++++
tools/perf/tests/tests.h | 1 +
4 files changed, 115 insertions(+), 1 deletion(-)
create mode 100644 tools/perf/tests/parse-no-sample-id-all.c
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index c5dc1ad..3a0ff7f 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -394,6 +394,8 @@ ifeq ($(ARCH),x86)
LIB_OBJS += $(OUTPUT)tests/perf-time-to-tsc.o
endif
LIB_OBJS += $(OUTPUT)tests/code-reading.o
+LIB_OBJS += $(OUTPUT)tests/sample-parsing.o
+LIB_OBJS += $(OUTPUT)tests/parse-no-sample-id-all.o
BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
@@ -439,7 +441,6 @@ PERFLIBS = $(LIB_FILE) $(LIBLK) $(LIBTRACEEVENT)
ifneq ($(OUTPUT),)
CFLAGS += -I$(OUTPUT)
endif
-LIB_OBJS += $(OUTPUT)tests/sample-parsing.o
ifdef NO_LIBELF
EXTLIBS := $(filter-out -lelf,$(EXTLIBS))
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 8bbeba3..1e67437 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -112,6 +112,10 @@ static struct test {
.func = test__keep_tracking,
},
{
+ .desc = "Test parsing with no sample_id_all bit set",
+ .func = test__parse_no_sample_id_all,
+ },
+ {
.func = NULL,
},
};
diff --git a/tools/perf/tests/parse-no-sample-id-all.c b/tools/perf/tests/parse-no-sample-id-all.c
new file mode 100644
index 0000000..e117b6c
--- /dev/null
+++ b/tools/perf/tests/parse-no-sample-id-all.c
@@ -0,0 +1,108 @@
+#include <sys/types.h>
+#include <stddef.h>
+
+#include "tests.h"
+
+#include "event.h"
+#include "evlist.h"
+#include "header.h"
+#include "util.h"
+
+static int process_event(struct perf_evlist **pevlist, union perf_event *event)
+{
+ struct perf_sample sample;
+
+ if (event->header.type == PERF_RECORD_HEADER_ATTR) {
+ if (perf_event__process_attr(NULL, event, pevlist)) {
+ pr_debug("perf_event__process_attr failed\n");
+ return -1;
+ }
+ return 0;
+ }
+
+ if (event->header.type >= PERF_RECORD_USER_TYPE_START)
+ return -1;
+
+ if (!*pevlist)
+ return -1;
+
+ if (perf_evlist__parse_sample(*pevlist, event, &sample)) {
+ pr_debug("perf_evlist__parse_sample failed\n");
+ return -1;
+ }
+
+ return 0;
+}
+
+static int process_events(union perf_event **events, size_t count)
+{
+ struct perf_evlist *evlist = NULL;
+ int err = 0;
+ size_t i;
+
+ for (i = 0; i < count && !err; i++)
+ err = process_event(&evlist, events[i]);
+
+ if (evlist)
+ perf_evlist__delete(evlist);
+
+ return err;
+}
+
+struct test_attr_event {
+ struct attr_event attr;
+ u64 id;
+};
+
+/**
+ * test__parse_no_sample_id_all - test parsing with no sample_id_all bit set.
+ *
+ * This function tests parsing data produced on kernel's that do not support the
+ * sample_id_all bit. Without the sample_id_all bit, non-sample events (such as
+ * mmap events) do not have an id sample appended, and consequently logic
+ * designed to determine the id will not work. That case happens when there is
+ * more than one selected event, so this test processes three events: 2
+ * attributes representing the selected events and one mmap event.
+ *
+ * Return: %0 on success, %-1 if the test fails.
+ */
+int test__parse_no_sample_id_all(void)
+{
+ int err;
+
+ struct test_attr_event event1 = {
+ .attr = {
+ .header = {
+ .type = PERF_RECORD_HEADER_ATTR,
+ .size = sizeof(struct test_attr_event),
+ },
+ },
+ .id = 1,
+ };
+ struct test_attr_event event2 = {
+ .attr = {
+ .header = {
+ .type = PERF_RECORD_HEADER_ATTR,
+ .size = sizeof(struct test_attr_event),
+ },
+ },
+ .id = 2,
+ };
+ struct mmap_event event3 = {
+ .header = {
+ .type = PERF_RECORD_MMAP,
+ .size = sizeof(struct mmap_event),
+ },
+ };
+ union perf_event *events[] = {
+ (union perf_event *)&event1,
+ (union perf_event *)&event2,
+ (union perf_event *)&event3,
+ };
+
+ err = process_events(events, ARRAY_SIZE(events));
+ if (err)
+ return -1;
+
+ return 0;
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index c048b58..e0ac713 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -39,5 +39,6 @@ int test__perf_time_to_tsc(void);
int test__code_reading(void);
int test__sample_parsing(void);
int test__keep_tracking(void);
+int test__parse_no_sample_id_all(void);
#endif /* TESTS_H */
--
1.8.1.4
From: Arnaldo Carvalho de Melo <[email protected]>
We had a hardcoded buffer for formatting histogram entries, truncating
long symbol names (C++ anyone?).
Fix it by using hists__sort_list_width() before formatting the first
histogram entry to calculate the max lenght needed by traversing the
overheads and columns lists (sort order).
Reported-by: Stephane Eranian <[email protected]>
Tested-by: Stephane Eranian <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/ui/stdio/hist.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 5b4fb33..194e2f4 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -350,9 +350,9 @@ static int hist_entry__period_snprintf(struct perf_hpp *hpp,
}
static int hist_entry__fprintf(struct hist_entry *he, size_t size,
- struct hists *hists, FILE *fp)
+ struct hists *hists,
+ char *bf, size_t bfsz, FILE *fp)
{
- char bf[512];
int ret;
struct perf_hpp hpp = {
.buf = bf,
@@ -360,8 +360,8 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
};
bool color = !symbol_conf.field_sep;
- if (size == 0 || size > sizeof(bf))
- size = hpp.size = sizeof(bf);
+ if (size == 0 || size > bfsz)
+ size = hpp.size = bfsz;
ret = hist_entry__period_snprintf(&hpp, he, color);
hist_entry__sort_snprintf(he, bf + ret, size - ret, hists);
@@ -392,6 +392,8 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
.ptr = hists_to_evsel(hists),
};
bool first = true;
+ size_t linesz;
+ char *line = NULL;
init_rem_hits();
@@ -479,6 +481,13 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
goto out;
print_entries:
+ linesz = hists__sort_list_width(hists) + 3 + 1;
+ line = malloc(linesz);
+ if (line == NULL) {
+ ret = -1;
+ goto out;
+ }
+
for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
float percent = h->stat.period * 100.0 /
@@ -490,10 +499,10 @@ print_entries:
if (percent < min_pcnt)
continue;
- ret += hist_entry__fprintf(h, max_cols, hists, fp);
+ ret += hist_entry__fprintf(h, max_cols, hists, line, linesz, fp);
if (max_rows && ++nr_rows >= max_rows)
- goto out;
+ break;
if (h->ms.map == NULL && verbose > 1) {
__map_groups__fprintf_maps(&h->thread->mg,
@@ -501,6 +510,8 @@ print_entries:
fprintf(fp, "%.10s end\n", graph_dotted_line);
}
}
+
+ free(line);
out:
free(rem_sq_bracket);
--
1.8.1.4
From: Adrian Hunter <[email protected]>
The perf_evlist__event2evsel() is changed to handle non-sample events
(such as mmap events) that have no id sample appended i.e. when
sample_id_all is not set.
Note that such events have a fixed format, so that the selected event
(evsel) they are associated with is immaterial.
Signed-off-by: Adrian Hunter <[email protected]>
Tested-by: David Ahern <[email protected]>
Acked-by: David Ahern <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/evlist.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index b8727ae..7101283 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -446,20 +446,25 @@ static int perf_evlist__event2id(struct perf_evlist *evlist,
static struct perf_evsel *perf_evlist__event2evsel(struct perf_evlist *evlist,
union perf_event *event)
{
+ struct perf_evsel *first = perf_evlist__first(evlist);
struct hlist_head *head;
struct perf_sample_id *sid;
int hash;
u64 id;
if (evlist->nr_entries == 1)
- return perf_evlist__first(evlist);
+ return first;
+
+ if (!first->attr.sample_id_all &&
+ event->header.type != PERF_RECORD_SAMPLE)
+ return first;
if (perf_evlist__event2id(evlist, event, &id))
return NULL;
/* Synthesized events have an id of zero */
if (!id)
- return perf_evlist__first(evlist);
+ return first;
hash = hash_64(id, PERF_EVLIST__HLIST_BITS);
head = &evlist->heads[hash];
--
1.8.1.4
From: Jiri Olsa <[email protected]>
Currently when processing events in the __perf_session__process_events
function we update a progress bar based on the file_size. During the
same processing we update the progress bar from within
flush_sample_queue which is based on number of samples count.
Having 2 different based updates is causing the progress bar to jump
heavily back and forth giving not much usefull info.
Fixing this by keeping only __perf_session__process_events based
progress bar update. And turning on flush_sample_queue progress bar
update only for final flushing.
This reduces the number of time the progress bar update function is
called and it significantly reduces the loading time for TUI, where the
progress bar update takes quite a lot of time.
Signed-off-by: Jiri Olsa <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/session.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 1fc0c62..476caa1 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -504,6 +504,7 @@ static int flush_sample_queue(struct perf_session *s,
u64 limit = os->next_flush;
u64 last_ts = os->last_sample ? os->last_sample->timestamp : 0ULL;
unsigned idx = 0, progress_next = os->nr_samples / 16;
+ bool show_progress = limit == ULLONG_MAX;
int ret;
if (!tool->ordered_samples || !limit)
@@ -526,7 +527,7 @@ static int flush_sample_queue(struct perf_session *s,
os->last_flush = iter->timestamp;
list_del(&iter->list);
list_add(&iter->list, &os->sample_cache);
- if (++idx >= progress_next) {
+ if (show_progress && (++idx >= progress_next)) {
progress_next += os->nr_samples / 16;
ui_progress__update(idx, os->nr_samples,
"Processing time ordered events...");
--
1.8.1.4
* Arnaldo Carvalho de Melo <[email protected]> wrote:
> From: Arnaldo Carvalho de Melo <[email protected]>
>
> Hi Ingo,
>
> Please consider pulling,
>
> - Arnaldo
>
> The following changes since commit 816434ec4a674fcdb3c2221a6dffdc8f34020550:
>
> Merge branch 'x86-spinlocks-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (2013-09-04 11:55:10 -0700)
>
> are available in the git repository at:
>
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-urgent-for-mingo
>
> for you to fetch changes up to 526fd8d4f770d18e99680ff87965e16bb8f1d806:
>
> perf session: Separate progress bar update when processing events (2013-09-05 16:19:02 -0300)
>
> ----------------------------------------------------------------
> perf/urgent fixes:
>
> . Fix parsing with no sample_id_all bit set, this regression prevents perf
> from reading old perf.data files generated in systems where
> perf_event_attr.sample_id_all isn't available, from Adrian Hunter.
>
> . Add signal checking to the inner 'perf trace' event processing loop, allowing
> faster response to control+C.
>
> . Fix formatting of long symbol names removing the hardcoding of a buffer
> size used to format histogram entries, which was truncating the lines.
>
> . Separate progress bar update when processing events, reducing potentially big
> overhead in not needed TUI progress bar screen updates, from Jiri Olsa.
>
> . Fix 'perf trace' build in architectures where MAP_32BIT is not defined, from
> Kyle McMartin.
>
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> ----------------------------------------------------------------
> Adrian Hunter (2):
> perf tools: Add test for parsing with no sample_id_all bit
> perf evlist: Fix parsing with no sample_id_all bit set
>
> Arnaldo Carvalho de Melo (2):
> perf trace: Check control+C more often
> perf hists: Fix formatting of long symbol names
>
> Jiri Olsa (1):
> perf session: Separate progress bar update when processing events
>
> Kyle McMartin (1):
> perf trace: Check if MAP_32BIT is defined
>
> tools/perf/Makefile | 3 +-
> tools/perf/builtin-trace.c | 5 ++
> tools/perf/tests/builtin-test.c | 4 ++
> tools/perf/tests/parse-no-sample-id-all.c | 108 ++++++++++++++++++++++++++++++
> tools/perf/tests/tests.h | 1 +
> tools/perf/ui/stdio/hist.c | 23 +++++--
> tools/perf/util/evlist.c | 9 ++-
> tools/perf/util/session.c | 3 +-
> 8 files changed, 146 insertions(+), 10 deletions(-)
> create mode 100644 tools/perf/tests/parse-no-sample-id-all.c
Pulled, thanks a lot Arnaldo!
Ingo