All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] perf session: Extend reader object to allow multiple readers
@ 2021-09-29  8:41 Alexey Bayduraev
  2021-09-29  8:41 ` [PATCH 1/6] perf session: Move reader structure to the top Alexey Bayduraev
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Alexey Bayduraev @ 2021-09-29  8:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
	Ingo Molnar, linux-kernel, Andi Kleen, Adrian Hunter,
	Alexander Antonov, Alexei Budankov, Riccardo Mancini

Patch set adds state info and decompressor object into reader object
that made possible to split reader__process_events function into three
logical parts: init/exit, map/unmap and single event reader which are
used in events reader loop. This approach allows reading multiple trace
files at the same time. 

The design and implementation are based on the prototype [1], [2].

The patch set was separated from [3] and already was rewieved by
Namhyung Kim and Riccardo Mancini. The difference from [3] is that
this patch set keeps reader object allocation on the stack.

Tested:

tools/perf/perf record -o prof.data -- matrix.gcc.g.O3
tools/perf/perf record -o prof.data -z -- matrix.gcc.g.O3
tools/perf/perf report -i prof.data
tools/perf/perf report -i prof.data --call-graph=callee
tools/perf/perf report -i prof.data --stdio --header
tools/perf/perf report -i prof.data -D --header

[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/
[3] https://lore.kernel.org/lkml/cover.1629186429.git.alexey.v.bayduraev@linux.intel.com/

Alexey Bayduraev (6):
  perf session: Move reader structure to the top
  perf session: Introduce reader_state in reader object
  perf session: Introduce decompressor into reader object
  perf session: Move init/exit into reader__init/reader__exit functions
  perf session: Move map/unmap into reader__mmap function
  perf session: Load single file for analysis

 tools/perf/util/session.c | 272 +++++++++++++++++++++++++-------------
 tools/perf/util/session.h |   2 +
 2 files changed, 179 insertions(+), 95 deletions(-)

-- 
2.19.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/6] perf session: Move reader structure to the top
  2021-09-29  8:41 [PATCH 0/6] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
@ 2021-09-29  8:41 ` Alexey Bayduraev
  2021-09-29  8:41 ` [PATCH 2/6] perf session: Introduce reader_state in reader object Alexey Bayduraev
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Alexey Bayduraev @ 2021-09-29  8:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
	Ingo Molnar, linux-kernel, Andi Kleen, Adrian Hunter,
	Alexander Antonov, Alexei Budankov, Riccardo Mancini

Moving reader structure to the top of the file. This is necessary
for the further use of this structure in the decompressor.

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 | 52 +++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 069c2cfdd3be..43bc58d465f2 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -36,6 +36,32 @@
 #include "units.h"
 #include <internal/lib.h>
 
+/*
+ * On 64bit we can mmap the data file in one go. No need for tiny mmap
+ * slices. On 32bit we use 32MB.
+ */
+#if BITS_PER_LONG == 64
+#define MMAP_SIZE ULLONG_MAX
+#define NUM_MMAPS 1
+#else
+#define MMAP_SIZE (32 * 1024 * 1024ULL)
+#define NUM_MMAPS 128
+#endif
+
+struct reader;
+
+typedef s64 (*reader_cb_t)(struct perf_session *session,
+			   union perf_event *event,
+			   u64 file_offset);
+
+struct reader {
+	int		 fd;
+	u64		 data_size;
+	u64		 data_offset;
+	reader_cb_t	 process;
+	bool		 in_place_update;
+};
+
 #ifdef HAVE_ZSTD_SUPPORT
 static int perf_session__process_compressed_event(struct perf_session *session,
 						  union perf_event *event, u64 file_offset)
@@ -2147,32 +2173,6 @@ static int __perf_session__process_decomp_events(struct perf_session *session)
 	return 0;
 }
 
-/*
- * On 64bit we can mmap the data file in one go. No need for tiny mmap
- * slices. On 32bit we use 32MB.
- */
-#if BITS_PER_LONG == 64
-#define MMAP_SIZE ULLONG_MAX
-#define NUM_MMAPS 1
-#else
-#define MMAP_SIZE (32 * 1024 * 1024ULL)
-#define NUM_MMAPS 128
-#endif
-
-struct reader;
-
-typedef s64 (*reader_cb_t)(struct perf_session *session,
-			   union perf_event *event,
-			   u64 file_offset);
-
-struct reader {
-	int		 fd;
-	u64		 data_size;
-	u64		 data_offset;
-	reader_cb_t	 process;
-	bool		 in_place_update;
-};
-
 static int
 reader__process_events(struct reader *rd, struct perf_session *session,
 		       struct ui_progress *prog)
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/6] perf session: Introduce reader_state in reader object
  2021-09-29  8:41 [PATCH 0/6] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
  2021-09-29  8:41 ` [PATCH 1/6] perf session: Move reader structure to the top Alexey Bayduraev
@ 2021-09-29  8:41 ` Alexey Bayduraev
  2021-09-29  8:41 ` [PATCH 3/6] perf session: Introduce decompressor into " Alexey Bayduraev
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Alexey Bayduraev @ 2021-09-29  8:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
	Ingo Molnar, linux-kernel, Andi Kleen, Adrian Hunter,
	Alexander Antonov, Alexei Budankov, Riccardo Mancini

We need all the state info about reader in separate object to load data
from multiple files, so we can keep multiple readers at the same time.
Adding struct reader_state and adding all items that need to be kept.

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 | 74 +++++++++++++++++++++++----------------
 1 file changed, 43 insertions(+), 31 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 43bc58d465f2..262efc18caac 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -54,12 +54,24 @@ typedef s64 (*reader_cb_t)(struct perf_session *session,
 			   union perf_event *event,
 			   u64 file_offset);
 
+struct reader_state {
+	char	*mmaps[NUM_MMAPS];
+	size_t	 mmap_size;
+	int	 mmap_idx;
+	char	*mmap_cur;
+	u64	 file_pos;
+	u64	 file_offset;
+	u64	 data_size;
+	u64	 head;
+};
+
 struct reader {
 	int		 fd;
 	u64		 data_size;
 	u64		 data_offset;
 	reader_cb_t	 process;
 	bool		 in_place_update;
+	struct reader_state state;
 };
 
 #ifdef HAVE_ZSTD_SUPPORT
