[PATCH 0/3] perf, tool: kvm record & event processing fixies

[PATCH 0/3] perf, tool: kvm record & event processing fixies

[Jiri Olsa]

hi,
sending fix for 'perf kvm record' to force guest kernel
machine specification option (patch 1). I'm not completely
sure there might be better solution for this like some
dummy guest machine record.

And 2 other bug fixies I cross when hunting the first
one (patch 2,3).

Attached patches:
 1/3 perf, tool: Force guest machine definition option
 2/3 perf, tool: Skip event correctly for unknown id/machine
 3/3 perf, tool: Fail on processing event with unknow size

thanks,
jirka
---
 tools/perf/builtin-kvm.c  |   10 ++++++++++
 tools/perf/util/session.c |   32 +++++++++-----------------------
 2 files changed, 19 insertions(+), 23 deletions(-)

[PATCH 1/3] perf, tool: Force guest machine definition option

[Jiri Olsa]

Running 'perf kvm record' without any of following options:
  --guestmount
  --guestvmlinux
  --guestkallsyms
  --guestmodules

is causing the guest machine to be ommited from the data file,
and all guest samples are counted in nr_unprocessable_samples.

This patch makes sure the 'perf kvm record' command is not
let through until one of the above option is specified.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/builtin-kvm.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 9fc6e0f..559d468 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -53,6 +53,16 @@ static int __cmd_record(int argc, const char **argv)
 	int rec_argc, i = 0, j;
 	const char **rec_argv;
 
+	if (perf_guest &&
+	    !(symbol_conf.guestmount ||
+	      symbol_conf.default_guest_vmlinux_name ||
+	      symbol_conf.default_guest_modules ||
+	      symbol_conf.default_guest_kallsyms)) {
+		ui__warning("You need to define guest with one of guestmount|"
+			    "guestvmlinux|guestkallsyms|guestmodules\n");
+		return -EINVAL;
+	}
+
 	rec_argc = argc + 2;
 	rec_argv = calloc(rec_argc + 1, sizeof(char *));
 	rec_argv[i++] = strdup("record");
-- 
1.7.1

Re: [PATCH 1/3] perf, tool: Force guest machine definition option

[David Ahern]

On 4/12/12 6:21 AM, Jiri Olsa wrote:
> Running 'perf kvm record' without any of following options:
>    --guestmount
>    --guestvmlinux
>    --guestkallsyms
>    --guestmodules
>
> is causing the guest machine to be ommited from the data file,
> and all guest samples are counted in nr_unprocessable_samples.

Even with guestmount specified you can hit this path -- e.g., no 
pid-based directory under guestmount for a VM.

David

Re: [PATCH 1/3] perf, tool: Force guest machine definition option

[Jiri Olsa]

On Thu, Apr 12, 2012 at 08:40:37AM -0600, David Ahern wrote:
> On 4/12/12 6:21 AM, Jiri Olsa wrote:
> >Running 'perf kvm record' without any of following options:
> >   --guestmount
> >   --guestvmlinux
> >   --guestkallsyms
> >   --guestmodules
> >
> >is causing the guest machine to be ommited from the data file,
> >and all guest samples are counted in nr_unprocessable_samples.
> 
> Even with guestmount specified you can hit this path -- e.g., no
> pid-based directory under guestmount for a VM.

hm, something's wrong anyway..

[root@dhcp-26-214 perf]# ./perf kvm --guest --guestkallsyms=./kallsyms record -p 1791
^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.013 MB perf.data.guest (~576 samples) ]
[root@dhcp-26-214 perf]# ./perf kvm --guest --guestkallsyms=./kallsyms report --stdio
Segmentation fault (core dumped)

it segfaults on guest mmap (attached bt) having NULL machine,
since no guest kernel buildid/dso was stored&loaded..
the host/guest machines code seems too 'complex' ;)

jirka


