[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20241114160450.295844-3-james.clark@linaro.org>
Date: Thu, 14 Nov 2024 16:04:49 +0000
From: James Clark <james.clark@...aro.org>
To: linux-perf-users@...r.kernel.org,
namhyung@...nel.org,
irogers@...gle.com,
thomas.falcon@...el.com
Cc: James Clark <james.clark@...aro.org>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
"Liang, Kan" <kan.liang@...ux.intel.com>,
linux-kernel@...r.kernel.org
Subject: [PATCH v2 2/2] libperf: evlist: Keep evsel idx contiguous on removal
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@...aro.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
Powered by blists - more mailing lists