[PATCH] perf tools: Fix gaps propagating maps

[Adrian Hunter <adrian.hunter@intel.com>]

A perf_evsel is a selected event containing the perf_event_attr
that is passed to perf_event_open().  A perf_evlist is a collection
of perf_evsel's.  A perf_evlist also has lists of cpus and threads
(pids) on which to open the event.  These lists are called 'maps'
and this patch is about how those 'maps' are propagated from the
perf_evlist to the perf_evsels.

Originally perf_evsels did not have their own thread 'maps', and
their cpu 'maps' were only used for checking.  Then threads were
added by:

	578e91ec04d0 ("perf evlist: Propagate thread maps through the evlist")

And then 'perf record' was changed to open events using the 'maps'
from perf_evsel not perf_evlist anymore, by:

	d988d5ee6478 ("perf evlist: Open event on evsel cpus and threads")

That exposed situations where the 'maps' were not getting propagated
correctly, such as when perf_evsel are added to the perf_evlist later.
This patch ensures 'maps' get propagated in that case, and also if 'maps'
are subsequently changed or set to NULL by perf_evlist__set_maps()
and perf_evlist__create_syswide_maps().

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

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index d51a5200c8af..00128cf7655b 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -124,8 +124,49 @@ void perf_evlist__delete(struct perf_evlist *evlist)
 	free(evlist);
 }
 
+static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
+					  struct perf_evsel *evsel,
+					  bool propagate)
+{
+	if (propagate) {
+		/*
+		 * 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) &&
+		    evsel->cpus != evlist->cpus) {
+			cpu_map__put(evsel->cpus);
+			evsel->cpus = cpu_map__get(evlist->cpus);
+		}
+
+		if (!evsel->threads)
+			evsel->threads = thread_map__get(evlist->threads);
+	} else {
+		if (evsel->cpus == evlist->cpus) {
+			cpu_map__put(evsel->cpus);
+			evsel->cpus = NULL;
+		}
+
+		if (evsel->threads == evlist->threads) {
+			thread_map__put(evsel->threads);
+			evsel->threads = NULL;
+		}
+	}
+}
+
+static void perf_evlist__propagate_maps(struct perf_evlist *evlist,
+					bool propagate)
+{
+	struct perf_evsel *evsel;
+
+	evlist__for_each(evlist, evsel)
+		__perf_evlist__propagate_maps(evlist, evsel, propagate);
+}
+
 void perf_evlist__add(struct perf_evlist *evlist, struct perf_evsel *entry)
 {
+	__perf_evlist__propagate_maps(evlist, entry, true);
+
 	entry->evlist = evlist;
 	list_add_tail(&entry->node, &evlist->entries);
 	entry->idx = evlist->nr_entries;
@@ -140,6 +181,10 @@ void perf_evlist__splice_list_tail(struct perf_evlist *evlist,
 				   int nr_entries)
 {
 	bool set_id_pos = !evlist->nr_entries;
+	struct perf_evsel *evsel;
+
+	__evlist__for_each(list, evsel)
+		__perf_evlist__propagate_maps(evlist, evsel, true);
 
 	list_splice_tail(list, &evlist->entries);
 	evlist->nr_entries += nr_entries;
@@ -1103,34 +1148,11 @@ 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)
-{
-	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->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);
-
-		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)
 {
+	if (evlist->threads || evlist->cpus)
+		return -1;
+
 	evlist->threads = thread_map__new_str(target->pid, target->tid,
 					      target->uid);
 
@@ -1145,7 +1167,11 @@ 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);
+	evlist->has_user_cpus = !!target->cpu_list;
+
+	perf_evlist__propagate_maps(evlist, true);
+
+	return 0;
 
 out_delete_threads:
 	thread_map__put(evlist->threads);
@@ -1157,17 +1183,32 @@ 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);
+	/*
+	 * First 'un-propagate' the current maps which allows the propagation to
+	 * work correctly even when changing the maps or setting them to NULL.
+	 */
+	perf_evlist__propagate_maps(evlist, false);
 
-	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;
+	}
 
-	if (evlist->threads)
+	if (threads != evlist->threads) {
 		thread_map__put(evlist->threads);
+		evlist->threads = threads;
+	}
 
-	evlist->threads = threads;
+	perf_evlist__propagate_maps(evlist, true);
 
-	return perf_evlist__propagate_maps(evlist, false);
+	return 0;
 }
 
 int perf_evlist__apply_filters(struct perf_evlist *evlist, struct perf_evsel **err_evsel)
@@ -1387,6 +1428,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;
 
 	/*
@@ -1398,20 +1441,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;
 }
 
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