[PATCH V2 00/14] perf tools: Fix gaps propagating maps

[PATCH V2 00/14] perf tools: Fix gaps propagating maps

[Adrian Hunter]

Hi

Here is V2 of "Fix gaps propagating maps" that fixes some problems
revealed by to d988d5ee6478 ("perf evlist: Open event on evsel cpus and threads")

Changes in V2:
	Split into multiple patches
	Add evsel->own_cpus to identify evsels with their own cpu map
	And consequently don't need to unpropagate

	perf tools: Fix perf_evlist__splice_list_tail not setting evlist
		New patch

	perf tools: Fix task exit test setting maps
		New patch

	perf tools: Fix software clock events test setting maps
		New patch


Adrian Hunter (14):
      perf tools: Simplify perf_evlist__propagate_maps logic
      perf tools: Simplify perf_evlist__set_maps logic
      perf tools: Remove redundant validation from perf_evlist__propagate_maps
      perf tools: Add evlist->has_user_cpus
      perf tools: Fix perf_evlist__splice_list_tail not setting evlist
      perf tools: Fix missing thread_map__put in perf_evlist__propagate_maps
      perf tools: Add evsel->own_cpus
      perf tools: Make perf_evlist__set_maps() more resilient
      perf tools: Make perf_evlist__create_maps() use perf_evlist__set_maps()
      perf tools: Factor out a function to propagate maps for a single evsel
      perf tools: Fix perf_evlist__add() not propagating maps
      perf tools: Fix perf_evlist__create_syswide_maps() not propagating maps
      perf tools: Fix task exit test setting maps
      perf tools: Fix software clock events test setting maps

 tools/perf/tests/sw-clock.c    |  22 +++++--
 tools/perf/tests/task-exit.c   |  22 +++++--
 tools/perf/util/evlist.c       | 134 +++++++++++++++++++++++------------------
 tools/perf/util/evlist.h       |   4 +-
 tools/perf/util/evsel.c        |   1 +
 tools/perf/util/evsel.h        |   1 +
 tools/perf/util/parse-events.c |   7 +--
 7 files changed, 119 insertions(+), 72 deletions(-)


Regards
Adrian

[PATCH V2 01/14] perf tools: Simplify perf_evlist__propagate_maps logic

[Adrian Hunter]

If evsel->cpus is to be reassigned then the current value
must be "put", which works even if it is NULL.  Simplify
the current logic by moving the "put" next to the assignment.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index d51a5200c8af..95e07ea3904c 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1113,11 +1113,10 @@ static int perf_evlist__propagate_maps(struct perf_evlist *evlist,
 		 * We already have cpus for evsel (via PMU sysfs) so
 		 * keep it, if there's no target cpu list defined.
 		 */
-		if (evsel->cpus && has_user_cpus)
+		if (!evsel->cpus || has_user_cpus) {
 			cpu_map__put(evsel->cpus);
-
-		if (!evsel->cpus || has_user_cpus)
 			evsel->cpus = cpu_map__get(evlist->cpus);
+		}
 
 		evsel->threads = thread_map__get(evlist->threads);
 
-- 
1.9.1

[PATCH V2 02/14] perf tools: Simplify perf_evlist__set_maps logic

[Adrian Hunter]

Don't need to check for NULL when "putting" evlist->maps and
evlist->threads because the "put" functions already do that.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 95e07ea3904c..9cb9296cc4f8 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1156,14 +1156,10 @@ int perf_evlist__set_maps(struct perf_evlist *evlist,
 			  struct cpu_map *cpus,
 			  struct thread_map *threads)
 {
-	if (evlist->cpus)
-		cpu_map__put(evlist->cpus);
-
+	cpu_map__put(evlist->cpus);
 	evlist->cpus = cpus;
 
-	if (evlist->threads)
-		thread_map__put(evlist->threads);
-
+	thread_map__put(evlist->threads);
 	evlist->threads = threads;
 
 	return perf_evlist__propagate_maps(evlist, false);
-- 
1.9.1

[PATCH V2 03/14] perf tools: Remove redundant validation from perf_evlist__propagate_maps

[Adrian Hunter]

The validation checks that the values that were just assigned, got
assigned i.e. the error can't ever happen.  Subsequent patches will
call this code in places where errors are not being returned.
Changing those code paths to return this non-existent error is
counter-productive, so just remove it.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 9cb9296cc4f8..840d5210be72 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1103,8 +1103,8 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
 	return perf_evlist__mmap_ex(evlist, pages, overwrite, 0, false);
 }
 
-static int perf_evlist__propagate_maps(struct perf_evlist *evlist,
-				       bool has_user_cpus)
+static void perf_evlist__propagate_maps(struct perf_evlist *evlist,
+					bool has_user_cpus)
 {
 	struct perf_evsel *evsel;
 
@@ -1119,13 +1119,7 @@ static int perf_evlist__propagate_maps(struct perf_evlist *evlist,
 		}
 
 		evsel->threads = thread_map__get(evlist->threads);
-
-		if ((evlist->cpus && !evsel->cpus) ||
-		    (evlist->threads && !evsel->threads))
-			return -ENOMEM;
 	}
-
-	return 0;
 }
 
 int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
@@ -1144,7 +1138,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
 	if (evlist->cpus == NULL)
 		goto out_delete_threads;
 