Program received signal SIGSEGV, Segmentation fault.
0x000000000046a35e in machine__mmap_name (self=0x0, bf=0x7fffffffbf30 "[kernel.kallsyms]", size=4096) at util/map.c:711
711                     snprintf(bf, size, "[%s.%d]", "guest.kernel.kallsyms", self->pid);
Missing separate debuginfos, use: debuginfo-install atk-2.2.0-2.fc16.x86_64 bzip2-libs-1.0.6-3.fc15.x86_64 cairo-1.10.2-4.fc16.x86_64 elfutils-libelf-0.153-1.fc16.x86_64 elfutils-libs-0.153-1.fc16.x86_64 fontconfig-2.8.0-4.fc16.x86_64 freetype-2.4.6-4.fc16.x86_64 gdk-pixbuf2-2.24.1-1.fc16.x86_64 glib2-2.30.2-1.fc16.x86_64 gtk2-2.24.8-3.fc16.x86_64 libX11-1.4.3-1.fc16.x86_64 libXau-1.0.6-2.fc15.x86_64 libXcomposite-0.4.3-2.fc15.x86_64 libXcursor-1.1.11-3.fc15.x86_64 libXdamage-1.1.3-2.fc15.x86_64 libXext-1.3.0-1.fc16.x86_64 libXfixes-5.0-1.fc16.x86_64 libXi-1.4.5-1.fc16.x86_64 libXinerama-1.1.1-2.fc15.x86_64 libXrandr-1.3.1-2.fc15.x86_64 libXrender-0.9.6-2.fc15.x86_64 libffi-3.0.10-1.fc16.x86_64 libpng-1.2.46-2.fc16.x86_64 libselinux-2.1.6-6.fc16.x86_64 libxcb-1.7-3.fc16.x86_64 newt-0.52.14-1.fc16.x86_64 nss-softokn-freebl-3.13.3-1.fc16.x86_64 pango-1.29.4-1.fc16.x86_64 perl-libs-5.14.2-197.fc16.x86_64 pixman-0.22.2-1.fc16.x86_64 python-libs-2.7.2-5.2.fc16.x86_64 slang-2.2.4-1.fc16.x86_64 xz-libs-5.1.1-1alpha.fc16.x86_64
(gdb) bt
#0  0x000000000046a35e in machine__mmap_name (self=0x0, bf=0x7fffffffbf30 "[kernel.kallsyms]", size=4096) at util/map.c:711
#1  0x0000000000443d84 in perf_event__process_kernel_mmap (tool=0x7fffffffdf40, event=0x7ffff7ff5168, machine=0x0) at util/event.c:550
#2  0x0000000000444265 in perf_event__process_mmap (tool=0x7fffffffdf40, event=0x7ffff7ff5168, sample=0x7fffffffd430, machine=0x0)
    at util/event.c:656
#3  0x000000000046d099 in perf_session_deliver_event (session=0x905570, event=0x7ffff7ff5168, sample=0x7fffffffd430, tool=
    0x7fffffffdf40, file_offset=360) at util/session.c:884
#4  0x000000000046d578 in perf_session__process_event (session=0x905570, event=0x7ffff7ff5168, tool=0x7fffffffdf40, file_offset=360)
    at util/session.c:990
#5  0x000000000046ddc6 in __perf_session__process_events (session=0x905570, data_offset=288, data_size=13824, file_size=14112, tool=
    0x7fffffffdf40) at util/session.c:1219
#6  0x000000000046df60 in perf_session__process_events (self=0x905570, tool=0x7fffffffdf40) at util/session.c:1258
#7  0x0000000000423b7d in __cmd_report (rep=0x7fffffffdf40) at builtin-report.c:367
#8  0x0000000000424f3e in cmd_report (argc=0, argv=0x9053a0, prefix=0x0) at builtin-report.c:752
#9  0x00000000004370c9 in __cmd_report (argc=2, argv=0x7fffffffe420) at builtin-kvm.c:84
#10 0x00000000004373ee in cmd_kvm (argc=2, argv=0x7fffffffe420, prefix=0x0) at builtin-kvm.c:131
#11 0x0000000000414ead in run_builtin (p=0x796488, argc=4, argv=0x7fffffffe420) at perf.c:273
#12 0x00000000004150a7 in handle_internal_command (argc=4, argv=0x7fffffffe420) at perf.c:345
#13 0x00000000004151f3 in run_argv (argcp=0x7fffffffe30c, argv=0x7fffffffe300) at perf.c:389
#14 0x0000000000415479 in main (argc=4, argv=0x7fffffffe420) at perf.c:487

Re: [PATCH 1/3] perf, tool: Force guest machine definition option

[David Ahern]

