diff options
| author | David Ahern <dsahern@gmail.com> | 2013-02-05 16:12:42 -0500 |
|---|---|---|
| committer | Arnaldo Carvalho de Melo <acme@redhat.com> | 2013-02-06 16:09:26 -0500 |
| commit | 0479b8b9cf4377df5d2c81506ce93326c31eff40 (patch) | |
| tree | dff61dc7107f5b6563915b02347597489687fc1c | |
| parent | 5936f54d6ca2857d81188dcdff8c61b8fc482f53 (diff) | |
perf evlist: Make event_copy local to mmaps
I am getting segfaults *after* the time sorting of perf samples where
the event type is off the charts:
(gdb) bt
\#0 0x0807b1b2 in hists__inc_nr_events (hists=0x80a99c4, type=1163281902) at util/hist.c:1225
\#1 0x08070795 in perf_session_deliver_event (session=0x80a9b90, event=0xf7a6aff8, sample=0xffffc318, tool=0xffffc520,
file_offset=0) at util/session.c:884
\#2 0x0806f9b9 in flush_sample_queue (s=0x80a9b90, tool=0xffffc520) at util/session.c:555
\#3 0x0806fc53 in process_finished_round (tool=0xffffc520, event=0x0, session=0x80a9b90) at util/session.c:645
This is bizarre because the event has already been processed once --
before it was added to the samples queue -- and the event was found to
be sane at that time.
There seem to be 2 causes:
1. perf_evlist__mmap_read updates the read location even though there
are outstanding references to events sitting in the mmap buffers via the
ordered samples queue.
2. There is a single evlist->event_copy for all evlist entries.
event_copy is used to handle an event wrapping at the mmap buffer
boundary.
This patch addresses the second problem - making event_copy local to
each perf_mmap. With this change my highly repeatable use case no longer
fails.
The first problem is much more complicated and will be the subject of a
future patch.
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1360098762-61827-1-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
| -rw-r--r-- | tools/perf/perf.h | 26 | ||||
| -rw-r--r-- | tools/perf/util/evlist.c | 4 | ||||
| -rw-r--r-- | tools/perf/util/evlist.h | 29 |
3 files changed, 30 insertions, 29 deletions
diff --git a/tools/perf/perf.h b/tools/perf/perf.h index 8f3bf388e414..c2206c87fc9f 100644 --- a/tools/perf/perf.h +++ b/tools/perf/perf.h | |||
| @@ -103,32 +103,6 @@ | |||
| 103 | #include "util/types.h" | 103 | #include "util/types.h" |
| 104 | #include <stdbool.h> | 104 | #include <stdbool.h> |
| 105 | 105 | ||
| 106 | struct perf_mmap { | ||
| 107 | void *base; | ||
| 108 | int mask; | ||
| 109 | unsigned int prev; | ||
| 110 | }; | ||
| 111 | |||
| 112 | static inline unsigned int perf_mmap__read_head(struct perf_mmap *mm) | ||
| 113 | { | ||
| 114 | struct perf_event_mmap_page *pc = mm->base; | ||
| 115 | int head = pc->data_head; | ||
| 116 | rmb(); | ||
| 117 | return head; | ||
| 118 | } | ||
| 119 | |||
| 120 | static inline void perf_mmap__write_tail(struct perf_mmap *md, | ||
| 121 | unsigned long tail) | ||
| 122 | { | ||
| 123 | struct perf_event_mmap_page *pc = md->base; | ||
| 124 | |||
| 125 | /* | ||
| 126 | * ensure all reads are done before we write the tail out. | ||
| 127 | */ | ||
| 128 | /* mb(); */ | ||
| 129 | pc->data_tail = tail; | ||
| 130 | } | ||
| 131 | |||
| 132 | /* | 106 | /* |
| 133 | * prctl(PR_TASK_PERF_EVENTS_DISABLE) will (cheaply) disable all | 107 | * prctl(PR_TASK_PERF_EVENTS_DISABLE) will (cheaply) disable all |
| 134 | * counters in the current task. | 108 | * counters in the current task. |
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index ecf123e080bb..bc4ad7977438 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c | |||
| @@ -375,7 +375,7 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx) | |||
| 375 | if ((old & md->mask) + size != ((old + size) & md->mask)) { | 375 | if ((old & md->mask) + size != ((old + size) & md->mask)) { |
| 376 | unsigned int offset = old; | 376 | unsigned int offset = old; |
| 377 | unsigned int len = min(sizeof(*event), size), cpy; | 377 | unsigned int len = min(sizeof(*event), size), cpy; |
| 378 | void *dst = &evlist->event_copy; | 378 | void *dst = &md->event_copy; |
| 379 | 379 | ||
| 380 | do { | 380 | do { |
| 381 | cpy = min(md->mask + 1 - (offset & md->mask), len); | 381 | cpy = min(md->mask + 1 - (offset & md->mask), len); |
| @@ -385,7 +385,7 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx) | |||
| 385 | len -= cpy; | 385 | len -= cpy; |
| 386 | } while (len); | 386 | } while (len); |
| 387 | 387 | ||
| 388 | event = &evlist->event_copy; | 388 | event = &md->event_copy; |
| 389 | } | 389 | } |
| 390 | 390 | ||
| 391 | old += size; | 391 | old += size; |
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h index 73579a25a93e..2dd07bd60b4f 100644 --- a/tools/perf/util/evlist.h +++ b/tools/perf/util/evlist.h | |||
| @@ -17,6 +17,13 @@ struct perf_record_opts; | |||
| 17 | #define PERF_EVLIST__HLIST_BITS 8 | 17 | #define PERF_EVLIST__HLIST_BITS 8 |
| 18 | #define PERF_EVLIST__HLIST_SIZE (1 << PERF_EVLIST__HLIST_BITS) | 18 | #define PERF_EVLIST__HLIST_SIZE (1 << PERF_EVLIST__HLIST_BITS) |
| 19 | 19 | ||
| 20 | struct perf_mmap { | ||
| 21 | void *base; | ||
| 22 | int mask; | ||
| 23 | unsigned int prev; | ||
| 24 | union perf_event event_copy; | ||
| 25 | }; | ||
| 26 | |||
| 20 | struct perf_evlist { | 27 | struct perf_evlist { |
| 21 | struct list_head entries; | 28 | struct list_head entries; |
| 22 | struct hlist_head heads[PERF_EVLIST__HLIST_SIZE]; | 29 | struct hlist_head heads[PERF_EVLIST__HLIST_SIZE]; |
| @@ -30,7 +37,6 @@ struct perf_evlist { | |||
| 30 | pid_t pid; | 37 | pid_t pid; |
| 31 | } workload; | 38 | } workload; |
| 32 | bool overwrite; | 39 | bool overwrite; |
| 33 | union perf_event event_copy; | ||
| 34 | struct perf_mmap *mmap; | 40 | struct perf_mmap *mmap; |
| 35 | struct pollfd *pollfd; | 41 | struct pollfd *pollfd; |
| 36 | struct thread_map *threads; | 42 | struct thread_map *threads; |
| @@ -136,4 +142,25 @@ static inline struct perf_evsel *perf_evlist__last(struct perf_evlist *evlist) | |||
| 136 | } | 142 | } |
| 137 | 143 | ||
| 138 | size_t perf_evlist__fprintf(struct perf_evlist *evlist, FILE *fp); | 144 | size_t perf_evlist__fprintf(struct perf_evlist *evlist, FILE *fp); |
| 145 | |||
| 146 | static inline unsigned int perf_mmap__read_head(struct perf_mmap *mm) | ||
| 147 | { | ||
| 148 | struct perf_event_mmap_page *pc = mm->base; | ||
| 149 | int head = pc->data_head; | ||
| 150 | rmb(); | ||
| 151 | return head; | ||
| 152 | } | ||
| 153 | |||
| 154 | static inline void perf_mmap__write_tail(struct perf_mmap *md, | ||
| 155 | unsigned long tail) | ||
| 156 | { | ||
| 157 | struct perf_event_mmap_page *pc = md->base; | ||
| 158 | |||
| 159 | /* | ||
| 160 | * ensure all reads are done before we write the tail out. | ||
| 161 | */ | ||
| 162 | /* mb(); */ | ||
| 163 | pc->data_tail = tail; | ||
| 164 | } | ||
| 165 | |||
| 139 | #endif /* __PERF_EVLIST_H */ | 166 | #endif /* __PERF_EVLIST_H */ |
