* [BUG] perf record: pipe mode still broken
@ 2014-06-05 16:06 Stephane Eranian
2014-06-05 17:00 ` [PATCH] perf tools: Fix pipe check regression in attr event callback Jiri Olsa
0 siblings, 1 reply; 6+ messages in thread
From: Stephane Eranian @ 2014-06-05 16:06 UTC (permalink / raw)
To: LKML
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Namhyung Kim, David Ahern,
Peter Zijlstra, mingo
Hi Jiri,
Somehow, I thought you had written a fix to avoid the problem below.
But when I try with tip.git, the problem is still there.
Could you push your fix ASAP.
Thanks.
$ perf record -o - noploop 2 | perf inject -b | perf report -i -
# To display the perf.data header info, please use
--header/--header-only options.
#
noploop for 2 seconds
0x1bd0 [0x28]: failed to process type: 9
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] perf tools: Fix pipe check regression in attr event callback
2014-06-05 16:06 [BUG] perf record: pipe mode still broken Stephane Eranian
@ 2014-06-05 17:00 ` Jiri Olsa
2014-06-05 20:21 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Olsa @ 2014-06-05 17:00 UTC (permalink / raw)
To: Stephane Eranian
Cc: LKML, Arnaldo Carvalho de Melo, Namhyung Kim, David Ahern,
Peter Zijlstra, mingo
On Thu, Jun 05, 2014 at 06:06:41PM +0200, Stephane Eranian wrote:
> Hi Jiri,
>
> Somehow, I thought you had written a fix to avoid the problem below.
> But when I try with tip.git, the problem is still there.
> Could you push your fix ASAP.
>
> Thanks.
>
> $ perf record -o - noploop 2 | perf inject -b | perf report -i -
> # To display the perf.data header info, please use
> --header/--header-only options.
> #
> noploop for 2 seconds
> 0x1bd0 [0x28]: failed to process type: 9
hum, I remember fixing another issue.. this one is
separated one, please try attached patch.
thanks,
jirka
---
The file factoring in builtin-inject.c object introduced regression
in attr event callback. The commit is:
3406912 perf inject: Handle output file via perf_data_file object
Following hunk reversed the logic:
- if (!inject->pipe_output)
+ if (&inject->output.is_pipe)
putting it back, following example now works:
$ perf record -o - kill | perf inject -b | perf report -i -
Reported-by: Stephane Eranian <eranian@google.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/builtin-inject.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 6a3af00..664010b 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -72,7 +72,7 @@ static int perf_event__repipe_attr(struct perf_tool *tool,
if (ret)
return ret;
- if (&inject->output.is_pipe)
+ if (!&inject->output.is_pipe)
return 0;
return perf_event__repipe_synth(tool, event);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] perf tools: Fix pipe check regression in attr event callback
2014-06-05 17:00 ` [PATCH] perf tools: Fix pipe check regression in attr event callback Jiri Olsa
@ 2014-06-05 20:21 ` Arnaldo Carvalho de Melo
2014-06-05 20:41 ` [PATCHv2] " Jiri Olsa
0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-06-05 20:21 UTC (permalink / raw)
To: Jiri Olsa
Cc: Stephane Eranian, LKML, Namhyung Kim, David Ahern, Peter Zijlstra, mingo
Em Thu, Jun 05, 2014 at 07:00:11PM +0200, Jiri Olsa escreveu:
> On Thu, Jun 05, 2014 at 06:06:41PM +0200, Stephane Eranian wrote:
> > Hi Jiri,
> >
> > Somehow, I thought you had written a fix to avoid the problem below.
> > But when I try with tip.git, the problem is still there.
> > Could you push your fix ASAP.
> >
> > Thanks.
> >
> > $ perf record -o - noploop 2 | perf inject -b | perf report -i -
> > # To display the perf.data header info, please use
> > --header/--header-only options.
> > #
> > noploop for 2 seconds
> > 0x1bd0 [0x28]: failed to process type: 9
>
> hum, I remember fixing another issue.. this one is
> separated one, please try attached patch.
>
> thanks,
> jirka
>
>
> ---
> The file factoring in builtin-inject.c object introduced regression
> in attr event callback. The commit is:
> 3406912 perf inject: Handle output file via perf_data_file object
>
> Following hunk reversed the logic:
> - if (!inject->pipe_output)
> + if (&inject->output.is_pipe)
Why do we need this '&'? this will always evaluate to true, as it will
get the address of a variable, also adding a ! before it will just it
eval to false always, or am I missing something? :-)
I think this should really be:
if (!inject->output.is_pipe)
No?
- Arnaldo
>
> putting it back, following example now works:
> $ perf record -o - kill | perf inject -b | perf report -i -
>
> Reported-by: Stephane Eranian <eranian@google.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/builtin-inject.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 6a3af00..664010b 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -72,7 +72,7 @@ static int perf_event__repipe_attr(struct perf_tool *tool,
> if (ret)
> return ret;
>
> - if (&inject->output.is_pipe)
> + if (!&inject->output.is_pipe)
> return 0;
>
> return perf_event__repipe_synth(tool, event);
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCHv2] perf tools: Fix pipe check regression in attr event callback
2014-06-05 20:21 ` Arnaldo Carvalho de Melo
@ 2014-06-05 20:41 ` Jiri Olsa
2014-06-05 21:34 ` Stephane Eranian
2014-06-12 12:01 ` [tip:perf/core] " tip-bot for Jiri Olsa
0 siblings, 2 replies; 6+ messages in thread
From: Jiri Olsa @ 2014-06-05 20:41 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Stephane Eranian, LKML, Namhyung Kim, David Ahern, Peter Zijlstra, mingo
On Thu, Jun 05, 2014 at 05:21:02PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 05, 2014 at 07:00:11PM +0200, Jiri Olsa escreveu:
> > On Thu, Jun 05, 2014 at 06:06:41PM +0200, Stephane Eranian wrote:
> > > Hi Jiri,
> > >
> > > Somehow, I thought you had written a fix to avoid the problem below.
> > > But when I try with tip.git, the problem is still there.
> > > Could you push your fix ASAP.
> > >
> > > Thanks.
> > >
> > > $ perf record -o - noploop 2 | perf inject -b | perf report -i -
> > > # To display the perf.data header info, please use
> > > --header/--header-only options.
> > > #
> > > noploop for 2 seconds
> > > 0x1bd0 [0x28]: failed to process type: 9
> >
> > hum, I remember fixing another issue.. this one is
> > separated one, please try attached patch.
> >
> > thanks,
> > jirka
> >
> >
> > ---
> > The file factoring in builtin-inject.c object introduced regression
> > in attr event callback. The commit is:
> > 3406912 perf inject: Handle output file via perf_data_file object
> >
> > Following hunk reversed the logic:
> > - if (!inject->pipe_output)
> > + if (&inject->output.is_pipe)
>
> Why do we need this '&'? this will always evaluate to true, as it will
> get the address of a variable, also adding a ! before it will just it
> eval to false always, or am I missing something? :-)
>
> I think this should really be:
>
> if (!inject->output.is_pipe)
>
> No?
aaarhg.. yes ;-) v2 attached
thanks,
jirka
---
The file factoring in builtin-inject.c object introduced regression
in attr event callback. The commit is:
3406912 perf inject: Handle output file via perf_data_file object
Following hunk reversed the logic:
- if (!inject->pipe_output)
+ if (&inject->output.is_pipe)
putting it back, following example now works:
$ perf record -o - kill | perf inject -b | perf report -i -
Plus removing extra '&' (kudos to Arnaldo)
Reported-by: Stephane Eranian <eranian@google.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/builtin-inject.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 6a3af00..16c7c11 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -72,7 +72,7 @@ static int perf_event__repipe_attr(struct perf_tool *tool,
if (ret)
return ret;
- if (&inject->output.is_pipe)
+ if (!inject->output.is_pipe)
return 0;
return perf_event__repipe_synth(tool, event);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCHv2] perf tools: Fix pipe check regression in attr event callback
2014-06-05 20:41 ` [PATCHv2] " Jiri Olsa
@ 2014-06-05 21:34 ` Stephane Eranian
2014-06-12 12:01 ` [tip:perf/core] " tip-bot for Jiri Olsa
1 sibling, 0 replies; 6+ messages in thread
From: Stephane Eranian @ 2014-06-05 21:34 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, LKML, Namhyung Kim, David Ahern,
Peter Zijlstra, mingo
On Thu, Jun 5, 2014 at 10:41 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Thu, Jun 05, 2014 at 05:21:02PM -0300, Arnaldo Carvalho de Melo wrote:
>> Em Thu, Jun 05, 2014 at 07:00:11PM +0200, Jiri Olsa escreveu:
>> > On Thu, Jun 05, 2014 at 06:06:41PM +0200, Stephane Eranian wrote:
>> > > Hi Jiri,
>> > >
>> > > Somehow, I thought you had written a fix to avoid the problem below.
>> > > But when I try with tip.git, the problem is still there.
>> > > Could you push your fix ASAP.
>> > >
>> > > Thanks.
>> > >
>> > > $ perf record -o - noploop 2 | perf inject -b | perf report -i -
>> > > # To display the perf.data header info, please use
>> > > --header/--header-only options.
>> > > #
>> > > noploop for 2 seconds
>> > > 0x1bd0 [0x28]: failed to process type: 9
>> >
>> > hum, I remember fixing another issue.. this one is
>> > separated one, please try attached patch.
>> >
>> > thanks,
>> > jirka
>> >
>> >
>> > ---
>> > The file factoring in builtin-inject.c object introduced regression
>> > in attr event callback. The commit is:
>> > 3406912 perf inject: Handle output file via perf_data_file object
>> >
>> > Following hunk reversed the logic:
>> > - if (!inject->pipe_output)
>> > + if (&inject->output.is_pipe)
>>
>> Why do we need this '&'? this will always evaluate to true, as it will
>> get the address of a variable, also adding a ! before it will just it
>> eval to false always, or am I missing something? :-)
>>
>> I think this should really be:
>>
>> if (!inject->output.is_pipe)
>>
>> No?
>
> aaarhg.. yes ;-) v2 attached
>
Had the same reaction as Arnaldo. Now, it makes more sense...
> thanks,
> jirka
>
> ---
> The file factoring in builtin-inject.c object introduced regression
> in attr event callback. The commit is:
> 3406912 perf inject: Handle output file via perf_data_file object
>
> Following hunk reversed the logic:
> - if (!inject->pipe_output)
> + if (&inject->output.is_pipe)
>
> putting it back, following example now works:
> $ perf record -o - kill | perf inject -b | perf report -i -
>
> Plus removing extra '&' (kudos to Arnaldo)
>
> Reported-by: Stephane Eranian <eranian@google.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/builtin-inject.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 6a3af00..16c7c11 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -72,7 +72,7 @@ static int perf_event__repipe_attr(struct perf_tool *tool,
> if (ret)
> return ret;
>
> - if (&inject->output.is_pipe)
> + if (!inject->output.is_pipe)
> return 0;
>
> return perf_event__repipe_synth(tool, event);
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip:perf/core] perf tools: Fix pipe check regression in attr event callback
2014-06-05 20:41 ` [PATCHv2] " Jiri Olsa
2014-06-05 21:34 ` Stephane Eranian
@ 2014-06-12 12:01 ` tip-bot for Jiri Olsa
1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Jiri Olsa @ 2014-06-12 12:01 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, paulus, eranian, hpa, mingo, jolsa, a.p.zijlstra,
acme, namhyung, fweisbec, dsahern, tglx, cjashfor
Commit-ID: a261e4a09a073451057e9dbe17783255ea94598d
Gitweb: http://git.kernel.org/tip/a261e4a09a073451057e9dbe17783255ea94598d
Author: Jiri Olsa <jolsa@kernel.org>
AuthorDate: Thu, 5 Jun 2014 18:51:44 +0200
Committer: Jiri Olsa <jolsa@kernel.org>
CommitDate: Mon, 9 Jun 2014 12:20:34 +0200
perf tools: Fix pipe check regression in attr event callback
The file factoring in builtin-inject.c object introduced regression
in attr event callback. The commit is:
3406912 perf inject: Handle output file via perf_data_file object
Following hunk reversed the logic:
- if (!inject->pipe_output)
+ if (&inject->output.is_pipe)
putting it back, following example now works:
$ perf record -o - kill | perf inject -b | perf report -i -
Plus removing extra '&' (kudos to Arnaldo)
Reported-by: Stephane Eranian <eranian@google.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20140605204117.GA1771@krava.redhat.com
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/builtin-inject.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 6a3af00..16c7c11 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -72,7 +72,7 @@ static int perf_event__repipe_attr(struct perf_tool *tool,
if (ret)
return ret;
- if (&inject->output.is_pipe)
+ if (!inject->output.is_pipe)
return 0;
return perf_event__repipe_synth(tool, event);
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-06-12 12:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-05 16:06 [BUG] perf record: pipe mode still broken Stephane Eranian
2014-06-05 17:00 ` [PATCH] perf tools: Fix pipe check regression in attr event callback Jiri Olsa
2014-06-05 20:21 ` Arnaldo Carvalho de Melo
2014-06-05 20:41 ` [PATCHv2] " Jiri Olsa
2014-06-05 21:34 ` Stephane Eranian
2014-06-12 12:01 ` [tip:perf/core] " tip-bot for Jiri Olsa
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).