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/[email protected]/
[3] https://lore.kernel.org/lkml/[email protected]/
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
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 <[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 | 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
Separating init/exit code into reader__init and reader__exit 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 | 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
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 <[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 | 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
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 <[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 | 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
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 <[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 | 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
>
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 <[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 | 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
>
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 <[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 | 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
>