On 4/13/12 5:32 AM, Jiri Olsa wrote:
>> Even with guestmount specified you can hit this path -- e.g., no
>> pid-based directory under guestmount for a VM.
>
> hm, something's wrong anyway..
>
> [root@dhcp-26-214 perf]# ./perf kvm --guest --guestkallsyms=./kallsyms record -p 1791
> ^C[ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.013 MB perf.data.guest (~576 samples) ]
> [root@dhcp-26-214 perf]# ./perf kvm --guest --guestkallsyms=./kallsyms report --stdio
> Segmentation fault (core dumped)
>
> it segfaults on guest mmap (attached bt) having NULL machine,
> since no guest kernel buildid/dso was stored&loaded..
> the host/guest machines code seems too 'complex' ;)

That was fixed on Monday I believe. Patch is in Arnaldo's urgent queue.


David

Re: [PATCH 1/3] perf, tool: Force guest machine definition option

[Jiri Olsa]

On Fri, Apr 13, 2012 at 06:21:54AM -0600, David Ahern wrote:
> On 4/13/12 5:32 AM, Jiri Olsa wrote:
> >>Even with guestmount specified you can hit this path -- e.g., no
> >>pid-based directory under guestmount for a VM.
> >
> >hm, something's wrong anyway..
> >
> >[root@dhcp-26-214 perf]# ./perf kvm --guest --guestkallsyms=./kallsyms record -p 1791
> >^C[ perf record: Woken up 1 times to write data ]
> >[ perf record: Captured and wrote 0.013 MB perf.data.guest (~576 samples) ]
> >[root@dhcp-26-214 perf]# ./perf kvm --guest --guestkallsyms=./kallsyms report --stdio
> >Segmentation fault (core dumped)
> >
> >it segfaults on guest mmap (attached bt) having NULL machine,
> >since no guest kernel buildid/dso was stored&loaded..
> >the host/guest machines code seems too 'complex' ;)
> 
> That was fixed on Monday I believe. Patch is in Arnaldo's urgent queue.

I get same error even on acme's urgent branch.. do you mean
below one?  It fixes the guest machine lookup for mmap event.

perf kvm: Finding struct machine fails for PERF_RECORD_MMAP
commit 7fb0a5ee8889488f7568ffddffeb66ddeb50917e
Author: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>
Date:   Mon Apr 9 13:52:23 2012 +0530

In my case it looks like for some reason the guest buildid DSO
is not stored in record phase (no hits maybe?), so report won't
create guest machine record at all and get NULL machine.

jirka

Re: [PATCH 1/3] perf, tool: Force guest machine definition option

[David Ahern]

On 4/13/12 6:41 AM, Jiri Olsa wrote:
> I get same error even on acme's urgent branch.. do you mean
> below one?  It fixes the guest machine lookup for mmap event.
>
> perf kvm: Finding struct machine fails for PERF_RECORD_MMAP
> commit 7fb0a5ee8889488f7568ffddffeb66ddeb50917e
> Author: Nikunj A. Dadhania<nikunj@linux.vnet.ibm.com>
> Date:   Mon Apr 9 13:52:23 2012 +0530

That's the one I was referring to.

>
> In my case it looks like for some reason the guest buildid DSO
> is not stored in record phase (no hits maybe?), so report won't
> create guest machine record at all and get NULL machine.

I see now -- different problem, but similar in that it's an mmap event 
and the pid is 0.

I need to take my daughter to school and won't get back to this for a 
while but what I am seeing is that on perf-record mmap events are 
generated with pid set to DEFAULT_GUEST_KERNEL_ID = 0 
(machines__create_guest_kernel_maps).

On the report side a machine has not been created for pid of 0, so the 
look up in perf_session__find_machine_for_cpumode fails.

David

Re: [PATCH 1/3] perf, tool: Force guest machine definition option

[Jiri Olsa]

On Fri, Apr 13, 2012 at 08:15:17AM -0600, David Ahern wrote:
> On 4/13/12 6:41 AM, Jiri Olsa wrote:
> >I get same error even on acme's urgent branch.. do you mean
> >below one?  It fixes the guest machine lookup for mmap event.
> >
> >perf kvm: Finding struct machine fails for PERF_RECORD_MMAP
> >commit 7fb0a5ee8889488f7568ffddffeb66ddeb50917e
> >Author: Nikunj A. Dadhania<nikunj@linux.vnet.ibm.com>
> >Date:   Mon Apr 9 13:52:23 2012 +0530
> 
> That's the one I was referring to.
> 
> >
> >In my case it looks like for some reason the guest buildid DSO
> >is not stored in record phase (no hits maybe?), so report won't
> >create guest machine record at all and get NULL machine.
> 
> I see now -- different problem, but similar in that it's an mmap
> event and the pid is 0.
> 
> I need to take my daughter to school and won't get back to this for
> a while but what I am seeing is that on perf-record mmap events are
> generated with pid set to DEFAULT_GUEST_KERNEL_ID = 0
> (machines__create_guest_kernel_maps).
> 
> On the report side a machine has not been created for pid of 0, so
> the look up in perf_session__find_machine_for_cpumode fails.
> 
> David

