2020-12-30 11:12:18

by Jiri Olsa

Subject: [PATCHv2] perf tools: Detect when pipe is passed as perf data

Currently we allow pipe input/output only through '-' string
being passed to '-o' or '-i' options, like:

# mkfifo perf.pipe
# perf record --no-buffering -e 'sched:sched_switch' -o - > perf.pipe &
[1] 354406
# cat perf.pipe | ./perf --no-pager script -i - | head -3
perf 354406 [000] 168190.164921: sched:sched_switch: perf:354406..
migration/0 12 [000] 168190.164928: sched:sched_switch: migration/0:..
perf 354406 [001] 168190.164981: sched:sched_switch: perf:354406..
...

This patch detects if given path is pipe and set the perf data
object accordingly, so it's possible now to do above with:

# mkfifo perf.pipe
# perf record --no-buffering -e 'sched:sched_switch' -o perf.pipe &
[1] 360188
# perf --no-pager script -i ./perf.pipe | head -3
perf 354442 [000] 168275.464895: sched:sched_switch: perf:354442..
migration/0 12 [000] 168275.464902: sched:sched_switch: migration/0:..
perf 354442 [001] 168275.464953: sched:sched_switch: perf:354442..

It's of course possible to combine any of above ways.

Signed-off-by: Jiri Olsa <[email protected]>
---
v2:
- removed O_CREAT|O_TRUNC flags from pipe's write end

tools/perf/util/data.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
index f29af4fc3d09..4dfa9e0f2fec 100644
--- a/tools/perf/util/data.c
+++ b/tools/perf/util/data.c
@@ -159,7 +159,7 @@ int perf_data__update_dir(struct perf_data *data)
return 0;
}

-static bool check_pipe(struct perf_data *data)
+static int check_pipe(struct perf_data *data)
{
struct stat st;
bool is_pipe = false;
@@ -172,6 +172,15 @@ static bool check_pipe(struct perf_data *data)
} else {
if (!strcmp(data->path, "-"))
is_pipe = true;
+ else if (!stat(data->path, &st) && S_ISFIFO(st.st_mode)) {
+ int flags = perf_data__is_read(data) ?
+ O_RDONLY : O_WRONLY;
+
+ fd = open(data->path, flags);
+ if (fd < 0)
+ return -EINVAL;
+ is_pipe = true;
+ }
}

if (is_pipe) {
@@ -190,7 +199,8 @@ static bool check_pipe(struct perf_data *data)
}
}

- return data->is_pipe = is_pipe;
+ data->is_pipe = is_pipe;
+ return 0;
}

static int check_backup(struct perf_data *data)
@@ -344,8 +354,11 @@ static int open_dir(struct perf_data *data)

