diff options
author | Ian Rogers <irogers@google.com> | 2025-04-14 10:41:26 -0700 |
---|---|---|
committer | Arnaldo Carvalho de Melo <acme@redhat.com> | 2025-04-25 12:31:15 -0300 |
commit | 07c35320337eec5744d7cf735e86ac6f52d3535f (patch) | |
tree | 4c10af438e57ad21e3bba37f30119b83464d1959 | |
parent | 84e629143b776793d05ef45d6ef9f59feae53fe4 (diff) |
perf intel-tpebs: Ensure events are opened, factor out finding
Factor out finding an tpebs_retire_lat from an evsel.
Don't blindly return when ignoring an open request, which happens after
the first open request, ensure the event was started on a fork of perf
record.
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
Signed-off-by: Ian Rogers <irogers@google.com>
Tested-by: Weilin Wang <weilin.wang@intel.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Andreas Färber <afaerber@suse.de>
Cc: Caleb Biggers <caleb.biggers@intel.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Perry Taylor <perry.taylor@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Falcon <thomas.falcon@intel.com>
Link: https://lore.kernel.org/r/20250414174134.3095492-9-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
-rw-r--r-- | tools/perf/util/intel-tpebs.c | 61 |
1 files changed, 37 insertions, 24 deletions
diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c index c4c818f32239..e42f3ec39a64 100644 --- a/tools/perf/util/intel-tpebs.c +++ b/tools/perf/util/intel-tpebs.c @@ -45,6 +45,8 @@ struct tpebs_retire_lat { int sum; /* Average of retire_latency, val = sum / count */ double val; + /* Has the event been sent to perf record? */ + bool started; }; static int evsel__tpebs_start_perf_record(struct evsel *evsel, int control_fd[], int ack_fd[]) @@ -94,6 +96,9 @@ static int evsel__tpebs_start_perf_record(struct evsel *evsel, int control_fd[], tpebs_cmd.out = -1; ret = start_command(&tpebs_cmd); zfree(&tpebs_cmd.argv); + list_for_each_entry(t, &tpebs_results, nd) + t->started = true; + return ret; } @@ -214,6 +219,19 @@ static struct tpebs_retire_lat *tpebs_retire_lat__new(struct evsel *evsel) return result; } +static struct tpebs_retire_lat *tpebs_retire_lat__find(struct evsel *evsel) +{ + struct tpebs_retire_lat *t; + + list_for_each_entry(t, &tpebs_results, nd) { + if (t->tpebs_name == evsel->name || + !strcmp(t->tpebs_name, evsel->name) || + (evsel->metric_id && !strcmp(t->tpebs_name, evsel->metric_id))) + return t; + } + return NULL; +} + /** * evsel__tpebs_prepare - create tpebs data structures ready for opening. * @evsel: retire_latency evsel, all evsels on its list will be prepared. @@ -221,16 +239,11 @@ static struct tpebs_retire_lat *tpebs_retire_lat__new(struct evsel *evsel) static int evsel__tpebs_prepare(struct evsel *evsel) { struct evsel *pos; - struct tpebs_retire_lat *tpebs_event; - - list_for_each_entry(tpebs_event, &tpebs_results, nd) { - if (!strcmp(tpebs_event->tpebs_name, evsel->name)) { - /* - * evsel, or an identically named one, was already - * prepared. - */ - return 0; - } + struct tpebs_retire_lat *tpebs_event = tpebs_retire_lat__find(evsel); + + if (tpebs_event) { + /* evsel, or an identically named one, was already prepared. */ + return 0; } tpebs_event = tpebs_retire_lat__new(evsel); if (!tpebs_event) @@ -262,12 +275,19 @@ int evsel__tpebs_open(struct evsel *evsel) { int ret; - /* - * We should only run tpebs_start when tpebs_recording is enabled. - * And we should only run it once with all the required events. - */ - if (tpebs_cmd.pid != 0 || !tpebs_recording) + /* We should only run tpebs_start when tpebs_recording is enabled. */ + if (!tpebs_recording) return 0; + /* Only start the events once. */ + if (tpebs_cmd.pid != 0) { + struct tpebs_retire_lat *t = tpebs_retire_lat__find(evsel); + + if (!t || !t->started) { + /* Fail, as the event wasn't started. */ + return -EBUSY; + } + return 0; + } ret = evsel__tpebs_prepare(evsel); if (ret) @@ -352,7 +372,6 @@ out: int tpebs_set_evsel(struct evsel *evsel, int cpu_map_idx, int thread) { __u64 val; - bool found = false; struct tpebs_retire_lat *t; struct perf_counts_values *count; @@ -367,19 +386,13 @@ int tpebs_set_evsel(struct evsel *evsel, int cpu_map_idx, int thread) tpebs_stop(); count = perf_counts(evsel->counts, cpu_map_idx, thread); - list_for_each_entry(t, &tpebs_results, nd) { - if (t->tpebs_name == evsel->name || - (evsel->metric_id && !strcmp(t->tpebs_name, evsel->metric_id))) { - found = true; - break; - } - } + t = tpebs_retire_lat__find(evsel); /* Set ena and run to non-zero */ count->ena = count->run = 1; count->lost = 0; - if (!found) { + if (!t) { /* * Set default value or 0 when retire_latency for this event is * not found from sampling data (record_tpebs not set or 0 |