@@ -2177,29 +2189,28 @@ static int
 reader__process_events(struct reader *rd, struct perf_session *session,
 		       struct ui_progress *prog)
 {
-	u64 data_size = rd->data_size;
-	u64 head, page_offset, file_offset, file_pos, size;
-	int err = 0, mmap_prot, mmap_flags, map_idx = 0;
-	size_t	mmap_size;
-	char *buf, *mmaps[NUM_MMAPS];
+	struct reader_state *st = &rd->state;
+	u64 page_offset, size;
+	int err = 0, mmap_prot, mmap_flags;
+	char *buf, **mmaps = st->mmaps;
 	union perf_event *event;
 	s64 skip;
 
 	page_offset = page_size * (rd->data_offset / page_size);
-	file_offset = page_offset;
-	head = rd->data_offset - page_offset;
+	st->file_offset = page_offset;
+	st->head = rd->data_offset - page_offset;
 
-	ui_progress__init_size(prog, data_size, "Processing events...");
+	ui_progress__init_size(prog, rd->data_size, "Processing events...");
 
-	data_size += rd->data_offset;
+	st->data_size = rd->data_size + rd->data_offset;
 
-	mmap_size = MMAP_SIZE;
-	if (mmap_size > data_size) {
-		mmap_size = data_size;
+	st->mmap_size = MMAP_SIZE;
+	if (st->mmap_size > st->data_size) {
+		st->mmap_size = st->data_size;
 		session->one_mmap = true;
 	}
 
-	memset(mmaps, 0, sizeof(mmaps));
+	memset(mmaps, 0, sizeof(st->mmaps));
 
 	mmap_prot  = PROT_READ;
 	mmap_flags = MAP_SHARED;
@@ -2211,35 +2222,36 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 		mmap_flags = MAP_PRIVATE;
 	}
 remap:
-	buf = mmap(NULL, mmap_size, mmap_prot, mmap_flags, rd->fd,
-		   file_offset);
+	buf = mmap(NULL, st->mmap_size, mmap_prot, mmap_flags, rd->fd,
+		   st->file_offset);
 	if (buf == MAP_FAILED) {
 		pr_err("failed to mmap file\n");
 		err = -errno;
 		goto out;
 	}
-	mmaps[map_idx] = buf;
-	map_idx = (map_idx + 1) & (ARRAY_SIZE(mmaps) - 1);
-	file_pos = file_offset + head;
+	mmaps[st->mmap_idx] = st->mmap_cur = buf;
+	st->mmap_idx = (st->mmap_idx + 1) & (ARRAY_SIZE(st->mmaps) - 1);
+	st->file_pos = st->file_offset + st->head;
 	if (session->one_mmap) {
 		session->one_mmap_addr = buf;
-		session->one_mmap_offset = file_offset;
+		session->one_mmap_offset = st->file_offset;
 	}
 
 more:
-	event = fetch_mmaped_event(head, mmap_size, buf, session->header.needs_swap);
+	event = fetch_mmaped_event(st->head, st->mmap_size, st->mmap_cur,
+				   session->header.needs_swap);
 	if (IS_ERR(event))
 		return PTR_ERR(event);
 
 	if (!event) {
-		if (mmaps[map_idx]) {
-			munmap(mmaps[map_idx], mmap_size);
-			mmaps[map_idx] = NULL;
+		if (mmaps[st->mmap_idx]) {
+			munmap(mmaps[st->mmap_idx], st->mmap_size);
+			mmaps[st->mmap_idx] = NULL;
 		}
 
-		page_offset = page_size * (head / page_size);
-		file_offset += page_offset;
-		head -= page_offset;
+		page_offset = page_size * (st->head / page_size);
+		st->file_offset += page_offset;
+		st->head -= page_offset;
 		goto remap;
 	}
 
@@ -2248,9 +2260,9 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 	skip = -EINVAL;
 
 	if (size < sizeof(struct perf_event_header) ||
-	    (skip = rd->process(session, event, file_pos)) < 0) {
+	    (skip = rd->process(session, event, st->file_pos)) < 0) {
 		pr_err("%#" PRIx64 " [%#x]: failed to process type: %d [%s]\n",
-		       file_offset + head, event->header.size,
+		       st->file_offset + st->head, event->header.size,
 		       event->header.type, strerror(-skip));
 		err = skip;
 		goto out;
@@ -2259,8 +2271,8 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 	if (skip)
 		size += skip;
 
-	head += size;
-	file_pos += size;
+	st->head += size;
+	st->file_pos += size;
 
 	err = __perf_session__process_decomp_events(session);
 	if (err)
@@ -2271,7 +2283,7 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 	if (session_done())
 		goto out;
 
-	if (file_pos < data_size)
+	if (st->file_pos < st->data_size)
 		goto more;
 
 out:
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/6] perf session: Introduce decompressor into reader object
  2021-09-29  8:41 [PATCH 0/6] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
  2021-09-29  8:41 ` [PATCH 1/6] perf session: Move reader structure to the top Alexey Bayduraev
  2021-09-29  8:41 ` [PATCH 2/6] perf session: Introduce reader_state in reader object Alexey Bayduraev
@ 2021-09-29  8:41 ` Alexey Bayduraev
  2021-10-04 13:53   ` Jiri Olsa
  2021-09-29  8:41 ` [PATCH 4/6] perf session: Move init/exit into reader__init/reader__exit functions Alexey Bayduraev
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Alexey Bayduraev @ 2021-09-29  8:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
	Ingo Molnar, linux-kernel, Andi Kleen, Adrian Hunter,
	Alexander Antonov, Alexei Budankov, Riccardo Mancini

Introduce decompressor into reader object so that decompression
could be executed separately for each data file. Adding active_reader
into perf_session object to select current decompressor.

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 | 48 +++++++++++++++++++++++++++++----------
 tools/perf/util/session.h |  2 ++
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 262efc18caac..3bea0830b3b8 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -71,6 +71,9 @@ struct reader {
 	u64		 data_offset;
 	reader_cb_t	 process;
 	bool		 in_place_update;
+	struct zstd_data zstd_data;
+	struct decomp	 *decomp;
+	struct decomp	 *decomp_last;
 	struct reader_state state;
 };
 
@@ -82,7 +85,10 @@ static int perf_session__process_compressed_event(struct perf_session *session,
 	size_t decomp_size, src_size;
 	u64 decomp_last_rem = 0;
 	size_t mmap_len, decomp_len = session->header.env.comp_mmap_len;
-	struct decomp *decomp, *decomp_last = session->decomp_last;
+	struct decomp *decomp, *decomp_last = session->active_reader ?
+		session->active_reader->decomp_last : session->decomp_last;
+	struct zstd_data *zstd_data = session->active_reader ?
+		&session->active_reader->zstd_data : &session->zstd_data;
 
 	if (decomp_last) {
 		decomp_last_rem = decomp_last->size - decomp_last->head;
@@ -109,7 +115,7 @@ static int perf_session__process_compressed_event(struct perf_session *session,
 	src = (void *)event + sizeof(struct perf_record_compressed);
 	src_size = event->pack.header.size - sizeof(struct perf_record_compressed);
 
-	decomp_size = zstd_decompress_stream(&(session->zstd_data), src, src_size,
+	decomp_size = zstd_decompress_stream(zstd_data, src, src_size,
 				&(decomp->data[decomp_last_rem]), decomp_len - decomp_last_rem);
 	if (!decomp_size) {
 		munmap(decomp, mmap_len);
@@ -119,12 +125,22 @@ static int perf_session__process_compressed_event(struct perf_session *session,
 
 	decomp->size += decomp_size;
 
-	if (session->decomp == NULL) {
-		session->decomp = decomp;
-		session->decomp_last = decomp;
+	if (session->active_reader) {
+		if (session->active_reader->decomp == NULL) {
+			session->active_reader->decomp = decomp;
+			session->active_reader->decomp_last = decomp;
+		} else {
+			session->active_reader->decomp_last->next = decomp;
+			session->active_reader->decomp_last = decomp;
+		}
 	} else {
-		session->decomp_last->next = decomp;
-		session->decomp_last = decomp;
+		if (session->decomp == NULL) {
+			session->decomp = decomp;
+			session->decomp_last = decomp;
+		} else {
+			session->decomp_last->next = decomp;
+			session->decomp_last = decomp;
+		}
 	}
 
 	pr_debug("decomp (B): %zd to %zd\n", src_size, decomp_size);
@@ -314,11 +330,11 @@ static void perf_session__delete_threads(struct perf_session *session)
 	machine__delete_threads(&session->machines.host);
 }
 
-static void perf_session__release_decomp_events(struct perf_session *session)
+static void perf_decomp__release_events(struct decomp *next)
 {
-	struct decomp *next, *decomp;
+	struct decomp *decomp;
 	size_t mmap_len;
-	next = session->decomp;
+
 	do {
 		decomp = next;
 		if (decomp == NULL)
@@ -337,7 +353,7 @@ 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);
-	perf_session__release_decomp_events(session);
+	perf_decomp__release_events(session->decomp);
 	perf_env__exit(&session->header.env);
 	machines__exit(&session->machines);
 	if (session->data) {
@@ -2155,7 +2171,8 @@ static int __perf_session__process_decomp_events(struct perf_session *session)
 {
 	s64 skip;
 	u64 size, file_pos = 0;
-	struct decomp *decomp = session->decomp_last;
+	struct decomp *decomp = session->active_reader ?
+		session->active_reader->decomp_last : session->decomp_last;
 
 	if (!decomp)
 		return 0;
@@ -2212,6 +2229,9 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 
 	memset(mmaps, 0, sizeof(st->mmaps));
 
+	if (zstd_init(&rd->zstd_data, 0))
+		return -1;
+
 	mmap_prot  = PROT_READ;
 	mmap_flags = MAP_SHARED;
 
@@ -2255,6 +2275,7 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 		goto remap;
 	}
 
+	session->active_reader = rd;
 	size = event->header.size;
 
 	skip = -EINVAL;
@@ -2287,6 +2308,9 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 		goto more;
 
 out:
+	session->active_reader = NULL;
+	perf_decomp__release_events(rd->decomp);
+	zstd_fini(&rd->zstd_data);
 	return err;
 }
 
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 5d8bd14a0a39..45f9f652a12a 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,7 @@ struct perf_session {
 	struct zstd_data	zstd_data;
 	struct decomp		*decomp;
 	struct decomp		*decomp_last;
+	struct reader           *active_reader;
 };
 
 struct decomp {
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/6] perf session: Move init/exit into reader__init/reader__exit functions
  2021-09-29  8:41 [PATCH 0/6] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
                   ` (2 preceding siblings ...)
  2021-09-29  8:41 ` [PATCH 3/6] perf session: Introduce decompressor into " Alexey Bayduraev
@ 2021-09-29  8:41 ` Alexey Bayduraev
  2021-09-29  8:41 ` [PATCH 5/6] perf session: Move map/unmap into reader__mmap function Alexey Bayduraev
  2021-09-29  8:41 ` [PATCH 6/6] perf session: Load single file for analysis Alexey Bayduraev
  5 siblings, 0 replies; 10+ messages in thread
From: Alexey Bayduraev @ 2021-09-29  8:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
	Ingo Molnar, linux-kernel, Andi Kleen, Adrian Hunter,
	Alexander Antonov, Alexei Budankov, Riccardo Mancini

Separating init/exit code into reader__init and reader__exit function.

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 | 42 ++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 3bea0830b3b8..85142d2a9a5a 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2203,28 +2203,23 @@ static int __perf_session__process_decomp_events(struct perf_session *session)
 }
 
 static int
-reader__process_events(struct reader *rd, struct perf_session *session,
-		       struct ui_progress *prog)
+reader__init(struct reader *rd, bool *one_mmap)
 {
 	struct reader_state *st = &rd->state;
-	u64 page_offset, size;
-	int err = 0, mmap_prot, mmap_flags;
-	char *buf, **mmaps = st->mmaps;
-	union perf_event *event;
-	s64 skip;
+	u64 page_offset;
+	char **mmaps = st->mmaps;
 
 	page_offset = page_size * (rd->data_offset / page_size);
 	st->file_offset = page_offset;
 	st->head = rd->data_offset - page_offset;
 
-	ui_progress__init_size(prog, rd->data_size, "Processing events...");
-
 	st->data_size = rd->data_size + rd->data_offset;
 
 	st->mmap_size = MMAP_SIZE;
 	if (st->mmap_size > st->data_size) {
 		st->mmap_size = st->data_size;
-		session->one_mmap = true;
+		if (one_mmap)
+			*one_mmap = true;
 	}
 
 	memset(mmaps, 0, sizeof(st->mmaps));
@@ -2232,6 +2227,27 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 	if (zstd_init(&rd->zstd_data, 0))
 		return -1;
 
+	return 0;
+}
+
+static void
+reader__exit(struct reader *rd)
+{
+	perf_decomp__release_events(rd->decomp);
+	zstd_fini(&rd->zstd_data);
+}
+
+static int
+reader__process_events(struct reader *rd, struct perf_session *session,
+		       struct ui_progress *prog)
+{
+	struct reader_state *st = &rd->state;
+	u64 page_offset, size;
+	int err = 0, mmap_prot, mmap_flags;
+	char *buf, **mmaps = st->mmaps;
+	union perf_event *event;
+	s64 skip;
+
 	mmap_prot  = PROT_READ;
 	mmap_flags = MAP_SHARED;
 
@@ -2309,8 +2325,6 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 
 out:
 	session->active_reader = NULL;
-	perf_decomp__release_events(rd->decomp);
-	zstd_fini(&rd->zstd_data);
 	return err;
 }
 
@@ -2342,6 +2356,9 @@ static int __perf_session__process_events(struct perf_session *session)
 
 	ui_progress__init_size(&prog, rd.data_size, "Processing events...");
 
+	err = reader__init(&rd, &session->one_mmap);
+	if (err)
+		goto out_err;
 	err = reader__process_events(&rd, session, &prog);
 	if (err)
 		goto out_err;
@@ -2364,6 +2381,7 @@ static int __perf_session__process_events(struct perf_session *session)
 	ordered_events__reinit(&session->ordered_events);
 	auxtrace__free_events(session);
 	session->one_mmap = false;
+	reader__exit(&rd);
 	return err;
 }
 
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/6] perf session: Move map/unmap into reader__mmap function
  2021-09-29  8:41 [PATCH 0/6] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
                   ` (3 preceding siblings ...)
  2021-09-29  8:41 ` [PATCH 4/6] perf session: Move init/exit into reader__init/reader__exit functions Alexey Bayduraev
