LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Leo Yan <leo.yan@linaro.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Coresight ML <coresight@lists.linaro.org>
Subject: Re: [PATCH] perf cs-etm: Correct CPU mode for samples
Date: Tue, 30 Oct 2018 11:32:26 -0300	[thread overview]
Message-ID: <20181030143226.GA23310@kernel.org> (raw)
In-Reply-To: <1540883908-17018-1-git-send-email-leo.yan@linaro.org>

Em Tue, Oct 30, 2018 at 03:18:28PM +0800, Leo Yan escreveu:
> Since commit 9042f5e3539e ("perf tools: Stop fallbacking to kallsyms
> for vdso symbols lookup"), the kernel address cannot be properly parsed
> to kernel symbol with command 'perf script -k vmlinux'.  The reason is
> CoreSight samples is always to set CPU mode as PERF_RECORD_MISC_USER,
> thus it fails to find corresponding map/dso in below flows:
> 
>   process_sample_event()
>     `-> machine__resolve()
> 	  `-> thread__find_map(thread, sample->cpumode, sample->ip, al);
> 
> In this flow it needs to pass argument 'sample->cpumode' to tell what's
> the CPU mode, before it always passed PERF_RECORD_MISC_USER but without
> any failure until the commit 9042f5e3539e ("perf tools: Stop fallbacking
> to kallsyms for vdso symbols lookup") has been merged.  The reason is
> even with the wrong CPU mode the function thread__find_map() firstly
> fails to find map but it will rollback to find kernel map for vdso
> symbols lookup.  In the latest code it has removed the fallback code,
> thus if CPU mode is PERF_RECORD_MISC_USER then it cannot find map
> anymore with kernel address.
> 
> This patch is to correct samples CPU mode setting, it creates a new
> helper function cs_etm__cpu_mode() to tell what's the CPU mode based on
> the address with the info from machine structure; this patch has a bit
> extension to check not only kernel and user mode, but also check for
> host/guest and hypervisor mode.  Finally this patch uses the function
> in instruction and branch samples and also apply in cs_etm__mem_access()
> for a minor polishing.

Mathieu, can I have your Acked-by, please? Leo, thanks for acting so
quickly on this one! 

Now processing coresight traces should be faster, less lookups :-)

- Arnaldo
 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  tools/perf/util/cs-etm.c | 39 ++++++++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 3b37d66..73430b7 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -244,6 +244,27 @@ static void cs_etm__free(struct perf_session *session)
>  	zfree(&aux);
>  }
>  
> +static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address)
> +{
> +	struct machine *machine;
> +
> +	machine = etmq->etm->machine;
> +
> +	if (address >= etmq->etm->kernel_start) {
> +		if (machine__is_host(machine))
> +			return PERF_RECORD_MISC_KERNEL;
> +		else
> +			return PERF_RECORD_MISC_GUEST_KERNEL;
> +	} else {
> +		if (machine__is_host(machine))
> +			return PERF_RECORD_MISC_USER;
> +		else if (perf_guest)
> +			return PERF_RECORD_MISC_GUEST_USER;
> +		else
> +			return PERF_RECORD_MISC_HYPERVISOR;
> +	}
> +}
> +
>  static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u64 address,
>  			      size_t size, u8 *buffer)
>  {
> @@ -258,10 +279,7 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u64 address,
>  		return -1;
>  
>  	machine = etmq->etm->machine;
> -	if (address >= etmq->etm->kernel_start)
> -		cpumode = PERF_RECORD_MISC_KERNEL;
> -	else
> -		cpumode = PERF_RECORD_MISC_USER;
> +	cpumode = cs_etm__cpu_mode(etmq, address);
>  
>  	thread = etmq->thread;
>  	if (!thread) {
> @@ -653,7 +671,7 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
>  	struct perf_sample sample = {.ip = 0,};
>  
>  	event->sample.header.type = PERF_RECORD_SAMPLE;
> -	event->sample.header.misc = PERF_RECORD_MISC_USER;
> +	event->sample.header.misc = cs_etm__cpu_mode(etmq, addr);
>  	event->sample.header.size = sizeof(struct perf_event_header);
>  
>  	sample.ip = addr;
> @@ -665,7 +683,7 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
>  	sample.cpu = etmq->packet->cpu;
>  	sample.flags = 0;
>  	sample.insn_len = 1;
> -	sample.cpumode = event->header.misc;
> +	sample.cpumode = event->sample.header.misc;
>  
>  	if (etm->synth_opts.last_branch) {
>  		cs_etm__copy_last_branch_rb(etmq);
> @@ -706,12 +724,15 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq)
>  		u64			nr;
>  		struct branch_entry	entries;
>  	} dummy_bs;
> +	u64 ip;
> +
> +	ip = cs_etm__last_executed_instr(etmq->prev_packet);
>  
>  	event->sample.header.type = PERF_RECORD_SAMPLE;
> -	event->sample.header.misc = PERF_RECORD_MISC_USER;
> +	event->sample.header.misc = cs_etm__cpu_mode(etmq, ip);
>  	event->sample.header.size = sizeof(struct perf_event_header);
>  
> -	sample.ip = cs_etm__last_executed_instr(etmq->prev_packet);
> +	sample.ip = ip;
>  	sample.pid = etmq->pid;
>  	sample.tid = etmq->tid;
>  	sample.addr = cs_etm__first_executed_instr(etmq->packet);
> @@ -720,7 +741,7 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq)
>  	sample.period = 1;
>  	sample.cpu = etmq->packet->cpu;
>  	sample.flags = 0;
> -	sample.cpumode = PERF_RECORD_MISC_USER;
> +	sample.cpumode = event->sample.header.misc;
>  
>  	/*
>  	 * perf report cannot handle events without a branch stack
> -- 
> 2.7.4

  reply	other threads:[~2018-10-30 14:32 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-30  7:18 Leo Yan
2018-10-30 14:32 ` Arnaldo Carvalho de Melo [this message]
2018-10-30 15:04   ` leo.yan
2018-10-30 15:11     ` Arnaldo Carvalho de Melo
2018-10-30 15:30       ` Adrian Hunter
2018-10-30 15:45         ` Arnaldo Carvalho de Melo
2018-10-31 22:04 ` [tip:perf/urgent] " tip-bot for Leo Yan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181030143226.GA23310@kernel.org \
    --to=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=coresight@lists.linaro.org \
    --cc=jolsa@redhat.com \
    --cc=leo.yan@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.poirier@linaro.org \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --subject='Re: [PATCH] perf cs-etm: Correct CPU mode for samples' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).