[GIT PULL 0/4] perf/urgent fixes

[GIT PULL 0/4] perf/urgent fixes

[Arnaldo Carvalho de Melo]

Hi Ingo,

	Please consider pulling,

- Arnaldo

The following changes since commit d7a702f0b1033cf402fef65bd6395072738f0844:

  perf/x86/intel/cqm: Do not access cpu_data() from CPU_UP_PREPARE handler (2015-08-12 11:37:23 +0200)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo

for you to fetch changes up to 09f4d78ab0af0973e1a49c10eb7bf977c68cc3aa:

  perf top: Show backtrace when handling a SIGSEGV on --stdio mode (2015-08-19 15:16:08 -0300)

----------------------------------------------------------------
perf/urgent fixes:

User visible:

- Fix buildid processing done at the end of a 'perf record' session, a
  problem that happened in workloads involving lots of small short-lived
  processes.  That code was not asking the perf_session layer to order
  the events.

  Make the code more robust to handle some of the problems with such
  out-of-order events and fix 'perf record' to ask for ordered events
  on systems where we have perf_event_attr.sample_id_all.  (Adrian Hunter)

- Show backtrace when handling a SIGSEGV in 'perf top --stdio' (Arnaldo Carvalho de Melo)

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

----------------------------------------------------------------
Adrian Hunter (3):
      perf tools: Avoid deadlock when map_groups are broken
      perf tools: Make fork event processing more resilient
      perf tools: Fix buildid processing

Arnaldo Carvalho de Melo (1):
      perf top: Show backtrace when handling a SIGSEGV on --stdio mode

 tools/perf/builtin-record.c | 11 +++++++++++
 tools/perf/builtin-top.c    |  4 ++--
 tools/perf/util/machine.c   | 20 ++++++++++++++++++--
 tools/perf/util/thread.c    |  6 ++++++
 4 files changed, 37 insertions(+), 4 deletions(-)

[PATCH 1/4] perf tools: Avoid deadlock when map_groups are broken

[Arnaldo Carvalho de Melo]

From: Adrian Hunter <adrian.hunter@intel.com>

Attempting to clone map groups onto themselves will deadlock.

