All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers
@ 2021-10-13  9:06 Alexey Bayduraev
  2021-10-13  9:06 ` [PATCH v4 1/8] perf session: Move all state items to reader object Alexey Bayduraev
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Alexey Bayduraev @ 2021-10-13  9:06 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

Changes in v4:
- set/unset active_decomp within reader__process_events

Changes in v3:
- removed struct reader_state in [PATCH v3 1/8]
- fixed repeating code in [PATCH v3 2/8]
- split [PATCH v2 4/5] to [PATCH v3 4/8], [PATCH v3 5/8]
- split [PATCH v2 5/5] to [PATCH v3 6/8] - [PATCH v3 8/8]

Changes in v2:
- introduced struct decomp_data suggested by Jiri Olsa
- removed unnecessary [PATCH v1 1/6]
- removed unnecessary extra line in [PATCH v2 4/5]
- removed unnecessary reader_state.eof flag in [PATCH v2 5/5]

Patchset moves state info and decompressor object into reader object
that made possible to split reader__process_events function into three
logical parts: init, 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 patchset was separated from [3].

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 (8):
  perf session: Move all state items to reader object
  perf session: Introduce decompressor in reader object
  perf session: Move init/release code to separate functions
  perf session: Move map code to separate function
  perf session: Move unmap code to reader__mmap
  perf session: Move event read code to separate function
  perf session: Introduce reader return codes
  perf session: Introduce reader EOF function

 tools/perf/util/session.c | 193 ++++++++++++++++++++++++++------------
 tools/perf/util/session.h |  10 +-
 2 files changed, 141 insertions(+), 62 deletions(-)

-- 
2.19.0


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

* [PATCH v4 1/8] perf session: Move all state items to reader object
  2021-10-13  9:06 [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
@ 2021-10-13  9:06 ` Alexey Bayduraev
  2021-10-13  9:06 ` [PATCH v4 2/8] perf session: Introduce decompressor in " Alexey Bayduraev
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Alexey Bayduraev @ 2021-10-13  9:06 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.
Moving all items that need to be kept from reader__process_events to
the reader object. Introducing mmap_cur to keep current mapping.

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

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 069c2cfdd3be..bf73e8c1f15c 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2171,6 +2171,13 @@ struct reader {
 	u64		 data_offset;
 	reader_cb_t	 process;
 	bool		 in_place_update;
+	char		 *mmaps[NUM_MMAPS];
+	size_t		 mmap_size;
+	int		 mmap_idx;
+	char		 *mmap_cur;
+	u64		 file_pos;
+	u64		 file_offset;
+	u64		 head;
 };
 
 static int
@@ -2178,28 +2185,27 @@ 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];
+	u64 page_offset, size;
+	int err = 0, mmap_prot, mmap_flags;
+	char *buf, **mmaps = rd->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;
+	rd->file_offset = page_offset;
+	rd->head = rd->data_offset - page_offset;
 
 	ui_progress__init_size(prog, data_size, "Processing events...");
 
 	data_size += rd->data_offset;
 
-	mmap_size = MMAP_SIZE;
-	if (mmap_size > data_size) {
-		mmap_size = data_size;
+	rd->mmap_size = MMAP_SIZE;
+	if (rd->mmap_size > data_size) {
+		rd->mmap_size = data_size;
 		session->one_mmap = true;
 	}
 
-	memset(mmaps, 0, sizeof(mmaps));
+	memset(mmaps, 0, sizeof(rd->mmaps));
 
 	mmap_prot  = PROT_READ;
 	mmap_flags = MAP_SHARED;
@@ -2211,35 +2217,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, rd->mmap_size, mmap_prot, mmap_flags, rd->fd,
+		   rd->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[rd->mmap_idx] = rd->mmap_cur = buf;
+	rd->mmap_idx = (rd->mmap_idx + 1) & (ARRAY_SIZE(rd->mmaps) - 1);
+	rd->file_pos = rd->file_offset + rd->head;
 	if (session->one_mmap) {
 		session->one_mmap_addr = buf;
-		session->one_mmap_offset = file_offset;
+		session->one_mmap_offset = rd->file_offset;
 	}
 
 more:
-	event = fetch_mmaped_event(head, mmap_size, buf, session->header.needs_swap);
+	event = fetch_mmaped_event(rd->head, rd->mmap_size, rd->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[rd->mmap_idx]) {
+			munmap(mmaps[rd->mmap_idx], rd->mmap_size);
+			mmaps[rd->mmap_idx] = NULL;
 		}
 
-		page_offset = page_size * (head / page_size);
-		file_offset += page_offset;
-		head -= page_offset;
+		page_offset = page_size * (rd->head / page_size);
+		rd->file_offset += page_offset;
+		rd->head -= page_offset;
 		goto remap;
 	}
 
@@ -2248,9 +2255,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, rd->file_pos)) < 0) {
 		pr_err("%#" PRIx64 " [%#x]: failed to process type: %d [%s]\n",
-		       file_offset + head, event->header.size,
+		       rd->file_offset + rd->head, event->header.size,
 		       event->header.type, strerror(-skip));
 		err = skip;
 		goto out;
@@ -2259,8 +2266,8 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 	if (skip)
 		size += skip;
 
-	head += size;
-	file_pos += size;
+	rd->head += size;
+	rd->file_pos += size;
 
 	err = __perf_session__process_decomp_events(session);
 	if (err)
@@ -2271,7 +2278,7 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 	if (session_done())
 		goto out;
 
-	if (file_pos < data_size)
+	if (rd->file_pos < data_size)
 		goto more;
 
 out:
-- 
2.19.0


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

* [PATCH v4 2/8] perf session: Introduce decompressor in reader object
  2021-10-13  9:06 [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
  2021-10-13  9:06 ` [PATCH v4 1/8] perf session: Move all state items to reader object Alexey Bayduraev