int perf_data__open(struct perf_data *data)
{
- if (check_pipe(data))
- return 0;
+ int err;
+
+ err = check_pipe(data);
+ if (err || data->is_pipe)
+ return err;

/* currently it allows stdio for pipe only */
data->use_stdio = false;
@@ -410,8 +423,10 @@ int perf_data__switch(struct perf_data *data,
{
int ret;

- if (check_pipe(data))
- return -EINVAL;
+ ret = check_pipe(data);
+ if (ret || data->is_pipe)
+ return ret;
+
if (perf_data__is_read(data))
return -EINVAL;

--
2.26.2

2021-01-06 01:36:12

by Stephane Eranian

Subject: Re: [PATCHv2] perf tools: Detect when pipe is passed as perf data

Hi,

On Wed, Dec 30, 2020 at 3:09 AM Jiri Olsa <[email protected]> wrote:
>
> Currently we allow pipe input/output only through '-' string
> being passed to '-o' or '-i' options, like:
>
It seems to me it would be useful to auto-detect that the perf.data
file is in pipe vs. file mode format.
Your patch detects the type of the file which is something different
from the format of its content.
Thanks.

> # mkfifo perf.pipe
> # perf record --no-buffering -e 'sched:sched_switch' -o - > perf.pipe &
> [1] 354406
> # cat perf.pipe | ./perf --no-pager script -i - | head -3
> perf 354406 [000] 168190.164921: sched:sched_switch: perf:354406..
> migration/0 12 [000] 168190.164928: sched:sched_switch: migration/0:..
> perf 354406 [001] 168190.164981: sched:sched_switch: perf:354406..
> ...
>
> This patch detects if given path is pipe and set the perf data
> object accordingly, so it's possible now to do above with:
>
> # mkfifo perf.pipe
> # perf record --no-buffering -e 'sched:sched_switch' -o perf.pipe &
> [1] 360188
> # perf --no-pager script -i ./perf.pipe | head -3
> perf 354442 [000] 168275.464895: sched:sched_switch: perf:354442..
> migration/0 12 [000] 168275.464902: sched:sched_switch: migration/0:..
> perf 354442 [001] 168275.464953: sched:sched_switch: perf:354442..
>
> It's of course possible to combine any of above ways.
>
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
> v2:
> - removed O_CREAT|O_TRUNC flags from pipe's write end
>
> tools/perf/util/data.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> index f29af4fc3d09..4dfa9e0f2fec 100644
> --- a/tools/perf/util/data.c
> +++ b/tools/perf/util/data.c
> @@ -159,7 +159,7 @@ int perf_data__update_dir(struct perf_data *data)
> return 0;
> }
>
> -static bool check_pipe(struct perf_data *data)
> +static int check_pipe(struct perf_data *data)
> {
> struct stat st;
> bool is_pipe = false;
> @@ -172,6 +172,15 @@ static bool check_pipe(struct perf_data *data)
> } else {
> if (!strcmp(data->path, "-"))
> is_pipe = true;
> + else if (!stat(data->path, &st) && S_ISFIFO(st.st_mode)) {
> + int flags = perf_data__is_read(data) ?
> + O_RDONLY : O_WRONLY;
> +
> + fd = open(data->path, flags);
> + if (fd < 0)
> + return -EINVAL;
> + is_pipe = true;
> + }
> }
>
> if (is_pipe) {
> @@ -190,7 +199,8 @@ static bool check_pipe(struct perf_data *data)
> }
> }
>
> - return data->is_pipe = is_pipe;
> + data->is_pipe = is_pipe;
> + return 0;
> }
>
> static int check_backup(struct perf_data *data)
> @@ -344,8 +354,11 @@ static int open_dir(struct perf_data *data)
>
> int perf_data__open(struct perf_data *data)
> {
> - if (check_pipe(data))
> - return 0;
> + int err;
> +
> + err = check_pipe(data);
> + if (err || data->is_pipe)
> + return err;
>
> /* currently it allows stdio for pipe only */
> data->use_stdio = false;
> @@ -410,8 +423,10 @@ int perf_data__switch(struct perf_data *data,
> {
> int ret;
>
> - if (check_pipe(data))
> - return -EINVAL;
> + ret = check_pipe(data);
> + if (ret || data->is_pipe)
> + return ret;
> +
> if (perf_data__is_read(data))
> return -EINVAL;
>
> --
> 2.26.2
>

2021-01-06 09:52:37

by Jiri Olsa

Subject: Re: [PATCHv2] perf tools: Detect when pipe is passed as perf data

On Tue, Jan 05, 2021 at 05:33:38PM -0800, Stephane Eranian wrote:
> Hi,
>
> On Wed, Dec 30, 2020 at 3:09 AM Jiri Olsa <[email protected]> wrote:
> >
> > Currently we allow pipe input/output only through '-' string
> > being passed to '-o' or '-i' options, like:
> >
> It seems to me it would be useful to auto-detect that the perf.data
> file is in pipe vs. file mode format.
> Your patch detects the type of the file which is something different
> from the format of its content.

hi,
it goes together with the format, once the output file
is pipe, the format is pipe as well

jirka

> Thanks.
>
> > # mkfifo perf.pipe
> > # perf record --no-buffering -e 'sched:sched_switch' -o - > perf.pipe &
> > [1] 354406
> > # cat perf.pipe | ./perf --no-pager script -i - | head -3
> > perf 354406 [000] 168190.164921: sched:sched_switch: perf:354406..
> > migration/0 12 [000] 168190.164928: sched:sched_switch: migration/0:..
> > perf 354406 [001] 168190.164981: sched:sched_switch: perf:354406..
> > ...
> >
> > This patch detects if given path is pipe and set the perf data
> > object accordingly, so it's possible now to do above with:
> >
> > # mkfifo perf.pipe
> > # perf record --no-buffering -e 'sched:sched_switch' -o perf.pipe &
> > [1] 360188
> > # perf --no-pager script -i ./perf.pipe | head -3
> > perf 354442 [000] 168275.464895: sched:sched_switch: perf:354442..
> > migration/0 12 [000] 168275.464902: sched:sched_switch: migration/0:..
> > perf 354442 [001] 168275.464953: sched:sched_switch: perf:354442..
> >
> > It's of course possible to combine any of above ways.
> >
> > Signed-off-by: Jiri Olsa <[email protected]>
> > ---
> > v2:
> > - removed O_CREAT|O_TRUNC flags from pipe's write end
> >
> > tools/perf/util/data.c | 27 +++++++++++++++++++++------
> > 1 file changed, 21 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> > index f29af4fc3d09..4dfa9e0f2fec 100644
> > --- a/tools/perf/util/data.c
> > +++ b/tools/perf/util/data.c
> > @@ -159,7 +159,7 @@ int perf_data__update_dir(struct perf_data *data)
> > return 0;
> > }
> >
> > -static bool check_pipe(struct perf_data *data)
> > +static int check_pipe(struct perf_data *data)
> > {
> > struct stat st;
> > bool is_pipe = false;
> > @@ -172,6 +172,15 @@ static bool check_pipe(struct perf_data *data)
> > } else {
> > if (!strcmp(data->path, "-"))
> > is_pipe = true;
> > + else if (!stat(data->path, &st) && S_ISFIFO(st.st_mode)) {
> > + int flags = perf_data__is_read(data) ?
> > + O_RDONLY : O_WRONLY;
> > +
> > + fd = open(data->path, flags);
> > + if (fd < 0)
> > + return -EINVAL;
> > + is_pipe = true;
> > + }
> > }
> >
> > if (is_pipe) {
> > @@ -190,7 +199,8 @@ static bool check_pipe(struct perf_data *data)
> > }
> > }
> >
> > - return data->is_pipe = is_pipe;
> > + data->is_pipe = is_pipe;
> > + return 0;
> > }
> >
> > static int check_backup(struct perf_data *data)
> > @@ -344,8 +354,11 @@ static int open_dir(struct perf_data *data)
> >
> > int perf_data__open(struct perf_data *data)
> > {
> > - if (check_pipe(data))
> > - return 0;
> > + int err;
> > +
> > + err = check_pipe(data);
> > + if (err || data->is_pipe)
> > + return err;
> >
> > /* currently it allows stdio for pipe only */
> > data->use_stdio = false;
> > @@ -410,8 +423,10 @@ int perf_data__switch(struct perf_data *data,
> > {
> > int ret;
> >
> > - if (check_pipe(data))
> > - return -EINVAL;
> > + ret = check_pipe(data);
> > + if (ret || data->is_pipe)
> > + return ret;
> > +
> > if (perf_data__is_read(data))
> > return -EINVAL;
> >
> > --
> > 2.26.2
> >
>

2021-01-11 07:29:43

by Stephane Eranian

Subject: Re: [PATCHv2] perf tools: Detect when pipe is passed as perf data

On Wed, Jan 6, 2021 at 1:49 AM Jiri Olsa <[email protected]> wrote:
>
> On Tue, Jan 05, 2021 at 05:33:38PM -0800, Stephane Eranian wrote:
> > Hi,
> >
> > On Wed, Dec 30, 2020 at 3:09 AM Jiri Olsa <[email protected]> wrote:
> > >
> > > Currently we allow pipe input/output only through '-' string
> > > being passed to '-o' or '-i' options, like:
> > >
> > It seems to me it would be useful to auto-detect that the perf.data
> > file is in pipe vs. file mode format.
> > Your patch detects the type of the file which is something different
> > from the format of its content.
>
> hi,
> it goes together with the format, once the output file
> is pipe, the format is pipe as well
>
What I was saying is if I do:
$ perf record -o - -a sleep 10 > perf.data
$ perf report -i perf.data
it should autodetect it is a pipe mode file.
Does it do that today?

> jirka
>
> > Thanks.
> >
> > > # mkfifo perf.pipe
> > > # perf record --no-buffering -e 'sched:sched_switch' -o - > perf.pipe &
> > > [1] 354406
> > > # cat perf.pipe | ./perf --no-pager script -i - | head -3
> > > perf 354406 [000] 168190.164921: sched:sched_switch: perf:354406..
> > > migration/0 12 [000] 168190.164928: sched:sched_switch: migration/0:..
> > > perf 354406 [001] 168190.164981: sched:sched_switch: perf:354406..
> > > ...
> > >
> > > This patch detects if given path is pipe and set the perf data
> > > object accordingly, so it's possible now to do above with:
> > >
> > > # mkfifo perf.pipe
> > > # perf record --no-buffering -e 'sched:sched_switch' -o perf.pipe &
> > > [1] 360188
> > > # perf --no-pager script -i ./perf.pipe | head -3
> > > perf 354442 [000] 168275.464895: sched:sched_switch: perf:354442..
> > > migration/0 12 [000] 168275.464902: sched:sched_switch: migration/0:..
> > > perf 354442 [001] 168275.464953: sched:sched_switch: perf:354442..
> > >
> > > It's of course possible to combine any of above ways.
> > >
> > > Signed-off-by: Jiri Olsa <[email protected]>
> > > ---
> > > v2:
> > > - removed O_CREAT|O_TRUNC flags from pipe's write end
> > >
> > > tools/perf/util/data.c | 27 +++++++++++++++++++++------
> > > 1 file changed, 21 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> > > index f29af4fc3d09..4dfa9e0f2fec 100644
> > > --- a/tools/perf/util/data.c
> > > +++ b/tools/perf/util/data.c
> > > @@ -159,7 +159,7 @@ int perf_data__update_dir(struct perf_data *data)
> > > return 0;
> > > }
> > >
> > > -static bool check_pipe(struct perf_data *data)
> > > +static int check_pipe(struct perf_data *data)
> > > {
> > > struct stat st;
> > > bool is_pipe = false;
> > > @@ -172,6 +172,15 @@ static bool check_pipe(struct perf_data *data)
> > > } else {
> > > if (!strcmp(data->path, "-"))
> > > is_pipe = true;
> > > + else if (!stat(data->path, &st) && S_ISFIFO(st.st_mode)) {
> > > + int flags = perf_data__is_read(data) ?
> > > + O_RDONLY : O_WRONLY;
> > > +
> > > + fd = open(data->path, flags);
> > > + if (fd < 0)
> > > + return -EINVAL;
> > > + is_pipe = true;
> > > + }
> > > }
> > >
> > > if (is_pipe) {
> > > @@ -190,7 +199,8 @@ static bool check_pipe(struct perf_data *data)
> > > }
> > > }
> > >
> > > - return data->is_pipe = is_pipe;
> > > + data->is_pipe = is_pipe;
> > > + return 0;
> > > }
> > >
> > > static int check_backup(struct perf_data *data)
> > > @@ -344,8 +354,11 @@ static int open_dir(struct perf_data *data)
> > >
> > > int perf_data__open(struct perf_data *data)
> > > {
> > > - if (check_pipe(data))
> > > - return 0;
> > > + int err;
> > > +
> > > + err = check_pipe(data);
> > > + if (err || data->is_pipe)
> > > + return err;
> > >
> > > /* currently it allows stdio for pipe only */
> > > data->use_stdio = false;
> > > @@ -410,8 +423,10 @@ int perf_data__switch(struct perf_data *data,
> > > {
> > > int ret;
> > >
> > > - if (check_pipe(data))
> > > - return -EINVAL;
> > > + ret = check_pipe(data);
> > > + if (ret || data->is_pipe)
> > > + return ret;
> > > +
> > > if (perf_data__is_read(data))
> > > return -EINVAL;
> > >
> > > --
> > > 2.26.2
> > >
> >
>

2021-01-11 11:01:16

by Jiri Olsa

Subject: Re: [PATCHv2] perf tools: Detect when pipe is passed as perf data

On Sun, Jan 10, 2021 at 11:13:18PM -0800, Stephane Eranian wrote:
> On Wed, Jan 6, 2021 at 1:49 AM Jiri Olsa <[email protected]> wrote:
> >
> > On Tue, Jan 05, 2021 at 05:33:38PM -0800, Stephane Eranian wrote:
> > > Hi,
> > >
> > > On Wed, Dec 30, 2020 at 3:09 AM Jiri Olsa <[email protected]> wrote:
> > > >
> > > > Currently we allow pipe input/output only through '-' string
> > > > being passed to '-o' or '-i' options, like:
> > > >
> > > It seems to me it would be useful to auto-detect that the perf.data
> > > file is in pipe vs. file mode format.
> > > Your patch detects the type of the file which is something different
> > > from the format of its content.
> >
> > hi,
> > it goes together with the format, once the output file
> > is pipe, the format is pipe as well
> >
> What I was saying is if I do:
> $ perf record -o - -a sleep 10 > perf.data
> $ perf report -i perf.data
> it should autodetect it is a pipe mode file.
> Does it do that today?

yes, your record stores pipe mode data to perf.data file,
and report detects pipe mode data even in normal file,
so we're fine here

jirka

>
> > jirka
> >
> > > Thanks.
> > >
> > > > # mkfifo perf.pipe
> > > > # perf record --no-buffering -e 'sched:sched_switch' -o - > perf.pipe &
> > > > [1] 354406
> > > > # cat perf.pipe | ./perf --no-pager script -i - | head -3
> > > > perf 354406 [000] 168190.164921: sched:sched_switch: perf:354406..
> > > > migration/0 12 [000] 168190.164928: sched:sched_switch: migration/0:..
> > > > perf 354406 [001] 168190.164981: sched:sched_switch: perf:354406..
> > > > ...
> > > >
> > > > This patch detects if given path is pipe and set the perf data
> > > > object accordingly, so it's possible now to do above with:
> > > >
> > > > # mkfifo perf.pipe
> > > > # perf record --no-buffering -e 'sched:sched_switch' -o perf.pipe &
> > > > [1] 360188
> > > > # perf --no-pager script -i ./perf.pipe | head -3
> > > > perf 354442 [000] 168275.464895: sched:sched_switch: perf:354442..
> > > > migration/0 12 [000] 168275.464902: sched:sched_switch: migration/0:..
> > > > perf 354442 [001] 168275.464953: sched:sched_switch: perf:354442..
> > > >
> > > > It's of course possible to combine any of above ways.
> > > >
> > > > Signed-off-by: Jiri Olsa <[email protected]>
> > > > ---
> > > > v2:
> > > > - removed O_CREAT|O_TRUNC flags from pipe's write end
> > > >
> > > > tools/perf/util/data.c | 27 +++++++++++++++++++++------
> > > > 1 file changed, 21 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> > > > index f29af4fc3d09..4dfa9e0f2fec 100644
> > > > --- a/tools/perf/util/data.c
> > > > +++ b/tools/perf/util/data.c
> > > > @@ -159,7 +159,7 @@ int perf_data__update_dir(struct perf_data *data)
> > > > return 0;
> > > > }
> > > >
> > > > -static bool check_pipe(struct perf_data *data)
> > > > +static int check_pipe(struct perf_data *data)
> > > > {
> > > > struct stat st;
> > > > bool is_pipe = false;
> > > > @@ -172,6 +172,15 @@ static bool check_pipe(struct perf_data *data)
> > > > } else {
> > > > if (!strcmp(data->path, "-"))
> > > > is_pipe = true;
> > > > + else if (!stat(data->path, &st) && S_ISFIFO(st.st_mode)) {
> > > > + int flags = perf_data__is_read(data) ?
> > > > + O_RDONLY : O_WRONLY;
> > > > +
> > > > + fd = open(data->path, flags);
> > > > + if (fd < 0)
> > > > + return -EINVAL;
> > > > + is_pipe = true;
> > > > + }
> > > > }
> > > >
> > > > if (is_pipe) {
> > > > @@ -190,7 +199,8 @@ static bool check_pipe(struct perf_data *data)
> > > > }
> > > > }
> > > >
> > > > - return data->is_pipe = is_pipe;
> > > > + data->is_pipe = is_pipe;
> > > > + return 0;
> > > > }
> > > >
> > > > static int check_backup(struct perf_data *data)
> > > > @@ -344,8 +354,11 @@ static int open_dir(struct perf_data *data)
> > > >
> > > > int perf_data__open(struct perf_data *data)
> > > > {
> > > > - if (check_pipe(data))
> > > > - return 0;
> > > > + int err;
> > > > +
> > > > + err = check_pipe(data);
> > > > + if (err || data->is_pipe)
> > > > + return err;
> > > >
> > > > /* currently it allows stdio for pipe only */
> > > > data->use_stdio = false;
> > > > @@ -410,8 +423,10 @@ int perf_data__switch(struct perf_data *data,
> > > > {
> > > > int ret;
> > > >
> > > > - if (check_pipe(data))
> > > > - return -EINVAL;
> > > > + ret = check_pipe(data);
> > > > + if (ret || data->is_pipe)
> > > > + return ret;
> > > > +
> > > > if (perf_data__is_read(data))
> > > > return -EINVAL;
> > > >
> > > > --
> > > > 2.26.2
> > > >
> > >
> >
>