From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755644AbcEXQ0Z (ORCPT ); Tue, 24 May 2016 12:26:25 -0400 Received: from mga03.intel.com ([134.134.136.65]:50458 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755478AbcEXQ0X (ORCPT ); Tue, 24 May 2016 12:26:23 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,360,1459839600"; d="scan'208";a="961385487" Subject: Re: [PATCH 1/3] perf pt: Mark PT return events as "return" To: Arnaldo Carvalho de Melo , Andi Kleen References: <1464051145-19968-1-git-send-email-andi@firstfloor.org> <20160524145702.GT8897@kernel.org> Cc: jolsa@kernel.org, linux-kernel@vger.kernel.org, Andi Kleen From: Adrian Hunter Message-ID: <574480AB.2000701@intel.com> Date: Tue, 24 May 2016 19:26:19 +0300 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20160524145702.GT8897@kernel.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 24/05/2016 5:57 p.m., Arnaldo Carvalho de Melo wrote: > Em Mon, May 23, 2016 at 05:52:23PM -0700, Andi Kleen escreveu: >> From: Andi Kleen >> >> 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? 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. > Can I have your Acked-by, please? > > - Arnaldo > >> Cc: adrian.hunter@intel.com >> Signed-off-by: Andi Kleen >> --- >> tools/perf/util/intel-pt.c | 53 +++++++++++++++++++++++++++++++++++++++++----- >> 1 file changed, 48 insertions(+), 5 deletions(-) >> >> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c >> index 137196990012..4f0b1d7adf50 100644 >> --- a/tools/perf/util/intel-pt.c >> +++ b/tools/perf/util/intel-pt.c >> @@ -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