From: "Hunter, Adrian" <adrian.hunter@intel.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>, Andi Kleen <ak@linux.intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"leo.yan@linaro.org" <leo.yan@linaro.org>,
David Miller <davem@davemloft.net>,
Mathieu Poirier <mathieu.poirier@linaro.org>
Subject: RE: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficient
Date: Mon, 5 Nov 2018 19:53:17 +0000 [thread overview]
Message-ID: <363DA0ED52042842948283D2FC38E4649C3133B4@IRSMSX106.ger.corp.intel.com> (raw)
In-Reply-To: <20181105193615.GG7077@kernel.org>
> -----Original Message-----
> From: Arnaldo Carvalho de Melo [mailto:acme@kernel.org]
> Sent: Monday, November 5, 2018 9:36 PM
> To: Hunter, Adrian <adrian.hunter@intel.com>
> Cc: Jiri Olsa <jolsa@redhat.com>; Andi Kleen <ak@linux.intel.com>; linux-
> kernel@vger.kernel.org; leo.yan@linaro.org; David Miller
> <davem@davemloft.net>; Mathieu Poirier <mathieu.poirier@linaro.org>
> Subject: Re: [PATCH 1/5] perf tools: Add fallback functions for cases where
> cpumode is insufficient
>
> Em Mon, Nov 05, 2018 at 07:21:44PM +0000, Hunter, Adrian escreveu:
> > > In Monday, November 5, 2018 7:30 PM, Arnaldo Carvalho de Melo wrote
> <SNIP>
> > > > +struct map *thread__find_map_fallback(struct thread *thread, u8
> > > cpumode,
> > > > + u64 addr, struct addr_location *al) {
>
> > > You named one as _fallback...
>
> > > > + struct map *map = thread__find_map(thread, cpumode, addr, al);
> > > > + struct machine *machine = thread->mg->machine;
> > > > + u8 addr_cpumode = machine__addr_cpumode(machine, cpumode,
> > > addr);
> > > > +
> > > > + if (map || addr_cpumode == cpumode)
> > > > + return map;
> > > > +
> > > > + return thread__find_map(thread, addr_cpumode, addr, al); }
> > > > +
> > > > struct symbol *thread__find_symbol(struct thread *thread, u8
> cpumode,
> > > > u64 addr, struct addr_location *al) { @@ -
> > > 1585,6 +1603,15 @@
> > > > struct symbol *thread__find_symbol(struct thread *thread, u8
> cpumode,
> > > > return al->sym;
> > > > }
>
> > > > +struct symbol *thread__find_symbol_fb(struct thread *thread, u8
> > > cpumode,
> > > > + u64 addr, struct addr_location *al)
>
> > > ... and the other as _fb, make it consistent, please.
>
> > ok
>
> > > > +{
> > > > + al->sym = NULL;
> > > > + if (thread__find_map_fallback(thread, cpumode, addr, al))
> > > > + al->sym = map__find_symbol(al->map, al->addr);
> > > > + return al->sym;
> > > > +}
>
> > > > /*
> > > > * Callers need to drop the reference to al->thread, obtained in
> > > > * machine__findnew_thread()
> > > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > > > index 111ae858cbcb..04edc0eac376 100644
> > > > --- a/tools/perf/util/machine.c
> > > > +++ b/tools/perf/util/machine.c
> > > > @@ -2542,6 +2542,46 @@ int machine__get_kernel_start(struct
> > > > machine
> > > *machine)
> > > > return err;
> > > > }
>
> > > > +/*
> > > > + * machine__single_ku_as - Machine has same address space for
> > > > +kernel
> > > and user.
> > > > + * @machine: machine object
> > > > + *
> > > > + * Some architectures have a single address space for kernel and
> > > > +user addresses,
> > > > + * which makes it possible to determine if an address is in
> > > > +kernel space or user
> > > > + * space.
> > > > + */
> > > > +static bool machine__single_ku_as(struct machine *machine) {
> > > > + return strcmp(perf_env__arch(machine->env), "sparc"); }
>
> > > Can we avoid having this strcmp be done repeatedly?
>
> > It is only done if there are mapping errors
>
> > > Can we avoid having this strcmp be done repeatedly? I.e. just make
> > this a
> > > boolean initialized at session start, when machine->env is setup, so
> > > we'd
> > > have:
>
> > > machine->single_address_space
>
> > > Instead of a function?
>
> > Sure thing.
>
> Thanks
>
> > >
> > > Also have you considered making this fallback to be performed only
> > > from code that is that arch specific?
> > >
> > > I.e. the code that supports branch samples/stacks is x86_86 specific
> > > at this point and thus only in that case we would call the routines
> > > that perform the fallback, which, in turn, wouldn't need to check for
> "sparc"?
>
> > I will look at it, but theoretically someone could be processing x86
> > data but doing it on a machine of a different architecture.
>
> Right, that should be supported, yes. What I meant was that when
> processing perf.data file with samples where the cpumode can't be inferred,
> we should use the fallback routines.
>
> It is super unfortunate that we have addresses without a accompanying
> cpumode :-\ Don't you think those coulde be fixed somehow? If this comes
> from things synthesized from Intel PT traces, then we can use the address
> ranges for kernel/userspace to derive that before hitting the core code, that
> would be fed with addr/cpumode pairs, just like we have hdr.misc &
> USER/KERNEL and the PERF_CONTEXT_ markers in callchains.
Yes we will probably need to look at that, but at the moment I would like a fix for stable.
next prev parent reply other threads:[~2018-11-05 19:53 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-31 9:10 [PATCH 0/5] perf tools: Fix for cases where cpumode is incorrect or insufficient Adrian Hunter
2018-10-31 9:10 ` [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficient Adrian Hunter
2018-11-05 17:29 ` Arnaldo Carvalho de Melo
2018-11-05 18:19 ` David Miller
2018-11-05 19:21 ` Hunter, Adrian
2018-11-05 19:36 ` Arnaldo Carvalho de Melo
2018-11-05 19:53 ` Hunter, Adrian [this message]
2018-11-05 20:31 ` Arnaldo Carvalho de Melo
2018-11-26 19:02 ` Arnaldo Carvalho de Melo
2018-11-26 19:41 ` Arnaldo Carvalho de Melo
2018-11-26 19:44 ` Arnaldo Carvalho de Melo
2018-10-31 9:10 ` [PATCH 2/5] perf tools: Use fallback for sample_addr_correlates_sym() cases Adrian Hunter
2018-11-03 17:46 ` Hunter, Adrian
2018-10-31 9:10 ` [PATCH 3/5] perf tools: Use fallbacks for branch stacks Adrian Hunter
2018-11-05 17:56 ` Arnaldo Carvalho de Melo
2018-10-31 9:10 ` [PATCH 4/5] perf intel-pt: Insert callchain context into synthesized callchains Adrian Hunter
2018-10-31 13:28 ` Arnaldo Carvalho de Melo
2018-10-31 14:15 ` Adrian Hunter
2018-10-31 14:20 ` Adrian Hunter
2018-10-31 14:27 ` Arnaldo Carvalho de Melo
2018-10-31 22:12 ` [tip:perf/urgent] " tip-bot for Adrian Hunter
2018-10-31 14:26 ` [PATCH 4/5] " Arnaldo Carvalho de Melo
2018-10-31 9:10 ` [PATCH 5/5] perf intel-pt/bts: Calculate cpumode for synthesized samples Adrian Hunter
2018-10-31 22:13 ` [tip:perf/urgent] " tip-bot for Adrian Hunter
2018-10-31 13:35 ` [PATCH 0/5] perf tools: Fix for cases where cpumode is incorrect or insufficient Jiri Olsa
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=363DA0ED52042842948283D2FC38E4649C3133B4@IRSMSX106.ger.corp.intel.com \
--to=adrian.hunter@intel.com \
--cc=acme@kernel.org \
--cc=ak@linux.intel.com \
--cc=davem@davemloft.net \
--cc=jolsa@redhat.com \
--cc=leo.yan@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.poirier@linaro.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).