@ 2021-10-13  9:06 ` Alexey Bayduraev
  2021-10-13  9:06 ` [PATCH v4 3/8] perf session: Move init/release code to separate functions Alexey Bayduraev
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Alexey Bayduraev @ 2021-10-13  9:06 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

Introducing decompressor data structure with pointers to decomp
objects and to zstd object. We cannot just move session->zstd_data to
decomp_data as session->zstd_data is used not only for decompression.
Adding decompressor data object to reader object and introducing
active_decomp into perf_session object to select current decompressor.
Thus decompression could be executed separately for each data file.

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 | 39 +++++++++++++++++++++++++--------------
 tools/perf/util/session.h | 10 ++++++++--
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index bf73e8c1f15c..ff09869ab075 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -44,7 +44,7 @@ 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_decomp->decomp_last;
 
 	if (decomp_last) {
 		decomp_last_rem = decomp_last->size - decomp_last->head;
@@ -71,7 +71,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(session->active_decomp->zstd_decomp, src, src_size,
 				&(decomp->data[decomp_last_rem]), decomp_len - decomp_last_rem);
 	if (!decomp_size) {
 		munmap(decomp, mmap_len);
@@ -81,13 +81,12 @@ 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;
-	} else {
-		session->decomp_last->next = decomp;
-		session->decomp_last = decomp;
-	}
+	if (session->active_decomp->decomp == NULL)
+		session->active_decomp->decomp = decomp;
+	else
+		session->active_decomp->decomp_last->next = decomp;
+
+	session->active_decomp->decomp_last = decomp;
 
 	pr_debug("decomp (B): %zd to %zd\n", src_size, decomp_size);
 
@@ -197,6 +196,8 @@ struct perf_session *__perf_session__new(struct perf_data *data,
 
 	session->repipe = repipe;
 	session->tool   = tool;
+	session->decomp_data.zstd_decomp = &session->zstd_data;
+	session->active_decomp = &session->decomp_data;
 	INIT_LIST_HEAD(&session->auxtrace_index);
 	machines__init(&session->machines);
 	ordered_events__init(&session->ordered_events,
@@ -276,11 +277,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)
@@ -299,7 +300,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_data.decomp);
 	perf_env__exit(&session->header.env);
 	machines__exit(&session->machines);
 	if (session->data) {
@@ -2117,7 +2118,7 @@ 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_decomp->decomp_last;
 
 	if (!decomp)
 		return 0;
@@ -2178,6 +2179,8 @@ struct reader {
 	u64		 file_pos;
 	u64		 file_offset;
 	u64		 head;
+	struct zstd_data   zstd_data;
+	struct decomp_data decomp_data;
 };
 
 static int
@@ -2207,6 +2210,11 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 
 	memset(mmaps, 0, sizeof(rd->mmaps));
 
+	if (zstd_init(&rd->zstd_data, 0))
+		return -1;
+	rd->decomp_data.zstd_decomp = &rd->zstd_data;
+	session->active_decomp = &rd->decomp_data;
+
 	mmap_prot  = PROT_READ;
 	mmap_flags = MAP_SHARED;
 
@@ -2282,6 +2290,7 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 		goto more;
 
 out:
+	session->active_decomp = &session->decomp_data;
 	return err;
 }
 
@@ -2334,6 +2343,8 @@ static int __perf_session__process_events(struct perf_session *session)
 	 */
 	ordered_events__reinit(&session->ordered_events);
 	auxtrace__free_events(session);
+	perf_decomp__release_events(rd.decomp_data.decomp);
+	zstd_fini(&rd.zstd_data);
 	session->one_mmap = false;
 	return err;
 }
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 5d8bd14a0a39..46c854292ad6 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -20,6 +20,12 @@ struct thread;
 struct auxtrace;
 struct itrace_synth_opts;
 
