From: Ian Rogers <irogers@google.com>
To: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	 Arnaldo Carvalho de Melo <acme@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	 Mark Rutland <mark.rutland@arm.com>,
	 Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>,  Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	 Kan Liang <kan.liang@linux.intel.com>,
	Weilin Wang <weilin.wang@intel.com>,
	 James Clark <james.clark@linaro.org>,
	Xu Yang <xu.yang_2@nxp.com>,
	 John Garry <john.g.garry@oracle.com>,
	Howard Chu <howardchu95@gmail.com>,
	 Levi Yun <yeoreum.yun@arm.com>,
	Dominique Martinet <asmadeus@codewreck.org>,
	 linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v1 06/14] perf intel-tpebs: Inline get_perf_record_args
Date: Mon, 24 Mar 2025 15:34:44 -0700	[thread overview]
Message-ID: <20250324223452.208081-7-irogers@google.com> (raw)
In-Reply-To: <20250324223452.208081-1-irogers@google.com>
Code is short enough to be inlined and there are no error cases when
made inline. Make the implicit NULL pointer at the end of the argv
explicit. Move the fixed number of arguments before the variable
number of arguments. Correctly size the argv allocation and zero when
feeing to avoid a dangling pointer.
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/intel-tpebs.c | 75 +++++++++++++----------------------
 1 file changed, 28 insertions(+), 47 deletions(-)
diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c
index e5ca58599b5f..3141b7d2e94f 100644
--- a/tools/perf/util/intel-tpebs.c
+++ b/tools/perf/util/intel-tpebs.c
@@ -46,72 +46,53 @@ struct tpebs_retire_lat {
 	double val;
 };
 
