* [PATCHv2] perf tools: Detect when pipe is passed as perf data
@ 2020-12-30 11:09 Jiri Olsa
2021-01-06 1:33 ` Stephane Eranian
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2020-12-30 11:09 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
Alexei Budankov
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 <jolsa@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCHv2] perf tools: Detect when pipe is passed as perf data 2020-12-30 11:09 [PATCHv2] perf tools: Detect when pipe is passed as perf data Jiri Olsa @ 2021-01-06 1:33 ` Stephane Eranian 2021-01-06 9:49 ` Jiri Olsa 0 siblings, 1 reply; 5+ messages in thread From: Stephane Eranian @ 2021-01-06 1:33 UTC (permalink / raw) To: Jiri Olsa Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan, Ian Rogers, Alexei Budankov Hi, On Wed, Dec 30, 2020 at 3:09 AM Jiri Olsa <jolsa@kernel.org> 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 <jolsa@kernel.org> > --- > 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCHv2] perf tools: Detect when pipe is passed as perf data 2021-01-06 1:33 ` Stephane Eranian @ 2021-01-06 9:49 ` Jiri Olsa 2021-01-11 7:13 ` Stephane Eranian 0 siblings, 1 reply; 5+ messages in thread From: Jiri Olsa @ 2021-01-06 9:49 UTC (permalink / raw) To: Stephane Eranian Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan, Ian Rogers, Alexei Budankov 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 <jolsa@kernel.org> 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 <jolsa@kernel.org> > > --- > > 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 > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCHv2] perf tools: Detect when pipe is passed as perf data 2021-01-06 9:49 ` Jiri Olsa @ 2021-01-11 7:13 ` Stephane Eranian 2021-01-11 10:56 ` Jiri Olsa 0 siblings, 1 reply; 5+ messages in thread From: Stephane Eranian @ 2021-01-11 7:13 UTC (permalink / raw) To: Jiri Olsa Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan, Ian Rogers, Alexei Budankov On Wed, Jan 6, 2021 at 1:49 AM Jiri Olsa <jolsa@redhat.com> 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 <jolsa@kernel.org> 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 <jolsa@kernel.org> > > > --- > > > 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 > > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCHv2] perf tools: Detect when pipe is passed as perf data 2021-01-11 7:13 ` Stephane Eranian @ 2021-01-11 10:56 ` Jiri Olsa 0 siblings, 0 replies; 5+ messages in thread From: Jiri Olsa @ 2021-01-11 10:56 UTC (permalink / raw) To: Stephane Eranian Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan, Ian Rogers, Alexei Budankov On Sun, Jan 10, 2021 at 11:13:18PM -0800, Stephane Eranian wrote: > On Wed, Jan 6, 2021 at 1:49 AM Jiri Olsa <jolsa@redhat.com> 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 <jolsa@kernel.org> 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 <jolsa@kernel.org> > > > > --- > > > > 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 > > > > > > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-11 10:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-30 11:09 [PATCHv2] perf tools: Detect when pipe is passed as perf data Jiri Olsa 2021-01-06 1:33 ` Stephane Eranian 2021-01-06 9:49 ` Jiri Olsa 2021-01-11 7:13 ` Stephane Eranian 2021-01-11 10:56 ` Jiri Olsa
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.