how about the patch below? it ensures there's machine record for
the guest. The segfault issue still stays..

jirka

---
Running 'perf kvm record' without any of following options:
 --guestmount
 --guestvmlinux
 --guestkallsyms
 --guestmodules

is causing the guest machine to be ommited from the data file,
and all guest samples are counted in nr_unprocessable_samples.

This patch makes sure the 'perf kvm record' command is not
let through if guest machine isn't defined.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/builtin-record.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 10b1f1f..0ae4237 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -462,6 +462,14 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 		return -1;
 	}
 
+	if (perf_guest &&
+	    !machines__find(&session->machines, DEFAULT_GUEST_KERNEL_ID)) {
+		ui__warning("You need to define guest with one of guestmount|"
+			    "guestvmlinux|guestkallsyms|guestmodules\n");
+		err = -EINVAL;
+		goto out_delete_session;
+	}
+
 	rec->session = session;
 
 	for (feat = HEADER_FIRST_FEATURE; feat < HEADER_LAST_FEATURE; feat++)
-- 
1.7.7.6

Re: [PATCH 1/3] perf, tool: Force guest machine definition option

[David Ahern]

On 4/15/12 8:05 AM, Jiri Olsa wrote:
> how about the patch below? it ensures there's machine record for
> the guest. The segfault issue still stays..
>
> jirka
>
> ---
> Running 'perf kvm record' without any of following options:
>   --guestmount
>   --guestvmlinux
>   --guestkallsyms
>   --guestmodules
>
> is causing the guest machine to be ommited from the data file,
> and all guest samples are counted in nr_unprocessable_samples.
>
> This patch makes sure the 'perf kvm record' command is not
> let through if guest machine isn't defined.


Doesn't work: it requires a 'default' vmlinux/modules/kallsyms even when 
a proper guestmount has been specified.

Today guestmount works for the report path because of the hook in 
machines__findnew() which creates a machine on first lookup. Really, the 
guestmount argument should not be required for the report which could be 
using a local symbol tree (symfs argument) -- e.g., capture on a host OS 
which could have kallsyms/module information for each VM / VM being 
profiled and then report file taken to another system for analysis where 
that system has access to symbols.

