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

perf intel-tpebs: Simplify tpebs_cmd 



No need to dynamically allocate when there is 1. tpebs_pid duplicates
tpebs_cmd.pid, so remove. Use 0 as the uninitialized value (PID == 0
is reserved for the kernel) rather than -1.

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-3-irogers@google.com


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

tools/perf/util/intel-tpebs.c

+18 −37
Original line number Diff line number Diff line
@@ -28,11 +28,10 @@ 
#define PERF_DATA		"-"



bool tpebs_recording;

static pid_t tpebs_pid = -1;

static size_t tpebs_event_size;

static LIST_HEAD(tpebs_results);

static pthread_t tpebs_reader_thread;

static struct child_process *tpebs_cmd;

static struct child_process tpebs_cmd;



struct tpebs_retire_lat {

	struct list_head nd;
@@ -83,16 +82,6 @@ static int get_perf_record_args(const char **record_argv, char buf[], 
	return 0;

}



static int prepare_run_command(const char **argv)

{

	tpebs_cmd = zalloc(sizeof(struct child_process));

	if (!tpebs_cmd)

		return -ENOMEM;

	tpebs_cmd->argv = argv;

	tpebs_cmd->out = -1;

	return 0;

}



static int start_perf_record(int control_fd[], int ack_fd[],

				const char *cpumap_buf)

{
@@ -110,10 +99,10 @@ static int start_perf_record(int control_fd[], int ack_fd[], 
	if (ret)

		goto out;



	ret = prepare_run_command(record_argv);

	if (ret)

		goto out;

	ret = start_command(tpebs_cmd);

	assert(tpebs_cmd.pid == 0);

	tpebs_cmd.argv = record_argv;

	tpebs_cmd.out = -1;

	ret = start_command(&tpebs_cmd);

out:

	free(record_argv);

	return ret;
@@ -156,14 +145,13 @@ static int process_feature_event(struct perf_session *session, 
	return 0;

}



static void *__sample_reader(void *arg)

static void *__sample_reader(void *arg __maybe_unused)

{

	struct child_process *child = arg;

	struct perf_session *session;

	struct perf_data data = {

		.mode = PERF_DATA_MODE_READ,

		.path = PERF_DATA,

		.file.fd = child->out,

		.file.fd = tpebs_cmd.out,

	};

	struct perf_tool tool;
@@ -189,12 +177,12 @@ static int tpebs_stop(void) 
	int ret = 0;



	/* Like tpebs_start, we should only run tpebs_end once. */

	if (tpebs_pid != -1) {

		kill(tpebs_cmd->pid, SIGTERM);

		tpebs_pid = -1;

	if (tpebs_cmd.pid != 0) {

		kill(tpebs_cmd.pid, SIGTERM);

		pthread_join(tpebs_reader_thread, NULL);

		close(tpebs_cmd->out);

		ret = finish_command(tpebs_cmd);

		close(tpebs_cmd.out);

		ret = finish_command(&tpebs_cmd);

		tpebs_cmd.pid = 0;

		if (ret == -ERR_RUN_COMMAND_WAITPID_SIGNAL)

			ret = 0;

	}
@@ -219,7 +207,7 @@ int tpebs_start(struct evlist *evsel_list) 
	 * 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_pid != -1 || !tpebs_recording)

	if (tpebs_cmd.pid != 0 || !tpebs_recording)

		return 0;



	cpu_map__snprint(evsel_list->core.user_requested_cpus, cpumap_buf, sizeof(cpumap_buf));
@@ -284,10 +272,11 @@ int tpebs_start(struct evlist *evsel_list) 
		ret = start_perf_record(control_fd, ack_fd, cpumap_buf);

		if (ret)

			goto out;

		tpebs_pid = tpebs_cmd->pid;

		if (pthread_create(&tpebs_reader_thread, NULL, __sample_reader, tpebs_cmd)) {

			kill(tpebs_cmd->pid, SIGTERM);

			close(tpebs_cmd->out);



		if (pthread_create(&tpebs_reader_thread, /*attr=*/NULL, __sample_reader,

				   /*arg=*/NULL)) {

			kill(tpebs_cmd.pid, SIGTERM);

			close(tpebs_cmd.out);

			pr_err("Could not create thread to process sample data.\n");

			ret = -1;

			goto out;
@@ -416,18 +405,10 @@ void tpebs_delete(void) 
{

	struct tpebs_retire_lat *r, *rtmp;



	if (tpebs_pid == -1)

		return;



	tpebs_stop();



	list_for_each_entry_safe(r, rtmp, &tpebs_results, nd) {

		list_del_init(&r->nd);

		tpebs_retire_lat__delete(r);

	}



	if (tpebs_cmd) {

		free(tpebs_cmd);

		tpebs_cmd = NULL;

	}

}