-	return perf_evlist__propagate_maps(evlist, !!target->cpu_list);
+	perf_evlist__propagate_maps(evlist, !!target->cpu_list);
+
+	return 0;
 
 out_delete_threads:
 	thread_map__put(evlist->threads);
@@ -1162,7 +1158,9 @@ int perf_evlist__set_maps(struct perf_evlist *evlist,
 	thread_map__put(evlist->threads);
 	evlist->threads = threads;
 
-	return perf_evlist__propagate_maps(evlist, false);
+	perf_evlist__propagate_maps(evlist, false);
+
+	return 0;
 }
 
 int perf_evlist__apply_filters(struct perf_evlist *evlist, struct perf_evsel **err_evsel)
-- 
1.9.1

[PATCH V2 04/14] perf tools: Add evlist->has_user_cpus

[Adrian Hunter]

Subsequent patches will need to call perf_evlist__propagate_maps
without reference to a "target".  Add evlist->has_user_cpus to
record whether the user has specified which cpus to target
(and therefore whether that list of cpus should override the
default settings for a selected event i.e. the cpu maps should
be propagated)

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c | 11 ++++++-----
 tools/perf/util/evlist.h |  1 +
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 840d5210be72..8bedf5cf366e 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1103,8 +1103,7 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
 	return perf_evlist__mmap_ex(evlist, pages, overwrite, 0, false);
 }
 
