Re: [BUG] perf script segfault

[Jiri Olsa <jolsa@redhat.com>]

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@redhat.com>
> 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)