Re: [PATCH 07/11] perf session: Add __perf_session__process_dir_events function

[Arnaldo Carvalho de Melo]

Em Fri, Mar 08, 2019 at 02:47:41PM +0100, Jiri Olsa escreveu:
> Adding __perf_session__process_dir_events function
> to process events over the directory data.
> 
> All directory events are pushed into sessions ordered
> data and flushed for processing.
> 
> Link: http://lkml.kernel.org/n/tip-n3zl0wo3z18tatv5x7epmjrh@xxxxxxxxxxxxxx
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
>  tools/perf/util/session.c | 88 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 86 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 0ec34227bd60..b55f4281b1da 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1876,8 +1876,6 @@ reader__process_events(struct reader *rd, struct perf_session *session,
>  	file_offset = page_offset;
>  	head = rd->data_offset - page_offset;
>  
> -	ui_progress__init_size(prog, data_size, "Processing events...");
> -
>  	data_size += rd->data_offset;
>  
>  	mmap_size = MMAP_SIZE;
> @@ -2006,6 +2004,89 @@ static int __perf_session__process_events(struct perf_session *session)
>  	return err;
>  }
>  
> +static s64 process_index(struct perf_session *session,
> +			 union perf_event *event,
> +			 u64 file_offset)
> +{
> +	struct perf_evlist *evlist = session->evlist;
> +	u64 timestamp;
> +	s64 ret;
> +
> +	if (session->header.needs_swap)
> +		event_swap(event, perf_evlist__sample_id_all(evlist));
> +
> +	if (event->header.type >= PERF_RECORD_HEADER_MAX)
> +		return -EINVAL;
> +
> +	events_stats__inc(&evlist->stats, event->header.type);
> +
> +	if (event->header.type >= PERF_RECORD_USER_TYPE_START)
> +		return perf_session__process_user_event(session, event, file_offset);
> +
> +	ret = perf_evlist__parse_sample_timestamp(evlist, event, &timestamp);
> +	if (ret)
> +		return ret;
> +
> +	return ordered_events__queue(&session->ordered_events, event,
> +				     timestamp, file_offset);
> +}
> +
> +static int __perf_session__process_dir_events(struct perf_session *session)
> +{
> +	struct perf_data *data = session->data;
> +	struct perf_tool *tool = session->tool;
> +	struct reader rd = {
> +		.fd		= perf_data__fd(session->data),
> +		.data_size	= session->header.data_size,
> +		.data_offset	= session->header.data_offset,
> +		.process	= process_simple,
> +	};
> +	int i, ret = 0;
> +	struct ui_progress prog;
> +	u64 total_size = perf_data__size(session->data);
> +
> +	perf_tool__fill_defaults(tool);
> +
> +	ui_progress__init_size(&prog, total_size, "Processing events...");
> +
> +	/* Read data from the header file.. */
> +	ret = reader__process_events(&rd, session, &prog);
> +	if (ret)
> +		goto out_err;
> +
> +	/* ... and continue with data files. */
> +	for (i = 0; i < data->dir.nr ; i++) {
> +		struct perf_data_file *file = &data->dir.files[i];
> +
> +		if (file->size == 0)
> +			continue;
> +
> +		rd = (struct reader) {
> +			.fd		= file->fd,
> +			.data_size	= file->size,
> +			.data_offset	= 0,
> +			.process	= process_index,
> +		};
> +
> +		ret = reader__process_events(&rd, session, &prog);
> +		if (ret)
> +			goto out_err;

Don't we have to have some handling of PERF_RECORD_FINISHED_ROUND here?
I.e. what happens if we fill th ordered events with just the contents
of, say, the first CPU and then have those events flushed and processed
before we start even looking at the events in the other CPUs?

I think some detailed explanation of what happens here is in need, no?

- Arnaldo

> +	}
> +
> +	ret = ordered_events__flush(&session->ordered_events, OE_FLUSH__FINAL);
> +
> +out_err:
> +	if (!tool->no_warn)
> +		perf_session__warn_about_errors(session);
> +
> +	/*
> +	 * We may switching perf.data output, make ordered_events
> +	 * reusable.
> +	 */
> +	ordered_events__reinit(&session->ordered_events);
> +	return ret;
> +}
> +
>  int perf_session__process_events(struct perf_session *session)
>  {
>  	if (perf_session__register_idle_thread(session) < 0)
> @@ -2014,6 +2095,9 @@ int perf_session__process_events(struct perf_session *session)
>  	if (perf_data__is_pipe(session->data))
>  		return __perf_session__process_pipe_events(session);
>  
> +	if (perf_data__is_dir(session->data))
> +		return __perf_session__process_dir_events(session);
> +
>  	return __perf_session__process_events(session);
>  }
>  
> -- 
> 2.17.2

-- 

- Arnaldo