Adding reader EOF return code and moving the check of EOF state to
reader__mmap. Adding reader OK and NODATA return codes to simplify
the code and separating reading code of single event into
reader__read_event function. Introducing read_event/remap 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 | 74 +++++++++++++++++++++++++--------------
1 file changed, 47 insertions(+), 27 deletions(-)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 6c825e4a9dfe..1915714747a1 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2167,6 +2167,12 @@ typedef s64 (*reader_cb_t)(struct perf_session *session,
union perf_event *event,
u64 file_offset);
+enum {
+ READER_OK,
+ READER_NODATA,
+ READER_EOF,
+};
+
struct reader_state {
char *mmaps[NUM_MMAPS];
size_t mmap_size;
@@ -2229,6 +2235,9 @@ reader__mmap(struct reader *rd, struct perf_session *session)
char *buf, **mmaps = st->mmaps;
u64 page_offset;
+ if (st->file_pos >= st->data_size)
+ return READER_EOF;
+
mmap_prot = PROT_READ;
mmap_flags = MAP_SHARED;
@@ -2257,36 +2266,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_decomp = &rd->decomp_data;
size = event->header.size;
@@ -2308,18 +2307,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_decomp = &session->decomp_data;
return err;
@@ -2356,9 +2349,36 @@ 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 < 0) {
goto out_err;
+ } else 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) {
+ goto out_err;
+ } else if (err == READER_NODATA) {
+ err = reader__mmap(&rd, session);
+ if (err < 0)
+ goto out_err;
+ else if (err == READER_EOF)
+ break;
+ }
+ }
+
/* do the final flush for ordered samples */
err = ordered_events__flush(oe, OE_FLUSH__FINAL);
if (err)
--
2.19.0
Em Tue, Oct 05, 2021 at 01:27:02PM +0300, Alexey Bayduraev escreveu:
> Adding reader EOF return code and moving the check of EOF state to
> reader__mmap. Adding reader OK and NODATA return codes to simplify
> the code and separating reading code of single event into
> reader__read_event function. Introducing read_event/remap loop
> in __perf_session__process_events.
You are listing a series of changes done into just one cset, isn't it
possible to break this down to ease review (now and when looking for
bugs in the future :) )?
- 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, 47 insertions(+), 27 deletions(-)
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 6c825e4a9dfe..1915714747a1 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -2167,6 +2167,12 @@ typedef s64 (*reader_cb_t)(struct perf_session *session,
> union perf_event *event,
> u64 file_offset);
>
> +enum {
> + READER_OK,
> + READER_NODATA,
> + READER_EOF,
> +};
> +
> struct reader_state {
> char *mmaps[NUM_MMAPS];
> size_t mmap_size;
> @@ -2229,6 +2235,9 @@ reader__mmap(struct reader *rd, struct perf_session *session)
> char *buf, **mmaps = st->mmaps;
> u64 page_offset;
>
> + if (st->file_pos >= st->data_size)
> + return READER_EOF;
> +
> mmap_prot = PROT_READ;
> mmap_flags = MAP_SHARED;
>
> @@ -2257,36 +2266,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_decomp = &rd->decomp_data;
> size = event->header.size;
> @@ -2308,18 +2307,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_decomp = &session->decomp_data;
> return err;
> @@ -2356,9 +2349,36 @@ 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 < 0) {
> goto out_err;
> + } else 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) {
> + goto out_err;
> + } else if (err == READER_NODATA) {
> + err = reader__mmap(&rd, session);
> + if (err < 0)
> + goto out_err;
> + else if (err == READER_EOF)
> + break;
> + }
> + }
> +
> /* do the final flush for ordered samples */
> err = ordered_events__flush(oe, OE_FLUSH__FINAL);
> if (err)
> --
> 2.19.0
--
- Arnaldo