[PATCH v2 2/2] libperf: evlist: Keep evsel idx contiguous on removal

[James Clark <james.clark@linaro.org>]

The only two places where evsels are removed end up fixing up the index.
Move it into libperf so that it's always done.

Signed-off-by: James Clark <james.clark@linaro.org>
---
 tools/lib/perf/evlist.c            | 17 +++++++------
 tools/lib/perf/tests/test-evlist.c | 41 ++++++++++++++++++++++++++++++
 tools/perf/builtin-record.c        | 18 +++----------
 3 files changed, 53 insertions(+), 23 deletions(-)

diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index 83c43dc13313..ffa1a28fb14f 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -52,15 +52,8 @@ static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
 		 * Empty cpu lists would eventually get opened as "any" so remove
 		 * genuinely empty ones before they're opened in the wrong place.
 		 */
-		if (perf_cpu_map__is_empty(evsel->cpus)) {
-			struct perf_evsel *next = perf_evlist__next(evlist, evsel);
-
+		if (perf_cpu_map__is_empty(evsel->cpus))
 			perf_evlist__remove(evlist, evsel);
-			/* Keep idx contiguous */
-			if (next)
-				list_for_each_entry_from(next, &evlist->entries, node)
-					next->idx--;
-		}
 	} else if (!evsel->own_cpus || evlist->has_user_cpus ||
 		(!evsel->requires_cpu && perf_cpu_map__has_any_cpu(evlist->user_requested_cpus))) {
 		/*
@@ -116,8 +109,16 @@ void perf_evlist__add(struct perf_evlist *evlist,
 void perf_evlist__remove(struct perf_evlist *evlist,
 			 struct perf_evsel *evsel)
 {
+	struct perf_evsel *next = perf_evlist__next(evlist, evsel);
+
 	list_del_init(&evsel->node);
 	evlist->nr_entries -= 1;
+
+	/* Keep idx contiguous */
+	if (!next)
+		return;
+	list_for_each_entry_from(next, &evlist->entries, node)
+		next->idx--;
 }
 
 struct perf_evlist *perf_evlist__new(void)
diff --git a/tools/lib/perf/tests/test-evlist.c b/tools/lib/perf/tests/test-evlist.c
index 10f70cb41ff1..06153820f408 100644
--- a/tools/lib/perf/tests/test-evlist.c
+++ b/tools/lib/perf/tests/test-evlist.c
@@ -571,6 +571,46 @@ static int test_stat_multiplexing(void)
 	return 0;
 }
 
+static int test_list_remove(void)
+{
+	struct perf_evlist *evlist;
+	struct perf_evsel *evsel, *evsel0, *evsel1, *evsel2;
+	struct perf_event_attr attr = {};
+	int idx = 0;
+
+	evlist = perf_evlist__new();
+	__T("failed to create evlist", evlist);
+
+	evsel0 = perf_evsel__new(&attr);
+	__T("failed to create evsel", evsel0);
+	evsel1 = perf_evsel__new(&attr);
+	__T("failed to create evsel", evsel1);
+	evsel2 = perf_evsel__new(&attr);
+	__T("failed to create evsel", evsel2);
+
+	perf_evlist__add(evlist, evsel0);
+	perf_evlist__add(evlist, evsel1);
+	perf_evlist__add(evlist, evsel2);
+
+	idx = 0;
+	perf_evlist__for_each_evsel(evlist, evsel)
+		__T("idx isn't contiguous", evsel->idx == idx++);
+
+	/* Test removing middle */
+	perf_evlist__remove(evlist, evsel1);
+	idx = 0;
+	perf_evlist__for_each_evsel(evlist, evsel)
+		__T("idx isn't contiguous", evsel->idx == idx++);
+
+	/* Test removing end */
+	perf_evlist__remove(evlist, evsel2);
+	idx = 0;
+	perf_evlist__for_each_evsel(evlist, evsel)
+		__T("idx isn't contiguous", evsel->idx == idx++);
+
+	return 0;
+}
+
 int test_evlist(int argc, char **argv)
 {
 	__T_START;
@@ -583,6 +623,7 @@ int test_evlist(int argc, char **argv)
 	test_mmap_thread();
 	test_mmap_cpus();
 	test_stat_multiplexing();
+	test_list_remove();
 
 	__T_END;
 	return tests_failed == 0 ? 0 : -1;
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 7e99743f7e42..2ebbb0fd0064 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1359,14 +1359,13 @@ static int record__mmap(struct record *rec)
 static int record__open(struct record *rec)
 {
 	char msg[BUFSIZ];
-	struct evsel *pos;
+	struct evsel *pos, *tmp;
 	struct evlist *evlist = rec->evlist;
 	struct perf_session *session = rec->session;
 	struct record_opts *opts = &rec->opts;
 	int rc = 0;
-	bool skipped = false;
 
-	evlist__for_each_entry(evlist, pos) {
+	evlist__for_each_entry_safe(evlist, tmp, pos) {
 try_again:
 		if (evsel__open(pos, pos->core.cpus, pos->core.threads) < 0) {
 			if (evsel__fallback(pos, &opts->target, errno, msg, sizeof(msg))) {
@@ -1383,23 +1382,12 @@ static int record__open(struct record *rec)
 			evsel__open_strerror(pos, &opts->target, errno, msg, sizeof(msg));
 			ui__error("Failure to open event '%s' on PMU '%s' which will be removed.\n%s\n",
 				  evsel__name(pos), evsel__pmu_name(pos), msg);
-			pos->skippable = true;
-			skipped = true;
+			evlist__remove(evlist, pos);
 		} else {
 			pos->supported = true;
 		}
 	}
 
-	if (skipped) {
-		struct evsel *tmp;
-		int idx = 0;
-
-		evlist__for_each_entry_safe(evlist, tmp, pos) {
-			if (pos->skippable)
-				evlist__remove(evlist, pos);
-			pos->core.idx = idx++;
-		}
-	}
 	if (symbol_conf.kptr_restrict && !evlist__exclude_kernel(evlist)) {
 		pr_warning(
 "WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted,\n"
-- 
2.34.1