Commit 81743920 authored by Ian Rogers's avatar Ian Rogers Committed by Arnaldo Carvalho de Melo
Browse files

perf intel-tpebs: Add mutex for tpebs_results 



Ensure sample reader isn't racing with events being added/removed.

Reviewed-by: default avatarKan Liang <kan.liang@linux.intel.com>
Signed-off-by: default avatarIan Rogers <irogers@google.com>
Tested-by: default avatarWeilin Wang <weilin.wang@intel.com>
Acked-by: default avatarNamhyung 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-12-irogers@google.com


Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent ea61db61

tools/perf/util/intel-tpebs.c

+46 −8
Original line number Diff line number Diff line
@@ -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();