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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox