Re: [PATCH V6 15/17] perf tools: Intel BTS to always update thread stack trace number
From: Adrian Hunter
Date: Mon Jun 22 2015 - 08:41:08 EST
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@xxxxxxxxx>
>> ---
>> 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@xxxxxxxxx>
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@xxxxxxxxx>
---
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/