Skip to contentSkip to conversation

perf/urgent fixes

GIT PULL New Arnaldo Carvalho de Melo 2 participants 6 messages Aug 19, 2015 – Aug 20, 2015 perf events

lore
Share & tools
lore

Arnaldo Carvalho de Melo

author maintainer
#0 lore raw

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)

----------------------------------------------------------------
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(-)
author maintainer
#1 lore raw

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

From: Adrian Hunter <[email protected]>

Attempting to clone map groups onto themselves will deadlock.

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

---
 tools/perf/util/thread.c | 6 ++++++
 1 file changed, 6 insertions(+)
tools/perf/util/thread.c +6 -0
index 28c4b746baa1..0a9ae8014729 100644
@@ -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)
Signature · 1 line
2.1.0
author maintainer
#2 lore raw

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

In reply to Arnaldo Carvalho de Melo

From: Adrian Hunter <[email protected]>

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.

---
 tools/perf/util/machine.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)
tools/perf/util/machine.c +18 -2
index 7ff682770fdb..f1a4c833121e 100644
@@ -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) {
Signature · 1 line
2.1.0
author maintainer
#3 lore raw

PATCH 3/4 perf tools: Fix buildid processing

In reply to Arnaldo Carvalho de Melo

From: Adrian Hunter <[email protected]>

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.

Cc Linus Torvalds <[email protected]>
[ Moved the sample_id_add to after trying to open the events, use pr_warning ]
---
 tools/perf/builtin-record.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
tools/perf/builtin-record.c +11 -0
index de165a1b9240..20b56eb987f8 100644
@@ -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,
 	},
 };
 
Signature · 1 line
2.1.0
author maintainer
#4 lore raw

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

In reply to Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <[email protected]>

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] #

---
 tools/perf/builtin-top.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/builtin-top.c +2 -2
index ecf319728f25..6135cc07213c 100644
@@ -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);
Signature · 1 line
2.1.0
#5 lore raw
In reply to Arnaldo Carvalho de Melo
Quoted 42 lines · 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)

----------------------------------------------------------------
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

Quoted 8 lines

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