Hello,
The perf inject processes the input data and produces an output with
injected data according to the given options. During the work, it
assumes the input and output files have the same format - either a
regular file or a pipe. This works for the obvious cases, but
sometimes makes a trouble when input and output have different
formats (like for debugging).
* changes in v2
- factor out perf_event__synthesize_for_pipe
- add a shell test for pipe operations
For example, this patchset fixed the following cases
1. input: pipe, output: file
# perf record -a -o - sleep 1 | perf inject -b -o perf-pipe.data
# perf report -i perf-pipe.data
2. input: file, output: pipe
# perf record -a -B sleep 1
# perf inject -b -i perf.data | perf report -i -
Thanks,
Namhyung
Namhyung Kim (5):
perf tools: Remove repipe argument from perf_session__new()
perf tools: Pass a fd to perf_file_header__read_pipe()
perf inject: Fix output from a pipe to a file
perf inject: Fix output from a file to a pipe
perf tools: Add pipe_test.sh to verify pipe operations
tools/perf/bench/synthesize.c | 4 +-
tools/perf/builtin-annotate.c | 2 +-
tools/perf/builtin-buildid-cache.c | 2 +-
tools/perf/builtin-buildid-list.c | 2 +-
tools/perf/builtin-c2c.c | 2 +-
tools/perf/builtin-diff.c | 4 +-
tools/perf/builtin-evlist.c | 2 +-
tools/perf/builtin-inject.c | 38 ++++++++++++++--
tools/perf/builtin-kmem.c | 2 +-
tools/perf/builtin-kvm.c | 4 +-
tools/perf/builtin-lock.c | 2 +-
tools/perf/builtin-mem.c | 3 +-
tools/perf/builtin-record.c | 40 +++--------------
tools/perf/builtin-report.c | 2 +-
tools/perf/builtin-sched.c | 4 +-
tools/perf/builtin-script.c | 4 +-
tools/perf/builtin-stat.c | 4 +-
tools/perf/builtin-timechart.c | 3 +-
tools/perf/builtin-top.c | 2 +-
tools/perf/builtin-trace.c | 2 +-
tools/perf/tests/shell/pipe_test.sh | 69 +++++++++++++++++++++++++++++
tools/perf/tests/topology.c | 4 +-
tools/perf/util/data-convert-bt.c | 2 +-
tools/perf/util/data-convert-json.c | 2 +-
tools/perf/util/header.c | 12 ++---
tools/perf/util/header.h | 2 +-
tools/perf/util/session.c | 11 ++---
tools/perf/util/session.h | 12 ++++-
tools/perf/util/synthetic-events.c | 53 +++++++++++++++++++++-
tools/perf/util/synthetic-events.h | 6 +++
30 files changed, 217 insertions(+), 84 deletions(-)
create mode 100755 tools/perf/tests/shell/pipe_test.sh
--
2.32.0.93.g670b81a890-goog
The repipe argument is only used by perf inject and the all others
passes 'false'. Let's remove it from the function signature and add
__perf_session__new() to be called from perf inject directly.
This is a preparation of the change the pipe input/output.
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/bench/synthesize.c | 4 ++--
tools/perf/builtin-annotate.c | 2 +-
tools/perf/builtin-buildid-cache.c | 2 +-
tools/perf/builtin-buildid-list.c | 2 +-
tools/perf/builtin-c2c.c | 2 +-
tools/perf/builtin-diff.c | 4 ++--
tools/perf/builtin-evlist.c | 2 +-
tools/perf/builtin-inject.c | 3 ++-
tools/perf/builtin-kmem.c | 2 +-
tools/perf/builtin-kvm.c | 4 ++--
tools/perf/builtin-lock.c | 2 +-
tools/perf/builtin-mem.c | 3 +--
tools/perf/builtin-record.c | 2 +-
tools/perf/builtin-report.c | 2 +-
tools/perf/builtin-sched.c | 4 ++--
tools/perf/builtin-script.c | 4 ++--
tools/perf/builtin-stat.c | 4 ++--
tools/perf/builtin-timechart.c | 3 +--
tools/perf/builtin-top.c | 2 +-
tools/perf/builtin-trace.c | 2 +-
tools/perf/tests/topology.c | 4 ++--
tools/perf/util/data-convert-bt.c | 2 +-
tools/perf/util/data-convert-json.c | 2 +-
tools/perf/util/session.c | 5 +++--
tools/perf/util/session.h | 12 ++++++++++--
25 files changed, 44 insertions(+), 36 deletions(-)
diff --git a/tools/perf/bench/synthesize.c b/tools/perf/bench/synthesize.c
index b2924e3181dc..05f7c923c745 100644
--- a/tools/perf/bench/synthesize.c
+++ b/tools/perf/bench/synthesize.c
@@ -117,7 +117,7 @@ static int run_single_threaded(void)
int err;
perf_set_singlethreaded();
- session = perf_session__new(NULL, false, NULL);
+ session = perf_session__new(NULL, NULL);
if (IS_ERR(session)) {
pr_err("Session creation failed.\n");
return PTR_ERR(session);
@@ -161,7 +161,7 @@ static int do_run_multi_threaded(struct target *target,
init_stats(&time_stats);
init_stats(&event_stats);
for (i = 0; i < multi_iterations; i++) {
- session = perf_session__new(NULL, false, NULL);
+ session = perf_session__new(NULL, NULL);
if (IS_ERR(session))
return PTR_ERR(session);
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index cebb861be3e3..05eb098cb0e3 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -596,7 +596,7 @@ int cmd_annotate(int argc, const char **argv)
data.path = input_name;
- annotate.session = perf_session__new(&data, false, &annotate.tool);
+ annotate.session = perf_session__new(&data, &annotate.tool);
if (IS_ERR(annotate.session))
return PTR_ERR(annotate.session);
diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index ecd0d3cb6f5c..0db3cfc04c47 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -443,7 +443,7 @@ int cmd_buildid_cache(int argc, const char **argv)
data.path = missing_filename;
data.force = force;
- session = perf_session__new(&data, false, NULL);
+ session = perf_session__new(&data, NULL);
if (IS_ERR(session))
return PTR_ERR(session);
}
diff --git a/tools/perf/builtin-buildid-list.c b/tools/perf/builtin-buildid-list.c
index 833405c27dae..cebadd632234 100644
--- a/tools/perf/builtin-buildid-list.c
+++ b/tools/perf/builtin-buildid-list.c
@@ -65,7 +65,7 @@ static int perf_session__list_build_ids(bool force, bool with_hits)
if (filename__fprintf_build_id(input_name, stdout) > 0)
goto out;
- session = perf_session__new(&data, false, &build_id__mark_dso_hit_ops);
+ session = perf_session__new(&data, &build_id__mark_dso_hit_ops);
if (IS_ERR(session))
return PTR_ERR(session);
diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index 6dea37f141b2..a812f32cf5d9 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -2790,7 +2790,7 @@ static int perf_c2c__report(int argc, const char **argv)
goto out;
}
- session = perf_session__new(&data, 0, &c2c.tool);
+ session = perf_session__new(&data, &c2c.tool);
if (IS_ERR(session)) {
err = PTR_ERR(session);
pr_debug("Error creating perf session\n");
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index f52b3a799e76..612060c53bd6 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -1156,7 +1156,7 @@ static int check_file_brstack(void)
int i;
data__for_each_file(i, d) {
- d->session = perf_session__new(&d->data, false, &pdiff.tool);
+ d->session = perf_session__new(&d->data, &pdiff.tool);
if (IS_ERR(d->session)) {
pr_err("Failed to open %s\n", d->data.path);
return PTR_ERR(d->session);
@@ -1188,7 +1188,7 @@ static int __cmd_diff(void)
ret = -EINVAL;
data__for_each_file(i, d) {
- d->session = perf_session__new(&d->data, false, &pdiff.tool);
+ d->session = perf_session__new(&d->data, &pdiff.tool);
if (IS_ERR(d->session)) {
ret = PTR_ERR(d->session);
pr_err("Failed to open %s\n", d->data.path);
diff --git a/tools/perf/builtin-evlist.c b/tools/perf/builtin-evlist.c
index 4617b32c9c97..b1076177c37f 100644
--- a/tools/perf/builtin-evlist.c
+++ b/tools/perf/builtin-evlist.c
@@ -42,7 +42,7 @@ static int __cmd_evlist(const char *file_name, struct perf_attr_details *details
};
bool has_tracepoint = false;
- session = perf_session__new(&data, 0, &tool);
+ session = perf_session__new(&data, &tool);
if (IS_ERR(session))
return PTR_ERR(session);
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 5d6f583e2cd3..3cffb12f01be 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -991,7 +991,8 @@ int cmd_inject(int argc, const char **argv)
}
data.path = inject.input_name;
- inject.session = perf_session__new(&data, inject.output.is_pipe, &inject.tool);
+ inject.session = __perf_session__new(&data, inject.output.is_pipe,
+ &inject.tool);
if (IS_ERR(inject.session))
return PTR_ERR(inject.session);
diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 0062445e8ead..da03a341c63c 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -1953,7 +1953,7 @@ int cmd_kmem(int argc, const char **argv)
data.path = input_name;
- kmem_session = session = perf_session__new(&data, false, &perf_kmem);
+ kmem_session = session = perf_session__new(&data, &perf_kmem);
if (IS_ERR(session))
return PTR_ERR(session);
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 1105c9e40a80..aa1b127ffb5b 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1093,7 +1093,7 @@ static int read_events(struct perf_kvm_stat *kvm)
};
kvm->tool = eops;
- kvm->session = perf_session__new(&file, false, &kvm->tool);
+ kvm->session = perf_session__new(&file, &kvm->tool);
if (IS_ERR(kvm->session)) {
pr_err("Initializing perf session failed\n");
return PTR_ERR(kvm->session);
@@ -1447,7 +1447,7 @@ static int kvm_events_live(struct perf_kvm_stat *kvm,
/*
* perf session
*/
- kvm->session = perf_session__new(&data, false, &kvm->tool);
+ kvm->session = perf_session__new(&data, &kvm->tool);
if (IS_ERR(kvm->session)) {
err = PTR_ERR(kvm->session);
goto out;
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 01326e370009..d70131b7b1b1 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -868,7 +868,7 @@ static int __cmd_report(bool display_info)
.force = force,
};
- session = perf_session__new(&data, false, &eops);
+ session = perf_session__new(&data, &eops);
if (IS_ERR(session)) {
pr_err("Initializing perf session failed\n");
return PTR_ERR(session);
diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index 0fd2a74dbaca..fcf65a59bea2 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -271,8 +271,7 @@ static int report_raw_events(struct perf_mem *mem)
.force = mem->force,
};
int ret;
- struct perf_session *session = perf_session__new(&data, false,
- &mem->tool);
+ struct perf_session *session = perf_session__new(&data, &mem->tool);
if (IS_ERR(session))
return PTR_ERR(session);
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 71efe6573ee7..199320046fff 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1680,7 +1680,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
signal(SIGUSR2, SIG_IGN);
}
- session = perf_session__new(data, false, tool);
+ session = perf_session__new(data, tool);
if (IS_ERR(session)) {
pr_err("Perf session creation failed.\n");
return PTR_ERR(session);
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 8639bbe0969d..21cfa3a96ee8 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1405,7 +1405,7 @@ int cmd_report(int argc, const char **argv)
data.force = symbol_conf.force;
repeat:
- session = perf_session__new(&data, false, &report.tool);
+ session = perf_session__new(&data, &report.tool);
if (IS_ERR(session))
return PTR_ERR(session);
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 954ce2f594e9..d4ffc331aacb 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1804,7 +1804,7 @@ static int perf_sched__read_events(struct perf_sched *sched)
};
int rc = -1;
- session = perf_session__new(&data, false, &sched->tool);
+ session = perf_session__new(&data, &sched->tool);
if (IS_ERR(session)) {
pr_debug("Error creating perf session");
return PTR_ERR(session);
@@ -3011,7 +3011,7 @@ static int perf_sched__timehist(struct perf_sched *sched)
symbol_conf.use_callchain = sched->show_callchain;
- session = perf_session__new(&data, false, &sched->tool);
+ session = perf_session__new(&data, &sched->tool);
if (IS_ERR(session))
return PTR_ERR(session);
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 2030936cc891..52cdb8c76ec8 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -3287,7 +3287,7 @@ int find_scripts(char **scripts_array, char **scripts_path_array, int num,
char *temp;
int i = 0;
- session = perf_session__new(&data, false, NULL);
+ session = perf_session__new(&data, NULL);
if (IS_ERR(session))
return PTR_ERR(session);
@@ -4000,7 +4000,7 @@ int cmd_script(int argc, const char **argv)
use_browser = 0;
}
- session = perf_session__new(&data, false, &script.tool);
+ session = perf_session__new(&data, &script.tool);
if (IS_ERR(session))
return PTR_ERR(session);
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f9f74a514315..014afffa7e72 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1993,7 +1993,7 @@ static int __cmd_record(int argc, const char **argv)
return -1;
}
- session = perf_session__new(data, false, NULL);
+ session = perf_session__new(data, NULL);
if (IS_ERR(session)) {
pr_err("Perf session creation failed\n");
return PTR_ERR(session);
@@ -2165,7 +2165,7 @@ static int __cmd_report(int argc, const char **argv)
perf_stat.data.path = input_name;
perf_stat.data.mode = PERF_DATA_MODE_READ;
- session = perf_session__new(&perf_stat.data, false, &perf_stat.tool);
+ session = perf_session__new(&perf_stat.data, &perf_stat.tool);
if (IS_ERR(session))
return PTR_ERR(session);
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 4e380e7b5230..43bf4d67edb0 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -1598,8 +1598,7 @@ static int __cmd_timechart(struct timechart *tchart, const char *output_name)
.force = tchart->force,
};
- struct perf_session *session = perf_session__new(&data, false,
- &tchart->tool);
+ struct perf_session *session = perf_session__new(&data, &tchart->tool);
int ret = -EINVAL;
if (IS_ERR(session))
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 2d570bfe7a56..30e4cba80b33 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1740,7 +1740,7 @@ int cmd_top(int argc, const char **argv)
signal(SIGWINCH, winch_sig);
}
- top.session = perf_session__new(NULL, false, NULL);
+ top.session = perf_session__new(NULL, NULL);
if (IS_ERR(top.session)) {
status = PTR_ERR(top.session);
goto out_delete_evlist;
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 7ec18ff57fc4..f963bd61de36 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -4205,7 +4205,7 @@ static int trace__replay(struct trace *trace)
/* add tid to output */
trace->multiple_threads = true;
- session = perf_session__new(&data, false, &trace->tool);
+ session = perf_session__new(&data, &trace->tool);
if (IS_ERR(session))
return PTR_ERR(session);
diff --git a/tools/perf/tests/topology.c b/tools/perf/tests/topology.c
index ec4e3b21b831..870bcc03e977 100644
--- a/tools/perf/tests/topology.c
+++ b/tools/perf/tests/topology.c
@@ -38,7 +38,7 @@ static int session_write_header(char *path)
.mode = PERF_DATA_MODE_WRITE,
};
- session = perf_session__new(&data, false, NULL);
+ session = perf_session__new(&data, NULL);
TEST_ASSERT_VAL("can't get session", !IS_ERR(session));
if (!perf_pmu__has_hybrid()) {
@@ -76,7 +76,7 @@ static int check_cpu_topology(char *path, struct perf_cpu_map *map)
int i;
struct aggr_cpu_id id;
- session = perf_session__new(&data, false, NULL);
+ session = perf_session__new(&data, NULL);
TEST_ASSERT_VAL("can't get session", !IS_ERR(session));
cpu__setup_cpunode_map();
diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
index cace349fb700..aa862a26d95c 100644
--- a/tools/perf/util/data-convert-bt.c
+++ b/tools/perf/util/data-convert-bt.c
@@ -1634,7 +1634,7 @@ int bt_convert__perf2ctf(const char *input, const char *path,
err = -1;
/* perf.data session */
- session = perf_session__new(&data, 0, &c.tool);
+ session = perf_session__new(&data, &c.tool);
if (IS_ERR(session))
return PTR_ERR(session);
diff --git a/tools/perf/util/data-convert-json.c b/tools/perf/util/data-convert-json.c
index 355cd1948bdf..f1ab6edba446 100644
--- a/tools/perf/util/data-convert-json.c
+++ b/tools/perf/util/data-convert-json.c
@@ -334,7 +334,7 @@ int bt_convert__perf2json(const char *input_name, const char *output_name,
goto err;
}
- session = perf_session__new(&data, false, &c.tool);
+ session = perf_session__new(&data, &c.tool);
if (IS_ERR(session)) {
fprintf(stderr, "Error creating perf session!\n");
goto err_fclose;
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index e9c929a39973..f76b18e3c061 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -185,8 +185,9 @@ static int ordered_events__deliver_event(struct ordered_events *oe,
session->tool, event->file_offset);
}
-struct perf_session *perf_session__new(struct perf_data *data,
- bool repipe, struct perf_tool *tool)
+struct perf_session *__perf_session__new(struct perf_data *data,
+ bool repipe,
+ struct perf_tool *tool)
{
int ret = -ENOMEM;
struct perf_session *session = zalloc(sizeof(*session));
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index e31ba4c92a6c..9d19d2a918c6 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -54,8 +54,16 @@ struct decomp {
struct perf_tool;
-struct perf_session *perf_session__new(struct perf_data *data,
- bool repipe, struct perf_tool *tool);
+struct perf_session *__perf_session__new(struct perf_data *data,
+ bool repipe,
+ struct perf_tool *tool);
+
+static inline struct perf_session *perf_session__new(struct perf_data *data,
+ struct perf_tool *tool)
+{
+ return __perf_session__new(data, false, tool);
+}
+
void perf_session__delete(struct perf_session *session);
void perf_event_header__bswap(struct perf_event_header *hdr);
--
2.32.0.93.g670b81a890-goog
Currently it unconditionally writes to stdout for repipe. But perf
inject can direct its output to a regular file. Then it needs to
write the header to the file as well.
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-inject.c | 1 +
tools/perf/util/header.c | 12 ++++++------
tools/perf/util/header.h | 2 +-
tools/perf/util/session.c | 8 ++++----
tools/perf/util/session.h | 4 ++--
5 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 3cffb12f01be..d99f4538d2fc 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -992,6 +992,7 @@ int cmd_inject(int argc, const char **argv)
data.path = inject.input_name;
inject.session = __perf_session__new(&data, inject.output.is_pipe,
+ perf_data__fd(&inject.output),
&inject.tool);
if (IS_ERR(inject.session))
return PTR_ERR(inject.session);
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 0158d2945bab..4f1a7b6df7ee 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3865,10 +3865,10 @@ static int perf_file_section__process(struct perf_file_section *section,
static int perf_file_header__read_pipe(struct perf_pipe_file_header *header,
struct perf_header *ph,
struct perf_data* data,
- bool repipe)
+ bool repipe, int repipe_fd)
{
struct feat_fd ff = {
- .fd = STDOUT_FILENO,
+ .fd = repipe_fd,
.ph = ph,
};
ssize_t ret;
@@ -3891,13 +3891,13 @@ static int perf_file_header__read_pipe(struct perf_pipe_file_header *header,
return 0;
}
-static int perf_header__read_pipe(struct perf_session *session)
+static int perf_header__read_pipe(struct perf_session *session, int repipe_fd)
{
struct perf_header *header = &session->header;
struct perf_pipe_file_header f_header;
if (perf_file_header__read_pipe(&f_header, header, session->data,
- session->repipe) < 0) {
+ session->repipe, repipe_fd) < 0) {
pr_debug("incompatible file format\n");
return -EINVAL;
}
@@ -3995,7 +3995,7 @@ static int evlist__prepare_tracepoint_events(struct evlist *evlist, struct tep_h
return 0;
}
-int perf_session__read_header(struct perf_session *session)
+int perf_session__read_header(struct perf_session *session, int repipe_fd)
{
struct perf_data *data = session->data;
struct perf_header *header = &session->header;
@@ -4016,7 +4016,7 @@ int perf_session__read_header(struct perf_session *session)
* We can read 'pipe' data event from regular file,
* check for the pipe header regardless of source.
*/
- err = perf_header__read_pipe(session);
+ err = perf_header__read_pipe(session, repipe_fd);
if (!err || perf_data__is_pipe(data)) {
data->is_pipe = true;
return err;
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index ae6b1cf19a7d..c9e3265832d9 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -115,7 +115,7 @@ struct perf_session;
struct perf_tool;
union perf_event;
-int perf_session__read_header(struct perf_session *session);
+int perf_session__read_header(struct perf_session *session, int repipe_fd);
int perf_session__write_header(struct perf_session *session,
struct evlist *evlist,
int fd, bool at_exit);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index f76b18e3c061..a40dd37ac71e 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -102,11 +102,11 @@ static int perf_session__deliver_event(struct perf_session *session,
struct perf_tool *tool,
u64 file_offset);
-static int perf_session__open(struct perf_session *session)
+static int perf_session__open(struct perf_session *session, int repipe_fd)
{
struct perf_data *data = session->data;
- if (perf_session__read_header(session) < 0) {
+ if (perf_session__read_header(session, repipe_fd) < 0) {
pr_err("incompatible file format (rerun with -v to learn more)\n");
return -1;
}
@@ -186,7 +186,7 @@ static int ordered_events__deliver_event(struct ordered_events *oe,
}
struct perf_session *__perf_session__new(struct perf_data *data,
- bool repipe,
+ bool repipe, int repipe_fd,
struct perf_tool *tool)
{
int ret = -ENOMEM;
@@ -211,7 +211,7 @@ struct perf_session *__perf_session__new(struct perf_data *data,
session->data = data;
if (perf_data__is_read(data)) {
- ret = perf_session__open(session);
+ ret = perf_session__open(session, repipe_fd);
if (ret < 0)
goto out_delete;
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 9d19d2a918c6..5d8bd14a0a39 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -55,13 +55,13 @@ struct decomp {
struct perf_tool;
struct perf_session *__perf_session__new(struct perf_data *data,
- bool repipe,
+ bool repipe, int repipe_fd,
struct perf_tool *tool);
static inline struct perf_session *perf_session__new(struct perf_data *data,
struct perf_tool *tool)
{
- return __perf_session__new(data, false, tool);
+ return __perf_session__new(data, false, -1, tool);
}
void perf_session__delete(struct perf_session *session);
--
2.32.0.93.g670b81a890-goog
It builds a test program and use it to verify pipe behavior with perf
record, inject and report.
$ perf test pipe -v
80: perf pipe recording and injection test :
--- start ---
test child forked, pid 1109301
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.000 MB - ]
1109315 1109315 -1 |test.file.MGNff
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.000 MB - ]
99.99% test.file.MGNff test.file.MGNffM [.] noploop
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.000 MB - ]
99.99% test.file.MGNff test.file.MGNffM [.] noploop
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.153 MB /tmp/perf.data.dmsnlx (3995 samples) ]
99.99% test.file.MGNff test.file.MGNffM [.] noploop
test child finished with 0
---- end ----
perf pipe recording and injection test: Ok
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/tests/shell/pipe_test.sh | 69 +++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)
create mode 100755 tools/perf/tests/shell/pipe_test.sh
diff --git a/tools/perf/tests/shell/pipe_test.sh b/tools/perf/tests/shell/pipe_test.sh
new file mode 100755
index 000000000000..b6a89c6896c3
--- /dev/null
+++ b/tools/perf/tests/shell/pipe_test.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+# perf pipe recording and injection test
+# SPDX-License-Identifier: GPL-2.0
+
+# skip if there's no compiler
+if ! [ -x "$(command -v cc)" ]; then
+ echo "failed: no compiler, install gcc"
+ exit 2
+fi
+
+file=$(mktemp /tmp/test.file.XXXXXX)
+data=$(mktemp /tmp/perf.data.XXXXXX)
+
+cat <<EOF | cc -o ${file} -x c -
+#include <signal.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+volatile int done;
+
+void sigalrm(int sig) {
+ done = 1;
+}
+
+__attribute__((noinline)) void noploop(void) {
+ while (!done)
+ continue;
+}
+
+int main(int argc, char *argv[]) {
+ int sec = 1;
+
+ if (argc > 1)
+ sec = atoi(argv[1]);
+
+ signal(SIGALRM, sigalrm);
+ alarm(sec);
+
+ noploop();
+ return 0;
+}
+EOF
+
+
+if ! perf record -e cycles:u -o - ${file} | perf report -i - --task | grep test.file; then
+ echo "cannot find the test file in the perf report"
+ exit 1
+fi
+
+if ! perf record -e cycles:u -o - ${file} | perf inject -b | perf report -i - | grep noploop; then
+ echo "cannot find noploop function in pipe #1"
+ exit 1
+fi
+
+perf record -e cycles:u -o - ${file} | perf inject -b -o ${data}
+if ! perf report -i ${data} | grep noploop; then
+ echo "cannot find noploop function in pipe #2"
+ exit 1
+fi
+
+perf record -e cycles:u -o ${data} ${file}
+if ! perf inject -b -i ${data} | perf report -i - | grep noploop; then
+ echo "cannot find noploop function in pipe #3"
+ exit 1
+fi
+
+
+rm -f ${file} ${data} ${data}.old
+exit 0
--
2.32.0.93.g670b81a890-goog
Sometimes it needs to save the perf inject data to a file for
debugging. But normally it assumes the same format for input and
output, so the end result cannot be used due to a broken format.
# perf record -a -o - sleep 1 | perf inject -b -o my.data
# perf report -i my.data --stdio
0x208 [0]: failed to process type: 0 [Invalid argument]
Error:
failed to process sample
# To display the perf.data header info, please use --header/--header-only options.
#
In this case, it thought the data has a regular file header since the
output is not a pipe. But actually it doesn't have one and has a pipe
file header. At the end of the session, it tries to rewrite the
regular file header with updated features and it overwrites the data
just follows the pipe header.
Fix it by checking either the input and the output is a pipe.
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-inject.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index d99f4538d2fc..7c126597d3f5 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -46,6 +46,7 @@ struct perf_inject {
bool jit_mode;
bool in_place_update;
bool in_place_update_dry_run;
+ bool is_pipe;
const char *input_name;
struct perf_data output;
u64 bytes_written;
@@ -126,7 +127,7 @@ static int perf_event__repipe_attr(struct perf_tool *tool,
if (ret)
return ret;
- if (!inject->output.is_pipe)
+ if (!inject->is_pipe)
return 0;
return perf_event__repipe_synth(tool, event);
@@ -825,14 +826,14 @@ static int __cmd_inject(struct perf_inject *inject)
if (!inject->itrace_synth_opts.set)
auxtrace_index__free(&session->auxtrace_index);
- if (!data_out->is_pipe && !inject->in_place_update)
+ if (!inject->is_pipe && !inject->in_place_update)
lseek(fd, output_data_offset, SEEK_SET);
ret = perf_session__process_events(session);
if (ret)
return ret;
- if (!data_out->is_pipe && !inject->in_place_update) {
+ if (!inject->is_pipe && !inject->in_place_update) {
if (inject->build_ids)
perf_header__set_feat(&session->header,
HEADER_BUILD_ID);
@@ -991,7 +992,10 @@ int cmd_inject(int argc, const char **argv)
}
data.path = inject.input_name;
- inject.session = __perf_session__new(&data, inject.output.is_pipe,
+ if (!strcmp(inject.input_name, "-") || inject.output.is_pipe)
+ inject.is_pipe = true;
+
+ inject.session = __perf_session__new(&data, inject.is_pipe,
perf_data__fd(&inject.output),
&inject.tool);
if (IS_ERR(inject.session))
--
2.32.0.93.g670b81a890-goog
When the input is a regular file but the output is a pipe, it should
write a pipe header. But just repiping would write a portion of the
existing header which is different in 'size' value. So we need to
prevent it and write a new pipe header along with other information
like event attributes and features.
This can handle something like this:
# perf record -a -B sleep 1
# perf inject -b -i perf.data | perf report -i -
Factor out perf_event__synthesize_for_pipe() to be shared between perf
record and inject.
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-inject.c | 28 ++++++++++++++--
tools/perf/builtin-record.c | 38 +++------------------
tools/perf/util/synthetic-events.c | 53 +++++++++++++++++++++++++++++-
tools/perf/util/synthetic-events.h | 6 ++++
4 files changed, 88 insertions(+), 37 deletions(-)
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 7c126597d3f5..0bdd824d8864 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -918,6 +918,7 @@ int cmd_inject(int argc, const char **argv)
.use_stdio = true,
};
int ret;
+ bool repipe = true;
struct option options[] = {
OPT_BOOLEAN('b', "build-ids", &inject.build_ids,
@@ -992,10 +993,18 @@ int cmd_inject(int argc, const char **argv)
}
data.path = inject.input_name;
- if (!strcmp(inject.input_name, "-") || inject.output.is_pipe)
+ if (!strcmp(inject.input_name, "-") || inject.output.is_pipe) {
inject.is_pipe = true;
+ /*
+ * Do not repipe header when input is a regular file
+ * since either it can rewrite the header at the end
+ * or write a new pipe header.
+ */
+ if (strcmp(inject.input_name, "-"))
+ repipe = false;
+ }
- inject.session = __perf_session__new(&data, inject.is_pipe,
+ inject.session = __perf_session__new(&data, repipe,
perf_data__fd(&inject.output),
&inject.tool);
if (IS_ERR(inject.session))
@@ -1004,6 +1013,21 @@ int cmd_inject(int argc, const char **argv)
if (zstd_init(&(inject.session->zstd_data), 0) < 0)
pr_warning("Decompression initialization failed.\n");
+ if (!data.is_pipe && inject.output.is_pipe) {
+ ret = perf_header__write_pipe(perf_data__fd(&inject.output));
+ if (ret < 0) {
+ pr_err("Couldn't write a new pipe header.\n");
+ goto out_delete;
+ }
+
+ ret = perf_event__synthesize_for_pipe(&inject.tool,
+ inject.session,
+ &inject.output,
+ perf_event__repipe);
+ if (ret < 0)
+ goto out_delete;
+ }
+
if (inject.build_ids && !inject.build_id_all) {
/*
* to make sure the mmap records are ordered correctly
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 199320046fff..9297de08b451 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1386,7 +1386,6 @@ static int record__synthesize(struct record *rec, bool tail)
struct perf_data *data = &rec->data;
struct record_opts *opts = &rec->opts;
struct perf_tool *tool = &rec->tool;
- int fd = perf_data__fd(data);
int err = 0;
event_op f = process_synthesized_event;
@@ -1394,41 +1393,12 @@ static int record__synthesize(struct record *rec, bool tail)
return 0;
if (data->is_pipe) {
- /*
- * We need to synthesize events first, because some
- * features works on top of them (on report side).
- */
- err = perf_event__synthesize_attrs(tool, rec->evlist,
- process_synthesized_event);
- if (err < 0) {
- pr_err("Couldn't synthesize attrs.\n");
- goto out;
- }
-
- err = perf_event__synthesize_features(tool, session, rec->evlist,
+ err = perf_event__synthesize_for_pipe(tool, session, data,
process_synthesized_event);
- if (err < 0) {
- pr_err("Couldn't synthesize features.\n");
- return err;
- }
+ if (err < 0)
+ goto out;
- if (have_tracepoints(&rec->evlist->core.entries)) {
- /*
- * FIXME err <= 0 here actually means that
- * there were no tracepoints so its not really
- * an error, just that we don't need to
- * synthesize anything. We really have to
- * return this more properly and also
- * propagate errors that now are calling die()
- */
- err = perf_event__synthesize_tracing_data(tool, fd, rec->evlist,
- process_synthesized_event);
- if (err <= 0) {
- pr_err("Couldn't record tracing data.\n");
- goto out;
- }
- rec->bytes_written += err;
- }
+ rec->bytes_written += err;
}
err = perf_event__synth_time_conv(record__pick_pc(rec), tool,
diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
index 35aa0c0f7cd9..a7e981b2d7de 100644
--- a/tools/perf/util/synthetic-events.c
+++ b/tools/perf/util/synthetic-events.c
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
+#include "util/cgroup.h"
+#include "util/data.h"
#include "util/debug.h"
#include "util/dso.h"
#include "util/event.h"
@@ -16,7 +18,6 @@
#include "util/synthetic-events.h"
#include "util/target.h"
#include "util/time-utils.h"
-#include "util/cgroup.h"
#include <linux/bitops.h>
#include <linux/kernel.h>
#include <linux/string.h>
@@ -2179,3 +2180,53 @@ int perf_event__synthesize_features(struct perf_tool *tool, struct perf_session
free(ff.buf);
return ret;
}
+
+int perf_event__synthesize_for_pipe(struct perf_tool *tool,
+ struct perf_session *session,
+ struct perf_data *data,
+ perf_event__handler_t process)
+{
+ int err;
+ int ret = 0;
+ struct evlist *evlist = session->evlist;
+
+ /*
+ * We need to synthesize events first, because some
+ * features works on top of them (on report side).
+ */
+ err = perf_event__synthesize_attrs(tool, evlist, process);
+ if (err < 0) {
+ pr_err("Couldn't synthesize attrs.\n");
+ return err;
+ }
+ ret += err;
+
+ err = perf_event__synthesize_features(tool, session, evlist, process);
+ if (err < 0) {
+ pr_err("Couldn't synthesize features.\n");
+ return err;
+ }
+ ret += err;
+
+ if (have_tracepoints(&evlist->core.entries)) {
+ int fd = perf_data__fd(data);
+
+ /*
+ * FIXME err <= 0 here actually means that
+ * there were no tracepoints so its not really
+ * an error, just that we don't need to
+ * synthesize anything. We really have to
+ * return this more properly and also
+ * propagate errors that now are calling die()
+ */
+ err = perf_event__synthesize_tracing_data(tool, fd, evlist,
+ process);
+ if (err <= 0) {
+ pr_err("Couldn't record tracing data.\n");
+ return err;
+ }
+ ret += err;
+ }
+
+ return ret;
+}
diff --git a/tools/perf/util/synthetic-events.h b/tools/perf/util/synthetic-events.h
index e7a3e9589738..c845e2b9b444 100644
--- a/tools/perf/util/synthetic-events.h
+++ b/tools/perf/util/synthetic-events.h
@@ -14,6 +14,7 @@ struct evsel;
struct machine;
struct perf_counts_values;
struct perf_cpu_map;
+struct perf_data;
struct perf_event_attr;
struct perf_event_mmap_page;
struct perf_sample;
@@ -101,4 +102,9 @@ static inline int perf_event__synthesize_bpf_events(struct perf_session *session
}
#endif // HAVE_LIBBPF_SUPPORT
+int perf_event__synthesize_for_pipe(struct perf_tool *tool,
+ struct perf_session *session,
+ struct perf_data *data,
+ perf_event__handler_t process);
+
#endif // __PERF_SYNTHETIC_EVENTS_H
--
2.32.0.93.g670b81a890-goog
On Tue, Jul 13, 2021 at 05:34:42PM -0700, Namhyung Kim wrote:
SNIP
> diff --git a/tools/perf/tests/shell/pipe_test.sh b/tools/perf/tests/shell/pipe_test.sh
> new file mode 100755
> index 000000000000..b6a89c6896c3
> --- /dev/null
> +++ b/tools/perf/tests/shell/pipe_test.sh
> @@ -0,0 +1,69 @@
> +#!/bin/sh
> +# perf pipe recording and injection test
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# skip if there's no compiler
> +if ! [ -x "$(command -v cc)" ]; then
> + echo "failed: no compiler, install gcc"
> + exit 2
> +fi
> +
> +file=$(mktemp /tmp/test.file.XXXXXX)
> +data=$(mktemp /tmp/perf.data.XXXXXX)
> +
> +cat <<EOF | cc -o ${file} -x c -
> +#include <signal.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +volatile int done;
> +
> +void sigalrm(int sig) {
> + done = 1;
> +}
> +
> +__attribute__((noinline)) void noploop(void) {
> + while (!done)
> + continue;
> +}
> +
> +int main(int argc, char *argv[]) {
> + int sec = 1;
> +
> + if (argc > 1)
> + sec = atoi(argv[1]);
> +
> + signal(SIGALRM, sigalrm);
> + alarm(sec);
> +
> + noploop();
> + return 0;
> +}
> +EOF
> +
> +
> +if ! perf record -e cycles:u -o - ${file} | perf report -i - --task | grep test.file; then
hi,
cycles:u will fail in virtual machine, could you use
cpu-clock instead or skip the test?
jirka
> + echo "cannot find the test file in the perf report"
> + exit 1
> +fi
> +
> +if ! perf record -e cycles:u -o - ${file} | perf inject -b | perf report -i - | grep noploop; then
> + echo "cannot find noploop function in pipe #1"
> + exit 1
> +fi
> +
> +perf record -e cycles:u -o - ${file} | perf inject -b -o ${data}
> +if ! perf report -i ${data} | grep noploop; then
> + echo "cannot find noploop function in pipe #2"
> + exit 1
> +fi
> +
> +perf record -e cycles:u -o ${data} ${file}
> +if ! perf inject -b -i ${data} | perf report -i - | grep noploop; then
> + echo "cannot find noploop function in pipe #3"
> + exit 1
> +fi
> +
> +
> +rm -f ${file} ${data} ${data}.old
> +exit 0
> --
> 2.32.0.93.g670b81a890-goog
>
Hi Jiri,
On Sun, Jul 18, 2021 at 2:38 PM Jiri Olsa <[email protected]> wrote:
>
> On Tue, Jul 13, 2021 at 05:34:42PM -0700, Namhyung Kim wrote:
>
> SNIP
>
> > diff --git a/tools/perf/tests/shell/pipe_test.sh b/tools/perf/tests/shell/pipe_test.sh
> > new file mode 100755
> > index 000000000000..b6a89c6896c3
> > --- /dev/null
> > +++ b/tools/perf/tests/shell/pipe_test.sh
> > @@ -0,0 +1,69 @@
> > +#!/bin/sh
> > +# perf pipe recording and injection test
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +# skip if there's no compiler
> > +if ! [ -x "$(command -v cc)" ]; then
> > + echo "failed: no compiler, install gcc"
> > + exit 2
> > +fi
> > +
> > +file=$(mktemp /tmp/test.file.XXXXXX)
> > +data=$(mktemp /tmp/perf.data.XXXXXX)
> > +
> > +cat <<EOF | cc -o ${file} -x c -
> > +#include <signal.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +
> > +volatile int done;
> > +
> > +void sigalrm(int sig) {
> > + done = 1;
> > +}
> > +
> > +__attribute__((noinline)) void noploop(void) {
> > + while (!done)
> > + continue;
> > +}
> > +
> > +int main(int argc, char *argv[]) {
> > + int sec = 1;
> > +
> > + if (argc > 1)
> > + sec = atoi(argv[1]);
> > +
> > + signal(SIGALRM, sigalrm);
> > + alarm(sec);
> > +
> > + noploop();
> > + return 0;
> > +}
> > +EOF
> > +
> > +
> > +if ! perf record -e cycles:u -o - ${file} | perf report -i - --task | grep test.file; then
>
> hi
> cycles:u will fail in virtual machine, could you use
> cpu-clock instead or skip the test?
You're right! I'll update to use task-clock.
Thanks,
Namhyung