lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 5 Oct 2021 15:06:17 -0300 From: Arnaldo Carvalho de Melo <acme@...nel.org> To: Alexey Bayduraev <alexey.v.bayduraev@...ux.intel.com> Cc: Jiri Olsa <jolsa@...hat.com>, Namhyung Kim <namhyung@...nel.org>, Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, linux-kernel <linux-kernel@...r.kernel.org>, Andi Kleen <ak@...ux.intel.com>, Adrian Hunter <adrian.hunter@...el.com>, Alexander Antonov <alexander.antonov@...ux.intel.com>, Alexei Budankov <abudankov@...wei.com>, Riccardo Mancini <rickyman7@...il.com> Subject: Re: [PATCH v2 1/5] perf session: Introduce reader_state in reader object 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 <jolsa@...nel.org> > Acked-by: Namhyung Kim <namhyung@...il.com> > Reviewed-by: Riccardo Mancini <rickyman7@...il.com> > Tested-by: Riccardo Mancini <rickyman7@...il.com> > Signed-off-by: Alexey Bayduraev <alexey.v.bayduraev@...ux.intel.com> > --- > 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
Powered by blists - more mailing lists