From: Jiri Olsa <[email protected]>
Ian came with the idea of having support to read the pipe
data also from file [1]. Currently pipe mode files fails
like:
$ perf record -o - sleep 1 > /tmp/perf.pipe.data
$ perf report -i /tmp/perf.pipe.data
incompatible file format (rerun with -v to learn more)
This patch adds the support to do that by trying the pipe
header first, and if its successfully detected, switching
the perf data to pipe mode.
[1] https://lore.kernel.org/lkml/[email protected]/
Original-patch-by: Ian Rogers <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/header.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 0ce47283a8a1..8ca709f938b8 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3574,7 +3574,7 @@ static int perf_header__read_pipe(struct perf_session *session)
return -EINVAL;
}
- return 0;
+ return f_header.size == sizeof(f_header) ? 0 : -1;
}
static int read_attr(int fd, struct perf_header *ph,
@@ -3676,7 +3676,7 @@ int perf_session__read_header(struct perf_session *session)
struct perf_file_header f_header;
struct perf_file_attr f_attr;
u64 f_id;
- int nr_attrs, nr_ids, i, j;
+ int nr_attrs, nr_ids, i, j, err;
int fd = perf_data__fd(data);
session->evlist = evlist__new();
@@ -3685,8 +3685,16 @@ int perf_session__read_header(struct perf_session *session)
session->evlist->env = &header->env;
session->machines.host.env = &header->env;
- if (perf_data__is_pipe(data))
- return perf_header__read_pipe(session);
+
+ /*
+ * We can read 'pipe' data event from regular file,
+ * check for the pipe header regardless of source.
+ */
+ err = perf_header__read_pipe(session);
+ if (!err || (err && perf_data__is_pipe(data))) {
+ data->is_pipe = true;
+ return err;
+ }
if (perf_file_header__read(&f_header, header, fd) < 0)
return -EINVAL;
--
2.25.4
On Fri, May 1, 2020 at 4:35 AM Jiri Olsa <[email protected]> wrote:
>
> From: Jiri Olsa <[email protected]>
>
> Ian came with the idea of having support to read the pipe
> data also from file [1]. Currently pipe mode files fails
> like:
>
> $ perf record -o - sleep 1 > /tmp/perf.pipe.data
> $ perf report -i /tmp/perf.pipe.data
> incompatible file format (rerun with -v to learn more)
>
> This patch adds the support to do that by trying the pipe
> header first, and if its successfully detected, switching
nit: s/its/it's/
> the perf data to pipe mode.
>
> [1] https://lore.kernel.org/lkml/[email protected]/
> Original-patch-by: Ian Rogers <[email protected]>
> Signed-off-by: Jiri Olsa <[email protected]>
Tested and works great, thanks!
Reviewed-by: Ian Rogers <[email protected]>
Thanks,
Ian
> ---
> tools/perf/util/header.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 0ce47283a8a1..8ca709f938b8 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -3574,7 +3574,7 @@ static int perf_header__read_pipe(struct perf_session *session)
> return -EINVAL;
> }
>
> - return 0;
> + return f_header.size == sizeof(f_header) ? 0 : -1;
> }
>
> static int read_attr(int fd, struct perf_header *ph,
> @@ -3676,7 +3676,7 @@ int perf_session__read_header(struct perf_session *session)
> struct perf_file_header f_header;
> struct perf_file_attr f_attr;
> u64 f_id;
> - int nr_attrs, nr_ids, i, j;
> + int nr_attrs, nr_ids, i, j, err;
> int fd = perf_data__fd(data);
>
> session->evlist = evlist__new();
> @@ -3685,8 +3685,16 @@ int perf_session__read_header(struct perf_session *session)
>
> session->evlist->env = &header->env;
> session->machines.host.env = &header->env;
> - if (perf_data__is_pipe(data))
> - return perf_header__read_pipe(session);
> +
> + /*
> + * We can read 'pipe' data event from regular file,
> + * check for the pipe header regardless of source.
> + */
> + err = perf_header__read_pipe(session);
> + if (!err || (err && perf_data__is_pipe(data))) {
> + data->is_pipe = true;
> + return err;
> + }
>
> if (perf_file_header__read(&f_header, header, fd) < 0)
> return -EINVAL;
> --
> 2.25.4
>
On Fri, May 01, 2020 at 01:34:47PM +0200, Jiri Olsa wrote:
> From: Jiri Olsa <[email protected]>
>
> Ian came with the idea of having support to read the pipe
> data also from file [1]. Currently pipe mode files fails
> like:
>
> $ perf record -o - sleep 1 > /tmp/perf.pipe.data
> $ perf report -i /tmp/perf.pipe.data
> incompatible file format (rerun with -v to learn more)
>
> This patch adds the support to do that by trying the pipe
> header first, and if its successfully detected, switching
> the perf data to pipe mode.
>
> [1] https://lore.kernel.org/lkml/[email protected]/
> Original-patch-by: Ian Rogers <[email protected]>
> Signed-off-by: Jiri Olsa <[email protected]>
actualy.. I found another issue while trying this on tracepoints:
# ./perf record -g -e 'raw_syscalls:sys_enter' -o - true > data
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.000 MB - ]
# ./perf script -i ./data
perf_event__process_tracing_data: tracing data size mismatch0x1034 [0xc]: failed to process type: 66
it's because some of the pipe synthesize code calls lseek, which
fails on pipe, but succeeds on normal file (with pipe data)
patch below fixes that for me, but I wonder there are other leftovers
like this.. I'll check on post it all together
jirka
---
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 8ca709f938b8..33e299674121 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3955,13 +3955,8 @@ int perf_event__process_tracing_data(struct perf_session *session,
{
ssize_t size_read, padding, size = event->tracing_data.size;
int fd = perf_data__fd(session->data);
- off_t offset = lseek(fd, 0, SEEK_CUR);
char buf[BUFSIZ];
- /* setup for reading amidst mmap */
- lseek(fd, offset + sizeof(struct perf_record_header_tracing_data),
- SEEK_SET);
-
size_read = trace_report(fd, &session->tevent,
session->repipe);
padding = PERF_ALIGN(size_read, sizeof(u64)) - size_read;
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index c11d89e0ee55..b75df19feaf1 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1543,7 +1543,8 @@ static s64 perf_session__process_user_event(struct perf_session *session,
return 0;
case PERF_RECORD_HEADER_TRACING_DATA:
/* setup for reading amidst mmap */
- lseek(fd, file_offset, SEEK_SET);
+ if (!perf_data__is_pipe(session->data))
+ lseek(fd, file_offset, SEEK_SET);
return tool->tracing_data(session, event);
case PERF_RECORD_HEADER_BUILD_ID:
return tool->build_id(session, event);
On Mon, May 4, 2020 at 3:57 PM Jiri Olsa <[email protected]> wrote:
>
> On Fri, May 01, 2020 at 01:34:47PM +0200, Jiri Olsa wrote:
> > From: Jiri Olsa <[email protected]>
> >
> > Ian came with the idea of having support to read the pipe
> > data also from file [1]. Currently pipe mode files fails
> > like:
> >
> > $ perf record -o - sleep 1 > /tmp/perf.pipe.data
> > $ perf report -i /tmp/perf.pipe.data
> > incompatible file format (rerun with -v to learn more)
> >
> > This patch adds the support to do that by trying the pipe
> > header first, and if its successfully detected, switching
> > the perf data to pipe mode.
> >
> > [1] https://lore.kernel.org/lkml/[email protected]/
> > Original-patch-by: Ian Rogers <[email protected]>
> > Signed-off-by: Jiri Olsa <[email protected]>
>
> actualy.. I found another issue while trying this on tracepoints:
>
> # ./perf record -g -e 'raw_syscalls:sys_enter' -o - true > data
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.000 MB - ]
> # ./perf script -i ./data
> perf_event__process_tracing_data: tracing data size mismatch0x1034 [0xc]: failed to process type: 66
>
> it's because some of the pipe synthesize code calls lseek, which
> fails on pipe, but succeeds on normal file (with pipe data)
>
> patch below fixes that for me, but I wonder there are other leftovers
> like this.. I'll check on post it all together
Thanks for testing! I wonder in the 2nd case whether a comment as to
why the seek isn't needed in pipe mode would be useful.
Ian
> jirka
>
>
> ---
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 8ca709f938b8..33e299674121 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -3955,13 +3955,8 @@ int perf_event__process_tracing_data(struct perf_session *session,
> {
> ssize_t size_read, padding, size = event->tracing_data.size;
> int fd = perf_data__fd(session->data);
> - off_t offset = lseek(fd, 0, SEEK_CUR);
> char buf[BUFSIZ];
>
> - /* setup for reading amidst mmap */
> - lseek(fd, offset + sizeof(struct perf_record_header_tracing_data),
> - SEEK_SET);
> -
> size_read = trace_report(fd, &session->tevent,
> session->repipe);
> padding = PERF_ALIGN(size_read, sizeof(u64)) - size_read;
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index c11d89e0ee55..b75df19feaf1 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1543,7 +1543,8 @@ static s64 perf_session__process_user_event(struct perf_session *session,
> return 0;
> case PERF_RECORD_HEADER_TRACING_DATA:
> /* setup for reading amidst mmap */
> - lseek(fd, file_offset, SEEK_SET);
> + if (!perf_data__is_pipe(session->data))
> + lseek(fd, file_offset, SEEK_SET);
> return tool->tracing_data(session, event);
> case PERF_RECORD_HEADER_BUILD_ID:
> return tool->build_id(session, event);
>