Re: [BUG] perf script segfault

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Tue, Mar 31, 2015 at 03:58:01PM +0200, Jiri Olsa escreveu:
> 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

If possible, yes, lemme see, yeah, thanks for the patch, testing it,
good that we can remove that from ordered_events!

- Arnaldo
 
> 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)