[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160620203622.GB8411@kernel.org>
Date: Mon, 20 Jun 2016 17:36:22 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Wang Nan <wangnan0@...wei.com>
Cc: linux-kernel@...r.kernel.org, pi3orama@....com,
He Kuang <hekuang@...wei.com>, Jiri Olsa <jolsa@...nel.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Zefan Li <lizefan@...wei.com>
Subject: Re: [PATCH v8 2/8] perf evlist: Introduce aux evlist
Em Mon, Jun 20, 2016 at 10:47:19AM +0000, Wang Nan escreveu:
> An auxiliary evlist is created by perf_evlist__new_aux() using an
> existing evlist as its parent. An auxiliary evlist can have its own
> 'struct perf_mmap', but can't have any other data. User should use its
> parent instead when accessing other data.
>
> Auxiliary evlists are containers of 'struct perf_mmap'. It is introduced
> to allow its parent evlist to map different events into separated mmaps.
>
> Following commits create an auxiliary evlist for overwritable
> events, because overwritable events need a read only and backwards ring
> buffer, which is different from normal events.
>
> To achieve this goal, this patch carefully changes 'evlist' to
> 'evlist->parent' in all functions in the path 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.
>
> A 'evlist->parent' pointer is added to 'struct perf_evlist' and points to
> the evlist itself for normal evlists.
>
> Children of one evlist are linked into it so one can find all children
> from its parent.
> To avoid potential complexity, forbid creating aux evlist from another
> aux evlist.
>
> Improve perf_evlist__munmap_filtered(), so when recording, if an event
> is terminated, unmap mmaps, from parent and children.
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> Cc: He Kuang <hekuang@...wei.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Masami Hiramatsu <mhiramat@...nel.org>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Zefan Li <lizefan@...wei.com>
> Cc: pi3orama@....com
> ---
> tools/perf/util/evlist.c | 49 +++++++++++++++++++++++++++++++++++++-----------
> tools/perf/util/evlist.h | 12 ++++++++++++
> 2 files changed, 50 insertions(+), 11 deletions(-)
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 1b918aa..b03cb3c 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -41,10 +41,12 @@ void perf_evlist__init(struct perf_evlist *evlist, struct cpu_map *cpus,
> for (i = 0; i < PERF_EVLIST__HLIST_SIZE; ++i)
> INIT_HLIST_HEAD(&evlist->heads[i]);
> INIT_LIST_HEAD(&evlist->entries);
> + INIT_LIST_HEAD(&evlist->children);
> perf_evlist__set_maps(evlist, cpus, threads);
> fdarray__init(&evlist->pollfd, 64);
> evlist->workload.pid = -1;
> evlist->backward = false;
> + evlist->parent = evlist;
> }
>
> struct perf_evlist *perf_evlist__new(void)
> @@ -487,13 +489,17 @@ static void perf_evlist__munmap_filtered(struct fdarray *fda, int fd,
> void *arg __maybe_unused)
> {
> struct perf_evlist *evlist = container_of(fda, struct perf_evlist, pollfd);
> + struct perf_evlist *child;
>
> perf_evlist__mmap_put(evlist, fda->priv[fd].idx);
> + list_for_each_entry(child, &evlist->children, list)
> + perf_evlist__mmap_put(child, fda->priv[fd].idx);
> +
> }
>
> int perf_evlist__filter_pollfd(struct perf_evlist *evlist, short revents_and_mask)
> {
> - return fdarray__filter(&evlist->pollfd, revents_and_mask,
> + return fdarray__filter(&evlist->parent->pollfd, revents_and_mask,
> perf_evlist__munmap_filtered, NULL);
> }
>
> @@ -1012,7 +1018,7 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
> struct perf_evsel *evsel;
> int revent;
>
> - evlist__for_each(evlist, evsel) {
> + evlist__for_each(evlist->parent, evsel) {
> int fd;
>
> if (evsel->overwrite != (evlist->overwrite && evlist->backward))
> @@ -1044,16 +1050,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, revent) < 0) {
> + __perf_evlist__add_pollfd(evlist->parent, fd, idx, revent) < 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);
> }
> }
> @@ -1094,13 +1100,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,
> @@ -1239,8 +1245,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),
> };
> @@ -1248,7 +1254,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;
> @@ -1259,7 +1265,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)
> @@ -1916,3 +1922,24 @@ 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;
> +
> + if (perf_evlist__is_aux(parent)) {
> + pr_err("Internal error: create aux evlist from another aux evlist\n");
> + return NULL;
> + }
> +
> + evlist = zalloc(sizeof(*evlist));
> + if (!evlist)
> + return NULL;
> +
> + perf_evlist__init(evlist, parent->cpus, parent->threads);
> + evlist->parent = parent->parent;
> + INIT_LIST_HEAD(&evlist->list);
> + list_add(&evlist->list, &parent->children);
> +
> + return evlist;
> +}
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 68cb136..5b50692 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -37,6 +37,10 @@ struct perf_mmap {
>
> struct perf_evlist {
> struct list_head entries;
> + union {
> + struct list_head children;
> + struct list_head list;
This is something new, right, i.e. having a list of evlists from a
parent evlist, one where there can be at most one level, i.e. no
grandchildrens...
Is this strictly needed? I.e. the existing code calling
perf_evlist__munmap_filtered() has just the parent pointer and thus, to
not change it we need to have a way to go the children?
Also, can you se a parent with more than one child?
- Arnaldo
> + };
> struct hlist_head heads[PERF_EVLIST__HLIST_SIZE];
> int nr_entries;
> int nr_groups;
> @@ -60,8 +64,14 @@ struct perf_evlist {
> struct perf_evsel *selected;
> struct events_stats stats;
> struct perf_env *env;
> + struct perf_evlist *parent;
> };
>
> +static inline bool perf_evlist__is_aux(struct perf_evlist *evlist)
> +{
> + return evlist->parent != evlist;
> +}
> +
> struct perf_evsel_str_handler {
> const char *name;
> void *handler;
> @@ -70,6 +80,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