+struct decomp_data {
+	struct decomp	 *decomp;
+	struct decomp	 *decomp_last;
+	struct zstd_data *zstd_decomp;
+};
+
 struct perf_session {
 	struct perf_header	header;
 	struct machines		machines;
@@ -39,8 +45,8 @@ struct perf_session {
 	u64			bytes_transferred;
 	u64			bytes_compressed;
 	struct zstd_data	zstd_data;
-	struct decomp		*decomp;
-	struct decomp		*decomp_last;
+	struct decomp_data	decomp_data;
+	struct decomp_data	*active_decomp;
 };
 
 struct decomp {
-- 
2.19.0


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

* [PATCH v4 3/8] perf session: Move init/release code to separate functions
  2021-10-13  9:06 [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
  2021-10-13  9:06 ` [PATCH v4 1/8] perf session: Move all state items to reader object Alexey Bayduraev
  2021-10-13  9:06 ` [PATCH v4 2/8] perf session: Introduce decompressor in " Alexey Bayduraev
@ 2021-10-13  9:06 ` Alexey Bayduraev
  2021-10-13  9:06 ` [PATCH v4 4/8] perf session: Move map code to separate function Alexey Bayduraev
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Alexey Bayduraev @ 2021-10-13  9:06 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/release code into reader__init and reader__release_decomp
functions. Removing a duplicate call to ui_progress__init_size, the same
call can be found in __perf_session__process_events. For multiple traces
ui_progress should be initialized by total size before reader__init calls.

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

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index ff09869ab075..396f8279757e 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2184,28 +2184,23 @@ struct reader {
 };
 
 static int
-reader__process_events(struct reader *rd, struct perf_session *session,
-		       struct ui_progress *prog)
+reader__init(struct reader *rd, bool *one_mmap)
 {
 	u64 data_size = rd->data_size;
-	u64 page_offset, size;
-	int err = 0, mmap_prot, mmap_flags;
-	char *buf, **mmaps = rd->mmaps;
-	union perf_event *event;
-	s64 skip;
+	u64 page_offset;
+	char **mmaps = rd->mmaps;
 
 	page_offset = page_size * (rd->data_offset / page_size);
 	rd->file_offset = page_offset;
 	rd->head = rd->data_offset - page_offset;
 
-	ui_progress__init_size(prog, data_size, "Processing events...");
-
 	data_size += rd->data_offset;
 
 	rd->mmap_size = MMAP_SIZE;
 	if (rd->mmap_size > data_size) {
 		rd->mmap_size = data_size;
-		session->one_mmap = true;
+		if (one_mmap)
+			*one_mmap = true;
 	}
 
 	memset(mmaps, 0, sizeof(rd->mmaps));
@@ -2213,6 +2208,31 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 	if (zstd_init(&rd->zstd_data, 0))
 		return -1;
 	rd->decomp_data.zstd_decomp = &rd->zstd_data;
+
+	return 0;
+}
+
+static void
+reader__release_decomp(struct reader *rd)
+{
+	perf_decomp__release_events(rd->decomp_data.decomp);
+	zstd_fini(&rd->zstd_data);
+}
+
+static int
+reader__process_events(struct reader *rd, struct perf_session *session,
+		       struct ui_progress *prog)
+{
+	u64 page_offset, size;
+	int err = 0, mmap_prot, mmap_flags;
+	char *buf, **mmaps = rd->mmaps;
+	union perf_event *event;
+	s64 skip;
+
+	err = reader__init(rd, &session->one_mmap);
+	if (err)
+		goto out;
+
 	session->active_decomp = &rd->decomp_data;
 
 	mmap_prot  = PROT_READ;
@@ -2286,7 +2306,7 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 	if (session_done())
 		goto out;
 
-	if (rd->file_pos < data_size)
+	if (rd->file_pos < rd->data_size + rd->data_offset)
 		goto more;
 
 out:
@@ -2343,8 +2363,7 @@ static int __perf_session__process_events(struct perf_session *session)
 	 */
 	ordered_events__reinit(&session->ordered_events);
 	auxtrace__free_events(session);
-	perf_decomp__release_events(rd.decomp_data.decomp);
-	zstd_fini(&rd.zstd_data);
+	reader__release_decomp(&rd);
 	session->one_mmap = false;
 	return err;
 }
-- 
2.19.0


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

* [PATCH v4 4/8] perf session: Move map code to separate function
  2021-10-13  9:06 [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
                   ` (2 preceding siblings ...)
  2021-10-13  9:06 ` [PATCH v4 3/8] perf session: Move init/release code to separate functions Alexey Bayduraev
@ 2021-10-13  9:06 ` Alexey Bayduraev
  2021-10-13  9:06 ` [PATCH v4 5/8] perf session: Move unmap code to reader__mmap Alexey Bayduraev
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Alexey Bayduraev @ 2021-10-13  9:06 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 code into separate reader__mmap 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 | 43 +++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 396f8279757e..50258319bb26 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2220,20 +2220,10 @@ reader__release_decomp(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)
 {
-	u64 page_offset, size;
-	int err = 0, mmap_prot, mmap_flags;
+	int mmap_prot, mmap_flags;
 	char *buf, **mmaps = rd->mmaps;
-	union perf_event *event;
-	s64 skip;
-
-	err = reader__init(rd, &session->one_mmap);
-	if (err)
-		goto out;
-
-	session->active_decomp = &rd->decomp_data;
 
 	mmap_prot  = PROT_READ;
 	mmap_flags = MAP_SHARED;
@@ -2244,13 +2234,12 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 		mmap_prot  |= PROT_WRITE;
 		mmap_flags = MAP_PRIVATE;
 	}
-remap:
+
 	buf = mmap(NULL, rd->mmap_size, mmap_prot, mmap_flags, rd->fd,
 		   rd->file_offset);
 	if (buf == MAP_FAILED) {
 		pr_err("failed to mmap file\n");
-		err = -errno;
-		goto out;
+		return -errno;
 	}
 	mmaps[rd->mmap_idx] = rd->mmap_cur = buf;
 	rd->mmap_idx = (rd->mmap_idx + 1) & (ARRAY_SIZE(rd->mmaps) - 1);
@@ -2260,6 +2249,30 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 		session->one_mmap_offset = rd->file_offset;
 	}
 
+	return 0;
+}
+
+static int
+reader__process_events(struct reader *rd, struct perf_session *session,
+		       struct ui_progress *prog)
+{
+	u64 page_offset, size;
+	int err = 0;
+	char **mmaps = rd->mmaps;
+	union perf_event *event;
+	s64 skip;
+
+	err = reader__init(rd, &session->one_mmap);
+	if (err)
+		goto out;
+
+	session->active_decomp = &rd->decomp_data;
+
+remap:
+	err = reader__mmap(rd, session);
+	if (err)
+		goto out;
+
 more:
 	event = fetch_mmaped_event(rd->head, rd->mmap_size, rd->mmap_cur,
 				   session->header.needs_swap);
-- 
2.19.0


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

* [PATCH v4 5/8] perf session: Move unmap code to reader__mmap
  2021-10-13  9:06 [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
                   ` (3 preceding siblings ...)
  2021-10-13  9:06 ` [PATCH v4 4/8] perf session: Move map code to separate function Alexey Bayduraev
@ 2021-10-13  9:06 ` Alexey Bayduraev
  2021-10-13  9:06 ` [PATCH v4 6/8] perf session: Move event read code to separate function Alexey Bayduraev
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Alexey Bayduraev @ 2021-10-13  9:06 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 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 | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 50258319bb26..d7b008837fd6 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2187,13 +2187,9 @@ static int
 reader__init(struct reader *rd, bool *one_mmap)
 {
 	u64 data_size = rd->data_size;
-	u64 page_offset;
 	char **mmaps = rd->mmaps;
 
-	page_offset = page_size * (rd->data_offset / page_size);
-	rd->file_offset = page_offset;
-	rd->head = rd->data_offset - page_offset;
-
+	rd->head = rd->data_offset;
 	data_size += rd->data_offset;
 
 	rd->mmap_size = MMAP_SIZE;
@@ -2224,6 +2220,7 @@ reader__mmap(struct reader *rd, struct perf_session *session)
 {
 	int mmap_prot, mmap_flags;
 	char *buf, **mmaps = rd->mmaps;
+	u64 page_offset;
 
 	mmap_prot  = PROT_READ;
 	mmap_flags = MAP_SHARED;
@@ -2235,6 +2232,15 @@ reader__mmap(struct reader *rd, struct perf_session *session)
 		mmap_flags = MAP_PRIVATE;
 	}
 
+	if (mmaps[rd->mmap_idx]) {
+		munmap(mmaps[rd->mmap_idx], rd->mmap_size);
+		mmaps[rd->mmap_idx] = NULL;
+	}
+
+	page_offset = page_size * (rd->head / page_size);
+	rd->file_offset += page_offset;
+	rd->head -= page_offset;
+
 	buf = mmap(NULL, rd->mmap_size, mmap_prot, mmap_flags, rd->fd,
 		   rd->file_offset);
 	if (buf == MAP_FAILED) {
@@ -2256,9 +2262,8 @@ static int
 reader__process_events(struct reader *rd, struct perf_session *session,
 		       struct ui_progress *prog)
 {
-	u64 page_offset, size;
+	u64 size;
 	int err = 0;
-	char **mmaps = rd->mmaps;
 	union perf_event *event;
 	s64 skip;
 
@@ -2279,17 +2284,8 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 	if (IS_ERR(event))
 		return PTR_ERR(event);
 
-	if (!event) {
-		if (mmaps[rd->mmap_idx]) {
-			munmap(mmaps[rd->mmap_idx], rd->mmap_size);
-			mmaps[rd->mmap_idx] = NULL;
-		}
-
-		page_offset = page_size * (rd->head / page_size);
-		rd->file_offset += page_offset;
-		rd->head -= page_offset;
+	if (!event)
 		goto remap;
-	}
 
 	size = event->header.size;
 
-- 
2.19.0


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

* [PATCH v4 6/8] perf session: Move event read code to separate function
  2021-10-13  9:06 [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
                   ` (4 preceding siblings ...)
  2021-10-13  9:06 ` [PATCH v4 5/8] perf session: Move unmap code to reader__mmap Alexey Bayduraev
@ 2021-10-13  9:06 ` Alexey Bayduraev
  2021-10-13  9:06 ` [PATCH v4 7/8] perf session: Introduce reader return codes Alexey Bayduraev
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Alexey Bayduraev @ 2021-10-13  9:06 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 reading code of single event into reader__read_event
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 | 46 ++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index d7b008837fd6..68d130fe51c2 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2259,33 +2259,21 @@ reader__mmap(struct reader *rd, struct perf_session *session)
 }
 
 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)
 {
 	u64 size;
 	int err = 0;
 	union perf_event *event;
 	s64 skip;
 
-	err = reader__init(rd, &session->one_mmap);
-	if (err)
-		goto out;
-
-	session->active_decomp = &rd->decomp_data;
-
-remap:
-	err = reader__mmap(rd, session);
-	if (err)
-		goto out;
-
-more:
 	event = fetch_mmaped_event(rd->head, rd->mmap_size, rd->mmap_cur,
 				   session->header.needs_swap);
 	if (IS_ERR(event))
 		return PTR_ERR(event);
 
 	if (!event)
-		goto remap;
+		return 1;
 
 	size = event->header.size;
 
@@ -2312,6 +2300,34 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 
 	ui_progress__update(prog, size);
 
+out:
+	return err;
+}
+
+static int
+reader__process_events(struct reader *rd, struct perf_session *session,
+		       struct ui_progress *prog)
+{
+	int err;
+
+	err = reader__init(rd, &session->one_mmap);
+	if (err)
+		goto out;
+
+	session->active_decomp = &rd->decomp_data;
+
+remap:
+	err = reader__mmap(rd, session);
+	if (err)
+		goto out;
+
+more:
+	err = reader__read_event(rd, session, prog);
+	if (err < 0)
+		goto out;
+	else if (err == 1)
+		goto remap;
+
 	if (session_done())
 		goto out;
 
-- 
2.19.0


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

* [PATCH v4 7/8] perf session: Introduce reader return codes
  2021-10-13  9:06 [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
                   ` (5 preceding siblings ...)
  2021-10-13  9:06 ` [PATCH v4 6/8] perf session: Move event read code to separate function Alexey Bayduraev
@ 2021-10-13  9:06 ` Alexey Bayduraev
  2021-10-13  9:06 ` [PATCH v4 8/8] perf session: Introduce reader EOF function Alexey Bayduraev
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Alexey Bayduraev @ 2021-10-13  9:06 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 reader READER_OK and READER_NODATA return codes to make
the code more clear.

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 | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 68d130fe51c2..9714881839e3 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2258,12 +2258,17 @@ reader__mmap(struct reader *rd, struct perf_session *session)
 	return 0;
 }
 
+enum {
+	READER_OK,
+	READER_NODATA,
+};
+
 static int
 reader__read_event(struct reader *rd, struct perf_session *session,
 		   struct ui_progress *prog)
 {
 	u64 size;
-	int err = 0;
+	int err = READER_OK;
 	union perf_event *event;
 	s64 skip;
 
@@ -2273,7 +2278,7 @@ reader__read_event(struct reader *rd, struct perf_session *session,
 		return PTR_ERR(event);
 
 	if (!event)
-		return 1;
+		return READER_NODATA;
 
 	size = event->header.size;
 
@@ -2325,7 +2330,7 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 	err = reader__read_event(rd, session, prog);
 	if (err < 0)
 		goto out;
-	else if (err == 1)
+	else if (err == READER_NODATA)
 		goto remap;
 
 	if (session_done())
-- 
2.19.0


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

* [PATCH v4 8/8] perf session: Introduce reader EOF function
  2021-10-13  9:06 [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
                   ` (6 preceding siblings ...)
  2021-10-13  9:06 ` [PATCH v4 7/8] perf session: Introduce reader return codes Alexey Bayduraev
@ 2021-10-13  9:06 ` Alexey Bayduraev
  2021-10-13  9:17 ` [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers Bayduraev, Alexey V
  2021-10-14  6:45 ` Jiri Olsa
  9 siblings, 0 replies; 11+ messages in thread
From: Alexey Bayduraev @ 2021-10-13  9:06 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

Introducing a function to check end-of-file status.

Signed-off-by: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
---
 tools/perf/util/session.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 9714881839e3..bca92ba7218a 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2309,6 +2309,12 @@ reader__read_event(struct reader *rd, struct perf_session *session,
 	return err;
 }
 
+static inline bool
+reader__eof(struct reader *rd)
+{
+	return (rd->file_pos >= rd->data_size + rd->data_offset);
+}
+
 static int
 reader__process_events(struct reader *rd, struct perf_session *session,
 		       struct ui_progress *prog)
@@ -2336,7 +2342,7 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 	if (session_done())
 		goto out;
 
-	if (rd->file_pos < rd->data_size + rd->data_offset)
+	if (!reader__eof(rd))
 		goto more;
 
 out:
-- 
2.19.0


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

* Re: [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers
  2021-10-13  9:06 [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
                   ` (7 preceding siblings ...)
  2021-10-13  9:06 ` [PATCH v4 8/8] perf session: Introduce reader EOF function Alexey Bayduraev
@ 2021-10-13  9:17 ` Bayduraev, Alexey V
  2021-10-14  6:45 ` Jiri Olsa
  9 siblings, 0 replies; 11+ messages in thread
From: Bayduraev, Alexey V @ 2021-10-13  9:17 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

On 13.10.2021 12:06, Alexey Bayduraev wrote:> Changes in v4:> - set/unset active_decomp within reader__process_events
Sorry, 

The subject should of course be [PATCH v4 0/8] ...

Regards,
Alexey

> 
> Changes in v3:
> - removed struct reader_state in [PATCH v3 1/8]
> - fixed repeating code in [PATCH v3 2/8]
> - split [PATCH v2 4/5] to [PATCH v3 4/8], [PATCH v3 5/8]
> - split [PATCH v2 5/5] to [PATCH v3 6/8] - [PATCH v3 8/8]
> 
> Changes in v2:
> - introduced struct decomp_data suggested by Jiri Olsa
> - removed unnecessary [PATCH v1 1/6]
> - removed unnecessary extra line in [PATCH v2 4/5]
> - removed unnecessary reader_state.eof flag in [PATCH v2 5/5]
> 
> Patchset moves state info and decompressor object into reader object
> that made possible to split reader__process_events function into three
> logical parts: init, 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 patchset was separated from [3].
> 
> 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 (8):
>   perf session: Move all state items to reader object
>   perf session: Introduce decompressor in reader object
>   perf session: Move init/release code to separate functions
>   perf session: Move map code to separate function
>   perf session: Move unmap code to reader__mmap
>   perf session: Move event read code to separate function
>   perf session: Introduce reader return codes
>   perf session: Introduce reader EOF function
> 
>  tools/perf/util/session.c | 193 ++++++++++++++++++++++++++------------
>  tools/perf/util/session.h |  10 +-
>  2 files changed, 141 insertions(+), 62 deletions(-)
> 

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

* Re: [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers
  2021-10-13  9:06 [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
                   ` (8 preceding siblings ...)
  2021-10-13  9:17 ` [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers Bayduraev, Alexey V
@ 2021-10-14  6:45 ` Jiri Olsa
  9 siblings, 0 replies; 11+ messages in thread
From: Jiri Olsa @ 2021-10-14  6:45 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, Oct 13, 2021 at 12:06:34PM +0300, Alexey Bayduraev wrote:
> Changes in v4:
> - set/unset active_decomp within reader__process_events

Reviewed-by: Jiri Olsa <jolsa@redhat.com>

thanks,
jirka

> 
> Changes in v3:
> - removed struct reader_state in [PATCH v3 1/8]
> - fixed repeating code in [PATCH v3 2/8]
> - split [PATCH v2 4/5] to [PATCH v3 4/8], [PATCH v3 5/8]
> - split [PATCH v2 5/5] to [PATCH v3 6/8] - [PATCH v3 8/8]
> 
> Changes in v2:
> - introduced struct decomp_data suggested by Jiri Olsa
> - removed unnecessary [PATCH v1 1/6]
> - removed unnecessary extra line in [PATCH v2 4/5]
> - removed unnecessary reader_state.eof flag in [PATCH v2 5/5]
> 
> Patchset moves state info and decompressor object into reader object
> that made possible to split reader__process_events function into three
> logical parts: init, 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 patchset was separated from [3].
> 
> 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 (8):
>   perf session: Move all state items to reader object
>   perf session: Introduce decompressor in reader object
>   perf session: Move init/release code to separate functions
>   perf session: Move map code to separate function
>   perf session: Move unmap code to reader__mmap
>   perf session: Move event read code to separate function
>   perf session: Introduce reader return codes
>   perf session: Introduce reader EOF function
> 
>  tools/perf/util/session.c | 193 ++++++++++++++++++++++++++------------
>  tools/perf/util/session.h |  10 +-
>  2 files changed, 141 insertions(+), 62 deletions(-)
> 
> -- 
> 2.19.0
> 


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

end of thread, other threads:[~2021-10-14  6:46 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-13  9:06 [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers Alexey Bayduraev
2021-10-13  9:06 ` [PATCH v4 1/8] perf session: Move all state items to reader object Alexey Bayduraev
2021-10-13  9:06 ` [PATCH v4 2/8] perf session: Introduce decompressor in " Alexey Bayduraev
2021-10-13  9:06 ` [PATCH v4 3/8] perf session: Move init/release code to separate functions Alexey Bayduraev
2021-10-13  9:06 ` [PATCH v4 4/8] perf session: Move map code to separate function Alexey Bayduraev
2021-10-13  9:06 ` [PATCH v4 5/8] perf session: Move unmap code to reader__mmap Alexey Bayduraev
2021-10-13  9:06 ` [PATCH v4 6/8] perf session: Move event read code to separate function Alexey Bayduraev
2021-10-13  9:06 ` [PATCH v4 7/8] perf session: Introduce reader return codes Alexey Bayduraev
2021-10-13  9:06 ` [PATCH v4 8/8] perf session: Introduce reader EOF function Alexey Bayduraev
2021-10-13  9:17 ` [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers Bayduraev, Alexey V
2021-10-14  6:45 ` 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.