2021-07-20 00:24:42

by Namhyung Kim

[permalink] [raw]
Subject: [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output

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


2021-07-20 00:28:56

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 2/5] perf tools: Pass a fd to perf_file_header__read_pipe()

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

2021-07-20 00:29:21

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 5/5] perf tools: Add pipe_test.sh to verify pipe operations

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

2021-07-20 00:30:56

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 3/5] perf inject: Fix output from a pipe to a file

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

2021-07-20 00:30:56

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 4/5] perf inject: Fix output from a file to a pipe

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

2021-07-20 09:09:34

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output

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 <[email protected]>

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
>

2021-08-02 13:18:43

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output

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 <[email protected]>

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

2021-08-02 16:35:19

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCHSET v3 0/5] perf inject: Fix broken data with mixed input/output

Hi Arnaldo,

On Mon, Aug 2, 2021 at 6:16 AM Arnaldo Carvalho de Melo <[email protected]> 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 <[email protected]>
>
> 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