@ 2021-09-29  8:41 ` Alexey Bayduraev
  2021-10-04 13:24   ` Jiri Olsa
  2021-09-29  8:41 ` [PATCH 6/6] perf session: Load single file for analysis Alexey Bayduraev
  5 siblings, 1 reply; 10+ messages in thread
From: Alexey Bayduraev @ 2021-09-29  8:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
	Ingo Molnar, linux-kernel, Andi Kleen, Adrian Hunter,
	Alexander Antonov, Alexei Budankov, Riccardo Mancini

Moving mapping and unmapping code into reader__mmap, so the mmap
code is located together. Moving head/file_offset computation into
reader__mmap function, so all the offset computation is located
together and in one place only.

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 | 60 +++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 25 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 85142d2a9a5a..5e08def72b41 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2206,12 +2206,9 @@ static int
 reader__init(struct reader *rd, bool *one_mmap)
 {
 	struct reader_state *st = &rd->state;
-	u64 page_offset;
 	char **mmaps = st->mmaps;
 
-	page_offset = page_size * (rd->data_offset / page_size);
-	st->file_offset = page_offset;
-	st->head = rd->data_offset - page_offset;
+	st->head = rd->data_offset;
 
 	st->data_size = rd->data_size + rd->data_offset;
 
@@ -2238,15 +2235,12 @@ reader__exit(struct reader *rd)
 }
 
 static int
-reader__process_events(struct reader *rd, struct perf_session *session,
-		       struct ui_progress *prog)
+reader__mmap(struct reader *rd, struct perf_session *session)
 {
 	struct reader_state *st = &rd->state;
-	u64 page_offset, size;
-	int err = 0, mmap_prot, mmap_flags;
+	int mmap_prot, mmap_flags;
 	char *buf, **mmaps = st->mmaps;
-	union perf_event *event;
-	s64 skip;
+	u64 page_offset;
 
 	mmap_prot  = PROT_READ;
 	mmap_flags = MAP_SHARED;
@@ -2257,20 +2251,45 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 		mmap_prot  |= PROT_WRITE;
 		mmap_flags = MAP_PRIVATE;
 	}
-remap:
+
+	if (mmaps[st->mmap_idx]) {
+		munmap(mmaps[st->mmap_idx], st->mmap_size);
+		mmaps[st->mmap_idx] = NULL;
+	}
+
+	page_offset = page_size * (st->head / page_size);
+	st->file_offset += page_offset;
+	st->head -= page_offset;
+
 	buf = mmap(NULL, st->mmap_size, mmap_prot, mmap_flags, rd->fd,
 		   st->file_offset);
 	if (buf == MAP_FAILED) {
 		pr_err("failed to mmap file\n");
-		err = -errno;
-		goto out;
+		return -errno;
 	}
 	mmaps[st->mmap_idx] = st->mmap_cur = buf;
 	st->mmap_idx = (st->mmap_idx + 1) & (ARRAY_SIZE(st->mmaps) - 1);
 	st->file_pos = st->file_offset + st->head;
+	return 0;
+}
+
+static int
+reader__process_events(struct reader *rd, struct perf_session *session,
+		       struct ui_progress *prog)
+{
+	struct reader_state *st = &rd->state;
+	u64 size;
+	int err = 0;
+	union perf_event *event;
+	s64 skip;
+
+remap:
+	err = reader__mmap(rd, session);
+	if (err)
+		goto out;
 	if (session->one_mmap) {
-		session->one_mmap_addr = buf;
-		session->one_mmap_offset = st->file_offset;
+		session->one_mmap_addr   = rd->state.mmap_cur;
+		session->one_mmap_offset = rd->state.file_offset;
 	}
 
 more:
@@ -2279,17 +2298,8 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 	if (IS_ERR(event))
 		return PTR_ERR(event);
 
-	if (!event) {
-		if (mmaps[st->mmap_idx]) {
-			munmap(mmaps[st->mmap_idx], st->mmap_size);
-			mmaps[st->mmap_idx] = NULL;
-		}
-
-		page_offset = page_size * (st->head / page_size);
-		st->file_offset += page_offset;
-		st->head -= page_offset;
+	if (!event)
 		goto remap;
-	}
 
 	session->active_reader = rd;
 	size = event->header.size;
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/6] perf session: Load single file for analysis
  2021-09-29  8:41 [PATCH 0/6] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
                   ` (4 preceding siblings ...)
  2021-09-29  8:41 ` [PATCH 5/6] perf session: Move map/unmap into reader__mmap function Alexey Bayduraev
@ 2021-09-29  8:41 ` Alexey Bayduraev
  2021-10-04 13:22   ` Jiri Olsa
  5 siblings, 1 reply; 10+ messages in thread
From: Alexey Bayduraev @ 2021-09-29  8:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
	Ingo Molnar, linux-kernel, Andi Kleen, Adrian Hunter,
	Alexander Antonov, Alexei Budankov, Riccardo Mancini

Adding EOF flag to reader state and moving the check to reader__mmap.
Separating reading code of single event into reader__read_event function.
Adding basic reader return codes to simplify the code and introducing
remap/read_event loop in __perf_session__process_events.

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 | 72 ++++++++++++++++++++++++---------------
 1 file changed, 45 insertions(+), 27 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 5e08def72b41..9a4fe78a7a54 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -63,6 +63,13 @@ struct reader_state {
 	u64	 file_offset;
 	u64	 data_size;
 	u64	 head;
+	bool	 eof;
+};
+
+enum {
+	READER_EOF,
+	READER_NODATA,
+	READER_OK,
 };
 
 struct reader {
@@ -2242,6 +2249,11 @@ reader__mmap(struct reader *rd, struct perf_session *session)
 	char *buf, **mmaps = st->mmaps;
 	u64 page_offset;
 
+	if (st->file_pos >= st->data_size) {
+		st->eof = true;
+		return READER_EOF;
+	}
+
 	mmap_prot  = PROT_READ;
 	mmap_flags = MAP_SHARED;
 
@@ -2270,36 +2282,26 @@ reader__mmap(struct reader *rd, struct perf_session *session)
 	mmaps[st->mmap_idx] = st->mmap_cur = buf;
 	st->mmap_idx = (st->mmap_idx + 1) & (ARRAY_SIZE(st->mmaps) - 1);
 	st->file_pos = st->file_offset + st->head;
-	return 0;
+	return READER_OK;
 }
 
 static int
-reader__process_events(struct reader *rd, struct perf_session *session,
-		       struct ui_progress *prog)
+reader__read_event(struct reader *rd, struct perf_session *session,
+		   struct ui_progress *prog)
 {
 	struct reader_state *st = &rd->state;
-	u64 size;
-	int err = 0;
+	int err = READER_OK;
 	union perf_event *event;
+	u64 size;
 	s64 skip;
 
-remap:
-	err = reader__mmap(rd, session);
-	if (err)
-		goto out;
-	if (session->one_mmap) {
-		session->one_mmap_addr   = rd->state.mmap_cur;
-		session->one_mmap_offset = rd->state.file_offset;
-	}
-
-more:
 	event = fetch_mmaped_event(st->head, st->mmap_size, st->mmap_cur,
 				   session->header.needs_swap);
 	if (IS_ERR(event))
 		return PTR_ERR(event);
 
 	if (!event)
-		goto remap;
+		return READER_NODATA;
 
 	session->active_reader = rd;
 	size = event->header.size;
@@ -2321,18 +2323,12 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 	st->head += size;
 	st->file_pos += size;
 
-	err = __perf_session__process_decomp_events(session);
-	if (err)
-		goto out;
+	skip = __perf_session__process_decomp_events(session);
+	if (skip)
+		err = skip;
 
 	ui_progress__update(prog, size);
 
-	if (session_done())
-		goto out;
-
-	if (st->file_pos < st->data_size)
-		goto more;
-
 out:
 	session->active_reader = NULL;
 	return err;
@@ -2369,9 +2365,31 @@ static int __perf_session__process_events(struct perf_session *session)
 	err = reader__init(&rd, &session->one_mmap);
 	if (err)
 		goto out_err;
-	err = reader__process_events(&rd, session, &prog);
-	if (err)
+	err = reader__mmap(&rd, session);
+	if (err != READER_OK) {
+		if (err == READER_EOF)
+			err = -EINVAL;
 		goto out_err;
+	}
+	if (session->one_mmap) {
+		session->one_mmap_addr   = rd.state.mmap_cur;
+		session->one_mmap_offset = rd.state.file_offset;
+	}
+
+	while (true) {
+		if (session_done())
+			break;
+
+		err = reader__read_event(&rd, session, &prog);
+		if (err < 0)
+			break;
+		if (err == READER_NODATA) {
+			err = reader__mmap(&rd, session);
+			if (err <= 0)
+				break;
+		}
+	}
+
 	/* do the final flush for ordered samples */
 	err = ordered_events__flush(oe, OE_FLUSH__FINAL);
 	if (err)
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 6/6] perf session: Load single file for analysis
  2021-09-29  8:41 ` [PATCH 6/6] perf session: Load single file for analysis Alexey Bayduraev
