2021-10-13 09:08:10

by Bayduraev, Alexey V

[permalink] [raw]
Subject: [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers

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/[email protected]/
[3] https://lore.kernel.org/lkml/[email protected]/

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


2021-10-13 09:09:11

by Bayduraev, Alexey V

[permalink] [raw]
Subject: [PATCH v4 1/8] perf session: Move all state items to reader object

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 <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Reviewed-by: Riccardo Mancini <[email protected]>
Tested-by: Riccardo Mancini <[email protected]>
Signed-off-by: Alexey Bayduraev <[email protected]>
---
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

2021-10-13 09:09:26

by Bayduraev, Alexey V

[permalink] [raw]
Subject: [PATCH v4 2/8] perf session: Introduce decompressor in reader object

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 <[email protected]>
Reviewed-by: Riccardo Mancini <[email protected]>
Tested-by: Riccardo Mancini <[email protected]>
Signed-off-by: Alexey Bayduraev <[email protected]>
---
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

2021-10-13 09:09:31

by Bayduraev, Alexey V

[permalink] [raw]
Subject: [PATCH v4 3/8] perf session: Move init/release code to separate functions

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 <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Reviewed-by: Riccardo Mancini <[email protected]>
Tested-by: Riccardo Mancini <[email protected]>
Signed-off-by: Alexey Bayduraev <[email protected]>
---
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

2021-10-13 09:09:35

by Bayduraev, Alexey V

[permalink] [raw]
Subject: [PATCH v4 4/8] perf session: Move map code to separate function

Moving mapping code into separate reader__mmap function.

Suggested-by: Jiri Olsa <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Reviewed-by: Riccardo Mancini <[email protected]>
Tested-by: Riccardo Mancini <[email protected]>
Signed-off-by: Alexey Bayduraev <[email protected]>
---
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

2021-10-13 09:10:09

by Bayduraev, Alexey V

[permalink] [raw]
Subject: [PATCH v4 5/8] perf session: Move unmap code to reader__mmap

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 <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Reviewed-by: Riccardo Mancini <[email protected]>
Tested-by: Riccardo Mancini <[email protected]>
Signed-off-by: Alexey Bayduraev <[email protected]>
---
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

2021-10-13 09:10:18

by Bayduraev, Alexey V

[permalink] [raw]
Subject: [PATCH v4 6/8] perf session: Move event read code to separate function

Separating reading code of single event into reader__read_event
function.

Suggested-by: Jiri Olsa <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Reviewed-by: Riccardo Mancini <[email protected]>
Tested-by: Riccardo Mancini <[email protected]>
Signed-off-by: Alexey Bayduraev <[email protected]>
---
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

2021-10-13 09:12:17

by Bayduraev, Alexey V

[permalink] [raw]
Subject: [PATCH v4 7/8] perf session: Introduce reader return codes

Adding reader READER_OK and READER_NODATA return codes to make
the code more clear.

Suggested-by: Jiri Olsa <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Reviewed-by: Riccardo Mancini <[email protected]>
Tested-by: Riccardo Mancini <[email protected]>
Signed-off-by: Alexey Bayduraev <[email protected]>
---
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

2021-10-13 09:12:37

by Bayduraev, Alexey V

[permalink] [raw]
Subject: [PATCH v4 8/8] perf session: Introduce reader EOF function

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

Signed-off-by: Alexey Bayduraev <[email protected]>
---
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

2021-10-13 09:18:26

by Bayduraev, Alexey V

[permalink] [raw]
Subject: Re: [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers

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/[email protected]/
> [3] https://lore.kernel.org/lkml/[email protected]/
>
> 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(-)
>

2021-10-14 06:48:02

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH v3 0/8] perf session: Extend reader object to allow multiple readers

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 <[email protected]>

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/[email protected]/
> [3] https://lore.kernel.org/lkml/[email protected]/
>
> 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
>