lkml.org 
[lkml]   [2018]   [Nov]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficient
    Date
    > -----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.

    \
     
     \ /
      Last update: 2018-11-05 20:53    [W:4.219 / U:0.076 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site