It only happens because of other bugs, but the code should protect
itself anyway.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/1439994561-27436-2-git-send-email-adrian.hunter@intel.com
[ Use pr_debug() instead of dump_fprintf() ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/thread.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 28c4b746baa1..0a9ae8014729 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -191,6 +191,12 @@ static int thread__clone_map_groups(struct thread *thread,
 	if (thread->pid_ == parent->pid_)
 		return 0;
 
+	if (thread->mg == parent->mg) {
+		pr_debug("broken map groups on thread %d/%d parent %d/%d\n",
+			 thread->pid_, thread->tid, parent->pid_, parent->tid);
+		return 0;
+	}
+
 	/* But this one is new process, copy maps. */
 	for (i = 0; i < MAP__NR_TYPES; ++i)
 		if (map_groups__clone(thread->mg, parent->mg, i) < 0)
-- 
2.1.0

[PATCH 2/4] perf tools: Make fork event processing more resilient

[Arnaldo Carvalho de Melo]

From: Adrian Hunter <adrian.hunter@intel.com>

When processing a fork event, the tools lookup the parent thread by its
tid.  In a couple of cases, it is possible for that thread to have the
wrong pid.

That can happen if the data is being processed out of order, or if the
(fork) event that would have removed the erroneous thread was lost.

Assume the latter case, print a dump message, remove the erroneous
thread, create a new one with the correct pid, and keep going.

Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Jiri Olsa <jolsa@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/1439994561-27436-3-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/machine.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 7ff682770fdb..f1a4c833121e 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1387,6 +1387,24 @@ int machine__process_fork_event(struct machine *machine, union perf_event *event
 							event->fork.ptid);
 	int err = 0;
 
+	if (dump_trace)
+		perf_event__fprintf_task(event, stdout);
+
+	/*
+	 * There may be an existing thread that is not actually the parent,
+	 * either because we are processing events out of order, or because the
+	 * (fork) event that would have removed the thread was lost. Assume the
+	 * latter case and continue on as best we can.
+	 */
+	if (parent->pid_ != (pid_t)event->fork.ppid) {
+		dump_printf("removing erroneous parent thread %d/%d\n",
+			    parent->pid_, parent->tid);
+		machine__remove_thread(machine, parent);
+		thread__put(parent);
+		parent = machine__findnew_thread(machine, event->fork.ppid,
+						 event->fork.ptid);
+	}
+
 	/* if a thread currently exists for the thread id remove it */
 	if (thread != NULL) {
 		machine__remove_thread(machine, thread);
@@ -1395,8 +1413,6 @@ int machine__process_fork_event(struct machine *machine, union perf_event *event
 
 	thread = machine__findnew_thread(machine, event->fork.pid,
 					 event->fork.tid);
-	if (dump_trace)
-		perf_event__fprintf_task(event, stdout);
 
 	if (thread == NULL || parent == NULL ||
 	    thread__fork(thread, parent, sample->time) < 0) {
-- 
2.1.0

[PATCH 3/4] perf tools: Fix buildid processing

[Arnaldo Carvalho de Melo]

From: Adrian Hunter <adrian.hunter@intel.com>

After recording, 'perf record' post-processes the data to determine
which buildids are needed.

That processing must process the data in time order, if possible,
because otherwise dependent events, like forks and mmaps, will not make
sense.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Jiri Olsa <jolsa@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/1439994561-27436-4-git-send-email-adrian.hunter@intel.com
[ Moved the sample_id_add to after trying to open the events, use pr_warning ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index de165a1b9240..20b56eb987f8 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -521,6 +521,15 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		goto out_child;
 	}
 
+	/*
+	 * Normally perf_session__new would do this, but it doesn't have the
+	 * evlist.
+	 */
+	if (rec->tool.ordered_events && !perf_evlist__sample_id_all(rec->evlist)) {
+		pr_warning("WARNING: No sample_id_all support, falling back to unordered processing\n");
+		rec->tool.ordered_events = false;
+	}
+
 	if (!rec->evlist->nr_groups)
 		perf_header__clear_feat(&session->header, HEADER_GROUP_DESC);
 
@@ -965,9 +974,11 @@ static struct record record = {
 	.tool = {
 		.sample		= process_sample_event,
 		.fork		= perf_event__process_fork,
+		.exit		= perf_event__process_exit,
 		.comm		= perf_event__process_comm,
 		.mmap		= perf_event__process_mmap,
 		.mmap2		= perf_event__process_mmap2,
+		.ordered_events	= true,
 	},
 };
 
-- 
2.1.0

[PATCH 4/4] perf top: Show backtrace when handling a SIGSEGV on --stdio mode

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

It was just freezing instead of informing about the SEGV, fix it and
also print a backtrace, just like in the TUI mode and in 'perf trace'.

Tested by provoking a NULL deref when pressing 'z':

     0.31%  libc-2.20.so     [.] malloc_consolidate
     0.31%  ld-2.20.so       [.] _dl_relocate_object
     0.28%  cc1              [.] ht_lookup
     0.28%  cc1              [.] ira_init_register_move_cost
  perf: Segmentation fault
  Obtained 7 stack frames.
  perf(dump_stack+0x32) [0x4d69f2]
  perf(sighandler_dump_stack+0x29) [0x4d6a89]
  /lib64/libc.so.6(+0x34960) [0x7f5064333960]
  perf() [0x438790]
  /lib64/libpthread.so.0(+0x752a) [0x7f50663dd52a]
  /lib64/libc.so.6(clone+0x6d) [0x7f50643ff22d]
  #

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-pewrpzqd29rgmhu2wkk7fhww@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index ecf319728f25..6135cc07213c 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -601,8 +601,8 @@ static void display_sig(int sig __maybe_unused)
 
 static void display_setup_sig(void)
 {
-	signal(SIGSEGV, display_sig);
-	signal(SIGFPE,  display_sig);
+	signal(SIGSEGV, sighandler_dump_stack);
+	signal(SIGFPE, sighandler_dump_stack);
 	signal(SIGINT,  display_sig);
 	signal(SIGQUIT, display_sig);
 	signal(SIGTERM, display_sig);
-- 
2.1.0

Re: [GIT PULL 0/4] perf/urgent fixes

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Hi Ingo,
> 
> 	Please consider pulling,
> 
> - Arnaldo
> 
> The following changes since commit d7a702f0b1033cf402fef65bd6395072738f0844:
> 
>   perf/x86/intel/cqm: Do not access cpu_data() from CPU_UP_PREPARE handler (2015-08-12 11:37:23 +0200)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo
> 
> for you to fetch changes up to 09f4d78ab0af0973e1a49c10eb7bf977c68cc3aa:
> 
>   perf top: Show backtrace when handling a SIGSEGV on --stdio mode (2015-08-19 15:16:08 -0300)
> 
> ----------------------------------------------------------------
> perf/urgent fixes:
> 
> User visible:
> 
> - Fix buildid processing done at the end of a 'perf record' session, a
>   problem that happened in workloads involving lots of small short-lived
>   processes.  That code was not asking the perf_session layer to order
>   the events.
> 
>   Make the code more robust to handle some of the problems with such
>   out-of-order events and fix 'perf record' to ask for ordered events
>   on systems where we have perf_event_attr.sample_id_all.  (Adrian Hunter)
> 
> - Show backtrace when handling a SIGSEGV in 'perf top --stdio' (Arnaldo Carvalho de Melo)
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Adrian Hunter (3):
>       perf tools: Avoid deadlock when map_groups are broken
>       perf tools: Make fork event processing more resilient
>       perf tools: Fix buildid processing
> 
> Arnaldo Carvalho de Melo (1):
>       perf top: Show backtrace when handling a SIGSEGV on --stdio mode
> 
>  tools/perf/builtin-record.c | 11 +++++++++++
>  tools/perf/builtin-top.c    |  4 ++--
>  tools/perf/util/machine.c   | 20 ++++++++++++++++++--
>  tools/perf/util/thread.c    |  6 ++++++
>  4 files changed, 37 insertions(+), 4 deletions(-)

Pulled, thanks a lot Arnaldo!

	Ingo