* [PATCH] perf session: Try to read pipe data from file
@ 2020-05-01 11:34 Jiri Olsa
2020-05-01 17:49 ` Ian Rogers
2020-05-04 22:57 ` Jiri Olsa
0 siblings, 2 replies; 4+ messages in thread
From: Jiri Olsa @ 2020-05-01 11:34 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
Peter Zijlstra, Michael Petlan
From: Jiri Olsa <jolsa@redhat.com>
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/20200409185744.255881-1-irogers@google.com/
Original-patch-by: Ian Rogers <irogers@google.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
| 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
--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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] perf session: Try to read pipe data from file
2020-05-01 11:34 [PATCH] perf session: Try to read pipe data from file Jiri Olsa
@ 2020-05-01 17:49 ` Ian Rogers
2020-05-04 22:57 ` Jiri Olsa
1 sibling, 0 replies; 4+ messages in thread
From: Ian Rogers @ 2020-05-01 17:49 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Michael Petlan
On Fri, May 1, 2020 at 4:35 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> From: Jiri Olsa <jolsa@redhat.com>
>
> 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/20200409185744.255881-1-irogers@google.com/
> Original-patch-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested and works great, thanks!
Reviewed-by: Ian Rogers <irogers@google.com>
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] perf session: Try to read pipe data from file
2020-05-01 11:34 [PATCH] perf session: Try to read pipe data from file Jiri Olsa
2020-05-01 17:49 ` Ian Rogers
@ 2020-05-04 22:57 ` Jiri Olsa
2020-05-04 23:27 ` Ian Rogers
1 sibling, 1 reply; 4+ messages in thread
From: Jiri Olsa @ 2020-05-04 22:57 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Ian Rogers, lkml, Ingo Molnar,
Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan
On Fri, May 01, 2020 at 01:34:47PM +0200, Jiri Olsa wrote:
> From: Jiri Olsa <jolsa@redhat.com>
>
> 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/20200409185744.255881-1-irogers@google.com/
> Original-patch-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
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);
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] perf session: Try to read pipe data from file
2020-05-04 22:57 ` Jiri Olsa
@ 2020-05-04 23:27 ` Ian Rogers
0 siblings, 0 replies; 4+ messages in thread
From: Ian Rogers @ 2020-05-04 23:27 UTC (permalink / raw)
To: Jiri Olsa
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Ingo Molnar,
Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan
On Mon, May 4, 2020 at 3:57 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Fri, May 01, 2020 at 01:34:47PM +0200, Jiri Olsa wrote:
> > From: Jiri Olsa <jolsa@redhat.com>
> >
> > 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/20200409185744.255881-1-irogers@google.com/
> > Original-patch-by: Ian Rogers <irogers@google.com>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>
> 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);
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-04 23:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-01 11:34 [PATCH] perf session: Try to read pipe data from file Jiri Olsa
2020-05-01 17:49 ` Ian Rogers
2020-05-04 22:57 ` Jiri Olsa
2020-05-04 23:27 ` Ian Rogers
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).