Messages in this thread Patch in this message | | | Date | Mon, 23 May 2016 16:48:42 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH v3 05/11] perf evlist: Introduce aux perf evlist |
| |
Em Mon, May 23, 2016 at 07:13:42AM +0000, Wang Nan escreveu: > Introduce auxiliary perf evlist. Such evlists created by perf_evlist__new_aux() > using an existing evlist. A 'parent' pointer points to the template.
You must define template above, otherwise I will not have how to compare the patch _intention_ to what the patch _does_.
That could help understanding the next paragraph, where it seems two evlists are at play, but I don't know which is which: > A 'evlist->parent' pointer is added to 'struct perf_evlist' and points to the > evlist itself for normal evlists (so in this patch changing 'evlist' to > 'evlist->parent' won't causes any error).
After reading the patch I understand that you're paving the way to allow that ,when reading the mmaps in an evlist, instead of reading from its mmaps, read from a "parent", i.e. at some point evlist->evlist != evlist, but why, just by reading this commit I can't figure it out :-\ > Following commits uses auxiliary evlist as container of 'struct perf_mmap', > creates auxiliary evlist for overwritable events, allow them to create > separated mmaps. To achieve this goal, this patch carefully changes 'evlist'
To begin with, _why_ do we need two evlists, you need to make this clear.
> to 'evlist->parent' in all functions in the patch of 'perf_evlist__mmap_ex', > except 'evlist->mmap' related operations, to make sure all evlist modifications > like pollfd and event id hash tables goes to original evlist.
I applied the patches before this one.
- Arnaldo > Signed-off-by: Wang Nan <wangnan0@huawei.com> > Cc: He Kuang <hekuang@huawei.com> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com> > Cc: Jiri Olsa <jolsa@kernel.org> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Zefan Li <lizefan@huawei.com> > Cc: pi3orama@163.com > --- > tools/perf/util/evlist.c | 31 +++++++++++++++++++++---------- > tools/perf/util/evlist.h | 3 +++ > 2 files changed, 24 insertions(+), 10 deletions(-) > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > index 1305910..af0bea7 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -45,6 +45,7 @@ void perf_evlist__init(struct perf_evlist *evlist, struct cpu_map *cpus, > fdarray__init(&evlist->pollfd, 64); > evlist->workload.pid = -1; > evlist->backward = false; > + evlist->parent = evlist; > } > > struct perf_evlist *perf_evlist__new(void) > @@ -994,7 +995,7 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx, > { > struct perf_evsel *evsel; > > - evlist__for_each(evlist, evsel) { > + evlist__for_each(evlist->parent, evsel) { > int fd; > > if (evsel->system_wide && thread) > @@ -1021,16 +1022,16 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx, > * Therefore don't add it for polling. > */ > if (!evsel->system_wide && > - __perf_evlist__add_pollfd(evlist, fd, idx) < 0) { > + __perf_evlist__add_pollfd(evlist->parent, fd, idx) < 0) { > perf_evlist__mmap_put(evlist, idx); > return -1; > } > > if (evsel->attr.read_format & PERF_FORMAT_ID) { > - if (perf_evlist__id_add_fd(evlist, evsel, cpu, thread, > + if (perf_evlist__id_add_fd(evlist->parent, evsel, cpu, thread, > fd) < 0) > return -1; > - perf_evlist__set_sid_idx(evlist, evsel, idx, cpu, > + perf_evlist__set_sid_idx(evlist->parent, evsel, idx, cpu, > thread); > } > } > @@ -1071,13 +1072,13 @@ static int perf_evlist__mmap_per_thread(struct perf_evlist *evlist, > struct mmap_params *mp) > { > int thread; > - int nr_threads = thread_map__nr(evlist->threads); > + int nr_threads = thread_map__nr(evlist->parent->threads); > > pr_debug2("perf event ring buffer mmapped per thread\n"); > for (thread = 0; thread < nr_threads; thread++) { > int output = -1; > > - auxtrace_mmap_params__set_idx(&mp->auxtrace_mp, evlist, thread, > + auxtrace_mmap_params__set_idx(&mp->auxtrace_mp, evlist->parent, thread, > false); > > if (perf_evlist__mmap_per_evsel(evlist, thread, mp, 0, thread, > @@ -1216,8 +1217,8 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages, > bool auxtrace_overwrite) > { > struct perf_evsel *evsel; > - const struct cpu_map *cpus = evlist->cpus; > - const struct thread_map *threads = evlist->threads; > + const struct cpu_map *cpus = evlist->parent->cpus; > + const struct thread_map *threads = evlist->parent->threads; > struct mmap_params mp = { > .prot = PROT_READ | (overwrite ? 0 : PROT_WRITE), > }; > @@ -1225,7 +1226,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages, > if (evlist->mmap == NULL && perf_evlist__alloc_mmap(evlist) < 0) > return -ENOMEM; > > - if (evlist->pollfd.entries == NULL && perf_evlist__alloc_pollfd(evlist) < 0) > + if (evlist->parent->pollfd.entries == NULL && perf_evlist__alloc_pollfd(evlist->parent) < 0) > return -ENOMEM; > > evlist->overwrite = overwrite; > @@ -1236,7 +1237,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages, > auxtrace_mmap_params__init(&mp.auxtrace_mp, evlist->mmap_len, > auxtrace_pages, auxtrace_overwrite); > > - evlist__for_each(evlist, evsel) { > + evlist__for_each(evlist->parent, evsel) { > if ((evsel->attr.read_format & PERF_FORMAT_ID) && > evsel->sample_id == NULL && > perf_evsel__alloc_id(evsel, cpu_map__nr(cpus), threads->nr) < 0) > @@ -1893,3 +1894,13 @@ perf_evlist__find_evsel_by_str(struct perf_evlist *evlist, > > return NULL; > } > + > +struct perf_evlist *perf_evlist__new_aux(struct perf_evlist *parent) > +{ > + struct perf_evlist *evlist = zalloc(sizeof(*evlist)); > + > + if (evlist != NULL) > + perf_evlist__init(evlist, parent->cpus, parent->threads); > + evlist->parent = parent->parent; > + return evlist; > +} > diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h > index d740fb8..0505012 100644 > --- a/tools/perf/util/evlist.h > +++ b/tools/perf/util/evlist.h > @@ -60,6 +60,7 @@ struct perf_evlist { > struct perf_evsel *selected; > struct events_stats stats; > struct perf_env *env; > + struct perf_evlist *parent; > }; > > struct perf_evsel_str_handler { > @@ -70,6 +71,8 @@ struct perf_evsel_str_handler { > struct perf_evlist *perf_evlist__new(void); > struct perf_evlist *perf_evlist__new_default(void); > struct perf_evlist *perf_evlist__new_dummy(void); > +struct perf_evlist *perf_evlist__new_aux(struct perf_evlist *); > + > void perf_evlist__init(struct perf_evlist *evlist, struct cpu_map *cpus, > struct thread_map *threads); > void perf_evlist__exit(struct perf_evlist *evlist); > -- > 1.8.3.4
| |