* [PATCH v4 01/16] perf intel-tpebs: Cleanup header
2025-04-09 6:07 [PATCH v4 00/16] Intel TPEBS min/max/mean/last support Ian Rogers
@ 2025-04-09 6:07 ` Ian Rogers
2025-04-09 6:07 ` [PATCH v4 02/16] perf intel-tpebs: Simplify tpebs_cmd Ian Rogers
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2025-04-09 6:07 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, Weilin Wang, James Clark,
Xu Yang, John Garry, Howard Chu, Levi Yun, Dominique Martinet,
linux-perf-users, linux-kernel
Remove arch conditional compilation. Arch conditional compilation
belongs in the arch/ directory.
Tidy header guards to match other files. Remove unneeded includes and
switch to forward declarations when necesary.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/Build | 2 +-
tools/perf/util/intel-tpebs.c | 1 +
tools/perf/util/intel-tpebs.h | 30 ++++++------------------------
3 files changed, 8 insertions(+), 25 deletions(-)
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 946bce6628f3..815274b199fd 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -161,7 +161,7 @@ perf-util-y += clockid.o
perf-util-y += list_sort.o
perf-util-y += mutex.o
perf-util-y += sharded_mutex.o
-perf-util-$(CONFIG_X86_64) += intel-tpebs.o
+perf-util-y += intel-tpebs.o
perf-util-$(CONFIG_LIBBPF) += bpf_map.o
perf-util-$(CONFIG_PERF_BPF_SKEL) += bpf_counter.o
diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c
index 2c421b475b3b..3503da28a12f 100644
--- a/tools/perf/util/intel-tpebs.c
+++ b/tools/perf/util/intel-tpebs.c
@@ -19,6 +19,7 @@
#include "tool.h"
#include "cpumap.h"
#include "metricgroup.h"
+#include "stat.h"
#include <sys/stat.h>
#include <sys/file.h>
#include <poll.h>
diff --git a/tools/perf/util/intel-tpebs.h b/tools/perf/util/intel-tpebs.h
index 766b3fbd79f1..63c16e759a71 100644
--- a/tools/perf/util/intel-tpebs.h
+++ b/tools/perf/util/intel-tpebs.h
@@ -2,34 +2,16 @@
/*
* intel_tpebs.h: Intel TEPBS support
*/
-#ifndef INCLUDE__PERF_INTEL_TPEBS_H__
-#define INCLUDE__PERF_INTEL_TPEBS_H__
+#ifndef __INTEL_TPEBS_H
+#define __INTEL_TPEBS_H
-#include "stat.h"
-#include "evsel.h"
-
-#ifdef HAVE_ARCH_X86_64_SUPPORT
+struct evlist;
+struct evsel;
extern bool tpebs_recording;
+
int tpebs_start(struct evlist *evsel_list);
void tpebs_delete(void);
int tpebs_set_evsel(struct evsel *evsel, int cpu_map_idx, int thread);
-#else
-
-static inline int tpebs_start(struct evlist *evsel_list __maybe_unused)
-{
- return 0;
-}
-
-static inline void tpebs_delete(void) {};
-
-static inline int tpebs_set_evsel(struct evsel *evsel __maybe_unused,
- int cpu_map_idx __maybe_unused,
- int thread __maybe_unused)
-{
- return 0;
-}
-
-#endif
-#endif
+#endif /* __INTEL_TPEBS_H */
--
2.49.0.504.g3bcea36a83-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 02/16] perf intel-tpebs: Simplify tpebs_cmd
2025-04-09 6:07 [PATCH v4 00/16] Intel TPEBS min/max/mean/last support Ian Rogers
2025-04-09 6:07 ` [PATCH v4 01/16] perf intel-tpebs: Cleanup header Ian Rogers
@ 2025-04-09 6:07 ` Ian Rogers
2025-04-09 6:07 ` [PATCH v4 03/16] perf intel-tpebs: Rename tpebs_start to evsel__tpebs_open Ian Rogers
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2025-04-09 6:07 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, Weilin Wang, James Clark,
Xu Yang, John Garry, Howard Chu, Levi Yun, Dominique Martinet,
linux-perf-users, linux-kernel
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.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/intel-tpebs.c | 55 ++++++++++++-----------------------
1 file changed, 18 insertions(+), 37 deletions(-)
diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c
index 3503da28a12f..74b43faab986 100644
--- a/tools/perf/util/intel-tpebs.c
+++ b/tools/perf/util/intel-tpebs.c
@@ -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;
- }
}
--
2.49.0.504.g3bcea36a83-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 03/16] perf intel-tpebs: Rename tpebs_start to evsel__tpebs_open
2025-04-09 6:07 [PATCH v4 00/16] Intel TPEBS min/max/mean/last support Ian Rogers
2025-04-09 6:07 ` [PATCH v4 01/16] perf intel-tpebs: Cleanup header Ian Rogers
2025-04-09 6:07 ` [PATCH v4 02/16] perf intel-tpebs: Simplify tpebs_cmd Ian Rogers
@ 2025-04-09 6:07 ` Ian Rogers
2025-04-09 6:07 ` [PATCH v4 04/16] perf intel-tpebs: Separate evsel__tpebs_prepare out of evsel__tpebs_open Ian Rogers
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2025-04-09 6:07 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, Weilin Wang, James Clark,
Xu Yang, John Garry, Howard Chu, Levi Yun, Dominique Martinet,
linux-perf-users, linux-kernel
Try to add more consistency to evsel by having tpebs_start renamed to
evsel__tpebs_open, passing the evsel that is being opened. The unusual
behavior of evsel__tpebs_open opening all events on the evlist is kept
and will be cleaned up further in later patches. The comments are
cleaned up as tpebs_start isn't called from evlist.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/evsel.c | 2 +-
tools/perf/util/intel-tpebs.c | 33 ++++++++++++++++-----------------
tools/perf/util/intel-tpebs.h | 2 +-
3 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 1974395492d7..121283f2f382 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2595,7 +2595,7 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
struct perf_cpu cpu;
if (evsel__is_retire_lat(evsel))
- return tpebs_start(evsel->evlist);
+ return evsel__tpebs_open(evsel);
err = __evsel__prepare_open(evsel, cpus, threads);
if (err)
diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c
index 74b43faab986..566e0ddcad88 100644
--- a/tools/perf/util/intel-tpebs.c
+++ b/tools/perf/util/intel-tpebs.c
@@ -12,6 +12,7 @@
#include <linux/zalloc.h>
#include <linux/err.h>
#include "sample.h"
+#include "counts.h"
#include "debug.h"
#include "evlist.h"
#include "evsel.h"
@@ -189,18 +190,16 @@ static int tpebs_stop(void)
return ret;
}
-/*
- * tpebs_start - start tpebs execution.
- * @evsel_list: retire_latency evsels in this list will be selected and sampled
- * to get the average retire_latency value.
- *
- * This function will be called from evlist level later when evlist__open() is
- * called consistently.
+/**
+ * evsel__tpebs_open - starts tpebs execution.
+ * @evsel: retire_latency evsel, all evsels on its list will be selected. Each
+ * evsel is sampled to get the average retire_latency value.
*/
-int tpebs_start(struct evlist *evsel_list)
+int evsel__tpebs_open(struct evsel *evsel)
{
int ret = 0;
- struct evsel *evsel;
+ struct evsel *pos;
+ struct evlist *evsel_list = evsel->evlist;
char cpumap_buf[50];
/*
@@ -215,25 +214,25 @@ int tpebs_start(struct evlist *evsel_list)
* Prepare perf record for sampling event retire_latency before fork and
* prepare workload
*/
- evlist__for_each_entry(evsel_list, evsel) {
+ evlist__for_each_entry(evsel_list, pos) {
int i;
char *name;
struct tpebs_retire_lat *new;
- if (!evsel->retire_lat)
+ if (!pos->retire_lat)
continue;
- pr_debug("tpebs: Retire_latency of event %s is required\n", evsel->name);
- for (i = strlen(evsel->name) - 1; i > 0; i--) {
- if (evsel->name[i] == 'R')
+ pr_debug("tpebs: Retire_latency of event %s is required\n", pos->name);
+ for (i = strlen(pos->name) - 1; i > 0; i--) {
+ if (pos->name[i] == 'R')
break;
}
- if (i <= 0 || evsel->name[i] != 'R') {
+ if (i <= 0 || pos->name[i] != 'R') {
ret = -1;
goto err;
}
- name = strdup(evsel->name);
+ name = strdup(pos->name);
if (!name) {
ret = -ENOMEM;
goto err;
@@ -247,7 +246,7 @@ int tpebs_start(struct evlist *evsel_list)
goto err;
}
new->name = name;
- new->tpebs_name = evsel->name;
+ new->tpebs_name = pos->name;
list_add_tail(&new->nd, &tpebs_results);
tpebs_event_size += 1;
}
diff --git a/tools/perf/util/intel-tpebs.h b/tools/perf/util/intel-tpebs.h
index 63c16e759a71..cc98203719c8 100644
--- a/tools/perf/util/intel-tpebs.h
+++ b/tools/perf/util/intel-tpebs.h
@@ -10,7 +10,7 @@ struct evsel;
extern bool tpebs_recording;
-int tpebs_start(struct evlist *evsel_list);
+int evsel__tpebs_open(struct evsel *evsel);
void tpebs_delete(void);
int tpebs_set_evsel(struct evsel *evsel, int cpu_map_idx, int thread);
--
2.49.0.504.g3bcea36a83-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 04/16] perf intel-tpebs: Separate evsel__tpebs_prepare out of evsel__tpebs_open
2025-04-09 6:07 [PATCH v4 00/16] Intel TPEBS min/max/mean/last support Ian Rogers
` (2 preceding siblings ...)
2025-04-09 6:07 ` [PATCH v4 03/16] perf intel-tpebs: Rename tpebs_start to evsel__tpebs_open Ian Rogers
@ 2025-04-09 6:07 ` Ian Rogers
2025-04-09 6:07 ` [PATCH v4 05/16] perf intel-tpebs: Move cpumap_buf " Ian Rogers
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2025-04-09 6:07 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, Weilin Wang, James Clark,
Xu Yang, John Garry, Howard Chu, Levi Yun, Dominique Martinet,
linux-perf-users, linux-kernel
Separate the creation of the tpebs_retire_lat result out of the
opening step. This is in preparation for adding a prepare operation
for evlists.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/intel-tpebs.c | 133 ++++++++++++++++++++++------------
1 file changed, 86 insertions(+), 47 deletions(-)
diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c
index 566e0ddcad88..2186818b2c9b 100644
--- a/tools/perf/util/intel-tpebs.c
+++ b/tools/perf/util/intel-tpebs.c
@@ -37,7 +37,7 @@ static struct child_process tpebs_cmd;
struct tpebs_retire_lat {
struct list_head nd;
/* Event name */
- const char *name;
+ char *name;
/* Event name with the TPEBS modifier R */
const char *tpebs_name;
/* Count of retire_latency values found in sample data */
@@ -190,6 +190,82 @@ static int tpebs_stop(void)
return ret;
}
+static char *evsel__tpebs_name(struct evsel *evsel)
+{
+ char *name, *modifier;
+
+ name = strdup(evsel->name);
+ if (!name)
+ return NULL;
+
+ modifier = strrchr(name, 'R');
+ if (!modifier) {
+ pr_err("Tpebs event missing modifier '%s'\n", name);
+ free(name);
+ return NULL;
+ }
+
+ *modifier = 'p';
+ return name;
+}
+
+static struct tpebs_retire_lat *tpebs_retire_lat__new(struct evsel *evsel)
+{
+ struct tpebs_retire_lat *result = zalloc(sizeof(*result));
+
+ if (!result)
+ return NULL;
+
+ result->tpebs_name = evsel->name;
+ result->name = evsel__tpebs_name(evsel);
+ if (!result->name) {
+ free(result);
+ return NULL;
+ }
+ list_add_tail(&result->nd, &tpebs_results);
+ tpebs_event_size++;
+ return result;
+}
+
+/**
+ * evsel__tpebs_prepare - create tpebs data structures ready for opening.
+ * @evsel: retire_latency evsel, all evsels on its list will be prepared.
+ */
+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;
+ }
+ }
+ tpebs_event = tpebs_retire_lat__new(evsel);
+ if (!tpebs_event)
+ return -ENOMEM;
+
+ /*
+ * Eagerly prepare all other evsels on the list to try to ensure that by
+ * open they are all known.
+ */
+ evlist__for_each_entry(evsel->evlist, pos) {
+ int ret;
+
+ if (pos == evsel || !pos->retire_lat)
+ continue;
+
+ ret = evsel__tpebs_prepare(pos);
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+
/**
* evsel__tpebs_open - starts tpebs execution.
* @evsel: retire_latency evsel, all evsels on its list will be selected. Each
@@ -197,10 +273,7 @@ static int tpebs_stop(void)
*/
int evsel__tpebs_open(struct evsel *evsel)
{
- int ret = 0;
- struct evsel *pos;
- struct evlist *evsel_list = evsel->evlist;
- char cpumap_buf[50];
+ int ret;
/*
* We should only run tpebs_start when tpebs_recording is enabled.
@@ -209,49 +282,13 @@ int evsel__tpebs_open(struct evsel *evsel)
if (tpebs_cmd.pid != 0 || !tpebs_recording)
return 0;
- cpu_map__snprint(evsel_list->core.user_requested_cpus, cpumap_buf, sizeof(cpumap_buf));
- /*
- * Prepare perf record for sampling event retire_latency before fork and
- * prepare workload
- */
- evlist__for_each_entry(evsel_list, pos) {
- int i;
- char *name;
- struct tpebs_retire_lat *new;
-
- if (!pos->retire_lat)
- continue;
-
- pr_debug("tpebs: Retire_latency of event %s is required\n", pos->name);
- for (i = strlen(pos->name) - 1; i > 0; i--) {
- if (pos->name[i] == 'R')
- break;
- }
- if (i <= 0 || pos->name[i] != 'R') {
- ret = -1;
- goto err;
- }
-
- name = strdup(pos->name);
- if (!name) {
- ret = -ENOMEM;
- goto err;
- }
- name[i] = 'p';
-
- new = zalloc(sizeof(*new));
- if (!new) {
- ret = -1;
- zfree(&name);
- goto err;
- }
- new->name = name;
- new->tpebs_name = pos->name;
- list_add_tail(&new->nd, &tpebs_results);
- tpebs_event_size += 1;
- }
+ ret = evsel__tpebs_prepare(evsel);
+ if (ret)
+ return ret;
if (tpebs_event_size > 0) {
+ struct evlist *evsel_list = evsel->evlist;
+ char cpumap_buf[50];
struct pollfd pollfd = { .events = POLLIN, };
int control_fd[2], ack_fd[2], len;
char ack_buf[8];
@@ -268,6 +305,9 @@ int evsel__tpebs_open(struct evsel *evsel)
goto out;
}
+ cpu_map__snprint(evsel_list->core.user_requested_cpus, cpumap_buf,
+ sizeof(cpumap_buf));
+
ret = start_perf_record(control_fd, ack_fd, cpumap_buf);
if (ret)
goto out;
@@ -321,7 +361,6 @@ int evsel__tpebs_open(struct evsel *evsel)
close(ack_fd[0]);
close(ack_fd[1]);
}
-err:
if (ret)
tpebs_delete();
return ret;
--
2.49.0.504.g3bcea36a83-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 05/16] perf intel-tpebs: Move cpumap_buf out of evsel__tpebs_open
2025-04-09 6:07 [PATCH v4 00/16] Intel TPEBS min/max/mean/last support Ian Rogers
` (3 preceding siblings ...)
2025-04-09 6:07 ` [PATCH v4 04/16] perf intel-tpebs: Separate evsel__tpebs_prepare out of evsel__tpebs_open Ian Rogers
@ 2025-04-09 6:07 ` Ian Rogers
2025-04-09 6:07 ` [PATCH v4 06/16] perf intel-tpebs: Reduce scope of tpebs_events_size Ian Rogers
2025-04-09 6:07 ` [PATCH v4 07/16] perf intel-tpebs: Inline get_perf_record_args Ian Rogers
6 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2025-04-09 6:07 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, Weilin Wang, James Clark,
Xu Yang, John Garry, Howard Chu, Levi Yun, Dominique Martinet,
linux-perf-users, linux-kernel
The buffer holds the cpumap to pass to the perf record command, so
move it down to the perf record function. Make this function an evsel
function given the need for the evsel for the cpumap.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/intel-tpebs.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c
index 2186818b2c9b..2b04deaf66ff 100644
--- a/tools/perf/util/intel-tpebs.c
+++ b/tools/perf/util/intel-tpebs.c
@@ -83,12 +83,15 @@ static int get_perf_record_args(const char **record_argv, char buf[],
return 0;
}
-static int start_perf_record(int control_fd[], int ack_fd[],
- const char *cpumap_buf)
+static int evsel__tpebs_start_perf_record(struct evsel *evsel, int control_fd[], int ack_fd[])
{
const char **record_argv;
int ret;
char buf[32];
+ char cpumap_buf[50];
+
+ 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]);
@@ -287,8 +290,6 @@ int evsel__tpebs_open(struct evsel *evsel)
return ret;
if (tpebs_event_size > 0) {
- struct evlist *evsel_list = evsel->evlist;
- char cpumap_buf[50];
struct pollfd pollfd = { .events = POLLIN, };
int control_fd[2], ack_fd[2], len;
char ack_buf[8];
@@ -305,10 +306,7 @@ int evsel__tpebs_open(struct evsel *evsel)
goto out;
}
- cpu_map__snprint(evsel_list->core.user_requested_cpus, cpumap_buf,
- sizeof(cpumap_buf));
-
- ret = start_perf_record(control_fd, ack_fd, cpumap_buf);
+ ret = evsel__tpebs_start_perf_record(evsel, control_fd, ack_fd);
if (ret)
goto out;
--
2.49.0.504.g3bcea36a83-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 06/16] perf intel-tpebs: Reduce scope of tpebs_events_size
2025-04-09 6:07 [PATCH v4 00/16] Intel TPEBS min/max/mean/last support Ian Rogers
` (4 preceding siblings ...)
2025-04-09 6:07 ` [PATCH v4 05/16] perf intel-tpebs: Move cpumap_buf " Ian Rogers
@ 2025-04-09 6:07 ` Ian Rogers
2025-04-09 6:07 ` [PATCH v4 07/16] perf intel-tpebs: Inline get_perf_record_args Ian Rogers
6 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2025-04-09 6:07 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, Weilin Wang, James Clark,
Xu Yang, John Garry, Howard Chu, Levi Yun, Dominique Martinet,
linux-perf-users, linux-kernel
Moved to record argument computation rather than being global.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/intel-tpebs.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c
index 2b04deaf66ff..e3bed86145b9 100644
--- a/tools/perf/util/intel-tpebs.c
+++ b/tools/perf/util/intel-tpebs.c
@@ -29,7 +29,6 @@
#define PERF_DATA "-"
bool tpebs_recording;
-static size_t tpebs_event_size;
static LIST_HEAD(tpebs_results);
static pthread_t tpebs_reader_thread;
static struct child_process tpebs_cmd;
@@ -86,15 +85,20 @@ static int get_perf_record_args(const char **record_argv, char buf[],
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;
@@ -226,7 +230,6 @@ static struct tpebs_retire_lat *tpebs_retire_lat__new(struct evsel *evsel)
return NULL;
}
list_add_tail(&result->nd, &tpebs_results);
- tpebs_event_size++;
return result;
}
@@ -289,7 +292,7 @@ int evsel__tpebs_open(struct evsel *evsel)
if (ret)
return ret;
- if (tpebs_event_size > 0) {
+ if (!list_empty(&tpebs_results)) {
struct pollfd pollfd = { .events = POLLIN, };
int control_fd[2], ack_fd[2], len;
char ack_buf[8];
--
2.49.0.504.g3bcea36a83-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 07/16] perf intel-tpebs: Inline get_perf_record_args
2025-04-09 6:07 [PATCH v4 00/16] Intel TPEBS min/max/mean/last support Ian Rogers
` (5 preceding siblings ...)
2025-04-09 6:07 ` [PATCH v4 06/16] perf intel-tpebs: Reduce scope of tpebs_events_size Ian Rogers
@ 2025-04-09 6:07 ` Ian Rogers
6 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2025-04-09 6:07 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, Weilin Wang, James Clark,
Xu Yang, John Garry, Howard Chu, Levi Yun, Dominique Martinet,
linux-perf-users, linux-kernel
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 e3bed86145b9..c4c818f32239 100644
--- a/tools/perf/util/intel-tpebs.c
+++ b/tools/perf/util/intel-tpebs.c
@@ -47,72 +47,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.504.g3bcea36a83-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread