Re: [PATCH v7 19/20] perf session: Load single file for analysis

From: Namhyung Kim
Date: Sat Jun 26 2021 - 20:49:38 EST


On Tue, Jun 22, 2021 at 1:43 AM Alexey Bayduraev
<alexey.v.bayduraev@xxxxxxxxxxxxxxx> 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
> reader remmap/read_event loop based on them.
>
> Design and implementation are based on the prototype [1], [2].
>
> [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@xxxxxxxxxx/
>
> Suggested-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> Signed-off-by: Alexey Bayduraev <alexey.v.bayduraev@xxxxxxxxxxxxxxx>
> ---
> tools/perf/util/session.c | 71 ++++++++++++++++++++++++---------------
> 1 file changed, 44 insertions(+), 27 deletions(-)
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 7d91205a6a47..fe25abf83b80 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -64,6 +64,12 @@ struct reader_state {
> u64 file_offset;
> u64 data_size;
> u64 head;
> + bool eof;
> +};
> +
> +enum {
> + READER_EOF = 0,
> + READER_OK = 1,

Just a nitpick, it might be better to add READER_NODATA state
to differentiate it from the real end-of-file state.

Thanks,
Namhyung


> };
>
> struct reader {
> @@ -2245,6 +2251,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;
>
> @@ -2273,36 +2284,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_EOF;
>
> session->active_reader = rd;
> size = event->header.size;
> @@ -2324,18 +2325,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;
> @@ -2379,9 +2374,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_EOF) {
> + 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
>