Perhaps what is needed is for the machine to be created for the pid == 
DEFAULT_GUEST_KERNEL_ID on first need. This is not the right place -- 
but shows the sentiment:

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 1efd3be..20420fc 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -824,6 +824,7 @@ static struct machine *
     perf_session__find_machine_for_cpumode(struct perf_session *session,
                            union perf_event *event)
  {
+    struct machine *mach;
     const u8 cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;

     if (cpumode == PERF_RECORD_MISC_GUEST_KERNEL && perf_guest) {
@@ -834,7 +835,10 @@ static struct machine *
         else
             pid = event->ip.pid;

-       return perf_session__find_machine(session, pid);
+       mach = perf_session__find_machine(session, pid);
+        if (!mach && pid == 0)
+            mach = machines__add(&session->machines, pid, "/dev/null");
+        return mach;
     }

     return perf_session__find_host_machine(session);

So basically it allows kernel maps to be attached to a pid 0 machine but 
that machine has no tree for further symbols. If nothing else perf-kvm 
does not segfault.

I am seeing other problems with perf-kvm too. e.g.,
perf kvm --guest --host --guestmount /tmp/guestmount  record -p 21234 -- 
sleep 10


Dumping the samples:
perf script -i perf.data.guest
--> shows no samples (2 'openssl speed' commands are running in the 
2-vcpu guest so something should be generated)

perf script -i perf.data.kvm

Failed to open [guest.kernel.kallsyms.21234]_text, continuing without 
symbols
         qemu-kvm 21234 cycles:  ffffffff8103edaa [unknown] 
([guest.kernel.kallsyms.21234]_text)

Anyways, perf-kvm needs some love. Gotta run.

David

[PATCH 2/3] perf, tool: Skip event correctly for unknown id/machine

[Jiri Olsa]

In case the perf_session__process_event function fails, we
estimate the next event offset.

This is not necessary for sample event failing on unknown ID
or machine. In such case we know proper size of the event,
so we dont need to guess. Also failure statistics are updated
correctly so we don't miss any information.

Forcing perf_session__process_event to return 0 in case of
unknown ID or machine.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/session.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 9412e3b..9189751 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -868,11 +868,11 @@ static int perf_session_deliver_event(struct perf_session *session,
 		dump_sample(session, event, sample);
 		if (evsel == NULL) {
 			++session->hists.stats.nr_unknown_id;
-			return -1;
+			return 0;
 		}
 		if (machine == NULL) {
 			++session->hists.stats.nr_unprocessable_samples;
-			return -1;
+			return 0;
 		}
 		return tool->sample(tool, event, sample, evsel, machine);
 	case PERF_RECORD_MMAP:
-- 
1.7.1

[tip:perf/urgent] perf session: Skip event correctly for unknown id/machine

[tip-bot for Jiri Olsa]

Commit-ID:  6782206b5dfece4c51f587b3ca1540a4027f87dd
Gitweb:     http://git.kernel.org/tip/6782206b5dfece4c51f587b3ca1540a4027f87dd
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Thu, 12 Apr 2012 14:21:01 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 12 Apr 2012 12:14:50 -0300

perf session: Skip event correctly for unknown id/machine

In case the perf_session__process_event function fails, we estimate the
next event offset.

This is not necessary for sample event failing on unknown ID or machine.
In such case we know proper size of the event, so we dont need to guess.
Also failure statistics are updated correctly so we don't miss any
information.

Forcing perf_session__process_event to return 0 in case of unknown ID or
machine.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1334233262-5679-3-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 00923cd..1efd3be 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -876,11 +876,11 @@ static int perf_session_deliver_event(struct perf_session *session,
 		dump_sample(session, event, sample);
 		if (evsel == NULL) {
 			++session->hists.stats.nr_unknown_id;
-			return -1;
+			return 0;
 		}
 		if (machine == NULL) {
 			++session->hists.stats.nr_unprocessable_samples;
-			return -1;
+			return 0;
 		}
 		return tool->sample(tool, event, sample, evsel, machine);
 	case PERF_RECORD_MMAP:

[PATCH 3/3] perf, tool: Fail on processing event with unknown size

[Jiri Olsa]

Currently if we cannot decide the size of the event, we guess next
event possition by:
  "... check alignment, and increment a single u64 in the hope
  to catch on again 'soon'"

This usually ends up with segfault or endless loop. It's better
to admit the failure right away, then pretend nothing happened.
It makes the life easier.. ;)

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/session.c |   28 +++++++---------------------
 1 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 9189751..fa0528b 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1100,16 +1100,9 @@ more:
 	}
 
 	if ((skip = perf_session__process_event(self, &event, tool, head)) < 0) {
-		dump_printf("%#" PRIx64 " [%#x]: skipping unknown header type: %d\n",
-			    head, event.header.size, event.header.type);
-		/*
-		 * assume we lost track of the stream, check alignment, and
-		 * increment a single u64 in the hope to catch on again 'soon'.
-		 */
-		if (unlikely(head & 7))
-			head &= ~7ULL;
-
-		size = 8;
+		pr_err("%#" PRIx64 " [%#x]: failed to process type: %d\n",
+		       head, event.header.size, event.header.type);
+		goto out_err;
 	}
 
 	head += size;
@@ -1218,17 +1211,10 @@ more:
 
 	if (size == 0 ||
 	    perf_session__process_event(session, event, tool, file_pos) < 0) {
-		dump_printf("%#" PRIx64 " [%#x]: skipping unknown header type: %d\n",
-			    file_offset + head, event->header.size,
-			    event->header.type);
-		/*
-		 * assume we lost track of the stream, check alignment, and
-		 * increment a single u64 in the hope to catch on again 'soon'.
-		 */
-		if (unlikely(head & 7))
-			head &= ~7ULL;
-
-		size = 8;
+		pr_err("%#" PRIx64 " [%#x]: failed to process type: %d\n",
+		       file_offset + head, event->header.size,
+		       event->header.type);
+		goto out_err;
 	}
 
 	head += size;
-- 
1.7.1

[PATCHv2 3/3] perf, tool: Fail on processing event with unknown size

[Jiri Olsa]

hi,
I compiled the initial version with old gcc and missed compiler
warnings, attaching fixed patch

jirka

---
Currently if we cannot decide the size of the event, we guess next
event possition by:
  "... check alignment, and increment a single u64 in the hope
  to catch on again 'soon'"

This usually ends up with segfault or endless loop. It's better
to admit the failure right away, then pretend nothing happened.
It makes the life easier ;)

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/session.c |   30 +++++++++---------------------
 1 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 1efd3be..4dcc8f3 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1108,16 +1108,10 @@ more:
 	}
 
 	if ((skip = perf_session__process_event(self, &event, tool, head)) < 0) {
-		dump_printf("%#" PRIx64 " [%#x]: skipping unknown header type: %d\n",
-			    head, event.header.size, event.header.type);
-		/*
-		 * assume we lost track of the stream, check alignment, and
-		 * increment a single u64 in the hope to catch on again 'soon'.
-		 */
-		if (unlikely(head & 7))
-			head &= ~7ULL;
-
-		size = 8;
+		pr_err("%#" PRIx64 " [%#x]: failed to process type: %d\n",
+		       head, event.header.size, event.header.type);
+		err = -EINVAL;
+		goto out_err;
 	}
 
 	head += size;