-static void perf_evlist__propagate_maps(struct perf_evlist *evlist,
-					bool has_user_cpus)
+static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
 {
 	struct perf_evsel *evsel;
 
@@ -1113,7 +1112,7 @@ static void perf_evlist__propagate_maps(struct perf_evlist *evlist,
 		 * We already have cpus for evsel (via PMU sysfs) so
 		 * keep it, if there's no target cpu list defined.
 		 */
-		if (!evsel->cpus || has_user_cpus) {
+		if (!evsel->cpus || evlist->has_user_cpus) {
 			cpu_map__put(evsel->cpus);
 			evsel->cpus = cpu_map__get(evlist->cpus);
 		}
@@ -1138,7 +1137,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
 	if (evlist->cpus == NULL)
 		goto out_delete_threads;
 
-	perf_evlist__propagate_maps(evlist, !!target->cpu_list);
+	evlist->has_user_cpus = !!target->cpu_list;
+
+	perf_evlist__propagate_maps(evlist);
 
 	return 0;
 
@@ -1158,7 +1159,7 @@ int perf_evlist__set_maps(struct perf_evlist *evlist,
 	thread_map__put(evlist->threads);
 	evlist->threads = threads;
 
-	perf_evlist__propagate_maps(evlist, false);
+	perf_evlist__propagate_maps(evlist);
 
 	return 0;
 }
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index b39a6198f4ac..2dd5715dfef6 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -42,6 +42,7 @@ struct perf_evlist {
 	int		 nr_mmaps;
 	bool		 overwrite;
 	bool		 enabled;
+	bool		 has_user_cpus;
 	size_t		 mmap_len;
 	int		 id_pos;
 	int		 is_pos;
-- 
1.9.1

[PATCH V2 05/14] perf tools: Fix perf_evlist__splice_list_tail not setting evlist

[Adrian Hunter]

Commit d49e46950772 ("perf evsel: Add a backpointer to the evlist
a evsel is in") updated perf_evlist__add() but not
perf_evlist__splice_list_tail().

This illustrates that it is better if perf_evlist__splice_list_tail()
calls perf_evlist__add() instead of duplicating the logic, so do that.
This will also simplify a subsequent fix for propagating maps.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c       | 15 +++++++--------
 tools/perf/util/evlist.h       |  3 +--
 tools/perf/util/parse-events.c |  3 +--
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 8bedf5cf366e..018b488d4c75 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -136,15 +136,14 @@ void perf_evlist__add(struct perf_evlist *evlist, struct perf_evsel *entry)
 }
 
 void perf_evlist__splice_list_tail(struct perf_evlist *evlist,
-				   struct list_head *list,
-				   int nr_entries)
+				   struct list_head *list)
 {
-	bool set_id_pos = !evlist->nr_entries;
+	struct perf_evsel *evsel, *temp;
 
-	list_splice_tail(list, &evlist->entries);
-	evlist->nr_entries += nr_entries;
-	if (set_id_pos)
-		perf_evlist__set_id_pos(evlist);
+	__evlist__for_each_safe(list, temp, evsel) {
+		list_del_init(&evsel->node);
+		perf_evlist__add(evlist, evsel);
+	}
 }
 
 void __perf_evlist__set_leader(struct list_head *list)
@@ -210,7 +209,7 @@ static int perf_evlist__add_attrs(struct perf_evlist *evlist,
 		list_add_tail(&evsel->node, &head);
 	}
 
-	perf_evlist__splice_list_tail(evlist, &head, nr_attrs);
+	perf_evlist__splice_list_tail(evlist, &head);
 
 	return 0;
 
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 2dd5715dfef6..3c6ec625d70a 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -180,8 +180,7 @@ bool perf_evlist__valid_sample_id_all(struct perf_evlist *evlist);
 bool perf_evlist__valid_read_format(struct perf_evlist *evlist);
 
 void perf_evlist__splice_list_tail(struct perf_evlist *evlist,
-				   struct list_head *list,
-				   int nr_entries);
+				   struct list_head *list);
 
 static inline struct perf_evsel *perf_evlist__first(struct perf_evlist *evlist)
 {
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index d826e6f515db..7e8ae21906e2 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1140,10 +1140,9 @@ int parse_events(struct perf_evlist *evlist, const char *str,
 	ret = parse_events__scanner(str, &data, PE_START_EVENTS);
 	perf_pmu__parse_cleanup();
 	if (!ret) {
-		int entries = data.idx - evlist->nr_entries;
 		struct perf_evsel *last;
 
-		perf_evlist__splice_list_tail(evlist, &data.list, entries);
+		perf_evlist__splice_list_tail(evlist, &data.list);
 		evlist->nr_groups += data.nr_groups;
 		last = perf_evlist__last(evlist);
 		last->cmdline_group_boundary = true;
-- 
1.9.1

[PATCH V2 06/14] perf tools: Fix missing thread_map__put in perf_evlist__propagate_maps

[Adrian Hunter]

perf_evlist__propagate_maps() incorrectly assumes evsel->threads
is NULL before reassigning it, but it won't be NULL when
perf_evlist__set_maps() is used to set different (or NULL) maps.
Thus thread_map__put must be used, which works even if
evsel->threads is NULL.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 018b488d4c75..c959c42080e3 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1116,6 +1116,7 @@ static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
 			evsel->cpus = cpu_map__get(evlist->cpus);
 		}
 
+		thread_map__put(evsel->threads);
 		evsel->threads = thread_map__get(evlist->threads);
 	}
 }
-- 
1.9.1

[PATCH V2 07/14] perf tools: Add evsel->own_cpus

[Adrian Hunter]

perf_evlist__propagate_maps() cannot easily tell if an evsel
has its own cpu map.  To make that simpler, keep a copy of
the PMU cpu map and adjust the propagation logic accordingly.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c       | 5 ++++-
 tools/perf/util/evsel.c        | 1 +
 tools/perf/util/evsel.h        | 1 +
 tools/perf/util/parse-events.c | 4 ++--
 4 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index c959c42080e3..6764e0eff849 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1111,9 +1111,12 @@ static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
 		 * We already have cpus for evsel (via PMU sysfs) so
 		 * keep it, if there's no target cpu list defined.
 		 */
-		if (!evsel->cpus || evlist->has_user_cpus) {
+		if (!evsel->own_cpus || evlist->has_user_cpus) {
 			cpu_map__put(evsel->cpus);
 			evsel->cpus = cpu_map__get(evlist->cpus);
+		} else if (evsel->cpus != evsel->own_cpus) {
+			cpu_map__put(evsel->cpus);
+			evsel->cpus = cpu_map__get(evsel->own_cpus);
 		}
 
 		thread_map__put(evsel->threads);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index c53f79123b37..5410483d5219 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1033,6 +1033,7 @@ void perf_evsel__exit(struct perf_evsel *evsel)
 	perf_evsel__free_config_terms(evsel);
 	close_cgroup(evsel->cgrp);
 	cpu_map__put(evsel->cpus);
+	cpu_map__put(evsel->own_cpus);
 	thread_map__put(evsel->threads);
 	zfree(&evsel->group_name);
 	zfree(&evsel->name);
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 298e6bbca200..ef8925f7211a 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -98,6 +98,7 @@ struct perf_evsel {
 	struct cgroup_sel	*cgrp;
 	void			*handler;
 	struct cpu_map		*cpus;
+	struct cpu_map		*own_cpus;
 	struct thread_map	*threads;
 	unsigned int		sample_size;
 	int			id_pos;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 7e8ae21906e2..21ed6ee63da9 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -287,8 +287,8 @@ __add_event(struct list_head *list, int *idx,
 	if (!evsel)
 		return NULL;
 
-	if (cpus)
-		evsel->cpus = cpu_map__get(cpus);
+	evsel->cpus     = cpu_map__get(cpus);
+	evsel->own_cpus = cpu_map__get(cpus);
 
 	if (name)
 		evsel->name = strdup(name);
-- 
1.9.1

[PATCH V2 08/14] perf tools: Make perf_evlist__set_maps() more resilient

[Adrian Hunter]

Make perf_evlist__set_maps() more resilient by allowing for the
possibility that one or another of the maps isn't being changed
and therefore should not be "put".

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 6764e0eff849..606fc6e63ec0 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1156,11 +1156,22 @@ int perf_evlist__set_maps(struct perf_evlist *evlist,
 			  struct cpu_map *cpus,
 			  struct thread_map *threads)
 {
-	cpu_map__put(evlist->cpus);
-	evlist->cpus = cpus;
+	/*
+	 * Allow for the possibility that one or another of the maps isn't being
+	 * changed i.e. don't put it.  Note we are assuming the maps that are
+	 * being applied are brand new and evlist is taking ownership of the
+	 * original reference count of 1.  If that is not the case it is up to
+	 * the caller to increase the reference count.
+	 */
+	if (cpus != evlist->cpus) {
+		cpu_map__put(evlist->cpus);
+		evlist->cpus = cpus;
+	}
 
-	thread_map__put(evlist->threads);
-	evlist->threads = threads;
+	if (threads != evlist->threads) {
+		thread_map__put(evlist->threads);
+		evlist->threads = threads;
+	}
 
 	perf_evlist__propagate_maps(evlist);
 
-- 
1.9.1

[PATCH V2 09/14] perf tools: Make perf_evlist__create_maps() use perf_evlist__set_maps()

[Adrian Hunter]

Since there is a function to set maps, perf_evlist__create_maps()
should use it.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 606fc6e63ec0..093ea73d863c 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1126,29 +1126,28 @@ static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
 
 int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
 {
-	evlist->threads = thread_map__new_str(target->pid, target->tid,
-					      target->uid);
+	struct cpu_map *cpus;
+	struct thread_map *threads;
 
-	if (evlist->threads == NULL)
+	threads = thread_map__new_str(target->pid, target->tid, target->uid);
+
+	if (!threads)
 		return -1;
 
 	if (target__uses_dummy_map(target))
-		evlist->cpus = cpu_map__dummy_new();
+		cpus = cpu_map__dummy_new();
 	else
-		evlist->cpus = cpu_map__new(target->cpu_list);
+		cpus = cpu_map__new(target->cpu_list);
 
-	if (evlist->cpus == NULL)
+	if (!cpus)
 		goto out_delete_threads;
 
 	evlist->has_user_cpus = !!target->cpu_list;
 
-	perf_evlist__propagate_maps(evlist);
-
-	return 0;
+	return perf_evlist__set_maps(evlist, cpus, threads);
 
 out_delete_threads:
-	thread_map__put(evlist->threads);
-	evlist->threads = NULL;
+	thread_map__put(threads);
 	return -1;
 }
 
-- 
1.9.1

[PATCH V2 10/14] perf tools: Factor out a function to propagate maps for a single evsel

[Adrian Hunter]

Subsequent fixes will need a function that just propagates maps
for a single evsel so factor it out.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 093ea73d863c..cddd5f25aa23 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1102,26 +1102,31 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
 	return perf_evlist__mmap_ex(evlist, pages, overwrite, 0, false);
 }
 
+static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
+					  struct perf_evsel *evsel)
+{
+	/*
+	 * We already have cpus for evsel (via PMU sysfs) so
+	 * keep it, if there's no target cpu list defined.
+	 */
+	if (!evsel->own_cpus || evlist->has_user_cpus) {
+		cpu_map__put(evsel->cpus);
+		evsel->cpus = cpu_map__get(evlist->cpus);
+	} else if (evsel->cpus != evsel->own_cpus) {
+		cpu_map__put(evsel->cpus);
+		evsel->cpus = cpu_map__get(evsel->own_cpus);
+	}
+
+	thread_map__put(evsel->threads);
+	evsel->threads = thread_map__get(evlist->threads);
+}
+
 static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
 {
 	struct perf_evsel *evsel;
 
-	evlist__for_each(evlist, evsel) {
-		/*
-		 * We already have cpus for evsel (via PMU sysfs) so
-		 * keep it, if there's no target cpu list defined.
-		 */
-		if (!evsel->own_cpus || evlist->has_user_cpus) {
-			cpu_map__put(evsel->cpus);
-			evsel->cpus = cpu_map__get(evlist->cpus);
-		} else if (evsel->cpus != evsel->own_cpus) {
-			cpu_map__put(evsel->cpus);
-			evsel->cpus = cpu_map__get(evsel->own_cpus);
-		}
-
-		thread_map__put(evsel->threads);
-		evsel->threads = thread_map__get(evlist->threads);
-	}
+	evlist__for_each(evlist, evsel)
+		__perf_evlist__propagate_maps(evlist, evsel);
 }
 
 int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
-- 
1.9.1

[PATCH V2 11/14] perf tools: Fix perf_evlist__add() not propagating maps

[Adrian Hunter]

If evsels are added after maps are created, then they won't
have any maps propagated to them.  Fix that.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c | 56 +++++++++++++++++++++++++-----------------------
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index cddd5f25aa23..e0a32e63825d 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -124,6 +124,33 @@ void perf_evlist__delete(struct perf_evlist *evlist)
 	free(evlist);
 }
 
+static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
+					  struct perf_evsel *evsel)
+{
+	/*
+	 * We already have cpus for evsel (via PMU sysfs) so
+	 * keep it, if there's no target cpu list defined.
+	 */
+	if (!evsel->own_cpus || evlist->has_user_cpus) {
+		cpu_map__put(evsel->cpus);
+		evsel->cpus = cpu_map__get(evlist->cpus);
+	} else if (evsel->cpus != evsel->own_cpus) {
+		cpu_map__put(evsel->cpus);
+		evsel->cpus = cpu_map__get(evsel->own_cpus);
+	}
+
+	thread_map__put(evsel->threads);
+	evsel->threads = thread_map__get(evlist->threads);
+}
+
+static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel;
+
+	evlist__for_each(evlist, evsel)
+		__perf_evlist__propagate_maps(evlist, evsel);
+}
+
 void perf_evlist__add(struct perf_evlist *evlist, struct perf_evsel *entry)
 {
 	entry->evlist = evlist;
@@ -133,6 +160,8 @@ void perf_evlist__add(struct perf_evlist *evlist, struct perf_evsel *entry)
 
 	if (!evlist->nr_entries++)
 		perf_evlist__set_id_pos(evlist);
+
+	__perf_evlist__propagate_maps(evlist, entry);
 }
 
 void perf_evlist__splice_list_tail(struct perf_evlist *evlist,
@@ -1102,33 +1131,6 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
 	return perf_evlist__mmap_ex(evlist, pages, overwrite, 0, false);
 }
 
-static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
-					  struct perf_evsel *evsel)
-{
-	/*
-	 * We already have cpus for evsel (via PMU sysfs) so
-	 * keep it, if there's no target cpu list defined.
-	 */
-	if (!evsel->own_cpus || evlist->has_user_cpus) {
-		cpu_map__put(evsel->cpus);
-		evsel->cpus = cpu_map__get(evlist->cpus);
-	} else if (evsel->cpus != evsel->own_cpus) {
-		cpu_map__put(evsel->cpus);
-		evsel->cpus = cpu_map__get(evsel->own_cpus);
-	}
-
-	thread_map__put(evsel->threads);
-	evsel->threads = thread_map__get(evlist->threads);
-}
-
-static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
-{
-	struct perf_evsel *evsel;
-
-	evlist__for_each(evlist, evsel)
-		__perf_evlist__propagate_maps(evlist, evsel);
-}
-
 int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
 {
 	struct cpu_map *cpus;
-- 
1.9.1

[PATCH V2 12/14] perf tools: Fix perf_evlist__create_syswide_maps() not propagating maps

[Adrian Hunter]

Fix it by making it call perf_evlist__set_maps() instead of
setting the maps itself.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index e0a32e63825d..fc2ac60d92f0 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1401,6 +1401,8 @@ void perf_evlist__close(struct perf_evlist *evlist)
 
 static int perf_evlist__create_syswide_maps(struct perf_evlist *evlist)
 {
+	struct cpu_map	  *cpus;
+	struct thread_map *threads;
 	int err = -ENOMEM;
 
 	/*
@@ -1412,20 +1414,22 @@ static int perf_evlist__create_syswide_maps(struct perf_evlist *evlist)
 	 * error, and we may not want to do that fallback to a
 	 * default cpu identity map :-\
 	 */
-	evlist->cpus = cpu_map__new(NULL);
-	if (evlist->cpus == NULL)
+	cpus = cpu_map__new(NULL);
+	if (!cpus)
 		goto out;
 
-	evlist->threads = thread_map__new_dummy();
-	if (evlist->threads == NULL)
-		goto out_free_cpus;
+	threads = thread_map__new_dummy();
+	if (!threads)
+		goto out_put;
 
-	err = 0;
+	err = perf_evlist__set_maps(evlist, cpus, threads);
+	if (err)
+		goto out_put;
 out:
 	return err;
-out_free_cpus:
-	cpu_map__put(evlist->cpus);
-	evlist->cpus = NULL;
+out_put:
+	cpu_map__put(cpus);
+	thread_map__put(threads);
 	goto out;
 }
 
-- 
1.9.1

[PATCH V2 13/14] perf tools: Fix task exit test setting maps

[Adrian Hunter]

The test titled "Test number of exit event of a simple workload"
was setting cpu/thread maps directly.  Make it use the proper
function perf_evlist__set_maps() especially now that it also
propagates the maps.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/task-exit.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/task-exit.c b/tools/perf/tests/task-exit.c
index 3a8fedef83bc..bd73003e012e 100644
--- a/tools/perf/tests/task-exit.c
+++ b/tools/perf/tests/task-exit.c
@@ -43,6 +43,8 @@ int test__task_exit(void)
 	};
 	const char *argv[] = { "true", NULL };
 	char sbuf[STRERR_BUFSIZE];
