| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150622143354.GA13937@kernel.org>
Date: Mon, 22 Jun 2015 11:33:54 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Adrian Hunter <adrian.hunter@...el.com>
Cc: Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
Jiri Olsa <jolsa@...hat.com>
Subject: Re: [PATCH V6 15/17] perf tools: Intel BTS to always update thread
stack trace number
Em Mon, Jun 22, 2015 at 03:38:35PM +0300, Adrian Hunter escreveu:
> 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@...el.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.
Thanks, I'll try and fold that into that patch, to keep bisect happy, as
refcount bugs sometimes are hard to find, I guess I'll even try writing
a generic perf based refcounting debugging tool at some point...
- Arnaldo
> From: Adrian Hunter <adrian.hunter@...el.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@...el.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/
Powered by blists - more mailing lists