We need all the state info about reader in separate object to load data
from multiple files, so we can keep multiple readers at the same time.
Adding struct reader_state and adding all items that need to be kept.
Suggested-by: Jiri Olsa <[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 | 74 +++++++++++++++++++++++----------------
1 file changed, 43 insertions(+), 31 deletions(-)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 069c2cfdd3be..f29b106b1b17 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2165,41 +2165,52 @@ typedef s64 (*reader_cb_t)(struct perf_session *session,
union perf_event *event,
u64 file_offset);
+struct reader_state {
+ char *mmaps[NUM_MMAPS];
+ size_t mmap_size;
+ int mmap_idx;
+ char *mmap_cur;
+ u64 file_pos;
+ u64 file_offset;
+ u64 data_size;
+ u64 head;
+};
+
struct reader {
int fd;
u64 data_size;
u64 data_offset;
reader_cb_t process;
bool in_place_update;
+ struct reader_state state;
};
static int
reader__process_events(struct reader *rd, struct perf_session *session,
struct ui_progress *prog)
{
- u64 data_size = rd->data_size;
- u64 head, page_offset, file_offset, file_pos, size;
- int err = 0, mmap_prot, mmap_flags, map_idx = 0;
- size_t mmap_size;
- char *buf, *mmaps[NUM_MMAPS];
+ struct reader_state *st = &rd->state;
+ u64 page_offset, size;
+ int err = 0, mmap_prot, mmap_flags;
+ char *buf, **mmaps = st->mmaps;
union perf_event *event;
s64 skip;
page_offset = page_size * (rd->data_offset / page_size);
- file_offset = page_offset;
- head = rd->data_offset - page_offset;
+ st->file_offset = page_offset;
+ st->head = rd->data_offset - page_offset;
- ui_progress__init_size(prog, data_size, "Processing events...");
+ ui_progress__init_size(prog, rd->data_size, "Processing events...");
- data_size += rd->data_offset;
+ st->data_size = rd->data_size + rd->data_offset;
- mmap_size = MMAP_SIZE;
- if (mmap_size > data_size) {
- mmap_size = data_size;
+ st->mmap_size = MMAP_SIZE;
+ if (st->mmap_size > st->data_size) {
+ st->mmap_size = st->data_size;
session->one_mmap = true;
}
- memset(mmaps, 0, sizeof(mmaps));
+ memset(mmaps, 0, sizeof(st->mmaps));
mmap_prot = PROT_READ;
mmap_flags = MAP_SHARED;
@@ -2211,35 +2222,36 @@ reader__process_events(struct reader *rd, struct perf_session *session,
mmap_flags = MAP_PRIVATE;
}
remap:
- buf = mmap(NULL, mmap_size, mmap_prot, mmap_flags, rd->fd,
- file_offset);
+ buf = mmap(NULL, st->mmap_size, mmap_prot, mmap_flags, rd->fd,
+ st->file_offset);
if (buf == MAP_FAILED) {
pr_err("failed to mmap file\n");
err = -errno;
goto out;
}
- mmaps[map_idx] = buf;
- map_idx = (map_idx + 1) & (ARRAY_SIZE(mmaps) - 1);
- file_pos = file_offset + head;
+ mmaps[st->mmap_idx] = st->mmap_cur = buf;
+ st->mmap_idx = (st->mmap_idx + 1) & (ARRAY_SIZE(st->mmaps) - 1);
+ st->file_pos = st->file_offset + st->head;
if (session->one_mmap) {
session->one_mmap_addr = buf;
- session->one_mmap_offset = file_offset;
+ session->one_mmap_offset = st->file_offset;
}
more:
- event = fetch_mmaped_event(head, mmap_size, buf, session->header.needs_swap);
+ event = fetch_mmaped_event(st->head, st->mmap_size, st->mmap_cur,
+ session->header.needs_swap);
if (IS_ERR(event))
return PTR_ERR(event);
if (!event) {
- if (mmaps[map_idx]) {
- munmap(mmaps[map_idx], mmap_size);
- mmaps[map_idx] = NULL;
+ if (mmaps[st->mmap_idx]) {
+ munmap(mmaps[st->mmap_idx], st->mmap_size);
+ mmaps[st->mmap_idx] = NULL;
}
- page_offset = page_size * (head / page_size);
- file_offset += page_offset;
- head -= page_offset;
+ page_offset = page_size * (st->head / page_size);
+ st->file_offset += page_offset;
+ st->head -= page_offset;
goto remap;
}
@@ -2248,9 +2260,9 @@ reader__process_events(struct reader *rd, struct perf_session *session,
skip = -EINVAL;
if (size < sizeof(struct perf_event_header) ||
- (skip = rd->process(session, event, file_pos)) < 0) {
+ (skip = rd->process(session, event, st->file_pos)) < 0) {
pr_err("%#" PRIx64 " [%#x]: failed to process type: %d [%s]\n",
- file_offset + head, event->header.size,
+ st->file_offset + st->head, event->header.size,
event->header.type, strerror(-skip));
err = skip;
goto out;
@@ -2259,8 +2271,8 @@ reader__process_events(struct reader *rd, struct perf_session *session,
if (skip)
size += skip;
- head += size;
- file_pos += size;
+ st->head += size;
+ st->file_pos += size;
err = __perf_session__process_decomp_events(session);
if (err)
@@ -2271,7 +2283,7 @@ reader__process_events(struct reader *rd, struct perf_session *session,
if (session_done())
goto out;
- if (file_pos < data_size)
+ if (st->file_pos < st->data_size)
goto more;
out:
--
2.19.0
Em Tue, Oct 05, 2021 at 01:26:58PM +0300, Alexey Bayduraev escreveu:
> We need all the state info about reader in separate object to load data
> from multiple files, so we can keep multiple readers at the same time.
> Adding struct reader_state and adding all items that need to be kept.
Why not pass 'struct reader' instead? "reader_state" looks too vague,
isn't the existing state the reader state? i.e. things like 'fd',
'data_size', etc in 'struct reader'.
Can we find better names to avoid this confusion?
- Arnaldo
> 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 | 74 +++++++++++++++++++++++----------------
> 1 file changed, 43 insertions(+), 31 deletions(-)
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 069c2cfdd3be..f29b106b1b17 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -2165,41 +2165,52 @@ typedef s64 (*reader_cb_t)(struct perf_session *session,
> union perf_event *event,
> u64 file_offset);
>
> +struct reader_state {
> + char *mmaps[NUM_MMAPS];
> + size_t mmap_size;
> + int mmap_idx;
> + char *mmap_cur;
> + u64 file_pos;
> + u64 file_offset;
> + u64 data_size;
> + u64 head;
> +};
> +
> struct reader {
> int fd;
> u64 data_size;
> u64 data_offset;
> reader_cb_t process;
> bool in_place_update;
> + struct reader_state state;
> };
>
> static int
> reader__process_events(struct reader *rd, struct perf_session *session,
> struct ui_progress *prog)
> {
> - u64 data_size = rd->data_size;
> - u64 head, page_offset, file_offset, file_pos, size;
> - int err = 0, mmap_prot, mmap_flags, map_idx = 0;
> - size_t mmap_size;
> - char *buf, *mmaps[NUM_MMAPS];
> + struct reader_state *st = &rd->state;
> + u64 page_offset, size;
> + int err = 0, mmap_prot, mmap_flags;
> + char *buf, **mmaps = st->mmaps;
> union perf_event *event;
> s64 skip;
>
> page_offset = page_size * (rd->data_offset / page_size);
> - file_offset = page_offset;
> - head = rd->data_offset - page_offset;
> + st->file_offset = page_offset;
> + st->head = rd->data_offset - page_offset;
>
> - ui_progress__init_size(prog, data_size, "Processing events...");
> + ui_progress__init_size(prog, rd->data_size, "Processing events...");
>
> - data_size += rd->data_offset;
> + st->data_size = rd->data_size + rd->data_offset;
>
> - mmap_size = MMAP_SIZE;
> - if (mmap_size > data_size) {
> - mmap_size = data_size;
> + st->mmap_size = MMAP_SIZE;
> + if (st->mmap_size > st->data_size) {
> + st->mmap_size = st->data_size;
> session->one_mmap = true;
> }
>
> - memset(mmaps, 0, sizeof(mmaps));
> + memset(mmaps, 0, sizeof(st->mmaps));
>
> mmap_prot = PROT_READ;
> mmap_flags = MAP_SHARED;
> @@ -2211,35 +2222,36 @@ reader__process_events(struct reader *rd, struct perf_session *session,
> mmap_flags = MAP_PRIVATE;
> }
> remap:
> - buf = mmap(NULL, mmap_size, mmap_prot, mmap_flags, rd->fd,
> - file_offset);
> + buf = mmap(NULL, st->mmap_size, mmap_prot, mmap_flags, rd->fd,
> + st->file_offset);
> if (buf == MAP_FAILED) {
> pr_err("failed to mmap file\n");
> err = -errno;
> goto out;
> }
> - mmaps[map_idx] = buf;
> - map_idx = (map_idx + 1) & (ARRAY_SIZE(mmaps) - 1);
> - file_pos = file_offset + head;
> + mmaps[st->mmap_idx] = st->mmap_cur = buf;
> + st->mmap_idx = (st->mmap_idx + 1) & (ARRAY_SIZE(st->mmaps) - 1);
> + st->file_pos = st->file_offset + st->head;
> if (session->one_mmap) {
> session->one_mmap_addr = buf;
> - session->one_mmap_offset = file_offset;
> + session->one_mmap_offset = st->file_offset;
> }
>
> more:
> - event = fetch_mmaped_event(head, mmap_size, buf, session->header.needs_swap);
> + event = fetch_mmaped_event(st->head, st->mmap_size, st->mmap_cur,
> + session->header.needs_swap);
> if (IS_ERR(event))
> return PTR_ERR(event);
>
> if (!event) {
> - if (mmaps[map_idx]) {
> - munmap(mmaps[map_idx], mmap_size);
> - mmaps[map_idx] = NULL;
> + if (mmaps[st->mmap_idx]) {
> + munmap(mmaps[st->mmap_idx], st->mmap_size);
> + mmaps[st->mmap_idx] = NULL;
> }
>
> - page_offset = page_size * (head / page_size);
> - file_offset += page_offset;
> - head -= page_offset;
> + page_offset = page_size * (st->head / page_size);
> + st->file_offset += page_offset;
> + st->head -= page_offset;
> goto remap;
> }
>
> @@ -2248,9 +2260,9 @@ reader__process_events(struct reader *rd, struct perf_session *session,
> skip = -EINVAL;
>
> if (size < sizeof(struct perf_event_header) ||
> - (skip = rd->process(session, event, file_pos)) < 0) {
> + (skip = rd->process(session, event, st->file_pos)) < 0) {
> pr_err("%#" PRIx64 " [%#x]: failed to process type: %d [%s]\n",
> - file_offset + head, event->header.size,
> + st->file_offset + st->head, event->header.size,
> event->header.type, strerror(-skip));
> err = skip;
> goto out;
> @@ -2259,8 +2271,8 @@ reader__process_events(struct reader *rd, struct perf_session *session,
> if (skip)
> size += skip;
>
> - head += size;
> - file_pos += size;
> + st->head += size;
> + st->file_pos += size;
>
> err = __perf_session__process_decomp_events(session);
> if (err)
> @@ -2271,7 +2283,7 @@ reader__process_events(struct reader *rd, struct perf_session *session,
> if (session_done())
> goto out;
>
> - if (file_pos < data_size)
> + if (st->file_pos < st->data_size)
> goto more;
>
> out:
> --
> 2.19.0