+	struct cpu_map *cpus;
+	struct thread_map *threads;
 
 	signal(SIGCHLD, sig_handler);
 
@@ -58,14 +60,23 @@ int test__task_exit(void)
 	 * perf_evlist__prepare_workload we'll fill in the only thread
 	 * we're monitoring, the one forked there.
 	 */
-	evlist->cpus = cpu_map__dummy_new();
-	evlist->threads = thread_map__new_by_tid(-1);
-	if (!evlist->cpus || !evlist->threads) {
+	cpus = cpu_map__dummy_new();
+	threads = thread_map__new_by_tid(-1);
+	if (!cpus || !threads) {
 		err = -ENOMEM;
 		pr_debug("Not enough memory to create thread/cpu maps\n");
-		goto out_delete_evlist;
+		goto out_free_maps;
+	}
+
+	err = perf_evlist__set_maps(evlist, cpus, threads);
+	if (err) {
+		pr_debug("Failed to set thread/cpu maps\n");
+		goto out_free_maps;
 	}
 
+	cpus	= NULL;
+	threads = NULL;
+
 	err = perf_evlist__prepare_workload(evlist, &target, argv, false,
 					    workload_exec_failed_signal);
 	if (err < 0) {
@@ -114,6 +125,9 @@ retry:
 		err = -1;
 	}
 
+out_free_maps:
+	cpu_map__put(cpus);
+	thread_map__put(threads);
 out_delete_evlist:
 	perf_evlist__delete(evlist);
 	return err;
-- 
1.9.1

[PATCH V2 14/14] perf tools: Fix software clock events test setting maps

[Adrian Hunter]

The test titled "Test software clock events have valid period values"
was setting cpu/thread maps directly.  Make it use the proper
function perf_evlist__set_maps() especially now that it also
propagates the maps.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/sw-clock.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/sw-clock.c b/tools/perf/tests/sw-clock.c
index 1aa21c90731b..f17ed784d756 100644
--- a/tools/perf/tests/sw-clock.c
+++ b/tools/perf/tests/sw-clock.c
@@ -34,6 +34,8 @@ static int __test__sw_clock_freq(enum perf_sw_ids clock_id)
 		.disabled = 1,
 		.freq = 1,
 	};
