From: Ian Rogers Date: Mon, 14 Apr 2025 17:41:29 +0000 (-0700) Subject: perf intel-tpebs: Add mutex for tpebs_results X-Git-Tag: v6.16-rc1~57^2~142 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=81743920491345dfde859eec677eaa094a01d28c;p=thirdparty%2Flinux.git perf intel-tpebs: Add mutex for tpebs_results Ensure sample reader isn't racing with events being added/removed. Reviewed-by: Kan Liang Signed-off-by: Ian Rogers Tested-by: Weilin Wang Acked-by: Namhyung Kim Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Alexandre Torgue Cc: Andreas Färber Cc: Caleb Biggers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Manivannan Sadhasivam Cc: Mark Rutland Cc: Maxime Coquelin Cc: Perry Taylor Cc: Peter Zijlstra Cc: Thomas Falcon Link: https://lore.kernel.org/r/20250414174134.3095492-12-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c index 8cbb3b3c00564..44e86d0f93432 100644 --- a/tools/perf/util/intel-tpebs.c +++ b/tools/perf/util/intel-tpebs.c @@ -16,6 +16,7 @@ #include "debug.h" #include "evlist.h" #include "evsel.h" +#include "mutex.h" #include "session.h" #include "tool.h" #include "cpumap.h" @@ -32,6 +33,7 @@ bool tpebs_recording; static LIST_HEAD(tpebs_results); static pthread_t tpebs_reader_thread; static struct child_process tpebs_cmd; +static struct mutex tpebs_mtx; struct tpebs_retire_lat { struct list_head nd; @@ -51,6 +53,19 @@ struct tpebs_retire_lat { static struct tpebs_retire_lat *tpebs_retire_lat__find(struct evsel *evsel); +static void tpebs_mtx_init(void) +{ + mutex_init(&tpebs_mtx); +} + +static struct mutex *tpebs_mtx_get(void) +{ + static pthread_once_t tpebs_mtx_once = PTHREAD_ONCE_INIT; + + pthread_once(&tpebs_mtx_once, tpebs_mtx_init); + return &tpebs_mtx; +} + static int evsel__tpebs_start_perf_record(struct evsel *evsel, int control_fd[], int ack_fd[]) { const char **record_argv; @@ -59,13 +74,15 @@ static int evsel__tpebs_start_perf_record(struct evsel *evsel, int control_fd[], char cpumap_buf[50]; struct tpebs_retire_lat *t; + mutex_lock(tpebs_mtx_get()); list_for_each_entry(t, &tpebs_results, nd) tpebs_event_size++; record_argv = malloc((10 + 2 * tpebs_event_size) * sizeof(*record_argv)); - if (!record_argv) + if (!record_argv) { + mutex_unlock(tpebs_mtx_get()); return -ENOMEM; - + } record_argv[i++] = "perf"; record_argv[i++] = "record"; record_argv[i++] = "-W"; @@ -101,6 +118,7 @@ static int evsel__tpebs_start_perf_record(struct evsel *evsel, int control_fd[], list_for_each_entry(t, &tpebs_results, nd) t->started = true; + mutex_unlock(tpebs_mtx_get()); return ret; } @@ -112,9 +130,12 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused, { struct tpebs_retire_lat *t; + mutex_lock(tpebs_mtx_get()); t = tpebs_retire_lat__find(evsel); - if (!t) + if (!t) { + mutex_unlock(tpebs_mtx_get()); return -EINVAL; + } /* * Need to handle per core results? We are assuming average retire * latency value will be used. Save the number of samples and the sum of @@ -123,6 +144,7 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused, t->count += 1; t->sum += sample->retire_lat; t->val = (double) t->sum / t->count; + mutex_unlock(tpebs_mtx_get()); return 0; } @@ -229,7 +251,6 @@ static struct tpebs_retire_lat *tpebs_retire_lat__new(struct evsel *evsel) return NULL; } result->evsel = evsel; - list_add_tail(&result->nd, &tpebs_results); return result; } @@ -282,15 +303,22 @@ static struct tpebs_retire_lat *tpebs_retire_lat__find(struct evsel *evsel) static int evsel__tpebs_prepare(struct evsel *evsel) { struct evsel *pos; - struct tpebs_retire_lat *tpebs_event = tpebs_retire_lat__find(evsel); + struct tpebs_retire_lat *tpebs_event; + mutex_lock(tpebs_mtx_get()); + tpebs_event = tpebs_retire_lat__find(evsel); if (tpebs_event) { /* evsel, or an identically named one, was already prepared. */ + mutex_unlock(tpebs_mtx_get()); return 0; } tpebs_event = tpebs_retire_lat__new(evsel); - if (!tpebs_event) + if (!tpebs_event) { + mutex_unlock(tpebs_mtx_get()); return -ENOMEM; + } + list_add_tail(&tpebs_event->nd, &tpebs_results); + mutex_unlock(tpebs_mtx_get()); /* * Eagerly prepare all other evsels on the list to try to ensure that by @@ -317,6 +345,7 @@ static int evsel__tpebs_prepare(struct evsel *evsel) int evsel__tpebs_open(struct evsel *evsel) { int ret; + bool tpebs_empty; /* We should only run tpebs_start when tpebs_recording is enabled. */ if (!tpebs_recording) @@ -336,7 +365,10 @@ int evsel__tpebs_open(struct evsel *evsel) if (ret) return ret; - if (!list_empty(&tpebs_results)) { + mutex_lock(tpebs_mtx_get()); + tpebs_empty = list_empty(&tpebs_results); + mutex_unlock(tpebs_mtx_get()); + if (!tpebs_empty) { struct pollfd pollfd = { .events = POLLIN, }; int control_fd[2], ack_fd[2], len; char ack_buf[8]; @@ -436,8 +468,10 @@ int evsel__tpebs_read(struct evsel *evsel, int cpu_map_idx, int thread) */ tpebs_stop(); + mutex_lock(tpebs_mtx_get()); t = tpebs_retire_lat__find(evsel); val = rint(t->val); + mutex_unlock(tpebs_mtx_get()); if (old_count) { count->val = old_count->val + val; @@ -460,9 +494,13 @@ int evsel__tpebs_read(struct evsel *evsel, int cpu_map_idx, int thread) */ void evsel__tpebs_close(struct evsel *evsel) { - struct tpebs_retire_lat *t = tpebs_retire_lat__find(evsel); + struct tpebs_retire_lat *t; + mutex_lock(tpebs_mtx_get()); + t = tpebs_retire_lat__find(evsel); + list_del_init(&t->nd); tpebs_retire_lat__delete(t); + mutex_unlock(tpebs_mtx_get()); if (list_empty(&tpebs_results)) tpebs_stop();