Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sun, 12 Sep 2021 22:44:48 +0200
From:   Jiri Olsa <jolsa@redhat.com>
To:     Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
Cc:     Arnaldo Carvalho de Melo <acme@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Andi Kleen <ak@linux.intel.com>,
        Adrian Hunter <adrian.hunter@intel.com>,
        Alexander Antonov <alexander.antonov@linux.intel.com>,
        Alexei Budankov <abudankov@huawei.com>,
        Riccardo Mancini <rickyman7@gmail.com>
Subject: Re: [PATCH v11 19/24] perf session: Introduce reader objects in
 session object
Message-ID: <YT5mwObzZ20BN5oz@krava>
References: <cover.1629186429.git.alexey.v.bayduraev@linux.intel.com>
 <117061f9be283a79d38068818b8a0038fbe578e1.1629186429.git.alexey.v.bayduraev@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <117061f9be283a79d38068818b8a0038fbe578e1.1629186429.git.alexey.v.bayduraev@linux.intel.com>
Precedence: bulk

On Tue, Aug 17, 2021 at 11:23:22AM +0300, Alexey Bayduraev wrote:
> Allow to allocate multiple reader objects, so we could load multiple
> data files located in data directory at the same time.
> 
> Design and implementation are based on the prototype [1], [2].
> 
> [1] git clone https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git -b perf/record_threads
> [2] https://lore.kernel.org/lkml/20180913125450.21342-1-jolsa@kernel.org/
> 
> Suggested-by: Jiri Olsa <jolsa@kernel.org>
> Acked-by: Namhyung Kim <namhyung@gmail.com>
> Reviewed-by: Riccardo Mancini <rickyman7@gmail.com>
> Tested-by: Riccardo Mancini <rickyman7@gmail.com>
> Signed-off-by: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
> ---
>  tools/perf/util/session.c | 33 +++++++++++++++++++++------------
>  tools/perf/util/session.h |  3 +++
>  2 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index a82204d4c4d6..1f92d3cfcb1d 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -343,6 +343,10 @@ void perf_session__delete(struct perf_session *session)
>  	auxtrace_index__free(&session->auxtrace_index);
>  	perf_session__destroy_kernel_maps(session);
>  	perf_session__delete_threads(session);
> +	if (session->readers) {
> +		zfree(&session->readers);
> +		session->nr_readers = 0;
> +	}
>  	perf_session__release_decomp_events(session);
>  	perf_env__exit(&session->header.env);
>  	machines__exit(&session->machines);
> @@ -2303,14 +2307,7 @@ static s64 process_simple(struct perf_session *session,
>  
>  static int __perf_session__process_events(struct perf_session *session)
>  {
> -	struct reader rd = {
> -		.fd		= perf_data__fd(session->data),
> -		.data_size	= session->header.data_size,
> -		.data_offset	= session->header.data_offset,
> -		.process	= process_simple,
> -		.path		= session->data->file.path,
> -		.in_place_update = session->data->in_place_update,
> -	};
> +	struct reader *rd;
>  	struct ordered_events *oe = &session->ordered_events;
>  	struct perf_tool *tool = session->tool;
>  	struct ui_progress prog;
> @@ -2318,12 +2315,24 @@ static int __perf_session__process_events(struct perf_session *session)
>  
>  	perf_tool__fill_defaults(tool);
>  
> -	if (rd.data_size == 0)
> -		return -1;
> +	rd = session->readers = zalloc(sizeof(struct reader));
> +	if (!rd)
> +		return -ENOMEM;
> +
> +	session->nr_readers = 1;
> +
> +	*rd = (struct reader) {
> +		.fd		 = perf_data__fd(session->data),
> +		.data_size	 = session->header.data_size,
> +		.data_offset	 = session->header.data_offset,
> +		.process	 = process_simple,
> +		.path		 = session->data->file.path,
> +		.in_place_update = session->data->in_place_update,
> +	};
>  
> -	ui_progress__init_size(&prog, rd.data_size, "Processing events...");
> +	ui_progress__init_size(&prog, rd->data_size, "Processing events...");
>  
> -	err = reader__process_events(&rd, session, &prog);
> +	err = reader__process_events(rd, session, &prog);
>  	if (err)
>  		goto out_err;

hm, leaking rd in here?

jirka

>  	/* do the final flush for ordered samples */
> diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
> index 308cf48e0945..b9b9468d8f17 100644
> --- a/tools/perf/util/session.h
> +++ b/tools/perf/util/session.h
> @@ -19,6 +19,7 @@ struct thread;
>  
>  struct auxtrace;
>  struct itrace_synth_opts;
> +struct reader;
>  
>  struct perf_session {
>  	struct perf_header	header;
> @@ -41,6 +42,8 @@ struct perf_session {
>  	struct zstd_data	zstd_data;
>  	struct decomp		*decomp;
>  	struct decomp		*decomp_last;
> +	struct reader		*readers;
> +	int			nr_readers;
>  };
>  
>  struct decomp {
> -- 
> 2.19.0
>