@ 2021-10-04 13:22   ` Jiri Olsa
  0 siblings, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2021-10-04 13:22 UTC (permalink / raw)
  To: Alexey Bayduraev
  Cc: Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Ingo Molnar, linux-kernel, Andi Kleen,
	Adrian Hunter, Alexander Antonov, Alexei Budankov,
	Riccardo Mancini

On Wed, Sep 29, 2021 at 11:41:54AM +0300, Alexey Bayduraev wrote:
> Adding EOF flag to reader state and moving the check to reader__mmap.
> Separating reading code of single event into reader__read_event function.
> Adding basic reader return codes to simplify the code and introducing
> remap/read_event loop in __perf_session__process_events.
> 
> 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 | 72 ++++++++++++++++++++++++---------------
>  1 file changed, 45 insertions(+), 27 deletions(-)
> 
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 5e08def72b41..9a4fe78a7a54 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -63,6 +63,13 @@ struct reader_state {
>  	u64	 file_offset;
>  	u64	 data_size;
>  	u64	 head;
> +	bool	 eof;

what's this flag for? I can't see it being used

jirka

> +};
> +
> +enum {
> +	READER_EOF,
> +	READER_NODATA,
> +	READER_OK,
>  };
>  
>  struct reader {
> @@ -2242,6 +2249,11 @@ reader__mmap(struct reader *rd, struct perf_session *session)
>  	char *buf, **mmaps = st->mmaps;
>  	u64 page_offset;
>  
> +	if (st->file_pos >= st->data_size) {
> +		st->eof = true;
> +		return READER_EOF;
> +	}
> +
>  	mmap_prot  = PROT_READ;
>  	mmap_flags = MAP_SHARED;
>  
> @@ -2270,36 +2282,26 @@ reader__mmap(struct reader *rd, struct perf_session *session)
>  	mmaps[st->mmap_idx] = st->mmap_cur = buf;
>  	st->mmap_idx = (st->mmap_idx + 1) & (ARRAY_SIZE(st->mmaps) - 1);
>  	st->file_pos = st->file_offset + st->head;
> -	return 0;
> +	return READER_OK;
>  }
>  
>  static int
> -reader__process_events(struct reader *rd, struct perf_session *session,
> -		       struct ui_progress *prog)
> +reader__read_event(struct reader *rd, struct perf_session *session,
> +		   struct ui_progress *prog)
>  {
>  	struct reader_state *st = &rd->state;
> -	u64 size;
> -	int err = 0;
> +	int err = READER_OK;
>  	union perf_event *event;
> +	u64 size;
>  	s64 skip;
>  
> -remap:
> -	err = reader__mmap(rd, session);
> -	if (err)
> -		goto out;
> -	if (session->one_mmap) {
> -		session->one_mmap_addr   = rd->state.mmap_cur;
> -		session->one_mmap_offset = rd->state.file_offset;
> -	}
> -
> -more:
>  	event = fetch_mmaped_event(st->head, st->mmap_size, st->mmap_cur,
>  				   session->header.needs_swap);
>  	if (IS_ERR(event))
>  		return PTR_ERR(event);
>  
>  	if (!event)
> -		goto remap;
> +		return READER_NODATA;
>  
>  	session->active_reader = rd;
>  	size = event->header.size;
> @@ -2321,18 +2323,12 @@ reader__process_events(struct reader *rd, struct perf_session *session,
>  	st->head += size;
>  	st->file_pos += size;
>  
> -	err = __perf_session__process_decomp_events(session);
> -	if (err)
> -		goto out;
> +	skip = __perf_session__process_decomp_events(session);
> +	if (skip)
> +		err = skip;
>  
>  	ui_progress__update(prog, size);
>  
> -	if (session_done())
> -		goto out;
> -
> -	if (st->file_pos < st->data_size)
> -		goto more;
> -
>  out:
>  	session->active_reader = NULL;
>  	return err;
> @@ -2369,9 +2365,31 @@ static int __perf_session__process_events(struct perf_session *session)
>  	err = reader__init(&rd, &session->one_mmap);
>  	if (err)
>  		goto out_err;
> -	err = reader__process_events(&rd, session, &prog);
> -	if (err)
> +	err = reader__mmap(&rd, session);
> +	if (err != READER_OK) {
> +		if (err == READER_EOF)
> +			err = -EINVAL;
>  		goto out_err;
> +	}
> +	if (session->one_mmap) {
> +		session->one_mmap_addr   = rd.state.mmap_cur;
> +		session->one_mmap_offset = rd.state.file_offset;
> +	}
> +
> +	while (true) {
> +		if (session_done())
> +			break;
> +
> +		err = reader__read_event(&rd, session, &prog);
> +		if (err < 0)
> +			break;
> +		if (err == READER_NODATA) {
> +			err = reader__mmap(&rd, session);
> +			if (err <= 0)
> +				break;
> +		}
> +	}
> +
>  	/* do the final flush for ordered samples */
>  	err = ordered_events__flush(oe, OE_FLUSH__FINAL);
>  	if (err)
> -- 
> 2.19.0
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/6] perf session: Move map/unmap into reader__mmap function
  2021-09-29  8:41 ` [PATCH 5/6] perf session: Move map/unmap into reader__mmap function Alexey Bayduraev
