Re: [BUG] perf script segfault

From: Jiri Olsa
Date: Tue Mar 31 2015 - 09:58:23 EST


On Tue, Mar 31, 2015 at 09:59:20AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Mar 30, 2015 at 08:45:33PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Mon, Mar 30, 2015 at 04:51:34PM -0600, David Ahern escreveu:
> > > tool was moved to ordered_events and is not initialized for pipe mode. I
> > > don't have time to look into it more than that before PTO on Wednesday.
>
> > I guess this one is enough, no? Checking with your example...
>
> So the following is better, can you give it a try, please?
>
> - Arnaldo
>
>
> From fbd7d154f01c47db71a3d8b0546911872aa1de54 Mon Sep 17 00:00:00 2001
> From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Date: Tue, 31 Mar 2015 09:53:50 -0300
> Subject: [PATCH 1/1] perf session: Always initialize ordered_events
>
> Even when it is not used to actually reorder events, some of its fields
> are used, like session->ordered_events->tool, to shorten function

hum, when did this happen? I somehow missed those changes.. :-\

I think we should have ordered_events struct free of perf_tool,
machines and perf_evlist structs

why dont we store it into perf_session? like in attached patch

plus I think the deliver function could take all the info from
session, because it's all already there...

---
static int ordered_events__deliver_event(struct ordered_events *oe,
struct ordered_event *event,
struct perf_sample *sample)
{
struct perf_session *session = container_of(oe, struct perf_session,
ordered_events);

return machines__deliver_event(&session->machines,
session->evlist, event->event,
sample,
session->tool,
event->file_offset);
}
---

jirka


---
diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c
index 6002fa3fcf77..1c464eff4d62 100644
--- a/tools/perf/util/ordered-events.c
+++ b/tools/perf/util/ordered-events.c
@@ -293,7 +293,7 @@ int ordered_events__flush(struct ordered_events *oe, enum oe_flush how)
}

void ordered_events__init(struct ordered_events *oe, struct machines *machines,
- struct perf_evlist *evlist, struct perf_tool *tool,
+ struct perf_evlist *evlist,
ordered_events__deliver_t deliver)
{
INIT_LIST_HEAD(&oe->events);
@@ -303,7 +303,6 @@ void ordered_events__init(struct ordered_events *oe, struct machines *machines,
oe->cur_alloc_size = 0;
oe->evlist = evlist;
oe->machines = machines;
- oe->tool = tool;
oe->deliver = deliver;
}

diff --git a/tools/perf/util/ordered-events.h b/tools/perf/util/ordered-events.h
index 173e13f28c08..b5008b07b5cc 100644
--- a/tools/perf/util/ordered-events.h
+++ b/tools/perf/util/ordered-events.h
@@ -41,7 +41,6 @@ struct ordered_events {
struct ordered_event *last;
struct machines *machines;
struct perf_evlist *evlist;
- struct perf_tool *tool;
ordered_events__deliver_t deliver;
int buffer_idx;
unsigned int nr_events;
@@ -54,7 +53,7 @@ int ordered_events__queue(struct ordered_events *oe, union perf_event *event,
void ordered_events__delete(struct ordered_events *oe, struct ordered_event *event);
int ordered_events__flush(struct ordered_events *oe, enum oe_flush how);
void ordered_events__init(struct ordered_events *oe, struct machines *machines,
- struct perf_evlist *evlsit, struct perf_tool *tool,
+ struct perf_evlist *evlsit,
ordered_events__deliver_t deliver);
void ordered_events__free(struct ordered_events *oe);

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index adf0740c563b..04795d8f83a1 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -96,8 +96,11 @@ static int ordered_events__deliver_event(struct ordered_events *oe,
struct ordered_event *event,
struct perf_sample *sample)
{
+ struct perf_session *session = container_of(oe, struct perf_session,
+ ordered_events);
+
return machines__deliver_event(oe->machines, oe->evlist, event->event,
- sample, oe->tool, event->file_offset);
+ sample, session->tool, event->file_offset);
}

struct perf_session *perf_session__new(struct perf_data_file *file,
@@ -109,6 +112,7 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
goto out;

session->repipe = repipe;
+ session->tool = tool;
machines__init(&session->machines);

if (file) {
@@ -141,7 +145,7 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
tool->ordered_events = false;
} else {
ordered_events__init(&session->ordered_events, &session->machines,
- session->evlist, tool, ordered_events__deliver_event);
+ session->evlist, ordered_events__deliver_event);
}

return session;
@@ -941,7 +945,7 @@ static s64 perf_session__process_user_event(struct perf_session *session,
u64 file_offset)
{
struct ordered_events *oe = &session->ordered_events;
- struct perf_tool *tool = oe->tool;
+ struct perf_tool *tool = session->tool;
int fd = perf_data_file__fd(session->file);
int err;

@@ -982,7 +986,7 @@ int perf_session__deliver_synth_event(struct perf_session *session,
struct perf_sample *sample)
{
struct perf_evlist *evlist = session->evlist;
- struct perf_tool *tool = session->ordered_events.tool;
+ struct perf_tool *tool = session->tool;

events_stats__inc(&evlist->stats, event->header.type);

@@ -1060,7 +1064,7 @@ static s64 perf_session__process_event(struct perf_session *session,
union perf_event *event, u64 file_offset)
{
struct perf_evlist *evlist = session->evlist;
- struct perf_tool *tool = session->ordered_events.tool;
+ struct perf_tool *tool = session->tool;
struct perf_sample sample;
int ret;

@@ -1165,7 +1169,7 @@ volatile int session_done;
static int __perf_session__process_pipe_events(struct perf_session *session)
{
struct ordered_events *oe = &session->ordered_events;
- struct perf_tool *tool = oe->tool;
+ struct perf_tool *tool = session->tool;
int fd = perf_data_file__fd(session->file);
union perf_event *event;
uint32_t size, cur_size = 0;
@@ -1298,7 +1302,7 @@ static int __perf_session__process_events(struct perf_session *session,
u64 file_size)
{
struct ordered_events *oe = &session->ordered_events;
- struct perf_tool *tool = oe->tool;
+ struct perf_tool *tool = session->tool;
int fd = perf_data_file__fd(session->file);
u64 head, page_offset, file_offset, file_pos, size;
int err, mmap_prot, mmap_flags, map_idx = 0;
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 1310998f8318..d5fa7b7916ef 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -26,6 +26,7 @@ struct perf_session {
u64 one_mmap_offset;
struct ordered_events ordered_events;
struct perf_data_file *file;
+ struct perf_tool *tool;
};

#define PRINT_IP_OPT_IP (1<<0)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/