linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output
@ 2021-07-19 22:31 Namhyung Kim
  2021-07-19 22:31 ` [PATCH 1/5] perf tools: Remove repipe argument from perf_session__new() Namhyung Kim
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Namhyung Kim @ 2021-07-19 22:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen, Ian Rogers, Adrian Hunter

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 v3
  - use task-clock:u in the pipe-test.sh

 * 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.402.g57bb445576-goog


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/5] perf tools: Remove repipe argument from perf_session__new()
  2021-07-19 22:31 [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output Namhyung Kim
@ 2021-07-19 22:31 ` Namhyung Kim
  2021-07-19 22:31 ` [PATCH 2/5] perf tools: Pass a fd to perf_file_header__read_pipe() Namhyung Kim
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2021-07-19 22:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen, Ian Rogers, Adrian Hunter

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 <namhyung@kernel.org>
---
 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 80450c0e8f36..d925096dd7f0 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 671a21c9ee4d..472cd12f10c6 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1681,7 +1681,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 6386af6a2612..7b36afac7c26 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 8c03a9862872..f777d7dd2037 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -3288,7 +3288,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);
 
@@ -4001,7 +4001,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 d25cb8088e8c..2c8c1dc7b095 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1996,7 +1996,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);
@@ -2168,7 +2168,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 02f8bb5dbc0f..a3ae9176a83e 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.402.g57bb445576-goog


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/5] perf tools: Pass a fd to perf_file_header__read_pipe()
  2021-07-19 22:31 [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output Namhyung Kim
  2021-07-19 22:31 ` [PATCH 1/5] perf tools: Remove repipe argument from perf_session__new() Namhyung Kim
