perf pt: Mark PT return events as "return"

From: Andi Kleen <[email protected]>

Move the get_main_thread function from db-export.c to thread.c so that it can be used elsewhere.

Signed-off-by Andi Kleen <[email protected]>

---
 tools/perf/util/db-export.c | 13 +------------
 tools/perf/util/db-export.h |  2 ++
 tools/perf/util/thread.c    | 11 +++++++++++
 tools/perf/util/thread.h    |  2 ++
 4 files changed, 16 insertions(+), 12 deletions(-)

tools/perf/util/db-export.c +1 -12

index 8d96c80cc67e..0fb0aeb0b09b 100644
@@ -233,17 +233,6 @@ int db_export__symbol(struct db_export *dbe, struct symbol *sym,
 	return 0;
 }
 
-static struct thread *get_main_thread(struct machine *machine, struct thread *thread)
-{
-	if (thread->pid_ == thread->tid)
-		return thread__get(thread);
-
-	if (thread->pid_ == -1)
-		return NULL;
-
-	return machine__find_thread(machine, thread->pid_, thread->pid_);
-}
-
 static int db_ids_from_al(struct db_export *dbe, struct addr_location *al,
 			  u64 *dso_db_id, u64 *sym_db_id, u64 *offset)
 {

@@ -383,7 +372,7 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
 	if (err)
 		return err;
 
-	main_thread = get_main_thread(al->machine, thread);
+	main_thread = thread__main_thread(al->machine, thread);
 	if (main_thread)
 		comm = machine__thread_exec_comm(al->machine, main_thread);
 

tools/perf/util/db-export.h +2 -0

index 67bc6b8ad2d6..1241e5773e13 100644
@@ -106,4 +106,6 @@ int db_export__branch_types(struct db_export *dbe);
 int db_export__call_path(struct db_export *dbe, struct call_path *cp);
 int db_export__call_return(struct db_export *dbe, struct call_return *cr);
 
+struct thread *db_export__main_thread(struct machine *machine, struct thread *thread);
+
 #endif

tools/perf/util/thread.c +11 -0

index 45fcb715a36b..ada58e6070bf 100644
@@ -265,3 +265,14 @@ void thread__find_cpumode_addr_location(struct thread *thread,
 			break;
 	}
 }
+
+struct thread *thread__main_thread(struct machine *machine, struct thread *thread)
+{
+	if (thread->pid_ == thread->tid)
+		return thread__get(thread);
+
+	if (thread->pid_ == -1)
+		return NULL;
+
+	return machine__find_thread(machine, thread->pid_, thread->pid_);
+}

tools/perf/util/thread.h +2 -0

index 45fba13c800b..08fcb14cf637 100644
@@ -81,6 +81,8 @@ void thread__insert_map(struct thread *thread, struct map *map);
 int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp);
 size_t thread__fprintf(struct thread *thread, FILE *fp);
 
+struct thread *thread__main_thread(struct machine *machine, struct thread *thread);
+
 void thread__find_addr_map(struct thread *thread,
 			   u8 cpumode, enum map_type type, u64 addr,
 			   struct addr_location *al);

Signature · 1 line

2.5.5

From: Andi Kleen <[email protected]>

With perf script --itrace=cr we can synthesize calls and returns out of a PT log. However both calls and returns are marked with the same event, called branches. This makes it difficult to read and post process, because calls and returns are somewhat diffferent.

Create a separate return event and mark the returns as return.

Cc [email protected]

Signed-off-by Andi Kleen <[email protected]>

---
 tools/perf/util/intel-pt.c | 53 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 48 insertions(+), 5 deletions(-)

tools/perf/util/intel-pt.c +48 -5

