From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Wang Nan <wangnan0@huawei.com>
Cc: pi3orama@163.com, linux-kernel@vger.kernel.org,
He Kuang <hekuang@huawei.com>,
Arnaldo Carvalho de Melo <acme@redhat.com>,
Jiri Olsa <jolsa@kernel.org>,
Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
Namhyung Kim <namhyung@kernel.org>, Zefan Li <lizefan@huawei.com>
Subject: Re: [PATCH v3 05/11] perf evlist: Introduce aux perf evlist
Date: Mon, 23 May 2016 16:48:42 -0300 [thread overview]
Message-ID: <20160523194842.GL8897@kernel.org> (raw)
In-Reply-To: <1463987628-163563-6-git-send-email-wangnan0@huawei.com>
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
next prev parent reply other threads:[~2016-05-23 19:48 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-23 7:13 [PATCH v3 00/11] perf tools: Support overwritable ring buffer Wang Nan
2016-05-23 7:13 ` [PATCH v3 01/11] perf tools: Add API to pause/resume a evlist Wang Nan
2016-05-23 21:24 ` Arnaldo Carvalho de Melo
2016-05-24 5:48 ` [tip:perf/urgent] perf evlist: Add API to pause/resume tip-bot for Wang Nan
2016-05-23 7:13 ` [PATCH v3 02/11] perf record: Prevent reading invalid data in record__mmap_read Wang Nan
2016-05-24 5:48 ` [tip:perf/urgent] " tip-bot for Wang Nan
2016-05-23 7:13 ` [PATCH v3 03/11] perf record: Rename variable to make code clear Wang Nan
2016-05-24 5:48 ` [tip:perf/urgent] " tip-bot for Wang Nan
2016-05-23 7:13 ` [PATCH v3 04/11] perf record: Read from backward ring buffer Wang Nan
2016-05-24 5:49 ` [tip:perf/urgent] " tip-bot for Wang Nan
2016-05-23 7:13 ` [PATCH v3 05/11] perf evlist: Introduce aux perf evlist Wang Nan
2016-05-23 19:48 ` Arnaldo Carvalho de Melo [this message]
2016-05-23 7:13 ` [PATCH v3 06/11] perf tools: Don't poll and mmap overwritable events Wang Nan
2016-05-23 7:13 ` [PATCH v3 07/11] perf tools: Enable overwrite settings Wang Nan
2016-05-23 7:13 ` [PATCH v3 08/11] perf record: Introduce rec->overwrite_evlist for overwritable events Wang Nan
2016-05-23 7:13 ` [PATCH v3 09/11] perf record: Toggle overwrite ring buffer for reading Wang Nan
2016-05-23 7:13 ` [PATCH v3 10/11] perf tools: Don't warn about out of order event if write_backward is used Wang Nan
2016-05-23 7:13 ` [PATCH v3 11/11] perf tools: Check write_backward during evlist config Wang Nan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160523194842.GL8897@kernel.org \
--to=acme@kernel.org \
--cc=acme@redhat.com \
--cc=hekuang@huawei.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lizefan@huawei.com \
--cc=masami.hiramatsu.pt@hitachi.com \
--cc=namhyung@kernel.org \
--cc=pi3orama@163.com \
--cc=wangnan0@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.