Re: [PATCH] perf: sample after exit loses thread correlation

[David Ahern <dsahern@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 888 bytes --]

On 7/29/13 4:29 AM, Adrian Hunter wrote:
>> When perf processes the EXIT event the thread is moved to the
>> dead_threads
>> list. When the SAMPLE event is processed no thread exists for the pid
>> so a new
>> one is created by machine__findnew_thread.
>
> In the case of per-cpu recording, it is normal for sample events to
> occur after the EXIT event simply because the EXIT event is recorded
> while the task is still running (in kernel).  It is therefore a mistake
> to move the thread to dead_threads just because of the EXIT event.
>
> Instead the thread should be marked as exiting and not moved to
> dead_threads until another thread starts on the same CPU. i.e. a comm,
> mmap, fork event with the same tid and same CPU, or any event with a
> different tid and same CPU.
>

Interesting idea -- delaying the move to the dead-threads list. 
Following solves the problem as well.


[-- Attachment #2: a.patch --]
[-- Type: text/plain, Size: 2037 bytes --]

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index f9f9d63..0d29b1b 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -994,11 +994,27 @@ out_problem:
 	return 0;
 }
 
+static void machine__remove_thread(struct machine *machine, struct thread *th)
+{
+	machine->last_match = NULL;
+	rb_erase(&th->rb_node, &machine->threads);
+	/*
+	 * We may have references to this thread, for instance in some hist_entry
+	 * instances, so just move them to a separate list.
+	 */
+	list_add_tail(&th->node, &machine->dead_threads);
+}
+
 int machine__process_fork_event(struct machine *machine, union perf_event *event)
 {
-	struct thread *thread = machine__findnew_thread(machine, event->fork.tid);
+	struct thread *thread = machine__find_thread(machine, event->fork.tid);
 	struct thread *parent = machine__findnew_thread(machine, event->fork.ptid);
 
+	/* if a thread currently exists for the thread id remove it */
+	if (thread != NULL)
+		machine__remove_thread(machine, thread);
+
+	thread = machine__findnew_thread(machine, event->fork.tid);
 	if (dump_trace)
 		perf_event__fprintf_task(event, stdout);
 
@@ -1011,27 +1027,12 @@ int machine__process_fork_event(struct machine *machine, union perf_event *event
 	return 0;
 }
 
-static void machine__remove_thread(struct machine *machine, struct thread *th)
-{
-	machine->last_match = NULL;
-	rb_erase(&th->rb_node, &machine->threads);
-	/*
-	 * We may have references to this thread, for instance in some hist_entry
-	 * instances, so just move them to a separate list.
-	 */
-	list_add_tail(&th->node, &machine->dead_threads);
-}
-
-int machine__process_exit_event(struct machine *machine, union perf_event *event)
+int machine__process_exit_event(struct machine *machine __maybe_unused,
+				union perf_event *event)
 {
-	struct thread *thread = machine__find_thread(machine, event->fork.tid);
-
 	if (dump_trace)
 		perf_event__fprintf_task(event, stdout);
 
-	if (thread != NULL)
-		machine__remove_thread(machine, thread);
-
 	return 0;
 }