index 137196990012..4f0b1d7adf50 100644
@@ -82,9 +82,12 @@ struct intel_pt {
 	u64 instructions_id;
 
 	bool sample_branches;
+	bool sample_returns;
 	u32 branches_filter;
 	u64 branches_sample_type;
+	u64 returns_sample_type;
 	u64 branches_id;
+	u64 returns_id;
 
 	bool sample_transactions;
 	u64 transactions_sample_type;

@@ -960,7 +963,8 @@ static int intel_pt_inject_event(union perf_event *event,
 	return perf_event__synthesize_sample(event, type, 0, sample, swapped);
 }
 
-static int intel_pt_synth_branch_sample(struct intel_pt_queue *ptq)
+static int intel_pt_synth_branch_sample(struct intel_pt_queue *ptq,
+					bool is_return)
 {
 	int ret;
 	struct intel_pt *pt = ptq->pt;

@@ -990,8 +994,13 @@ static int intel_pt_synth_branch_sample(struct intel_pt_queue *ptq)
 	sample.pid = ptq->pid;
 	sample.tid = ptq->tid;
 	sample.addr = ptq->state->to_ip;
-	sample.id = ptq->pt->branches_id;
-	sample.stream_id = ptq->pt->branches_id;
+	if (is_return) {
+		sample.id = ptq->pt->returns_id;
+		sample.stream_id = ptq->pt->returns_id;
+	} else {
+		sample.id = ptq->pt->branches_id;
+		sample.stream_id = ptq->pt->branches_id;
+	}
 	sample.period = 1;
 	sample.cpu = ptq->cpu;
 	sample.flags = ptq->flags;

@@ -1014,6 +1023,8 @@ static int intel_pt_synth_branch_sample(struct intel_pt_queue *ptq)
 
 	if (pt->synth_opts.inject) {
 		ret = intel_pt_inject_event(event, &sample,
+					    is_return ?
+					    pt->returns_sample_type :
 					    pt->branches_sample_type,
 					    pt->synth_needs_swap);
 		if (ret)

@@ -1241,7 +1252,13 @@ static int intel_pt_sample(struct intel_pt_queue *ptq)
 		thread_stack__set_trace_nr(ptq->thread, state->trace_nr);
 
 	if (pt->sample_branches) {
-		err = intel_pt_synth_branch_sample(ptq);
+		err = intel_pt_synth_branch_sample(ptq, false);
+		if (err)
+			return err;
+	}
+
+	if (pt->sample_returns) {
+		err = intel_pt_synth_branch_sample(ptq, true);
 		if (err)
 			return err;
 	}

@@ -1956,7 +1973,33 @@ static int intel_pt_synth_events(struct intel_pt *pt,
 		}
 		pt->sample_branches = true;
 		pt->branches_sample_type = attr.sample_type;
-		pt->branches_id = id;
+		pt->branches_id = id++;
+	}
+	if (pt->synth_opts.returns) {
+		attr.config = PERF_COUNT_HW_BRANCH_INSTRUCTIONS;
+		attr.sample_period = 1;
+		attr.sample_type |= PERF_SAMPLE_ADDR;
+		attr.sample_type &= ~(u64)PERF_SAMPLE_CALLCHAIN;
+		attr.sample_type &= ~(u64)PERF_SAMPLE_BRANCH_STACK;
+		pr_debug("Synthesizing 'return' event with id %" PRIu64 " sample type %#" PRIx64 "\n",
+			 id, (u64)attr.sample_type);
+		err = intel_pt_synth_event(session, &attr, id);
+		if (err) {
+			pr_err("%s: failed to synthesize 'return' event type\n",
+			       __func__);
+			return err;
+		}
+		pt->sample_returns = true;
+		pt->returns_sample_type = attr.sample_type;
+		pt->returns_id = id;
+		evlist__for_each(evlist, evsel) {
+			if (evsel->id && evsel->id[0] == pt->returns_id) {
+				if (evsel->name)
+					zfree(&evsel->name);
+				evsel->name = strdup("return");
+				break;
+			}
+		}
 	}
 
 	pt->synth_needs_swap = evsel->needs_swap;

Signature · 1 line

2.5.5

Em Mon, May 23, 2016 at 05:52:23PM -0700, Andi Kleen escreveu:

Quoted 8 lines

From: Andi Kleen <[email protected]>

With perf script --itrace=cr we can synthesize calls and returns out of a PT log. However both calls and returns are marked with the same event, called branches. This makes it difficult to read and post process, because calls and returns are somewhat diffferent.

Create a separate return event and mark the returns as return.

Adrian, you voiced some concerns about this patch, are those settled?

Can I have your Acked-by, please?

- Arnaldo

Quoted 110 lines

Cc [email protected]

Signed-off-by Andi Kleen <[email protected]>

---
  tools/perf/util/intel-pt.c | 53 +++++++++++++++++++++++++++++++++++++++++-----
  1 file changed, 48 insertions(+), 5 deletions(-)

tools/perf/util/intel-pt.c +48 -6

index 137196990012..4f0b1d7adf50 100644
@@ -82,9 +82,12 @@ struct intel_pt {
  	u64 instructions_id;
 
  	bool sample_branches;
+	bool sample_returns;
  	u32 branches_filter;
  	u64 branches_sample_type;
+	u64 returns_sample_type;
  	u64 branches_id;
+	u64 returns_id;
 
  	bool sample_transactions;
  	u64 transactions_sample_type;

@@ -960,7 +963,8 @@ static int intel_pt_inject_event(union perf_event *event,
  	return perf_event__synthesize_sample(event, type, 0, sample, swapped);
  }
 
-static int intel_pt_synth_branch_sample(struct intel_pt_queue *ptq)
+static int intel_pt_synth_branch_sample(struct intel_pt_queue *ptq,
+					bool is_return)
  {
  	int ret;
  	struct intel_pt *pt = ptq->pt;

@@ -990,8 +994,13 @@ static int intel_pt_synth_branch_sample(struct intel_pt_queue *ptq)
  	sample.pid = ptq->pid;
  	sample.tid = ptq->tid;
  	sample.addr = ptq->state->to_ip;
-	sample.id = ptq->pt->branches_id;
-	sample.stream_id = ptq->pt->branches_id;
+	if (is_return) {
+		sample.id = ptq->pt->returns_id;
+		sample.stream_id = ptq->pt->returns_id;
+	} else {
+		sample.id = ptq->pt->branches_id;
+		sample.stream_id = ptq->pt->branches_id;
+	}
  	sample.period = 1;
  	sample.cpu = ptq->cpu;
  	sample.flags = ptq->flags;

@@ -1014,6 +1023,8 @@ static int intel_pt_synth_branch_sample(struct intel_pt_queue *ptq)
 
  	if (pt->synth_opts.inject) {
  		ret = intel_pt_inject_event(event, &sample,
+					    is_return ?
+					    pt->returns_sample_type :
  					    pt->branches_sample_type,
  					    pt->synth_needs_swap);
  		if (ret)

@@ -1241,7 +1252,13 @@ static int intel_pt_sample(struct intel_pt_queue *ptq)
  		thread_stack__set_trace_nr(ptq->thread, state->trace_nr);
 
  	if (pt->sample_branches) {
-		err = intel_pt_synth_branch_sample(ptq);
+		err = intel_pt_synth_branch_sample(ptq, false);
+		if (err)
+			return err;
+	}
+
+	if (pt->sample_returns) {
+		err = intel_pt_synth_branch_sample(ptq, true);
  		if (err)
  			return err;
  	}

@@ -1956,7 +1973,33 @@ static int intel_pt_synth_events(struct intel_pt *pt,
  		}
  		pt->sample_branches = true;
  		pt->branches_sample_type = attr.sample_type;
-		pt->branches_id = id;
+		pt->branches_id = id++;
+	}
+	if (pt->synth_opts.returns) {
+		attr.config = PERF_COUNT_HW_BRANCH_INSTRUCTIONS;
+		attr.sample_period = 1;
+		attr.sample_type |= PERF_SAMPLE_ADDR;
+		attr.sample_type &= ~(u64)PERF_SAMPLE_CALLCHAIN;
+		attr.sample_type &= ~(u64)PERF_SAMPLE_BRANCH_STACK;
+		pr_debug("Synthesizing 'return' event with id %" PRIu64 " sample type %#" PRIx64 "\n",
+			 id, (u64)attr.sample_type);
+		err = intel_pt_synth_event(session, &attr, id);
+		if (err) {
+			pr_err("%s: failed to synthesize 'return' event type\n",
+			       __func__);
+			return err;
+		}
+		pt->sample_returns = true;
+		pt->returns_sample_type = attr.sample_type;
+		pt->returns_id = id;
+		evlist__for_each(evlist, evsel) {
+			if (evsel->id && evsel->id[0] == pt->returns_id) {
+				if (evsel->name)
+					zfree(&evsel->name);
+				evsel->name = strdup("return");
+				break;
+			}
+		}
  	}
 
  	pt->synth_needs_swap = evsel->needs_swap;
--

2.5.5

Quoted 8 lines

Em Mon, May 23, 2016 at 05:52:23PM -0700, Andi Kleen escreveu:

Quoted 7 lines

From: Andi Kleen <[email protected]>

With perf script --itrace=cr we can synthesize calls and returns out of a PT log. However both calls and returns are marked with the same event, called branches. This makes it difficult to read and post process, because calls and returns are somewhat diffferent.

Quoted 3 lines

Quoted 1 line

Create a separate return event and mark the returns as return.

Adrian, you voiced some concerns about this patch, are those settled?

What about beautifying the sample flags i.e. instead of displaying the letters interpret them into something more human readable e.g.

bc call
br return
bo conditional jump
b jump
bci software interrupt
bri return from interrupt
bcs system call
brs return from system call
by asynchronous branch
bcyi hardware interrupt
bA transaction abort
bB trace begin
bE trace end

In Tx (x) can turn up on a number of those too.

Quoted 114 lines

Can I have your Acked-by, please?

- Arnaldo

Quoted 110 lines

Cc [email protected]

Signed-off-by Andi Kleen <[email protected]>

---
  tools/perf/util/intel-pt.c | 53 +++++++++++++++++++++++++++++++++++++++++-----
  1 file changed, 48 insertions(+), 5 deletions(-)

tools/perf/util/intel-pt.c +48 -6

index 137196990012..4f0b1d7adf50 100644
@@ -82,9 +82,12 @@ struct intel_pt {
  	u64 instructions_id;
 
  	bool sample_branches;
+	bool sample_returns;
  	u32 branches_filter;
  	u64 branches_sample_type;
+	u64 returns_sample_type;
  	u64 branches_id;
+	u64 returns_id;
 
  	bool sample_transactions;
  	u64 transactions_sample_type;

@@ -960,7 +963,8 @@ static int intel_pt_inject_event(union perf_event *event,
  	return perf_event__synthesize_sample(event, type, 0, sample, swapped);
  }
 
-static int intel_pt_synth_branch_sample(struct intel_pt_queue *ptq)
+static int intel_pt_synth_branch_sample(struct intel_pt_queue *ptq,
+					bool is_return)
  {
  	int ret;
  	struct intel_pt *pt = ptq->pt;

@@ -990,8 +994,13 @@ static int intel_pt_synth_branch_sample(struct intel_pt_queue *ptq)
  	sample.pid = ptq->pid;
  	sample.tid = ptq->tid;
  	sample.addr = ptq->state->to_ip;
-	sample.id = ptq->pt->branches_id;
-	sample.stream_id = ptq->pt->branches_id;
+	if (is_return) {
+		sample.id = ptq->pt->returns_id;
+		sample.stream_id = ptq->pt->returns_id;
+	} else {
+		sample.id = ptq->pt->branches_id;
+		sample.stream_id = ptq->pt->branches_id;
+	}
  	sample.period = 1;
  	sample.cpu = ptq->cpu;
  	sample.flags = ptq->flags;

@@ -1014,6 +1023,8 @@ static int intel_pt_synth_branch_sample(struct intel_pt_queue *ptq)
 
  	if (pt->synth_opts.inject) {
  		ret = intel_pt_inject_event(event, &sample,
+					    is_return ?
+					    pt->returns_sample_type :
  					    pt->branches_sample_type,
  					    pt->synth_needs_swap);
  		if (ret)

@@ -1241,7 +1252,13 @@ static int intel_pt_sample(struct intel_pt_queue *ptq)
  		thread_stack__set_trace_nr(ptq->thread, state->trace_nr);
 
  	if (pt->sample_branches) {
-		err = intel_pt_synth_branch_sample(ptq);
+		err = intel_pt_synth_branch_sample(ptq, false);
+		if (err)
+			return err;
+	}
+
+	if (pt->sample_returns) {
+		err = intel_pt_synth_branch_sample(ptq, true);
  		if (err)
  			return err;
  	}

@@ -1956,7 +1973,33 @@ static int intel_pt_synth_events(struct intel_pt *pt,
  		}
  		pt->sample_branches = true;
  		pt->branches_sample_type = attr.sample_type;
-		pt->branches_id = id;
+		pt->branches_id = id++;
+	}
+	if (pt->synth_opts.returns) {
+		attr.config = PERF_COUNT_HW_BRANCH_INSTRUCTIONS;
+		attr.sample_period = 1;
+		attr.sample_type |= PERF_SAMPLE_ADDR;
+		attr.sample_type &= ~(u64)PERF_SAMPLE_CALLCHAIN;
+		attr.sample_type &= ~(u64)PERF_SAMPLE_BRANCH_STACK;
+		pr_debug("Synthesizing 'return' event with id %" PRIu64 " sample type %#" PRIx64 "\n",
+			 id, (u64)attr.sample_type);
+		err = intel_pt_synth_event(session, &attr, id);
+		if (err) {
+			pr_err("%s: failed to synthesize 'return' event type\n",
+			       __func__);
+			return err;
+		}
+		pt->sample_returns = true;
+		pt->returns_sample_type = attr.sample_type;
+		pt->returns_id = id;
+		evlist__for_each(evlist, evsel) {
+			if (evsel->id && evsel->id[0] == pt->returns_id) {
+				if (evsel->name)
+					zfree(&evsel->name);
+				evsel->name = strdup("return");
+				break;
+			}
+		}
  	}
 
  	pt->synth_needs_swap = evsel->needs_swap;
--

2.5.5

Quoted 2 lines

What about beautifying the sample flags i.e. instead of displaying the letters interpret them into something more human readable e.g.

Looks like a good idea, but can't this be done on top of what he sent?

Quoted 4 lines

Quoted 2 lines

What about beautifying the sample flags i.e. instead of displaying the letters interpret them into something more human readable e.g.

Looks like a good idea, but can't this be done on top of what he sent?

Yes it both makes sense together.

It looks like it synths 2 samples for each return (one "branches" and one "return"), which I would expect would mess up the thread_stack pairing calls and returns. Did you intend to have 2 samples?