Em Wed, Oct 31, 2018 at 11:10:39AM +0200, Adrian Hunter escreveu: > For branch stacks or branch samples, the sample cpumode might not be > correct because it applies only to the sample 'ip' and not necessary to > 'addr' or branch stack addresses. Add fallback functions that can be used > to deal with those cases > > Signed-off-by: Adrian Hunter <adrian.hun...@intel.com> > Cc: sta...@vger.kernel.org # 4.19 > --- > tools/perf/util/event.c | 27 ++++++++++++++++++++++++++ > tools/perf/util/machine.c | 40 +++++++++++++++++++++++++++++++++++++++ > tools/perf/util/machine.h | 2 ++ > tools/perf/util/thread.h | 4 ++++ > 4 files changed, 73 insertions(+) > > diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c > index bc646185f8d9..52b24b8ce29e 100644 > --- a/tools/perf/util/event.c > +++ b/tools/perf/util/event.c > @@ -1576,6 +1576,24 @@ struct map *thread__find_map(struct thread *thread, u8 > cpumode, u64 addr, > return al->map; > } > > +/* > + * For branch stacks or branch samples, the sample cpumode might not be > correct > + * because it applies only to the sample 'ip' and not necessary to 'addr' or > + * branch stack addresses. If possible, use a fallback to deal with those > cases. > + */ > +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. > +{ > + 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? 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? 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"? > + > +u8 machine__addr_cpumode(struct machine *machine, u8 cpumode, u64 addr) > +{ > + u8 addr_cpumode = cpumode; > + bool kernel_ip; > + > + if (!machine__single_ku_as(machine)) > + goto out; > + > + kernel_ip = machine__kernel_ip(machine, addr); > + switch (cpumode) { > + case PERF_RECORD_MISC_KERNEL: > + case PERF_RECORD_MISC_USER: > + addr_cpumode = kernel_ip ? PERF_RECORD_MISC_KERNEL : > + PERF_RECORD_MISC_USER; > + break; > + case PERF_RECORD_MISC_GUEST_KERNEL: > + case PERF_RECORD_MISC_GUEST_USER: > + addr_cpumode = kernel_ip ? PERF_RECORD_MISC_GUEST_KERNEL : > + PERF_RECORD_MISC_GUEST_USER; > + break; > + default: > + break; > + } > +out: > + return addr_cpumode; > +} > + > struct dso *machine__findnew_dso(struct machine *machine, const char > *filename) > { > return dsos__findnew(&machine->dsos, filename); > diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h > index d856b85862e2..20d464135ed1 100644 > --- a/tools/perf/util/machine.h > +++ b/tools/perf/util/machine.h > @@ -99,6 +99,8 @@ static inline bool machine__kernel_ip(struct machine > *machine, u64 ip) > return ip >= kernel_start; > } > > +u8 machine__addr_cpumode(struct machine *machine, u8 cpumode, u64 addr); > + > struct thread *machine__find_thread(struct machine *machine, pid_t pid, > pid_t tid); > struct comm *machine__thread_exec_comm(struct machine *machine, > diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h > index 36c09a9904e6..e28f9dc1110a 100644 > --- a/tools/perf/util/thread.h > +++ b/tools/perf/util/thread.h > @@ -96,9 +96,13 @@ struct thread *thread__main_thread(struct machine > *machine, struct thread *threa > > struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr, > struct addr_location *al); > +struct map *thread__find_map_fallback(struct thread *thread, u8 cpumode, > + u64 addr, struct addr_location *al); > > struct symbol *thread__find_symbol(struct thread *thread, u8 cpumode, > u64 addr, struct addr_location *al); > +struct symbol *thread__find_symbol_fb(struct thread *thread, u8 cpumode, > + u64 addr, struct addr_location *al); > > void thread__find_cpumode_addr_location(struct thread *thread, u64 addr, > struct addr_location *al); > -- > 2.17.1