* [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@elte.hu
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@elte.hu
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@elte.hu
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@elte.hu
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@elte.hu
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 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.