@ 2021-07-19 22:31 ` Namhyung Kim
  2021-07-19 22:31 ` [PATCH 3/5] perf inject: Fix output from a pipe to a file Namhyung Kim
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2021-07-19 22:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen, Ian Rogers, Adrian Hunter

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 <namhyung@kernel.org>
---
 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 44249027507a..f280b3a38646 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.402.g57bb445576-goog


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/5] perf inject: Fix output from a pipe to a file
  2021-07-19 22:31 [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output Namhyung Kim
  2021-07-19 22:31 ` [PATCH 1/5] perf tools: Remove repipe argument from perf_session__new() Namhyung Kim
  2021-07-19 22:31 ` [PATCH 2/5] perf tools: Pass a fd to perf_file_header__read_pipe() Namhyung Kim
@ 2021-07-19 22:31 ` Namhyung Kim
  2021-07-19 22:31 ` [PATCH 4/5] perf inject: Fix output from a file to a pipe Namhyung Kim
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2021-07-19 22:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen, Ian Rogers, Adrian Hunter

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 <namhyung@kernel.org>
---
 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.402.g57bb445576-goog


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/5] perf inject: Fix output from a file to a pipe
  2021-07-19 22:31 [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output Namhyung Kim
                   ` (2 preceding siblings ...)
  2021-07-19 22:31 ` [PATCH 3/5] perf inject: Fix output from a pipe to a file Namhyung Kim
@ 2021-07-19 22:31 ` Namhyung Kim
  2021-07-19 22:31 ` [PATCH 5/5] perf tools: Add pipe_test.sh to verify pipe operations Namhyung Kim
  2021-07-20  9:01 ` [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output Jiri Olsa
  5 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2021-07-19 22:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen, Ian Rogers, Adrian Hunter

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 <namhyung@kernel.org>
---
 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 472cd12f10c6..548c1dbde6c5 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1387,7 +1387,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;
 
@@ -1395,41 +1394,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.402.g57bb445576-goog


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 5/5] perf tools: Add pipe_test.sh to verify pipe operations
  2021-07-19 22:31 [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output Namhyung Kim
                   ` (3 preceding siblings ...)
  2021-07-19 22:31 ` [PATCH 4/5] perf inject: Fix output from a file to a pipe Namhyung Kim
@ 2021-07-19 22:31 ` Namhyung Kim
  2021-07-20  9:01 ` [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output Jiri Olsa
  5 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2021-07-19 22:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen, Ian Rogers, Adrian Hunter

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 <namhyung@kernel.org>
---
 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..1b32b4f28391
--- /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 task-clock: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 task-clock: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 task-clock: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 task-clock: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.402.g57bb445576-goog


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output
  2021-07-19 22:31 [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output Namhyung Kim
                   ` (4 preceding siblings ...)
  2021-07-19 22:31 ` [PATCH 5/5] perf tools: Add pipe_test.sh to verify pipe operations Namhyung Kim
@ 2021-07-20  9:01 ` Jiri Olsa
  2021-08-02 13:16   ` Arnaldo Carvalho de Melo
  5 siblings, 1 reply; 9+ messages in thread
From: Jiri Olsa @ 2021-07-20  9:01 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, LKML,
	Andi Kleen, Ian Rogers, Adrian Hunter

On Mon, Jul 19, 2021 at 03:31:48PM -0700, Namhyung Kim wrote:
> 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 v3
>   - use task-clock:u in the pipe-test.sh

Acked-by: Jiri Olsa <jolsa@redhat.com>

thanks,
jirka

> 
>  * 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.402.g57bb445576-goog
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output
  2021-07-20  9:01 ` [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output Jiri Olsa
@ 2021-08-02 13:16   ` Arnaldo Carvalho de Melo
  2021-08-02 16:33     ` Namhyung Kim
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-08-02 13:16 UTC (permalink / raw)
  To: Namhyung Kim, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen, Ian Rogers, Adrian Hunter

Em Tue, Jul 20, 2021 at 11:01:43AM +0200, Jiri Olsa escreveu:
> On Mon, Jul 19, 2021 at 03:31:48PM -0700, Namhyung Kim wrote:
> > 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 v3
> >   - use task-clock:u in the pipe-test.sh
 
> Acked-by: Jiri Olsa <jolsa@redhat.com>

Thanks, applied.

Had to do some adjustments due to minor conflicts, can you please check
tmp.perf/core?

- Arnaldo
 
> thanks,
> jirka
> 
> > 
> >  * 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.402.g57bb445576-goog
> > 
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output
  2021-08-02 13:16   ` Arnaldo Carvalho de Melo
@ 2021-08-02 16:33     ` Namhyung Kim
  0 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2021-08-02 16:33 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen,
	Ian Rogers, Adrian Hunter

Hi Arnaldo,

On Mon, Aug 2, 2021 at 6:16 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> Em Tue, Jul 20, 2021 at 11:01:43AM +0200, Jiri Olsa escreveu:
> > On Mon, Jul 19, 2021 at 03:31:48PM -0700, Namhyung Kim wrote:
> > > 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 v3
> > >   - use task-clock:u in the pipe-test.sh
>
> > Acked-by: Jiri Olsa <jolsa@redhat.com>
>
> Thanks, applied.
>
> Had to do some adjustments due to minor conflicts, can you please check
> tmp.perf/core?

Thanks, it looks good to me!
Namhyung

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-08-02 16:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-19 22:31 [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output Namhyung Kim
2021-07-19 22:31 ` [PATCH 1/5] perf tools: Remove repipe argument from perf_session__new() Namhyung Kim
2021-07-19 22:31 ` [PATCH 2/5] perf tools: Pass a fd to perf_file_header__read_pipe() Namhyung Kim
2021-07-19 22:31 ` [PATCH 3/5] perf inject: Fix output from a pipe to a file Namhyung Kim
2021-07-19 22:31 ` [PATCH 4/5] perf inject: Fix output from a file to a pipe Namhyung Kim
2021-07-19 22:31 ` [PATCH 5/5] perf tools: Add pipe_test.sh to verify pipe operations Namhyung Kim
2021-07-20  9:01 ` [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output Jiri Olsa
2021-08-02 13:16   ` Arnaldo Carvalho de Melo
2021-08-02 16:33     ` Namhyung Kim

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).