+	struct cpu_map *cpus;
+	struct thread_map *threads;
 
 	attr.sample_freq = 500;
 
@@ -50,14 +52,23 @@ static int __test__sw_clock_freq(enum perf_sw_ids clock_id)
 	}
 	perf_evlist__add(evlist, evsel);
 
-	evlist->cpus = cpu_map__dummy_new();
-	evlist->threads = thread_map__new_by_tid(getpid());
-	if (!evlist->cpus || !evlist->threads) {
+	cpus = cpu_map__dummy_new();
+	threads = thread_map__new_by_tid(getpid());
+	if (!cpus || !threads) {
 		err = -ENOMEM;
 		pr_debug("Not enough memory to create thread/cpu maps\n");
-		goto out_delete_evlist;
+		goto out_free_maps;
+	}
+
+	err = perf_evlist__set_maps(evlist, cpus, threads);
+	if (err) {
+		pr_debug("Failed to set thread/cpu maps\n");
+		goto out_free_maps;
 	}
 
+	cpus	= NULL;
+	threads = NULL;
+
 	if (perf_evlist__open(evlist)) {
 		const char *knob = "/proc/sys/kernel/perf_event_max_sample_rate";
 
@@ -107,6 +118,9 @@ next_event:
 		err = -1;
 	}
 
+out_free_maps:
+	cpu_map__put(cpus);
+	thread_map__put(threads);
 out_delete_evlist:
 	perf_evlist__delete(evlist);
 	return err;
-- 
1.9.1

Re: [PATCH V2 07/14] perf tools: Add evsel->own_cpus

[Jiri Olsa]

On Mon, Sep 07, 2015 at 05:27:49PM +0300, Adrian Hunter wrote:
> perf_evlist__propagate_maps() cannot easily tell if an evsel
> has its own cpu map.  To make that simpler, keep a copy of
> the PMU cpu map and adjust the propagation logic accordingly.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/util/evlist.c       | 5 ++++-
>  tools/perf/util/evsel.c        | 1 +
>  tools/perf/util/evsel.h        | 1 +
>  tools/perf/util/parse-events.c | 4 ++--
>  4 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index c959c42080e3..6764e0eff849 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1111,9 +1111,12 @@ static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
>  		 * We already have cpus for evsel (via PMU sysfs) so
>  		 * keep it, if there's no target cpu list defined.
>  		 */
> -		if (!evsel->cpus || evlist->has_user_cpus) {
> +		if (!evsel->own_cpus || evlist->has_user_cpus) {
>  			cpu_map__put(evsel->cpus);
>  			evsel->cpus = cpu_map__get(evlist->cpus);
> +		} else if (evsel->cpus != evsel->own_cpus) {
> +			cpu_map__put(evsel->cpus);
> +			evsel->cpus = cpu_map__get(evsel->own_cpus);

hum, so (evsel->cpus != evsel->own_cpus) could happen only when:
  - evsel->own_cpus != NULL
  - we overloaded evsel->cpus with evlist->cpus via perf_evlist__propagate_maps
  - we changed evlist->has_user_cpus = false
  - we recall perf_evlist__propagate_maps

I'm missing usecase for that, or something else ;-)

thanks,
jirka

Re: [PATCH V2 03/14] perf tools: Remove redundant validation from perf_evlist__propagate_maps

[Jiri Olsa]

On Mon, Sep 07, 2015 at 05:27:45PM +0300, Adrian Hunter wrote:
> The validation checks that the values that were just assigned, got
> assigned i.e. the error can't ever happen.  Subsequent patches will
> call this code in places where errors are not being returned.
> Changing those code paths to return this non-existent error is
> counter-productive, so just remove it.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/util/evlist.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 9cb9296cc4f8..840d5210be72 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1103,8 +1103,8 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
>  	return perf_evlist__mmap_ex(evlist, pages, overwrite, 0, false);
>  }
>  
> -static int perf_evlist__propagate_maps(struct perf_evlist *evlist,
> -				       bool has_user_cpus)
> +static void perf_evlist__propagate_maps(struct perf_evlist *evlist,
> +					bool has_user_cpus)
>  {
>  	struct perf_evsel *evsel;
>  
> @@ -1119,13 +1119,7 @@ static int perf_evlist__propagate_maps(struct perf_evlist *evlist,
>  		}
>  
>  		evsel->threads = thread_map__get(evlist->threads);
> -
> -		if ((evlist->cpus && !evsel->cpus) ||
> -		    (evlist->threads && !evsel->threads))
> -			return -ENOMEM;
>  	}
> -
> -	return 0;
>  }
>  
>  int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
> @@ -1144,7 +1138,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
>  	if (evlist->cpus == NULL)
>  		goto out_delete_threads;
>  
> -	return perf_evlist__propagate_maps(evlist, !!target->cpu_list);
> +	perf_evlist__propagate_maps(evlist, !!target->cpu_list);
> +
> +	return 0;
>  
>  out_delete_threads:
>  	thread_map__put(evlist->threads);
> @@ -1162,7 +1158,9 @@ int perf_evlist__set_maps(struct perf_evlist *evlist,
>  	thread_map__put(evlist->threads);
>  	evlist->threads = threads;
>  
> -	return perf_evlist__propagate_maps(evlist, false);
> +	perf_evlist__propagate_maps(evlist, false);
> +
> +	return 0;

perf_evlist__set_maps could now also return void

jirka

Re: [PATCH V2 00/14] perf tools: Fix gaps propagating maps

[Jiri Olsa]

On Mon, Sep 07, 2015 at 05:27:42PM +0300, Adrian Hunter wrote:
> Hi
> 
> Here is V2 of "Fix gaps propagating maps" that fixes some problems
> revealed by to d988d5ee6478 ("perf evlist: Open event on evsel cpus and threads")
> 
> Changes in V2:
> 	Split into multiple patches
> 	Add evsel->own_cpus to identify evsels with their own cpu map
> 	And consequently don't need to unpropagate
> 
> 	perf tools: Fix perf_evlist__splice_list_tail not setting evlist
> 		New patch
> 
> 	perf tools: Fix task exit test setting maps
> 		New patch
> 
> 	perf tools: Fix software clock events test setting maps
> 		New patch
> 
> 
> Adrian Hunter (14):
>       perf tools: Simplify perf_evlist__propagate_maps logic
>       perf tools: Simplify perf_evlist__set_maps logic
>       perf tools: Remove redundant validation from perf_evlist__propagate_maps
>       perf tools: Add evlist->has_user_cpus
>       perf tools: Fix perf_evlist__splice_list_tail not setting evlist
>       perf tools: Fix missing thread_map__put in perf_evlist__propagate_maps
>       perf tools: Add evsel->own_cpus
>       perf tools: Make perf_evlist__set_maps() more resilient
>       perf tools: Make perf_evlist__create_maps() use perf_evlist__set_maps()
>       perf tools: Factor out a function to propagate maps for a single evsel
>       perf tools: Fix perf_evlist__add() not propagating maps
>       perf tools: Fix perf_evlist__create_syswide_maps() not propagating maps
>       perf tools: Fix task exit test setting maps
>       perf tools: Fix software clock events test setting maps

other than my 2 comments, I'm ok with this patchset, nice! ;-)

thanks,
jirka

Re: [PATCH V2 07/14] perf tools: Add evsel->own_cpus

[Adrian Hunter]

On 08/09/15 09:45, Jiri Olsa wrote:
> On Mon, Sep 07, 2015 at 05:27:49PM +0300, Adrian Hunter wrote:
>> perf_evlist__propagate_maps() cannot easily tell if an evsel
>> has its own cpu map.  To make that simpler, keep a copy of
>> the PMU cpu map and adjust the propagation logic accordingly.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  tools/perf/util/evlist.c       | 5 ++++-
>>  tools/perf/util/evsel.c        | 1 +
>>  tools/perf/util/evsel.h        | 1 +
>>  tools/perf/util/parse-events.c | 4 ++--
>>  4 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>> index c959c42080e3..6764e0eff849 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -1111,9 +1111,12 @@ static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
>>  		 * We already have cpus for evsel (via PMU sysfs) so
>>  		 * keep it, if there's no target cpu list defined.
>>  		 */
>> -		if (!evsel->cpus || evlist->has_user_cpus) {
>> +		if (!evsel->own_cpus || evlist->has_user_cpus) {
>>  			cpu_map__put(evsel->cpus);
>>  			evsel->cpus = cpu_map__get(evlist->cpus);
>> +		} else if (evsel->cpus != evsel->own_cpus) {
>> +			cpu_map__put(evsel->cpus);
>> +			evsel->cpus = cpu_map__get(evsel->own_cpus);
> 
> hum, so (evsel->cpus != evsel->own_cpus) could happen only when:
>   - evsel->own_cpus != NULL
>   - we overloaded evsel->cpus with evlist->cpus via perf_evlist__propagate_maps
>   - we changed evlist->has_user_cpus = false
>   - we recall perf_evlist__propagate_maps
> 
> I'm missing usecase for that, or something else ;-)

That's true but the idea is to establish rules (invariants) that are always
true.  Like an evsel either has its own cpu map or the same as the evlist.

Re: [PATCH V2 03/14] perf tools: Remove redundant validation from perf_evlist__propagate_maps

[Adrian Hunter]

On 08/09/15 09:53, Jiri Olsa wrote:
> On Mon, Sep 07, 2015 at 05:27:45PM +0300, Adrian Hunter wrote:
>> The validation checks that the values that were just assigned, got
>> assigned i.e. the error can't ever happen.  Subsequent patches will
>> call this code in places where errors are not being returned.
>> Changing those code paths to return this non-existent error is
>> counter-productive, so just remove it.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  tools/perf/util/evlist.c | 18 ++++++++----------
>>  1 file changed, 8 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>> index 9cb9296cc4f8..840d5210be72 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -1103,8 +1103,8 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
>>  	return perf_evlist__mmap_ex(evlist, pages, overwrite, 0, false);
>>  }
>>  
>> -static int perf_evlist__propagate_maps(struct perf_evlist *evlist,
>> -				       bool has_user_cpus)
>> +static void perf_evlist__propagate_maps(struct perf_evlist *evlist,
>> +					bool has_user_cpus)
>>  {
>>  	struct perf_evsel *evsel;
>>  
>> @@ -1119,13 +1119,7 @@ static int perf_evlist__propagate_maps(struct perf_evlist *evlist,
>>  		}
>>  
>>  		evsel->threads = thread_map__get(evlist->threads);
>> -
>> -		if ((evlist->cpus && !evsel->cpus) ||
>> -		    (evlist->threads && !evsel->threads))
>> -			return -ENOMEM;
>>  	}
>> -
>> -	return 0;
>>  }
>>  
>>  int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
>> @@ -1144,7 +1138,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
>>  	if (evlist->cpus == NULL)
>>  		goto out_delete_threads;
>>  
>> -	return perf_evlist__propagate_maps(evlist, !!target->cpu_list);
>> +	perf_evlist__propagate_maps(evlist, !!target->cpu_list);
>> +
>> +	return 0;
>>  
>>  out_delete_threads:
>>  	thread_map__put(evlist->threads);
>> @@ -1162,7 +1158,9 @@ int perf_evlist__set_maps(struct perf_evlist *evlist,
>>  	thread_map__put(evlist->threads);
>>  	evlist->threads = threads;
>>  
>> -	return perf_evlist__propagate_maps(evlist, false);
>> +	perf_evlist__propagate_maps(evlist, false);
>> +
>> +	return 0;
> 
> perf_evlist__set_maps could now also return void

And most of the callers aren't checking the return value either, so yes that
needs to be tidied up.  I will make a V3

Re: [PATCH V2 07/14] perf tools: Add evsel->own_cpus

[Jiri Olsa]

On Tue, Sep 08, 2015 at 10:09:10AM +0300, Adrian Hunter wrote:
> On 08/09/15 09:45, Jiri Olsa wrote:
> > On Mon, Sep 07, 2015 at 05:27:49PM +0300, Adrian Hunter wrote:
> >> perf_evlist__propagate_maps() cannot easily tell if an evsel
> >> has its own cpu map.  To make that simpler, keep a copy of
> >> the PMU cpu map and adjust the propagation logic accordingly.
> >>
> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >> ---
> >>  tools/perf/util/evlist.c       | 5 ++++-
> >>  tools/perf/util/evsel.c        | 1 +
> >>  tools/perf/util/evsel.h        | 1 +
> >>  tools/perf/util/parse-events.c | 4 ++--
> >>  4 files changed, 8 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> >> index c959c42080e3..6764e0eff849 100644
> >> --- a/tools/perf/util/evlist.c
> >> +++ b/tools/perf/util/evlist.c
> >> @@ -1111,9 +1111,12 @@ static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
> >>  		 * We already have cpus for evsel (via PMU sysfs) so
> >>  		 * keep it, if there's no target cpu list defined.
> >>  		 */
> >> -		if (!evsel->cpus || evlist->has_user_cpus) {
> >> +		if (!evsel->own_cpus || evlist->has_user_cpus) {
> >>  			cpu_map__put(evsel->cpus);
> >>  			evsel->cpus = cpu_map__get(evlist->cpus);
> >> +		} else if (evsel->cpus != evsel->own_cpus) {
> >> +			cpu_map__put(evsel->cpus);
> >> +			evsel->cpus = cpu_map__get(evsel->own_cpus);
> > 
> > hum, so (evsel->cpus != evsel->own_cpus) could happen only when:
> >   - evsel->own_cpus != NULL
> >   - we overloaded evsel->cpus with evlist->cpus via perf_evlist__propagate_maps
> >   - we changed evlist->has_user_cpus = false
> >   - we recall perf_evlist__propagate_maps
> > 
> > I'm missing usecase for that, or something else ;-)
> 
> That's true but the idea is to establish rules (invariants) that are always
> true.  Like an evsel either has its own cpu map or the same as the evlist.

ook, jirka

Re: [PATCH V2 07/14] perf tools: Add evsel->own_cpus

[Arnaldo Carvalho de Melo]

Em Tue, Sep 08, 2015 at 09:38:54AM +0200, Jiri Olsa escreveu:
> On Tue, Sep 08, 2015 at 10:09:10AM +0300, Adrian Hunter wrote:
> > On 08/09/15 09:45, Jiri Olsa wrote:
> > > On Mon, Sep 07, 2015 at 05:27:49PM +0300, Adrian Hunter wrote:
> > >> +++ b/tools/perf/util/evlist.c
> > >> @@ -1111,9 +1111,12 @@ static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
> > >>  		 * We already have cpus for evsel (via PMU sysfs) so
> > >>  		 * keep it, if there's no target cpu list defined.
> > >>  		 */
> > >> -		if (!evsel->cpus || evlist->has_user_cpus) {
> > >> +		if (!evsel->own_cpus || evlist->has_user_cpus) {
> > >>  			cpu_map__put(evsel->cpus);
> > >>  			evsel->cpus = cpu_map__get(evlist->cpus);
> > >> +		} else if (evsel->cpus != evsel->own_cpus) {
> > >> +			cpu_map__put(evsel->cpus);
> > >> +			evsel->cpus = cpu_map__get(evsel->own_cpus);
> > > 
> > > hum, so (evsel->cpus != evsel->own_cpus) could happen only when:
> > >   - evsel->own_cpus != NULL
> > >   - we overloaded evsel->cpus with evlist->cpus via perf_evlist__propagate_maps
> > >   - we changed evlist->has_user_cpus = false
> > >   - we recall perf_evlist__propagate_maps

> > > I'm missing usecase for that, or something else ;-)

> > That's true but the idea is to establish rules (invariants) that are always
> > true.  Like an evsel either has its own cpu map or the same as the evlist.
 
> ook, jirka

Yup, I haven't even looked at the code that much, but at least Adrian's
explanation matches my expectations 8-)

- Arnaldo