| 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 linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20160523194842.GL8897@kernel.org>
Date: Mon, 23 May 2016 16:48:42 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Wang Nan <wangnan0@...wei.com>
Cc: pi3orama@....com, linux-kernel@...r.kernel.org,
He Kuang <hekuang@...wei.com>,
Arnaldo Carvalho de Melo <acme@...hat.com>,
Jiri Olsa <jolsa@...nel.org>,
Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
Namhyung Kim <namhyung@...nel.org>,
Zefan Li <lizefan@...wei.com>
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@...wei.com>
> Cc: He Kuang <hekuang@...wei.com>
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Zefan Li <lizefan@...wei.com>
> Cc: pi3orama@....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
Powered by blists - more mailing lists