Re: [PATCH V6 15/17] perf tools: Intel BTS to always update thread stack trace number

[Adrian Hunter <adrian.hunter@intel.com>]

On 19/06/15 19:11, Arnaldo Carvalho de Melo wrote:
> Em Fri, May 29, 2015 at 04:33:43PM +0300, Adrian Hunter escreveu:
>> The enhanced thread stack is used by higher layers but still requires
>> the trace number.  The trace number is used to distinguish discontinuous
>> sections of trace (for example from Snapshot mode or Sample mode), which
>> cause the thread stack to be flushed.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  tools/perf/util/intel-bts.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/util/intel-bts.c b/tools/perf/util/intel-bts.c
>> index b068860..cd7bde3 100644
>> --- a/tools/perf/util/intel-bts.c
>> +++ b/tools/perf/util/intel-bts.c
>> @@ -27,6 +27,8 @@
>>  #include "machine.h"
>>  #include "session.h"
>>  #include "util.h"
>> +#include "thread.h"
>> +#include "thread-stack.h"
>>  #include "debug.h"
>>  #include "tsc.h"
>>  #include "auxtrace.h"
>> @@ -443,19 +445,22 @@ static int intel_bts_process_buffer(struct intel_bts_queue *btsq,
>>  
>>  static int intel_bts_process_queue(struct intel_bts_queue *btsq, u64 *timestamp)
>>  {
>> -	struct auxtrace_buffer *buffer = btsq->buffer;
>> +	struct auxtrace_buffer *buffer = btsq->buffer, *old_buffer = buffer;
>>  	struct auxtrace_queue *queue;
>> +	struct thread *thread;
>>  	int err;
>>  
>>  	if (btsq->done)
>>  		return 1;
>>  
>>  	if (btsq->pid == -1) {
>> -		struct thread *thread;
>> -
>> -		thread = machine__find_thread(btsq->bts->machine, -1, btsq->tid);
>> +		thread = machine__find_thread(btsq->bts->machine, -1,
>> +					      btsq->tid);
>>  		if (thread)
>>  			btsq->pid = thread->pid_;
>> +	} else {
>> +		thread = machine__findnew_thread(btsq->bts->machine, btsq->pid,
>> +						 btsq->tid);
> 
> Humm, so what will be done with the reference count you got from
> machine__findnew_thread()? You have to drop it when you're done with
> using this thread.
> 

Thought I fixed that. Went looking and, yes, the chunks got lost when
rolling V6 of the patches. Anyway here are the fixes as a separate patch.

From: Adrian Hunter <adrian.hunter@intel.com>
Date: Mon, 22 Jun 2015 15:02:04 +0300
Subject: [PATCH] perf tools: Fix missing thread__put()s

Processing for Intel BTS and Intel PT are using machine__find_thread() and
machine__findnew_thread() which increase the struct thread reference count.
Add missing thread__put()s when finished with that struct thread reference.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/intel-bts.c | 36 ++++++++++++++++++++++++------------
 tools/perf/util/intel-pt.c  |  5 +++--
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/intel-bts.c b/tools/perf/util/intel-bts.c
index cd7bde33b635..dce99cfb1309 100644
--- a/tools/perf/util/intel-bts.c
+++ b/tools/perf/util/intel-bts.c
@@ -318,6 +318,7 @@ static int intel_bts_get_next_insn(struct
intel_bts_queue *btsq, u64 ip)
 	ssize_t len;
 	int x86_64;
 	uint8_t cpumode;
+	int err = -1;

 	bufsz = intel_pt_insn_max_size();

@@ -332,11 +333,11 @@ static int intel_bts_get_next_insn(struct
intel_bts_queue *btsq, u64 ip)

 	thread__find_addr_map(thread, cpumode, MAP__FUNCTION, ip, &al);
 	if (!al.map || !al.map->dso)
-		return -1;
+		goto out_put;

 	len = dso__data_read_addr(al.map->dso, al.map, machine, ip, buf, bufsz);
 	if (len <= 0)
-		return -1;
+		goto out_put;

 	/* Load maps to ensure dso->is_64_bit has been updated */
 	map__load(al.map, machine->symbol_filter);
@@ -344,9 +345,12 @@ static int intel_bts_get_next_insn(struct
intel_bts_queue *btsq, u64 ip)
 	x86_64 = al.map->dso->is_64_bit;

 	if (intel_pt_get_insn(buf, len, x86_64, &btsq->intel_pt_insn))
-		return -1;
+		goto out_put;

-	return 0;
+	err = 0;
+out_put:
+	thread__put(thread);
+	return err;
 }

 static int intel_bts_synth_error(struct intel_bts *bts, int cpu, pid_t pid,
@@ -471,24 +475,31 @@ static int intel_bts_process_queue(struct
intel_bts_queue *btsq, u64 *timestamp)
 	if (!buffer) {
 		if (!btsq->bts->sampling_mode)
 			btsq->done = 1;
-		return 1;
+		err = 1;
+		goto out_put;
 	}

 	/* Currently there is no support for split buffers */
-	if (buffer->consecutive)
-		return -EINVAL;
+	if (buffer->consecutive) {
+		err = -EINVAL;
+		goto out_put;
+	}

 	if (!buffer->data) {
 		int fd = perf_data_file__fd(btsq->bts->session->file);

 		buffer->data = auxtrace_buffer__get_data(buffer, fd);
-		if (!buffer->data)
-			return -ENOMEM;
+		if (!buffer->data) {
+			err = -ENOMEM;
+			goto out_put;
+		}
 	}

 	if (btsq->bts->snapshot_mode && !buffer->consecutive &&
-	    intel_bts_do_fix_overlap(queue, buffer))
-		return -ENOMEM;
+	    intel_bts_do_fix_overlap(queue, buffer)) {
+		err = -ENOMEM;
+		goto out_put;
+	}

 	if (!btsq->bts->synth_opts.callchain && thread &&
 	    (!old_buffer || btsq->bts->sampling_mode ||
@@ -507,7 +518,8 @@ static int intel_bts_process_queue(struct
intel_bts_queue *btsq, u64 *timestamp)
 		if (!btsq->bts->sampling_mode)
 			btsq->done = 1;
 	}
-
+out_put:
+	thread__put(thread);
 	return err;
 }

diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
index 751c43a1fbcc..8c8559615666 100644
--- a/tools/perf/util/intel-pt.c
+++ b/tools/perf/util/intel-pt.c
@@ -200,7 +200,7 @@ static void intel_pt_use_buffer_pid_tid(struct
intel_pt_queue *ptq,
 	intel_pt_log("queue %u cpu %d pid %d tid %d\n",
 		     ptq->queue_nr, ptq->cpu, ptq->pid, ptq->tid);

-	ptq->thread = NULL;
+	thread__zput(ptq->thread);

 	if (ptq->tid != -1) {
 		if (ptq->pid != -1)
@@ -713,6 +713,7 @@ static void intel_pt_free_queue(void *priv)

 	if (!ptq)
 		return;
+	thread__zput(ptq->thread);
 	intel_pt_decoder_free(ptq->decoder);
 	zfree(&ptq->event_buf);
 	zfree(&ptq->chain);
@@ -726,7 +727,7 @@ static void intel_pt_set_pid_tid_cpu(struct intel_pt *pt,

 	if (queue->tid == -1 || pt->have_sched_switch) {
 		ptq->tid = machine__get_current_tid(pt->machine, ptq->cpu);
-		ptq->thread = NULL;
+		thread__zput(ptq->thread);
 	}

 	if (!ptq->thread && ptq->tid != -1)
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/