@ 2021-10-04 13:24   ` Jiri Olsa
  0 siblings, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2021-10-04 13:24 UTC (permalink / raw)
  To: Alexey Bayduraev
  Cc: Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Ingo Molnar, linux-kernel, Andi Kleen,
	Adrian Hunter, Alexander Antonov, Alexei Budankov,
	Riccardo Mancini

On Wed, Sep 29, 2021 at 11:41:53AM +0300, Alexey Bayduraev wrote:
> Moving mapping and unmapping code into reader__mmap, so the mmap
> code is located together. Moving head/file_offset computation into
> reader__mmap function, so all the offset computation is located
> together and in one place only.
> 
> 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 | 60 +++++++++++++++++++++++----------------
>  1 file changed, 35 insertions(+), 25 deletions(-)
> 
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 85142d2a9a5a..5e08def72b41 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -2206,12 +2206,9 @@ static int
>  reader__init(struct reader *rd, bool *one_mmap)
>  {
>  	struct reader_state *st = &rd->state;
> -	u64 page_offset;
>  	char **mmaps = st->mmaps;
>  
> -	page_offset = page_size * (rd->data_offset / page_size);
> -	st->file_offset = page_offset;
> -	st->head = rd->data_offset - page_offset;
> +	st->head = rd->data_offset;
>  

extra line, please remove

jirka

>  	st->data_size = rd->data_size + rd->data_offset;
>  
> @@ -2238,15 +2235,12 @@ reader__exit(struct reader *rd)
>  }
>  
>  static int
> -reader__process_events(struct reader *rd, struct perf_session *session,
> -		       struct ui_progress *prog)
> +reader__mmap(struct reader *rd, struct perf_session *session)
>  {
>  	struct reader_state *st = &rd->state;
> -	u64 page_offset, size;
> -	int err = 0, mmap_prot, mmap_flags;
> +	int mmap_prot, mmap_flags;
>  	char *buf, **mmaps = st->mmaps;
> -	union perf_event *event;
> -	s64 skip;
> +	u64 page_offset;
>  
>  	mmap_prot  = PROT_READ;
>  	mmap_flags = MAP_SHARED;
> @@ -2257,20 +2251,45 @@ reader__process_events(struct reader *rd, struct perf_session *session,
>  		mmap_prot  |= PROT_WRITE;
>  		mmap_flags = MAP_PRIVATE;
>  	}
> -remap:
> +
> +	if (mmaps[st->mmap_idx]) {
> +		munmap(mmaps[st->mmap_idx], st->mmap_size);
> +		mmaps[st->mmap_idx] = NULL;
> +	}
> +
> +	page_offset = page_size * (st->head / page_size);
> +	st->file_offset += page_offset;
> +	st->head -= page_offset;
> +
>  	buf = mmap(NULL, st->mmap_size, mmap_prot, mmap_flags, rd->fd,
>  		   st->file_offset);
>  	if (buf == MAP_FAILED) {
>  		pr_err("failed to mmap file\n");
> -		err = -errno;
> -		goto out;
> +		return -errno;
>  	}
>  	mmaps[st->mmap_idx] = st->mmap_cur = buf;
>  	st->mmap_idx = (st->mmap_idx + 1) & (ARRAY_SIZE(st->mmaps) - 1);
>  	st->file_pos = st->file_offset + st->head;
> +	return 0;
> +}
> +
> +static int
> +reader__process_events(struct reader *rd, struct perf_session *session,
> +		       struct ui_progress *prog)
> +{
> +	struct reader_state *st = &rd->state;
> +	u64 size;
> +	int err = 0;
> +	union perf_event *event;
> +	s64 skip;
> +
> +remap:
> +	err = reader__mmap(rd, session);
> +	if (err)
> +		goto out;
>  	if (session->one_mmap) {
> -		session->one_mmap_addr = buf;
> -		session->one_mmap_offset = st->file_offset;
> +		session->one_mmap_addr   = rd->state.mmap_cur;
> +		session->one_mmap_offset = rd->state.file_offset;
>  	}
>  
>  more:
> @@ -2279,17 +2298,8 @@ reader__process_events(struct reader *rd, struct perf_session *session,
>  	if (IS_ERR(event))
>  		return PTR_ERR(event);
>  
> -	if (!event) {
> -		if (mmaps[st->mmap_idx]) {
> -			munmap(mmaps[st->mmap_idx], st->mmap_size);
> -			mmaps[st->mmap_idx] = NULL;
> -		}
> -
> -		page_offset = page_size * (st->head / page_size);
> -		st->file_offset += page_offset;
> -		st->head -= page_offset;
> +	if (!event)
>  		goto remap;
> -	}
>  
>  	session->active_reader = rd;
>  	size = event->header.size;
> -- 
> 2.19.0
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/6] perf session: Introduce decompressor into reader object
  2021-09-29  8:41 ` [PATCH 3/6] perf session: Introduce decompressor into " Alexey Bayduraev
