From: Weilin Wang <weilin.wang@intel.com>
Another thread is required to synchronize between perf stat and perf record
when we pass data through pipe.
Signed-off-by: Weilin Wang <weilin.wang@intel.com>
Reviewed-by: Ian Rogers <irogers@google.com>
---
tools/perf/builtin-stat.c | 51 +++++++++++++++++++++--------------
tools/perf/util/stat-shadow.c | 4 +--
2 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index fc887d9aaa66..14488cb0cfc8 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -802,9 +802,9 @@ static int process_feature_event(struct perf_session *session,
return 0;
}
-static int __cmd_script(struct child_process *cmd __maybe_unused)
+static void *__cmd_script(void *arg __maybe_unused)
{
- int err = 0;
+ struct child_process *cmd = arg;
struct perf_session *session;
struct perf_data data = {
.mode = PERF_DATA_MODE_READ,
@@ -819,29 +819,15 @@ static int __cmd_script(struct child_process *cmd __maybe_unused)
.attr = perf_event__process_attr,
},
};
- struct tpebs_event *e;
-
- list_for_each_entry(e, &stat_config.tpebs_events, nd) {
- struct tpebs_retire_lat *new = malloc(sizeof(struct tpebs_retire_lat));
- if (!new)
- return -1;
- new->event.name = strdup(e->name);
- new->event.tpebs_name = strdup(e->tpebs_name);
- new->count = 0;
- new->sum = 0;
- list_add_tail(&new->event.nd, &stat_config.tpebs_results);
- }
-
- kill(cmd->pid, SIGTERM);
session = perf_session__new(&data, &script.tool);
if (IS_ERR(session))
- return PTR_ERR(session);
+ return NULL;
script.session = session;
- err = perf_session__process_events(session);
+ perf_session__process_events(session);
perf_session__delete(session);
- return err;
+ return NULL;
}
static int __run_perf_stat(int argc, const char **argv, int run_idx)
@@ -861,16 +847,37 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
int err;
bool second_pass = false;
struct child_process cmd;
+ pthread_t thread_script;
/* Prepare perf record for sampling event retire_latency before fork and
* prepare workload */
if (stat_config.tpebs_event_size > 0) {
int ret;
+ struct tpebs_event *e;
pr_debug("perf stat pid = %d\n", getpid());
+ list_for_each_entry(e, &stat_config.tpebs_events, nd) {
+ struct tpebs_retire_lat *new = malloc(sizeof(struct tpebs_retire_lat));
+
+ if (!new)
+ return -1;
+ new->event.name = strdup(e->name);
+ new->event.tpebs_name = strdup(e->tpebs_name);
+ new->count = 0;
+ new->sum = 0;
+ list_add_tail(&new->event.nd, &stat_config.tpebs_results);
+ }
ret = prepare_perf_record(&cmd);
if (ret)
return ret;
+ if (pthread_create(&thread_script, NULL, __cmd_script, &cmd)) {
+ kill(cmd.pid, SIGTERM);
+ close(cmd.out);
+ pr_err("Could not create thread to process sample data.\n");
+ return -1;
+ }
+ /* Wait for perf record initialization a little bit.*/
+ sleep(2);
}
if (forks) {
@@ -1081,8 +1088,12 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
if (stat_config.tpebs_event_size > 0) {
int ret;
- ret = __cmd_script(&cmd);
+ kill(cmd.pid, SIGTERM);
+ pthread_join(thread_script, NULL);
close(cmd.out);
+ ret = finish_command(&cmd);
+ if (ret != -ERR_RUN_COMMAND_WAITPID_SIGNAL)
+ return ret;
}
if (stat_config.walltime_run_table)
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 8fc3415de106..bc77e9e02892 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -361,8 +361,8 @@ static int prepare_retire_lat(struct expr_parse_ctx *pctx,
int ret = 0;
struct tpebs_retire_lat *t;
- list_for_each_entry(t, retire_lats, nd) {
- ret = expr__add_id_val(pctx, strdup(t->tpebs_name), t->val);
+ list_for_each_entry(t, retire_lats, event.nd) {
+ ret = expr__add_id_val(pctx, strdup(t->event.tpebs_name), t->val);
if (ret < 0)
return ret;
}
--
2.43.0On Tue, Mar 26, 2024 at 1:29 PM <weilin.wang@intel.com> wrote:
>
> From: Weilin Wang <weilin.wang@intel.com>
>
> Another thread is required to synchronize between perf stat and perf record
> when we pass data through pipe.
It seems better if you can squash this commit to the patch 2/6.
Thanks,
Namhyung
>
> Signed-off-by: Weilin Wang <weilin.wang@intel.com>
> Reviewed-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/builtin-stat.c | 51 +++++++++++++++++++++--------------
> tools/perf/util/stat-shadow.c | 4 +--
> 2 files changed, 33 insertions(+), 22 deletions(-)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index fc887d9aaa66..14488cb0cfc8 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -802,9 +802,9 @@ static int process_feature_event(struct perf_session *session,
> return 0;
> }
>
> -static int __cmd_script(struct child_process *cmd __maybe_unused)
> +static void *__cmd_script(void *arg __maybe_unused)
> {
> - int err = 0;
> + struct child_process *cmd = arg;
> struct perf_session *session;
> struct perf_data data = {
> .mode = PERF_DATA_MODE_READ,
> @@ -819,29 +819,15 @@ static int __cmd_script(struct child_process *cmd __maybe_unused)
> .attr = perf_event__process_attr,
> },
> };
> - struct tpebs_event *e;
> -
> - list_for_each_entry(e, &stat_config.tpebs_events, nd) {
> - struct tpebs_retire_lat *new = malloc(sizeof(struct tpebs_retire_lat));
>
> - if (!new)
> - return -1;
> - new->event.name = strdup(e->name);
> - new->event.tpebs_name = strdup(e->tpebs_name);
> - new->count = 0;
> - new->sum = 0;
> - list_add_tail(&new->event.nd, &stat_config.tpebs_results);
> - }
> -
> - kill(cmd->pid, SIGTERM);
> session = perf_session__new(&data, &script.tool);
> if (IS_ERR(session))
> - return PTR_ERR(session);
> + return NULL;
> script.session = session;
> - err = perf_session__process_events(session);
> + perf_session__process_events(session);
> perf_session__delete(session);
>
> - return err;
> + return NULL;
> }
>
> static int __run_perf_stat(int argc, const char **argv, int run_idx)
> @@ -861,16 +847,37 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
> int err;
> bool second_pass = false;
> struct child_process cmd;
> + pthread_t thread_script;
>
> /* Prepare perf record for sampling event retire_latency before fork and
> * prepare workload */
> if (stat_config.tpebs_event_size > 0) {
> int ret;
> + struct tpebs_event *e;
>
> pr_debug("perf stat pid = %d\n", getpid());
> + list_for_each_entry(e, &stat_config.tpebs_events, nd) {
> + struct tpebs_retire_lat *new = malloc(sizeof(struct tpebs_retire_lat));
> +
> + if (!new)
> + return -1;
> + new->event.name = strdup(e->name);
> + new->event.tpebs_name = strdup(e->tpebs_name);
> + new->count = 0;
> + new->sum = 0;
> + list_add_tail(&new->event.nd, &stat_config.tpebs_results);
> + }
> ret = prepare_perf_record(&cmd);
> if (ret)
> return ret;
> + if (pthread_create(&thread_script, NULL, __cmd_script, &cmd)) {
> + kill(cmd.pid, SIGTERM);
> + close(cmd.out);
> + pr_err("Could not create thread to process sample data.\n");
> + return -1;
> + }
> + /* Wait for perf record initialization a little bit.*/
> + sleep(2);
> }
>
> if (forks) {
> @@ -1081,8 +1088,12 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
> if (stat_config.tpebs_event_size > 0) {
> int ret;
>
> - ret = __cmd_script(&cmd);
> + kill(cmd.pid, SIGTERM);
> + pthread_join(thread_script, NULL);
> close(cmd.out);
> + ret = finish_command(&cmd);
> + if (ret != -ERR_RUN_COMMAND_WAITPID_SIGNAL)
> + return ret;
> }
>
> if (stat_config.walltime_run_table)
> diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
> index 8fc3415de106..bc77e9e02892 100644
> --- a/tools/perf/util/stat-shadow.c
> +++ b/tools/perf/util/stat-shadow.c
> @@ -361,8 +361,8 @@ static int prepare_retire_lat(struct expr_parse_ctx *pctx,
> int ret = 0;
> struct tpebs_retire_lat *t;
>
> - list_for_each_entry(t, retire_lats, nd) {
> - ret = expr__add_id_val(pctx, strdup(t->tpebs_name), t->val);
> + list_for_each_entry(t, retire_lats, event.nd) {
> + ret = expr__add_id_val(pctx, strdup(t->event.tpebs_name), t->val);
> if (ret < 0)
> return ret;
> }
> --
> 2.43.0
>
© 2016 - 2026 Red Hat, Inc.