Skip to content

Commit 0479b8b

Browse files
dsahernacmel
authored andcommitted
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 <[email protected]> Cc: Frederic Weisbecker <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Jiri Olsa <[email protected]> Cc: Namhyung Kim <[email protected]> Cc: Peter Zijlstra <[email protected]> Link: http://lkml.kernel.org/r/[email protected] Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
1 parent 5936f54 commit 0479b8b

File tree

3 files changed

+30
-29
lines changed

3 files changed

+30
-29
lines changed

tools/perf/perf.h

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -103,32 +103,6 @@
103103
#include "util/types.h"
104104
#include <stdbool.h>
105105

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-
132106
/*
133107
* prctl(PR_TASK_PERF_EVENTS_DISABLE) will (cheaply) disable all
134108
* counters in the current task.

tools/perf/util/evlist.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
375375
if ((old & md->mask) + size != ((old + size) & md->mask)) {
376376
unsigned int offset = old;
377377
unsigned int len = min(sizeof(*event), size), cpy;
378-
void *dst = &evlist->event_copy;
378+
void *dst = &md->event_copy;
379379

380380
do {
381381
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)
385385
len -= cpy;
386386
} while (len);
387387

388-
event = &evlist->event_copy;
388+
event = &md->event_copy;
389389
}
390390

391391
old += size;

tools/perf/util/evlist.h

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ struct perf_record_opts;
1717
#define PERF_EVLIST__HLIST_BITS 8
1818
#define PERF_EVLIST__HLIST_SIZE (1 << PERF_EVLIST__HLIST_BITS)
1919

20+
struct perf_mmap {
21+
void *base;
22+
int mask;
23+
unsigned int prev;
24+
union perf_event event_copy;
25+
};
26+
2027
struct perf_evlist {
2128
struct list_head entries;
2229
struct hlist_head heads[PERF_EVLIST__HLIST_SIZE];
@@ -30,7 +37,6 @@ struct perf_evlist {
3037
pid_t pid;
3138
} workload;
3239
bool overwrite;
33-
union perf_event event_copy;
3440
struct perf_mmap *mmap;
3541
struct pollfd *pollfd;
3642
struct thread_map *threads;
@@ -136,4 +142,25 @@ static inline struct perf_evsel *perf_evlist__last(struct perf_evlist *evlist)
136142
}
137143

138144
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+
139166
#endif /* __PERF_EVLIST_H */

0 commit comments

Comments
 (0)