-static int get_perf_record_args(const char **record_argv, char buf[],
-				const char *cpumap_buf)
+static int evsel__tpebs_start_perf_record(struct evsel *evsel, int control_fd[], int ack_fd[])
 {
-	struct tpebs_retire_lat *e;
-	int i = 0;
+	const char **record_argv;
+	int tpebs_event_size = 0, i = 0, ret;
+	char control_fd_buf[32];
+	char cpumap_buf[50];
+	struct tpebs_retire_lat *t;
+
+	list_for_each_entry(t, &tpebs_results, nd)
+		tpebs_event_size++;
 
-	pr_debug("tpebs: Prepare perf record for retire_latency\n");
+	record_argv = malloc((10 + 2 * tpebs_event_size) * sizeof(*record_argv));
+	if (!record_argv)
+		return -ENOMEM;
 
 	record_argv[i++] = "perf";
 	record_argv[i++] = "record";
 	record_argv[i++] = "-W";
 	record_argv[i++] = "--synth=no";
-	record_argv[i++] = buf;
 
-	if (!cpumap_buf) {
-		pr_err("tpebs: Require cpumap list to run sampling\n");
-		return -ECANCELED;
-	}
-	/* Use -C when cpumap_buf is not "-1" */
-	if (strcmp(cpumap_buf, "-1")) {
+	scnprintf(control_fd_buf, sizeof(control_fd_buf), "--control=fd:%d,%d",
+		  control_fd[0], ack_fd[1]);
+	record_argv[i++] = control_fd_buf;
+
+	record_argv[i++] = "-o";
+	record_argv[i++] = PERF_DATA;
+
+	if (!perf_cpu_map__is_any_cpu_or_is_empty(evsel->evlist->core.user_requested_cpus)) {
+		cpu_map__snprint(evsel->evlist->core.user_requested_cpus, cpumap_buf,
+				 sizeof(cpumap_buf));
 		record_argv[i++] = "-C";
 		record_argv[i++] = cpumap_buf;
 	}
 
-	list_for_each_entry(e, &tpebs_results, nd) {
+	list_for_each_entry(t, &tpebs_results, nd) {
 		record_argv[i++] = "-e";
-		record_argv[i++] = e->name;
+		record_argv[i++] = t->name;
 	}
-
-	record_argv[i++] = "-o";
-	record_argv[i++] = PERF_DATA;
-
-	return 0;
-}
-
-static int evsel__tpebs_start_perf_record(struct evsel *evsel, int control_fd[], int ack_fd[])
-{
-	const char **record_argv;
-	size_t tpebs_event_size = 0;
-	int ret;
-	char buf[32];
-	char cpumap_buf[50];
-	struct tpebs_retire_lat *t;
-
-	cpu_map__snprint(evsel->evlist->core.user_requested_cpus, cpumap_buf,
-			 sizeof(cpumap_buf));
-
-	scnprintf(buf, sizeof(buf), "--control=fd:%d,%d", control_fd[0], ack_fd[1]);
-
-	list_for_each_entry(t, &tpebs_results, nd)
-		tpebs_event_size++;
-
-	record_argv = calloc(12 + 2 * tpebs_event_size, sizeof(char *));
-	if (!record_argv)
-		return -ENOMEM;
-
-	ret = get_perf_record_args(record_argv, buf, cpumap_buf);
-	if (ret)
-		goto out;
+	record_argv[i++] = NULL;
+	assert(i == 10 + 2 * tpebs_event_size || i == 8 + 2 * tpebs_event_size);
+	/* Note, no workload given so system wide is implied. */
 
 	assert(tpebs_cmd.pid == 0);
 	tpebs_cmd.argv = record_argv;
 	tpebs_cmd.out = -1;
 	ret = start_command(&tpebs_cmd);
-out:
-	free(record_argv);
+	zfree(&tpebs_cmd.argv);
 	return ret;
 }
 
-- 
2.49.0.395.g12beb8f557-goog
next prev parent reply	other threads:[~2025-03-24 22:35 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-24 22:34 [PATCH v1 00/14] Intel TPEBS min/max/mean/last support Ian Rogers
2025-03-24 22:34 ` [PATCH v1 01/14] perf intel-tpebs: Simplify tpebs_cmd Ian Rogers
2025-03-24 22:34 ` [PATCH v1 02/14] perf intel-tpebs: Rename tpebs_start to evsel__tpebs_open Ian Rogers
2025-03-24 22:34 ` [PATCH v1 03/14] perf intel-tpebs: Separate evsel__tpebs_prepare out of evsel__tpebs_open Ian Rogers
2025-03-24 22:34 ` [PATCH v1 04/14] perf intel-tpebs: Move cpumap_buf " Ian Rogers
2025-03-24 22:34 ` [PATCH v1 05/14] perf intel-tpebs: Reduce scope of tpebs_events_size Ian Rogers
2025-03-24 22:34 ` Ian Rogers [this message]
2025-03-24 22:34 ` [PATCH v1 07/14] perf intel-tpebs: Ensure events are opened, factor out finding Ian Rogers
2025-03-24 22:34 ` [PATCH v1 08/14] perf intel-tpebs: Refactor tpebs_results list Ian Rogers
2025-03-24 22:34 ` [PATCH v1 09/14] perf intel-tpebs: Add support for updating counts in evsel__tpebs_read Ian Rogers
2025-03-24 22:34 ` [PATCH v1 10/14] perf intel-tpebs: Add mutex for tpebs_results Ian Rogers
2025-03-24 22:34 ` [PATCH v1 11/14] perf intel-tpebs: Don't close record on read Ian Rogers
2025-03-24 22:34 ` [PATCH v1 12/14] perf intel-tpebs: Use stats for retirement latency statistics Ian Rogers
2025-03-24 22:34 ` [PATCH v1 13/14] perf stat: Add mean, min, max and last --tpebs-mode options Ian Rogers
2025-03-24 22:34 ` [PATCH v1 14/14] perf pmu-events: Add retirement latency to JSON events inside of perf Ian Rogers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox
  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):
  git send-email \
    --in-reply-to=20250324223452.208081-7-irogers@google.com \
    --to=irogers@google.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=asmadeus@codewreck.org \
    --cc=howardchu95@gmail.com \
    --cc=james.clark@linaro.org \
    --cc=john.g.garry@oracle.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=weilin.wang@intel.com \
    --cc=xu.yang_2@nxp.com \
    --cc=yeoreum.yun@arm.com \
    /path/to/YOUR_REPLY
  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
  Be sure your reply has a Subject: header at the top and a blank line
  before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).