@ 2021-10-04 13:53   ` Jiri Olsa
  0 siblings, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2021-10-04 13:53 UTC (permalink / raw)
  To: Alexey Bayduraev
  Cc: Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Ingo Molnar, linux-kernel, Andi Kleen,
	Adrian Hunter, Alexander Antonov, Alexei Budankov,
	Riccardo Mancini

On Wed, Sep 29, 2021 at 11:41:51AM +0300, Alexey Bayduraev wrote:
> Introduce decompressor into reader object so that decompression
> could be executed separately for each data file. Adding active_reader
> into perf_session object to select current decompressor.
> 
> 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 | 48 +++++++++++++++++++++++++++++----------
>  tools/perf/util/session.h |  2 ++
>  2 files changed, 38 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 262efc18caac..3bea0830b3b8 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -71,6 +71,9 @@ struct reader {
>  	u64		 data_offset;
>  	reader_cb_t	 process;
>  	bool		 in_place_update;
> +	struct zstd_data zstd_data;
> +	struct decomp	 *decomp;
> +	struct decomp	 *decomp_last;

so reader and session share the same fields, I think it'd be better
if we put them to the new struct.. perhaps have something like:

  struct compress {
    struct zstd_data zstd_data;
    struct decomp    *decomp;
    struct decomp    *decomp_last;
  }

  struct session {
    ...
    struct compress *compress;   /* initialized to &compress_lcl */
    struct compress compress_lcl;
  }

  struct reader {
    ...
    struct compress compress;
  }


reader__process_events will set session->compress to &reader->compress

perf_session__process_compressed_event would always work with
compress pointer.. this would ease up following code as well

also I don't like the idea sharing the whole reader with session,
it'll be abused.. let's give up gradually ;-)

thanks,
jirka

>  	struct reader_state state;
>  };
>  
> @@ -82,7 +85,10 @@ static int perf_session__process_compressed_event(struct perf_session *session,
>  	size_t decomp_size, src_size;
>  	u64 decomp_last_rem = 0;
>  	size_t mmap_len, decomp_len = session->header.env.comp_mmap_len;
> -	struct decomp *decomp, *decomp_last = session->decomp_last;
> +	struct decomp *decomp, *decomp_last = session->active_reader ?
> +		session->active_reader->decomp_last : session->decomp_last;
> +	struct zstd_data *zstd_data = session->active_reader ?
> +		&session->active_reader->zstd_data : &session->zstd_data;
>  
>  	if (decomp_last) {
>  		decomp_last_rem = decomp_last->size - decomp_last->head;
> @@ -109,7 +115,7 @@ static int perf_session__process_compressed_event(struct perf_session *session,
>  	src = (void *)event + sizeof(struct perf_record_compressed);
>  	src_size = event->pack.header.size - sizeof(struct perf_record_compressed);
>  
> -	decomp_size = zstd_decompress_stream(&(session->zstd_data), src, src_size,
> +	decomp_size = zstd_decompress_stream(zstd_data, src, src_size,
>  				&(decomp->data[decomp_last_rem]), decomp_len - decomp_last_rem);
>  	if (!decomp_size) {
>  		munmap(decomp, mmap_len);
> @@ -119,12 +125,22 @@ static int perf_session__process_compressed_event(struct perf_session *session,
>  
>  	decomp->size += decomp_size;
>  
> -	if (session->decomp == NULL) {
> -		session->decomp = decomp;
> -		session->decomp_last = decomp;
> +	if (session->active_reader) {
> +		if (session->active_reader->decomp == NULL) {
> +			session->active_reader->decomp = decomp;
> +			session->active_reader->decomp_last = decomp;
> +		} else {
> +			session->active_reader->decomp_last->next = decomp;
> +			session->active_reader->decomp_last = decomp;
> +		}
>  	} else {
> -		session->decomp_last->next = decomp;
> -		session->decomp_last = decomp;
> +		if (session->decomp == NULL) {
> +			session->decomp = decomp;
> +			session->decomp_last = decomp;
> +		} else {
> +			session->decomp_last->next = decomp;
> +			session->decomp_last = decomp;
> +		}
>  	}
>  
>  	pr_debug("decomp (B): %zd to %zd\n", src_size, decomp_size);
> @@ -314,11 +330,11 @@ static void perf_session__delete_threads(struct perf_session *session)
>  	machine__delete_threads(&session->machines.host);
>  }
>  
> -static void perf_session__release_decomp_events(struct perf_session *session)
> +static void perf_decomp__release_events(struct decomp *next)
>  {
> -	struct decomp *next, *decomp;
> +	struct decomp *decomp;
>  	size_t mmap_len;
> -	next = session->decomp;
> +
>  	do {
>  		decomp = next;
>  		if (decomp == NULL)
> @@ -337,7 +353,7 @@ 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);
> -	perf_session__release_decomp_events(session);
> +	perf_decomp__release_events(session->decomp);
>  	perf_env__exit(&session->header.env);
>  	machines__exit(&session->machines);
>  	if (session->data) {
> @@ -2155,7 +2171,8 @@ static int __perf_session__process_decomp_events(struct perf_session *session)
>  {
>  	s64 skip;
>  	u64 size, file_pos = 0;
> -	struct decomp *decomp = session->decomp_last;
> +	struct decomp *decomp = session->active_reader ?
> +		session->active_reader->decomp_last : session->decomp_last;
>  
>  	if (!decomp)
>  		return 0;
> @@ -2212,6 +2229,9 @@ reader__process_events(struct reader *rd, struct perf_session *session,
>  
>  	memset(mmaps, 0, sizeof(st->mmaps));
>  
> +	if (zstd_init(&rd->zstd_data, 0))
> +		return -1;
> +
>  	mmap_prot  = PROT_READ;
>  	mmap_flags = MAP_SHARED;
>  
> @@ -2255,6 +2275,7 @@ reader__process_events(struct reader *rd, struct perf_session *session,
>  		goto remap;
>  	}
>  
> +	session->active_reader = rd;
>  	size = event->header.size;
>  
>  	skip = -EINVAL;
> @@ -2287,6 +2308,9 @@ reader__process_events(struct reader *rd, struct perf_session *session,
>  		goto more;
>  
>  out:
> +	session->active_reader = NULL;
> +	perf_decomp__release_events(rd->decomp);
> +	zstd_fini(&rd->zstd_data);
>  	return err;
>  }
>  
> diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
> index 5d8bd14a0a39..45f9f652a12a 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,7 @@ struct perf_session {
>  	struct zstd_data	zstd_data;
>  	struct decomp		*decomp;
>  	struct decomp		*decomp_last;
> +	struct reader           *active_reader;
>  };
>  
>  struct decomp {
> -- 
> 2.19.0
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-10-04 13:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29  8:41 [PATCH 0/6] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
2021-09-29  8:41 ` [PATCH 1/6] perf session: Move reader structure to the top Alexey Bayduraev
2021-09-29  8:41 ` [PATCH 2/6] perf session: Introduce reader_state in reader object Alexey Bayduraev
2021-09-29  8:41 ` [PATCH 3/6] perf session: Introduce decompressor into " Alexey Bayduraev
2021-10-04 13:53   ` Jiri Olsa
2021-09-29  8:41 ` [PATCH 4/6] perf session: Move init/exit into reader__init/reader__exit functions Alexey Bayduraev
2021-09-29  8:41 ` [PATCH 5/6] perf session: Move map/unmap into reader__mmap function Alexey Bayduraev
2021-10-04 13:24   ` Jiri Olsa
2021-09-29  8:41 ` [PATCH 6/6] perf session: Load single file for analysis Alexey Bayduraev
2021-10-04 13:22   ` Jiri Olsa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.