@@ -1226,17 +1220,11 @@ more:
 
 	if (size == 0 ||
 	    perf_session__process_event(session, event, tool, file_pos) < 0) {
-		dump_printf("%#" PRIx64 " [%#x]: skipping unknown header type: %d\n",
-			    file_offset + head, event->header.size,
-			    event->header.type);
-		/*
-		 * assume we lost track of the stream, check alignment, and
-		 * increment a single u64 in the hope to catch on again 'soon'.
-		 */
-		if (unlikely(head & 7))
-			head &= ~7ULL;
-
-		size = 8;
+		pr_err("%#" PRIx64 " [%#x]: failed to process type: %d\n",
+		       file_offset + head, event->header.size,
+		       event->header.type);
+		err = -EINVAL;
+		goto out_err;
 	}
 
 	head += size;
-- 
1.7.7.6

[tip:perf/core] perf session: Fail on processing event with unknown size

[tip-bot for Jiri Olsa]

Commit-ID:  9389a46043c8f091dc8f8d8e25a5c1355f8bcc9b
Gitweb:     http://git.kernel.org/tip/9389a46043c8f091dc8f8d8e25a5c1355f8bcc9b
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Mon, 16 Apr 2012 20:42:51 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 4 May 2012 11:53:22 -0300

perf session: Fail on processing event with unknown size

Currently if we cannot decide the size of the event, we guess next
event possition by:
  "... check alignment, and increment a single u64 in the hope
  to catch on again 'soon'"

This usually ends up with segfault or endless loop. It's better
to admit the failure right away, then pretend nothing happened.
It makes the life easier ;)

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20120416184251.GA11503@m.brq.redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c |   30 +++++++++---------------------
 1 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 9412e3b..f992ae3 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1100,16 +1100,10 @@ more:
 	}
 
 	if ((skip = perf_session__process_event(self, &event, tool, head)) < 0) {
-		dump_printf("%#" PRIx64 " [%#x]: skipping unknown header type: %d\n",
-			    head, event.header.size, event.header.type);
-		/*
-		 * assume we lost track of the stream, check alignment, and
-		 * increment a single u64 in the hope to catch on again 'soon'.
-		 */
-		if (unlikely(head & 7))
-			head &= ~7ULL;
-
-		size = 8;
+		pr_err("%#" PRIx64 " [%#x]: failed to process type: %d\n",
+		       head, event.header.size, event.header.type);
+		err = -EINVAL;
+		goto out_err;
 	}
 
 	head += size;
@@ -1218,17 +1212,11 @@ more:
 
 	if (size == 0 ||
 	    perf_session__process_event(session, event, tool, file_pos) < 0) {
-		dump_printf("%#" PRIx64 " [%#x]: skipping unknown header type: %d\n",
-			    file_offset + head, event->header.size,
-			    event->header.type);
-		/*
-		 * assume we lost track of the stream, check alignment, and
-		 * increment a single u64 in the hope to catch on again 'soon'.
-		 */
-		if (unlikely(head & 7))
-			head &= ~7ULL;
-
-		size = 8;
+		pr_err("%#" PRIx64 " [%#x]: failed to process type: %d\n",
+		       file_offset + head, event->header.size,
+		       event->header.type);
+		err = -EINVAL;
+		goto out_err;
 	}
 
 	head += size;