perf tools: Fix for cases where cpumode is incorrect or insufficient
Adrian Hunter
reviewercover perf tools: Fix for cases where cpumode is incorrect or insufficient
Hi
These patches probably deal with most cases except the one fixed by David Miller's "perf callchain: Honour the ordering of PERF_CONTEXT_{USER,KERNEL,etc}" patch, and also cat_backtrace() which looks like it has the same problem, and the cs-etm fix.
Adrian Hunter (5):
perf tools: Add fallback functions for cases where cpumode is insufficient perf tools: Use fallback for sample_addr_correlates_sym() cases perf tools: Use fallbacks for branch stacks
perf intel-pt: Insert callchain context into synthesized callchains perf intel-pt/bts: Calculate cpumode for synthesized samples
tools/perf/builtin-script.c | 12 +++--- tools/perf/util/event.c | 29 +++++++++++++- tools/perf/util/intel-bts.c | 17 ++++++--- tools/perf/util/intel-pt.c | 28 ++++++++------ tools/perf/util/machine.c | 40 ++++++++++++++++++++ tools/perf/util/machine.h | 2 + .../util/scripting-engines/trace-event-python.c | 16 ++++---- tools/perf/util/thread-stack.c | 44 +++++++++++++++++----- tools/perf/util/thread-stack.h | 2 +- tools/perf/util/thread.h | 4 ++ 10 files changed, 153 insertions(+), 41 deletions(-)
Regards
Adrian
Adrian Hunter
reviewerPATCH 1/5 perf tools: Add fallback functions for cases where cpumode is insufficient
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
--- 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(+)
tools/perf/util/event.c +27 -0
@@ -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) +{ +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) +{ + 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()
tools/perf/util/machine.c +40 -0
@@ -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" ); +} + +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);
tools/perf/util/machine.h +2 -0
@@ -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,
tools/perf/util/thread.h +4 -0
@@ -96,9 +96,13 @@ struct thread *thread__main_thread(struct machine *machine, struct thread *threastruct 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);
Signature · 1 line
2.17.1
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficient
Em Wed, Oct 31, 2018 at 11:10:39AM +0200, Adrian Hunter escreveu:
Quoted 20 lines
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 <[email protected]>Cc [email protected] # 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(+)tools/perf/util/event.c +0
-0@@ -1576,6 +1576,24 @@ struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,return al->map;
tools/perf/util/event.c struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr, · line 1586
Quoted 10 lines
tools/perf/util/event.c +8
-0} +/* +* 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...
Quoted 14 lines
tools/perf/util/event.c +10
-0+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) {
tools/perf/util/event.c struct symbol *thread__find_symbol(struct thread *thread, u8 cpumode, · line 1607
Quoted 5 lines
tools/perf/util/event.c +2
-0@@ -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.
Quoted 20 lines
tools/perf/util/event.c +7
-0+{ + 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() tools/perf/util/machine.c +2
-0@@ -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.
tools/perf/util/machine.c int machine__get_kernel_start(struct machine *machine) · line 2556
Quoted 10 lines
tools/perf/util/machine.c +10
-0+* @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"?
Quoted 64 lines
tools/perf/util/machine.c +28
-0+ +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);tools/perf/util/machine.h +2
-0@@ -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,tools/perf/util/thread.h +4
-0@@ -96,9 +96,13 @@ struct thread *thread__main_thread(struct machine *machine, struct thread *threastruct 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);Signature · 1 line
2.17.1
Re: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficient
From: Arnaldo Carvalho de Melo <[email protected]>
Date: Mon, 5 Nov 2018 14:29:46 -0300
Quoted 21 lines
Quoted 6 lines
tools/perf/util/machine.c +2
-0@@ -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. tools/perf/util/machine.c int machine__get_kernel_start(struct machine *machine) · line 2556
Quoted 10 lines
tools/perf/util/machine.c +10
-0+* @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->envis setup, so we'd have:
Quoted 3 lines
machine->single_address_spaceInstead of a function?
Agreed, doing this every time is wasteful.
We could also make it a define in some arch/foo file.
Hunter, Adrian
reviewerRE: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficient
In reply to Arnaldo Carvalho de MeloQuoted 66 lines
-----Original Message-----
From: Arnaldo Carvalho de Melo [mailto:[email protected]]
Sent: Monday, November 5, 2018 7:30 PM
To: Hunter, Adrian <[email protected]>
Cc: Jiri Olsa <[email protected]>; Andi Kleen <[email protected]>; linux- [email protected]; [email protected]; David Miller <[email protected]>; Mathieu Poirier <[email protected]>
Subject: Re: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficientEm Wed, Oct 31, 2018 at 11:10:39AM +0200, Adrian Hunter escreveu:
tools/perf/util/event.c struct map *thread__find_map(struct thread
Quoted 20 lines
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 <[email protected]>Cc [email protected] # 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(+)tools/perf/util/event.c index renamed from tools/perf/util/event.c +0
-0
bc646185f8d9..52b24b8ce29e100644tools/perf/util/event.c +0
-0@@ -1576,6 +1576,24 @@ struct map *thread__find_map(struct thread*thread, u8 cpumode, u64 addr,
tools/perf/util/event.c struct map *thread__find_map(struct thread · line 1584
Quoted 9 lines
tools/perf/util/event.c +6
-0return 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.
tools/perf/util/event.c struct map *thread__find_map(struct thread · line 1586
Quoted 2 lines
tools/perf/util/event.c +2
-0+*/ +struct map *thread__find_map_fallback(struct thread *thread, u8cpumode,
tools/perf/util/event.c struct map *thread__find_map(struct thread · line 1587
Quoted 1 line
tools/perf/util/event.c +1
-0+ u64 addr, struct addr_location *al) {You named one as
_fallback...tools/perf/util/event.c struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
Quoted 3 lines
tools/perf/util/event.c +3
-0+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);
tools/perf/util/event.c struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
Quoted 8 lines
tools/perf/util/event.c +6
-0+ +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 @@
Quoted 4 lines
tools/perf/util/event.c +0
-0struct symbol *thread__find_symbol(struct thread *thread, u8 cpumode,return
al->sym;
}
Quoted 5 lines
Quoted 1 line
+struct symbol *thread__find_symbol_fb(struct thread *thread, u8
cpumode,
tools/perf/util/event.c
Quoted 1 line
tools/perf/util/event.c +1
-0+ u64 addr, struct addr_location *al)... and the other as
_fb, make it consistent, please.
ok
Quoted 23 lines
tools/perf/util/machine.c int machine__get_kernel_start(struct machine
Quoted 16 lines
tools/perf/util/event.c +7
-0+{ + 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() tools/perf/util/machine.c +0
-0@@ -2542,6 +2542,46 @@ int machine__get_kernel_start(struct machine*machine)
tools/perf/util/machine.c int machine__get_kernel_start(struct machine *machine)
Quoted 5 lines
tools/perf/util/machine.c +2
-0return err; } +/* +* machine__single_ku_as - Machine has same address space for kernel and user.
Quoted 12 lines
tools/perf/util/machine.c int machine__get_kernel_start(struct machine · line 2556
Quoted 10 lines
tools/perf/util/machine.c +10
-0+* @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
Quoted 7 lines
Can we avoid having this strcmp be done repeatedly? I.e. just make this a boolean initialized at session start, when
machine->envis setup, so we'd have:machine->single_address_spaceInstead of a function?
Sure thing.
Quoted 7 lines
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/stacksisx86_86specific 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.
Quoted 74 lines
Quoted 10 lines
tools/perf/util/machine.c +9
-0+ +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; +tools/perf/util/machine.c int machine__get_kernel_start(struct machine *machine)
Quoted 10 lines
tools/perf/util/machine.c +10
-0+ 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:tools/perf/util/machine.h static inline bool machine__kernel_ip(struct machine
Quoted 17 lines
tools/perf/util/machine.c +9
-0+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.hb/tools/perf/util/machine.hindexd856b85862e2..20d464135ed1100644tools/perf/util/machine.h +0
-0@@ -99,6 +99,8 @@ static inline bool machine__kernel_ip(struct machine*machine, u64 ip)
tools/perf/util/machine.h static inline bool machine__kernel_ip(struct machine *machine, u64 ip) · line 106
Quoted 9 lines
tools/perf/util/machine.h +3
-0return 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
tools/perf/util/thread.h struct thread *thread__main_thread(struct machine · line 98
Quoted 8 lines
--git
a/tools/perf/util/thread.hb/tools/perf/util/thread.hindex36c09a9904e6..e28f9dc1110a100644tools/perf/util/thread.h +0
-0@@ -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, u64addr,
tools/perf/util/thread.h struct thread *thread__main_thread(struct machine · line 100
Quoted 2 lines
tools/perf/util/thread.h +1
-0struct addr_location *al); +struct map *thread__find_map_fallback(struct thread *thread, u8cpumode,
tools/perf/util/thread.h struct thread *thread__main_thread(struct machine *machine, struct thread *threa
Quoted 5 lines
tools/perf/util/thread.h +2
-0+ 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, u8cpumode,
tools/perf/util/thread.h struct thread *thread__main_thread(struct machine *machine, struct thread *threa
Quoted 3 lines
tools/perf/util/thread.h +1
-0+ u64 addr, struct addr_location *al); void thread__find_cpumode_addr_location(struct thread *thread, u64addr,
Quoted 3 lines
struct
addr_location*al);
--
2.17.1
Arnaldo Carvalho de Melo
maintainerRe: [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:
Quoted 1 line
Quoted 1 line
In Monday, November 5, 2018 7:30 PM, Arnaldo Carvalho de Melo wrote
<SNIP>
Quoted 72 lines
Quoted 23 lines
Quoted 1 line
+struct map *thread__find_map_fallback(struct thread *thread, u8
cpumode,
Review
Quoted 1 line
(quoted diff) +1
-0+ u64 addr, struct addr_location *al) {You named one as
_fallback...tools/perf/util/event.c struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
Quoted 3 lines
tools/perf/util/event.c +3
-0+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);
tools/perf/util/event.c struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
Quoted 8 lines
tools/perf/util/event.c +6
-0+ +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 @@
Quoted 3 lines
struct symbol *thread__find_symbol(struct thread *thread, u8 cpumode,
returnal->sym;
}Quoted 5 lines
Quoted 1 line
+struct symbol *thread__find_symbol_fb(struct thread *thread, u8
cpumode,
Review
Quoted 1 line
(quoted diff) +1
-0+ u64 addr, struct addr_location *al)... and the other as
_fb, make it consistent, please.ok
Quoted 22 lines
tools/perf/util/machine.c int machine__get_kernel_start(struct machine
Quoted 15 lines
tools/perf/util/event.c +6
-0+{ + 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() tools/perf/util/machine.c +0
-0@@ -2542,6 +2542,46 @@ int machine__get_kernel_start(struct machine*machine)
tools/perf/util/machine.c int machine__get_kernel_start(struct machine *machine)
Quoted 5 lines
tools/perf/util/machine.c +2
-0return err; } +/* +* machine__single_ku_as - Machine has same address space for kernel and user.
Quoted 12 lines
tools/perf/util/machine.c int machine__get_kernel_start(struct machine · line 2556
Quoted 10 lines
tools/perf/util/machine.c +10
-0+* @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
Quoted 3 lines
Can we avoid having this strcmp be done repeatedly? I.e. just make this a boolean initialized at session start, when
machine->envis setup, so we'd have:
Quoted 5 lines
Quoted 3 lines
machine->single_address_spaceInstead of a function?
Sure thing.
Thanks
Quoted 10 lines
Quoted 7 lines
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/stacksisx86_86specific 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.
- Arnaldo
Hunter, Adrian
reviewerRE: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficient
Quoted 115 lines
-----Original Message-----
From: Arnaldo Carvalho de Melo [mailto:[email protected]]
Sent: Monday, November 5, 2018 9:36 PM
To: Hunter, Adrian <[email protected]>
Cc: Jiri Olsa <[email protected]>; Andi Kleen <[email protected]>; linux- [email protected]; [email protected]; David Miller <[email protected]>; Mathieu Poirier <[email protected]>
Subject: Re: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficientEm Mon, Nov 05, 2018 at 07:21:44PM +0000, Hunter, Adrian escreveu:
Quoted 1 line
Quoted 1 line
In Monday, November 5, 2018 7:30 PM, Arnaldo Carvalho de Melo wrote
<SNIP>
Quoted 10 lines
Quoted 10 lines
Quoted 1 line
+struct map *thread__find_map_fallback(struct thread *thread, u8
cpumode,
Review
Quoted 1 line
(quoted diff) +1
-0+ u64 addr, struct addr_location *al) {You named one as
_fallback...tools/perf/util/event.c struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
Quoted 3 lines
tools/perf/util/event.c +3
-0+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);
Quoted 7 lines
Quoted 7 lines
tools/perf/util/event.c struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
Quoted 7 lines
tools/perf/util/event.c +6
-0+ +if (map || addr_cpumode == cpumode) +return map; + +return thread__find_map (thread, addr_cpumode, addr, al); } + struct symbol *thread__find_symbol(struct thread *thread, u8cpumode,
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
u64 addr, struct
addr_location*al) { @@ -1585,6 +1603,15 @@
Quoted 1 line
struct symbol *thread__find_symbol(struct thread *thread, u8
cpumode,
Quoted 56 lines
Quoted 8 lines
tools/perf/util/event.c struct symbol *thread__find_symbol(struct thread *thread, u8 cpumode,
Quoted 4 lines
tools/perf/util/event.c +1
-0return al->sym; } +struct symbol *thread__find_symbol_fb(struct thread *thread, u8cpumode,
Review
Quoted 1 line
(quoted diff) +1
-0+ u64 addr, struct addr_location *al)... and the other as
_fb, make it consistent, please.ok
Quoted 24 lines
Quoted 15 lines
tools/perf/util/event.c +6
-0+{ + 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() tools/perf/util/machine.c +0
-0@@ -2542,6 +2542,46 @@ int machine__get_kernel_start(structQuoted 1 line
machine
*machine)
tools/perf/util/machine.c int machine__get_kernel_start(struct machine *machine)
Quoted 6 lines
tools/perf/util/machine.c +3
-0return err; } +/* +* machine__single_ku_as - Machine has same address space for +kernel and user.
Quoted 12 lines
tools/perf/util/machine.c
Quoted 10 lines
+ * @machine: machine object
+ *
+ * Some architectures have a single address space for kernel andtools/perf/util/machine.c +7
-0+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
Quoted 1 line
Can we avoid having this strcmp be done repeatedly? I.e. just make
this a
Quoted 3 lines
boolean initialized at session start, when
machine->envis setup, so we'd
have:Quoted 5 lines
Quoted 3 lines
machine->single_address_spaceInstead of a function?
Sure thing.
Thanks
Quoted 7 lines
Quoted 7 lines
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/stacksisx86_86specific 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"?
Quoted 2 lines
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.
Quoted 6 lines
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.
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficient
Em Mon, Nov 05, 2018 at 07:53:17PM +0000, Hunter, Adrian escreveu:
Quoted 27 lines
Quoted 27 lines
-----Original Message-----
From: Arnaldo Carvalho de Melo [mailto:[email protected]]
Sent: Monday, November 5, 2018 9:36 PM
To: Hunter, Adrian <[email protected]>
Cc: Jiri Olsa <[email protected]>; Andi Kleen <[email protected]>; linux- [email protected]; [email protected]; David Miller <[email protected]>; Mathieu Poirier <[email protected]>
Subject: Re: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficientEm Mon, Nov 05, 2018 at 07:21:44PM +0000, Hunter, Adrian escreveu:
Quoted 7 lines
Quoted 7 lines
In Monday, November 5, 2018 7:30 PM, Arnaldo Carvalho de Melo wrote 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/stacksisx86_86specific 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"?
Quoted 2 lines
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.
Quoted 8 lines
Quoted 6 lines
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.
Ok, with that check for archs like sparc, fair enough, and its great that you consider doing the better fix on top of it, later, thanks!
- Arnaldo
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficient
In reply to Hunter, AdrianEm Mon, Nov 05, 2018 at 07:53:17PM +0000, Hunter, Adrian escreveu:
Quoted 91 lines
Quoted 91 lines
-----Original Message-----
From: Arnaldo Carvalho de Melo [mailto:[email protected]]
Sent: Monday, November 5, 2018 9:36 PM
To: Hunter, Adrian <[email protected]>
Cc: Jiri Olsa <[email protected]>; Andi Kleen <[email protected]>; linux- [email protected]; [email protected]; David Miller <[email protected]>; Mathieu Poirier <[email protected]>
Subject: Re: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficientEm Mon, Nov 05, 2018 at 07:21:44PM +0000, Hunter, Adrian escreveu:
Quoted 1 line
Quoted 1 line
In Monday, November 5, 2018 7:30 PM, Arnaldo Carvalho de Melo wrote
<SNIP>
Quoted 10 lines
Quoted 10 lines
Quoted 1 line
+struct map *thread__find_map_fallback(struct thread *thread, u8
cpumode,
Review
Quoted 1 line
(quoted diff) +1
-0+ u64 addr, struct addr_location *al) {You named one as
_fallback...tools/perf/util/event.c struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
Quoted 3 lines
tools/perf/util/event.c +3
-0+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);
Quoted 7 lines
Quoted 7 lines
tools/perf/util/event.c struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
Quoted 7 lines
tools/perf/util/event.c +6
-0+ +if (map || addr_cpumode == cpumode) +return map; + +return thread__find_map (thread, addr_cpumode, addr, al); } + struct symbol *thread__find_symbol(struct thread *thread, u8cpumode,
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
u64 addr, struct
addr_location*al) { @@ -1585,6 +1603,15 @@
Quoted 1 line
struct symbol *thread__find_symbol(struct thread *thread, u8
cpumode,
Quoted 55 lines
Quoted 8 lines
tools/perf/util/event.c struct symbol *thread__find_symbol(struct thread *thread, u8 cpumode,
Quoted 4 lines
tools/perf/util/event.c +1
-0return al->sym; } +struct symbol *thread__find_symbol_fb(struct thread *thread, u8cpumode,
Review
Quoted 1 line
(quoted diff) +1
-0+ u64 addr, struct addr_location *al)... and the other as
_fb, make it consistent, please.ok
Quoted 24 lines
Quoted 15 lines
tools/perf/util/event.c +6
-0+{ + 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() tools/perf/util/machine.c +0
-0@@ -2542,6 +2542,46 @@ int machine__get_kernel_start(structQuoted 1 line
machine
*machine)
tools/perf/util/machine.c int machine__get_kernel_start(struct machine *machine)
Quoted 6 lines
tools/perf/util/machine.c +3
-0return err; } +/* +* machine__single_ku_as - Machine has same address space for +kernel and user.
Quoted 12 lines
tools/perf/util/machine.c
Quoted 10 lines
+ * @machine: machine object
+ *
+ * Some architectures have a single address space for kernel andtools/perf/util/machine.c +7
-0+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
Quoted 1 line
Can we avoid having this strcmp be done repeatedly? I.e. just make
this a
Quoted 3 lines
boolean initialized at session start, when
machine->envis setup, so we'd
have:
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
machine->single_address_spaceInstead of a function?
Sure thing.
Have you sent an update to this patch series addressing the concerns? I think that renaming that _fb to __fallback and having this machine->single_address_space is what is missing, I'll try to take a stab at this now and have it in my perf/core branch soon.
But you have this already done, please send it.
- Arnaldo
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficient
Em Mon, Nov 26, 2018 at 04:02:17PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 18 lines
Em Mon, Nov 05, 2018 at 07:53:17PM +0000, Hunter, Adrian escreveu:
Quoted 16 lines
Quoted 16 lines
From: Arnaldo Carvalho de Melo [mailto:[email protected]] Em Mon, Nov 05, 2018 at 07:21:44PM +0000, Hunter, Adrian escreveu:
Quoted 14 lines
Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+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
Quoted 7 lines
Can we avoid having this strcmp be done repeatedly? I.e. just make this a boolean initialized at session start, when
machine->envis setup, so we'd have:machine->single_address_spaceInstead of a function?
Quoted 8 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Sure thing.
Have you sent an update to this patch series addressing the concerns? I think that renaming that
_fbto__fallbackand having thismachine->single_address_spaceis what is missing, I'll try to take a stab at this now and have it in my perf/core branch soon.But you have this already done, please send it.
So, here is the patch on top of yours and at the end a patch that I'm inserting just before yours, please check and ack, thanks:
tools/perf/util/machine.c +1 -13
@@ -2592,25 +2592,13 @@ 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" ); -}u8 machine__addr_cpumode (struct machine *machine,u8 cpumode,u64 addr) {u8 addr_cpumode = cpumode;bool kernel_ip; -if (!machine__single_ku_as (machine)) +if (!machine->env->single_address_space) goto out; kernel_ip =machine__kernel_ip (machine, addr);
commit 5aabc18da787f7f6785b4027c63a89592e4e2c03
Author: Arnaldo Carvalho de Melo <[email protected]> Date: Mon Nov 26 16:25:25 2018 -0300
perf env: Record if a arch has a single user/kernel address space
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. Some don't, e.g.: sparc.
Cache that info in perf_env so that, for instance, code needing to fallback failed symbol lookups at the kernel space in single address space arches can lookup at userspace.
tools/perf/util/env.c +10 -0
@@ -8,6 +8,16 @@struct perf_env perf_env; +/* +* 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. Some don't, e.g.: sparc +*/ +void perf_env__init_single_address_space (struct perf_env *env) +{ + env->single_address_space =strcmp (perf_env__arch (env),"sparc" ); +} +void perf_env__exit (struct perf_env *env) {int i;
tools/perf/util/env.h +8 -0
@@ -60,6 +60,12 @@ struct perf_env {struct cpu_topology_map *cpu;struct cpu_cache_level *caches;int caches_cnt; +/* +* 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. Some don't, e.g.: sparc +*/ +bool single_address_space;struct numa_node *numa_nodes;struct memory_node *memory_nodes;unsigned long long memory_bsize;
@@ -72,6 +78,8 @@ void perf_env__exit(struct perf_env *env);int perf_env__set_cmdline (struct perf_env *env,int argc,const char *argv[]); +void perf_env__init_single_address_space (struct perf_env *env); +int perf_env__read_cpu_topology_map (struct perf_env *env);void cpu_cache_level__free (struct cpu_cache_level *cache);
tools/perf/util/session.c +2 -0
@@ -150,6 +150,8 @@ struct perf_session *perf_session__new(struct perf_data *data, session->machines.host.env = &perf_env; } +perf_env__init_single_address_space (session->machines.host.env); +if (!data ||perf_data__is_write (data)) {/* * In O_RDONLY mode this will be performed when reading the
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 1/5] perf tools: Add fallback functions for cases where cpumode is insufficient
Em Mon, Nov 26, 2018 at 04:41:45PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 26 lines
Em Mon, Nov 26, 2018 at 04:02:17PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 24 lines
Em Mon, Nov 05, 2018 at 07:53:17PM +0000, Hunter, Adrian escreveu:
Quoted 13 lines
Quoted 12 lines
From: Arnaldo Carvalho de Melo [mailto:[email protected]] Em Mon, Nov 05, 2018 at 07:21:44PM +0000, Hunter, Adrian escreveu:
Quoted 10 lines
Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+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
Quoted 3 lines
Can we avoid having this strcmp be done repeatedly? I.e. just make this a boolean initialized at session start, when
machine->envis setup, so we'd have:Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
machine->single_address_spaceInstead of a function?
Sure thing.
Have you sent an update to this patch series addressing the concerns? I think that renaming that
_fbto__fallbackand having thismachine->single_address_spaceis what is missing, I'll try to take a stab at this now and have it in my perf/core branch soon.
Quoted 4 lines
Quoted 1 line
But you have this already done, please send it.
So, here is the patch on top of yours and at the end a patch that I'm inserting just before yours, please check and ack, thanks:
Argh, nevermind, just saw your V2, /me goes to read it ;-\
- Arnaldo
Adrian Hunter
reviewerPATCH 2/5 perf tools: Use fallback for sample_addr_correlates_sym() cases
In reply to Adrian Hunterthread__resolve() is used in the sample_addr_correlates_sym() cases where 'addr' is a destination of a branch which does not necessarily have the same cpumode as the 'ip'. Use the fallback function in that case.
This patch depends on patch "perf tools: Add fallback functions for cases where cpumode is insufficient".
--- tools/perf/util/event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/event.c +1 -1
@@ -1705,7 +1705,7 @@ bool sample_addr_correlates_sym(struct perf_event_attr *attr)void thread__resolve (struct thread *thread,struct addr_location *al,struct perf_sample *sample) { -thread__find_map (thread, sample->cpumode, sample->addr, al); +thread__find_map_fallback (thread, sample->cpumode, sample->addr, al); al->cpu = sample->cpu; al->sym =NULL ;
Signature · 1 line
2.17.1
Hunter, Adrian
reviewerRE: [PATCH 2/5] perf tools: Use fallback for sample_addr_correlates_sym() cases
Quoted 14 lines
-----Original Message-----
From: Hunter, Adrian
Sent: Wednesday, October 31, 2018 11:11 AM
To: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>; Andi Kleen <[email protected]>; linux- [email protected]; [email protected]; David Miller <[email protected]>; Mathieu Poirier <[email protected]>
Subject: [PATCH 2/5] perf tools: Use fallback forsample_addr_correlates_sym()cases
thread__resolve()is used in thesample_addr_correlates_sym()cases where 'addr' is a destination of a branch which does not necessarily have the same cpumode as the 'ip'. Use the fallback function in that case.
Quoted 5 lines
This patch depends on patch "perf tools: Add fallback functions for cases where cpumode is insufficient".
Signed-off-by Adrian Hunter <[email protected]>Cc [email protected] # 4.19
Any comments?
Quoted 21 lines
--- tools/perf/util/event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)tools/perf/util/event.c +1
-1@@ -1705,7 +1705,7 @@ bool sample_addr_correlates_sym(struct perf_event_attr *attr)void thread__resolve (struct thread *thread,struct addr_location *al,struct perf_sample *sample) { -thread__find_map (thread, sample->cpumode, sample->addr, al); +thread__find_map_fallback (thread, sample->cpumode, sample- >addr, al);tools/perf/util/event.c +0
-1al->cpu = sample->cpu; al->sym =NULL ; --2.17.1
Adrian Hunter
reviewerPATCH 3/5 perf tools: Use fallbacks for branch stacks
In reply to Adrian HunterBranch stacks do not necessarily have the same cpumode as the 'ip'. Use the fallback functions in those cases.
This patch depends on patch "perf tools: Add fallback functions for cases where cpumode is insufficient".
--- tools/perf/builtin-script.c | 12 ++++++------ .../util/scripting-engines/trace-event-python.c | 16 ++++++++-------- 2 files changed, 14 insertions(+), 14 deletions(-)
tools/perf/builtin-script.c +6 -6
@@ -728,8 +728,8 @@ static int perf_sample__fprintf_brstack(struct perf_sample *sample,if (PRINT_FIELD (DSO )) {memset (&alf,0 ,sizeof (alf));memset (&alt,0 ,sizeof (alt)); -thread__find_map (thread, sample->cpumode, from, &alf); -thread__find_map (thread, sample->cpumode, to, &alt); +thread__find_map_fallback (thread, sample->cpumode, from, &alf); +thread__find_map_fallback (thread, sample->cpumode, to, &alt); } printed +=fprintf (fp," 0x%" PRIx64, from);
@@ -775,8 +775,8 @@ static int perf_sample__fprintf_brstacksym(struct perf_sample *sample, from = br->entries[i].from; to = br->entries[i].to; -thread__find_symbol (thread, sample->cpumode, from, &alf); -thread__find_symbol (thread, sample->cpumode, to, &alt); +thread__find_symbol_fb (thread, sample->cpumode, from, &alf); +thread__find_symbol_fb (thread, sample->cpumode, to, &alt); printed +=symbol__fprintf_symname_offs (alf.sym, &alf, fp);if (PRINT_FIELD (DSO )) {
@@ -820,11 +820,11 @@ static int perf_sample__fprintf_brstackoff(struct perf_sample *sample, from = br->entries[i].from; to = br->entries[i].to; -if (thread__find_map (thread, sample->cpumode, from, &alf) && +if (thread__find_map_fallback (thread, sample->cpumode, from, &alf) && !alf.map->dso->adjust_symbols) from =map__map_ip (alf.map, from); -if (thread__find_map (thread, sample->cpumode, to, &alt) && +if (thread__find_map_fallback (thread, sample->cpumode, to, &alt) && !alt.map->dso->adjust_symbols) to =map__map_ip (alt.map, to);
tools/perf/util/scripting-engines/trace-event-python.c +8 -8
@@ -494,14 +494,14 @@ static PyObject *python_process_brstack(struct perf_sample *sample,pydict_set_item_string_decref (pyelem,"cycles" ,PyLong_FromUnsignedLongLong (br->entries[i].flags.cycles)); -thread__find_map (thread, sample->cpumode, - br->entries[i].from, &al); +thread__find_map_fallback (thread, sample->cpumode, + br->entries[i].from, &al); dsoname =get_dsoname (al.map);pydict_set_item_string_decref (pyelem,"from_dsoname" ,_PyUnicode_FromString (dsoname)); -thread__find_map (thread, sample->cpumode, - br->entries[i].to, &al); +thread__find_map_fallback (thread, sample->cpumode, + br->entries[i].to, &al); dsoname =get_dsoname (al.map);pydict_set_item_string_decref (pyelem,"to_dsoname" ,_PyUnicode_FromString (dsoname));
@@ -576,14 +576,14 @@ static PyObject *python_process_brstacksym(struct perf_sample *sample,if (!pyelem)Py_FatalError ("couldn't create Python dictionary" ); -thread__find_symbol (thread, sample->cpumode, - br->entries[i].from, &al); +thread__find_symbol_fb (thread, sample->cpumode, + br->entries[i].from, &al);get_symoff (al.sym, &al, true, bf,sizeof (bf));pydict_set_item_string_decref (pyelem,"from" ,_PyUnicode_FromString (bf)); -thread__find_symbol (thread, sample->cpumode, - br->entries[i].to, &al); +thread__find_symbol_fb (thread, sample->cpumode, + br->entries[i].to, &al);get_symoff (al.sym, &al, true, bf,sizeof (bf));pydict_set_item_string_decref (pyelem,"to" ,_PyUnicode_FromString (bf));
Signature · 1 line
2.17.1
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 3/5] perf tools: Use fallbacks for branch stacks
Em Wed, Oct 31, 2018 at 11:10:41AM +0200, Adrian Hunter escreveu:
Quoted 5 lines
Branch stacks do not necessarily have the same cpumode as the 'ip'. Use the fallback functions in those cases.
This patch depends on patch "perf tools: Add fallback functions for cases
Quoted 1 line
where cpumode is insufficient".
This one is ok, i.e. only in places where we need a fallback we do it.
- Arnaldo
Quoted 91 lines
Signed-off-by Adrian Hunter <[email protected]>Cc [email protected] # 4.19--- tools/perf/builtin-script.c | 12 ++++++------ .../util/scripting-engines/trace-event-python.c | 16 ++++++++-------- 2 files changed, 14 insertions(+), 14 deletions(-)tools/perf/builtin-script.c +6
-6@@ -728,8 +728,8 @@ static int perf_sample__fprintf_brstack(struct perf_sample *sample,if (PRINT_FIELD (DSO )) {memset (&alf,0 ,sizeof (alf));memset (&alt,0 ,sizeof (alt)); -thread__find_map (thread, sample->cpumode, from, &alf); -thread__find_map (thread, sample->cpumode, to, &alt); +thread__find_map_fallback (thread, sample->cpumode, from, &alf); +thread__find_map_fallback (thread, sample->cpumode, to, &alt); } printed +=fprintf (fp," 0x%" PRIx64, from);@@ -775,8 +775,8 @@ static int perf_sample__fprintf_brstacksym(struct perf_sample *sample, from = br->entries[i].from; to = br->entries[i].to; -thread__find_symbol (thread, sample->cpumode, from, &alf); -thread__find_symbol (thread, sample->cpumode, to, &alt); +thread__find_symbol_fb (thread, sample->cpumode, from, &alf); +thread__find_symbol_fb (thread, sample->cpumode, to, &alt); printed +=symbol__fprintf_symname_offs (alf.sym, &alf, fp);if (PRINT_FIELD (DSO )) {@@ -820,11 +820,11 @@ static int perf_sample__fprintf_brstackoff(struct perf_sample *sample, from = br->entries[i].from; to = br->entries[i].to; -if (thread__find_map (thread, sample->cpumode, from, &alf) && +if (thread__find_map_fallback (thread, sample->cpumode, from, &alf) && !alf.map->dso->adjust_symbols) from =map__map_ip (alf.map, from); -if (thread__find_map (thread, sample->cpumode, to, &alt) && +if (thread__find_map_fallback (thread, sample->cpumode, to, &alt) && !alt.map->dso->adjust_symbols) to =map__map_ip (alt.map, to);tools/perf/util/scripting-engines/trace-event-python.c +8
-8@@ -494,14 +494,14 @@ static PyObject *python_process_brstack(struct perf_sample *sample,pydict_set_item_string_decref (pyelem,"cycles" ,PyLong_FromUnsignedLongLong (br->entries[i].flags.cycles)); -thread__find_map (thread, sample->cpumode, - br->entries[i].from, &al); +thread__find_map_fallback (thread, sample->cpumode, + br->entries[i].from, &al); dsoname =get_dsoname (al.map);pydict_set_item_string_decref (pyelem,"from_dsoname" ,_PyUnicode_FromString (dsoname)); -thread__find_map (thread, sample->cpumode, - br->entries[i].to, &al); +thread__find_map_fallback (thread, sample->cpumode, + br->entries[i].to, &al); dsoname =get_dsoname (al.map);pydict_set_item_string_decref (pyelem,"to_dsoname" ,_PyUnicode_FromString (dsoname));@@ -576,14 +576,14 @@ static PyObject *python_process_brstacksym(struct perf_sample *sample,if (!pyelem)Py_FatalError ("couldn't create Python dictionary" ); -thread__find_symbol (thread, sample->cpumode, - br->entries[i].from, &al); +thread__find_symbol_fb (thread, sample->cpumode, + br->entries[i].from, &al);get_symoff (al.sym, &al, true, bf,sizeof (bf));pydict_set_item_string_decref (pyelem,"from" ,_PyUnicode_FromString (bf)); -thread__find_symbol (thread, sample->cpumode, - br->entries[i].to, &al); +thread__find_symbol_fb (thread, sample->cpumode, + br->entries[i].to, &al);get_symoff (al.sym, &al, true, bf,sizeof (bf));pydict_set_item_string_decref (pyelem,"to" ,_PyUnicode_FromString (bf));Signature · 1 line
2.17.1
Adrian Hunter
reviewerPATCH 4/5 perf intel-pt: Insert callchain context into synthesized callchains
In reply to Adrian HunterIn the absence of a fallback, callchains must encode also the callchain context. Do that now there is no fallback.
--- tools/perf/util/intel-pt.c | 6 +++-- tools/perf/util/thread-stack.c | 44 +++++++++++++++++++++++++++------- tools/perf/util/thread-stack.h | 2 +- 3 files changed, 40 insertions(+), 12 deletions(-)
tools/perf/util/intel-pt.c +4 -2
@@ -759,7 +759,8 @@ static struct intel_pt_queue *intel_pt_alloc_queue(struct intel_pt *pt,if (pt->synth_opts.callchain) {size_t sz =sizeof (struct ip_callchain ); - sz += pt->synth_opts.callchain_sz *sizeof (u64); +/* Add 1 to callchain_sz for callchain context */ + sz += (pt->synth_opts.callchain_sz +1 ) *sizeof (u64); ptq->chain =zalloc (sz);if (!ptq->chain) goto out_free;
@@ -1160,7 +1161,8 @@ static void intel_pt_prep_sample(struct intel_pt *pt,if (pt->synth_opts.callchain) {thread_stack__sample (ptq->thread, ptq->chain, - pt->synth_opts.callchain_sz, sample->ip); + pt->synth_opts.callchain_sz +1 , + sample->ip, pt->kernel_start); sample->callchain = ptq->chain; }
tools/perf/util/thread-stack.c +35 -9
55 lines not on this page · raw message
tools/perf/util/thread-stack.h +1 -1
@@ -84,7 +84,7 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, u64 to_ip, u16 insn_len, u64 trace_nr);void thread_stack__set_trace_nr (struct thread *thread,u64 trace_nr);void thread_stack__sample (struct thread *thread,struct ip_callchain *chain, -size_t sz,u64 ip); +size_t sz,u64 ip,u64 kernel_start);int thread_stack__flush (struct thread *thread);void thread_stack__free (struct thread *thread);size_t thread_stack__depth (struct thread *thread);
Signature · 1 line
2.17.1
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 4/5] perf intel-pt: Insert callchain context into synthesized callchains
Em Wed, Oct 31, 2018 at 11:10:42AM +0200, Adrian Hunter escreveu:
Quoted 2 lines
In the absence of a fallback, callchains must encode also the callchain context. Do that now there is no fallback.
So, this one is independent of the first 3 patches, right? Ok, applying it first, I'll relook the first ones next.
- Arnaldo
Quoted 107 lines
Signed-off-by Adrian Hunter <[email protected]>Cc [email protected] # 4.19--- tools/perf/util/intel-pt.c | 6 +++-- tools/perf/util/thread-stack.c | 44 +++++++++++++++++++++++++++------- tools/perf/util/thread-stack.h | 2 +- 3 files changed, 40 insertions(+), 12 deletions(-)tools/perf/util/intel-pt.c +4
-2@@ -759,7 +759,8 @@ static struct intel_pt_queue *intel_pt_alloc_queue(struct intel_pt *pt,if (pt->synth_opts.callchain) {size_t sz =sizeof (struct ip_callchain ); - sz += pt->synth_opts.callchain_sz *sizeof (u64); +/* Add 1 to callchain_sz for callchain context */ + sz += (pt->synth_opts.callchain_sz +1 ) *sizeof (u64); ptq->chain =zalloc (sz);if (!ptq->chain) goto out_free;@@ -1160,7 +1161,8 @@ static void intel_pt_prep_sample(struct intel_pt *pt,if (pt->synth_opts.callchain) {thread_stack__sample (ptq->thread, ptq->chain, - pt->synth_opts.callchain_sz, sample->ip); + pt->synth_opts.callchain_sz +1 , + sample->ip, pt->kernel_start); sample->callchain = ptq->chain; }tools/perf/util/thread-stack.c +35
-9@@ -310,20 +310,46 @@ void thread_stack__free(struct thread *thread) } } +static inline u64 callchain_context (u64 ip,u64 kernel_start) +{ +return ip < kernel_start ?PERF_CONTEXT_USER :PERF_CONTEXT_KERNEL ; +} +void thread_stack__sample (struct thread *thread,struct ip_callchain *chain, -size_t sz,u64 ip) +size_t sz,u64 ip,u64 kernel_start) { -size_t i; +u64 context =callchain_context (ip, kernel_start); +u64 last_context; +size_t i, j; -if (!thread || !thread->ts) - chain->nr =1 ; -else - chain->nr =min (sz, thread->ts->cnt +1 ); +if (sz <2 ) { + chain->nr =0 ; +return ; + } - chain->ips[0 ] = ip; + chain->ips[0 ] = context; + chain->ips[1 ] = ip; + +if (!thread || !thread->ts) { + chain->nr =2 ; +return ; + } + + last_context = context; + +for (i =2 , j =0 ; i < sz && j < thread->ts->cnt; i++, j++) { + ip = thread->ts->stack[thread->ts->cnt - j].ret_addr; + context =callchain_context (ip, kernel_start); +if (context != last_context) { +if (i >= sz -1 ) +break ; + chain->ips[i++] = context; + last_context = context; + } + chain->ips[i] = ip; + } -for (i =1 ; i < chain->nr; i++) - chain->ips[i] = thread->ts->stack[thread->ts->cnt - i].ret_addr; + chain->nr = i; } struct call_return_processor *tools/perf/util/thread-stack.h +1
-1@@ -84,7 +84,7 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, u64 to_ip, u16 insn_len, u64 trace_nr);void thread_stack__set_trace_nr (struct thread *thread,u64 trace_nr);void thread_stack__sample (struct thread *thread,struct ip_callchain *chain, -size_t sz,u64 ip); +size_t sz,u64 ip,u64 kernel_start);int thread_stack__flush (struct thread *thread);void thread_stack__free (struct thread *thread);size_t thread_stack__depth (struct thread *thread);Signature · 1 line
2.17.1
Adrian Hunter
reviewerRe: [PATCH 4/5] perf intel-pt: Insert callchain context into synthesized callchains
Quoted 5 lines · Arnaldo Carvalho de Melo
Em Wed, Oct 31, 2018 at 11:10:42AM +0200, Adrian Hunter escreveu:
Quoted 2 lines
In the absence of a fallback, callchains must encode also the callchain context. Do that now there is no fallback.
So, this one is independent of the first 3 patches, right?
Yes. I was just going to test it separately when I noticed I had screwed up my earlier testing. When I re-tested I discovered this patch has an off-by-one error:
tools/perf/util/thread-stack.c +1 -1
@@ -337,7 +337,7 @@ void thread_stack__sample(struct thread *thread, struct ip_callchain *chain, last_context = context; -for (i =2 , j =0 ; i < sz && j < thread->ts->cnt; i++, j++) { +for (i =2 , j =1 ; i < sz && j <= thread->ts->cnt; i++, j++) { ip = thread->ts->stack[thread->ts->cnt - j].ret_addr; context =callchain_context (ip, kernel_start);if (context != last_context) {
Shall I send V2?
Quoted 113 lines
So, this one is independent of the first 3 patches, right? Ok, applying it first, I'll relook the first ones next.
- Arnaldo
Quoted 107 lines
Signed-off-by Adrian Hunter <[email protected]>Cc [email protected] # 4.19--- tools/perf/util/intel-pt.c | 6 +++-- tools/perf/util/thread-stack.c | 44 +++++++++++++++++++++++++++------- tools/perf/util/thread-stack.h | 2 +- 3 files changed, 40 insertions(+), 12 deletions(-)tools/perf/util/intel-pt.c +4
-2@@ -759,7 +759,8 @@ static struct intel_pt_queue *intel_pt_alloc_queue(struct intel_pt *pt,if (pt->synth_opts.callchain) {size_t sz =sizeof (struct ip_callchain ); - sz += pt->synth_opts.callchain_sz *sizeof (u64); +/* Add 1 to callchain_sz for callchain context */ + sz += (pt->synth_opts.callchain_sz +1 ) *sizeof (u64); ptq->chain =zalloc (sz);if (!ptq->chain) goto out_free;@@ -1160,7 +1161,8 @@ static void intel_pt_prep_sample(struct intel_pt *pt,if (pt->synth_opts.callchain) {thread_stack__sample (ptq->thread, ptq->chain, - pt->synth_opts.callchain_sz, sample->ip); + pt->synth_opts.callchain_sz +1 , + sample->ip, pt->kernel_start); sample->callchain = ptq->chain; }tools/perf/util/thread-stack.c +35
-9@@ -310,20 +310,46 @@ void thread_stack__free(struct thread *thread) } } +static inline u64 callchain_context (u64 ip,u64 kernel_start) +{ +return ip < kernel_start ?PERF_CONTEXT_USER :PERF_CONTEXT_KERNEL ; +} +void thread_stack__sample (struct thread *thread,struct ip_callchain *chain, -size_t sz,u64 ip) +size_t sz,u64 ip,u64 kernel_start) { -size_t i; +u64 context =callchain_context (ip, kernel_start); +u64 last_context; +size_t i, j; -if (!thread || !thread->ts) - chain->nr =1 ; -else - chain->nr =min (sz, thread->ts->cnt +1 ); +if (sz <2 ) { + chain->nr =0 ; +return ; + } - chain->ips[0 ] = ip; + chain->ips[0 ] = context; + chain->ips[1 ] = ip; + +if (!thread || !thread->ts) { + chain->nr =2 ; +return ; + } + + last_context = context; + +for (i =2 , j =0 ; i < sz && j < thread->ts->cnt; i++, j++) { + ip = thread->ts->stack[thread->ts->cnt - j].ret_addr; + context =callchain_context (ip, kernel_start); +if (context != last_context) { +if (i >= sz -1 ) +break ; + chain->ips[i++] = context; + last_context = context; + } + chain->ips[i] = ip; + } -for (i =1 ; i < chain->nr; i++) - chain->ips[i] = thread->ts->stack[thread->ts->cnt - i].ret_addr; + chain->nr = i; } struct call_return_processor *tools/perf/util/thread-stack.h +1
-1@@ -84,7 +84,7 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, u64 to_ip, u16 insn_len, u64 trace_nr);void thread_stack__set_trace_nr (struct thread *thread,u64 trace_nr);void thread_stack__sample (struct thread *thread,struct ip_callchain *chain, -size_t sz,u64 ip); +size_t sz,u64 ip,u64 kernel_start);int thread_stack__flush (struct thread *thread);void thread_stack__free (struct thread *thread);size_t thread_stack__depth (struct thread *thread);Signature · 1 line
2.17.1
Adrian Hunter
reviewerRe: [PATCH 4/5] perf intel-pt: Insert callchain context into synthesized callchains
Quoted 25 lines · Adrian Hunter
Quoted 5 lines · Arnaldo Carvalho de Melo
Em Wed, Oct 31, 2018 at 11:10:42AM +0200, Adrian Hunter escreveu:
Quoted 2 lines
In the absence of a fallback, callchains must encode also the callchain context. Do that now there is no fallback.
So, this one is independent of the first 3 patches, right?
Yes. I was just going to test it separately when I noticed I had screwed up my earlier testing. When I re-tested I discovered this patch has an off-by-one error:
tools/perf/util/thread-stack.c +1
-1@@ -337,7 +337,7 @@ void thread_stack__sample(struct thread *thread, struct ip_callchain *chain, last_context = context; -for (i =2 , j =0 ; i < sz && j < thread->ts->cnt; i++, j++) { +for (i =2 , j =1 ; i < sz && j <= thread->ts->cnt; i++, j++) { ip = thread->ts->stack[thread->ts->cnt - j].ret_addr; context =callchain_context (ip, kernel_start);if (context != last_context) {
Quoted 1 line
Shall I send V2?
I have attached it
From 19a9fbdbd3729c7f14c9c1f0ac79c18cce91ff60 Mon Sep 17 00:00:00 2001
From: Adrian Hunter <[email protected]>
Date: Wed, 31 Oct 2018 09:31:25 +0200
Subject: [PATCH V2 4/5] perf intel-pt: Insert callchain context into
synthesized callchains
In the absence of a fallback, callchains must encode also the callchain context. Do that now there is no fallback.
---
Changes in V2:
Fix off-by-one error
tools/perf/util/intel-pt.c | 6 +++-- tools/perf/util/thread-stack.c | 44 +++++++++++++++++++++++++++------- tools/perf/util/thread-stack.h | 2 +- 3 files changed, 40 insertions(+), 12 deletions(-)
tools/perf/util/intel-pt.c +4 -2
@@ -759,7 +759,8 @@ static struct intel_pt_queue *intel_pt_alloc_queue(struct intel_pt *pt,if (pt->synth_opts.callchain) {size_t sz =sizeof (struct ip_callchain ); - sz += pt->synth_opts.callchain_sz *sizeof (u64); +/* Add 1 to callchain_sz for callchain context */ + sz += (pt->synth_opts.callchain_sz +1 ) *sizeof (u64); ptq->chain =zalloc (sz);if (!ptq->chain) goto out_free;
@@ -1160,7 +1161,8 @@ static void intel_pt_prep_sample(struct intel_pt *pt,if (pt->synth_opts.callchain) {thread_stack__sample (ptq->thread, ptq->chain, - pt->synth_opts.callchain_sz, sample->ip); + pt->synth_opts.callchain_sz +1 , + sample->ip, pt->kernel_start); sample->callchain = ptq->chain; }
tools/perf/util/thread-stack.c +35 -9
55 lines not on this page · raw message
tools/perf/util/thread-stack.h +1 -1
@@ -84,7 +84,7 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, u64 to_ip, u16 insn_len, u64 trace_nr);void thread_stack__set_trace_nr (struct thread *thread,u64 trace_nr);void thread_stack__sample (struct thread *thread,struct ip_callchain *chain, -size_t sz,u64 ip); +size_t sz,u64 ip,u64 kernel_start);int thread_stack__flush (struct thread *thread);void thread_stack__free (struct thread *thread);size_t thread_stack__depth (struct thread *thread);
Signature · 1 line
2.17.1
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 4/5] perf intel-pt: Insert callchain context into synthesized callchains
Em Wed, Oct 31, 2018 at 04:20:45PM +0200, Adrian Hunter escreveu:
Quoted 26 lines · Adrian Hunter
Quoted 25 lines · Arnaldo Carvalho de Melo
Quoted 5 lines
Em Wed, Oct 31, 2018 at 11:10:42AM +0200, Adrian Hunter escreveu:
Quoted 2 lines
In the absence of a fallback, callchains must encode also the callchain context. Do that now there is no fallback.
So, this one is independent of the first 3 patches, right?
Yes. I was just going to test it separately when I noticed I had screwed up my earlier testing. When I re-tested I discovered this patch has an off-by-one error:
tools/perf/util/thread-stack.c +1
-1@@ -337,7 +337,7 @@ void thread_stack__sample(struct thread *thread, struct ip_callchain *chain, last_context = context; -for (i =2 , j =0 ; i < sz && j < thread->ts->cnt; i++, j++) { +for (i =2 , j =1 ; i < sz && j <= thread->ts->cnt; i++, j++) { ip = thread->ts->stack[thread->ts->cnt - j].ret_addr; context =callchain_context (ip, kernel_start);if (context != last_context) {
Quoted 3 lines
Quoted 1 line
Shall I send V2?
I have attached it
Thanks!
Quoted 124 lines
Quoted 1 line
From
19a9fbdbd3729c7f14c9c1f0ac79c18cce91ff60Mon Sep 17 00:00:00 2001From: Adrian Hunter <[email protected]>
Date: Wed, 31 Oct 2018 09:31:25 +0200
Subject: [PATCH V2 4/5] perf intel-pt: Insert callchain context into
synthesized callchainsIn the absence of a fallback, callchains must encode also the callchain context. Do that now there is no fallback.
Signed-off-by Adrian Hunter <[email protected]>Cc [email protected] # 4.19---
Changes in V2:
Fix off-by-one error
tools/perf/util/intel-pt.c | 6 +++-- tools/perf/util/thread-stack.c | 44 +++++++++++++++++++++++++++------- tools/perf/util/thread-stack.h | 2 +- 3 files changed, 40 insertions(+), 12 deletions(-)tools/perf/util/intel-pt.c +4
-2@@ -759,7 +759,8 @@ static struct intel_pt_queue *intel_pt_alloc_queue(struct intel_pt *pt,if (pt->synth_opts.callchain) {size_t sz =sizeof (struct ip_callchain ); - sz += pt->synth_opts.callchain_sz *sizeof (u64); +/* Add 1 to callchain_sz for callchain context */ + sz += (pt->synth_opts.callchain_sz +1 ) *sizeof (u64); ptq->chain =zalloc (sz);if (!ptq->chain) goto out_free;@@ -1160,7 +1161,8 @@ static void intel_pt_prep_sample(struct intel_pt *pt,if (pt->synth_opts.callchain) {thread_stack__sample (ptq->thread, ptq->chain, - pt->synth_opts.callchain_sz, sample->ip); + pt->synth_opts.callchain_sz +1 , + sample->ip, pt->kernel_start); sample->callchain = ptq->chain; }tools/perf/util/thread-stack.c +35
-9@@ -310,20 +310,46 @@ void thread_stack__free(struct thread *thread) } } +static inline u64 callchain_context (u64 ip,u64 kernel_start) +{ +return ip < kernel_start ?PERF_CONTEXT_USER :PERF_CONTEXT_KERNEL ; +} +void thread_stack__sample (struct thread *thread,struct ip_callchain *chain, -size_t sz,u64 ip) +size_t sz,u64 ip,u64 kernel_start) { -size_t i; +u64 context =callchain_context (ip, kernel_start); +u64 last_context; +size_t i, j; -if (!thread || !thread->ts) - chain->nr =1 ; -else - chain->nr =min (sz, thread->ts->cnt +1 ); +if (sz <2 ) { + chain->nr =0 ; +return ; + } - chain->ips[0 ] = ip; + chain->ips[0 ] = context; + chain->ips[1 ] = ip; + +if (!thread || !thread->ts) { + chain->nr =2 ; +return ; + } + + last_context = context; + +for (i =2 , j =1 ; i < sz && j <= thread->ts->cnt; i++, j++) { + ip = thread->ts->stack[thread->ts->cnt - j].ret_addr; + context =callchain_context (ip, kernel_start); +if (context != last_context) { +if (i >= sz -1 ) +break ; + chain->ips[i++] = context; + last_context = context; + } + chain->ips[i] = ip; + } -for (i =1 ; i < chain->nr; i++) - chain->ips[i] = thread->ts->stack[thread->ts->cnt - i].ret_addr; + chain->nr = i; } struct call_return_processor *tools/perf/util/thread-stack.h +1
-1@@ -84,7 +84,7 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, u64 to_ip, u16 insn_len, u64 trace_nr);void thread_stack__set_trace_nr (struct thread *thread,u64 trace_nr);void thread_stack__sample (struct thread *thread,struct ip_callchain *chain, -size_t sz,u64 ip); +size_t sz,u64 ip,u64 kernel_start);int thread_stack__flush (struct thread *thread);void thread_stack__free (struct thread *thread);size_t thread_stack__depth (struct thread *thread);Signature · 1 line
2.17.1
tip-bot for Adrian Hunter
merge bot[tip:perf/urgent] perf intel-pt: Insert callchain context into synthesized callchains
In reply to Adrian Hunterperf intel-pt: Insert callchain context into synthesized callchains
Commit-ID: 242483068b4b9ad02f1653819b6e683577681e0e Gitweb: https://git.kernel.org/tip/242483068b4b9ad02f1653819b6e683577681e0e Author: Adrian Hunter <[email protected]> AuthorDate: Wed, 31 Oct 2018 11:10:42 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Wed, 31 Oct 2018 12:54:27 -0300
perf intel-pt: Insert callchain context into synthesized callchains
In the absence of a fallback, callchains must encode also the callchain context. Do that now there is no fallback.
--- tools/perf/util/intel-pt.c | 6 ++++-- tools/perf/util/thread-stack.c | 44 +++++++++++++++++++++++++++++++++--------- tools/perf/util/thread-stack.h | 2 +- 3 files changed, 40 insertions(+), 12 deletions(-)
tools/perf/util/intel-pt.c +4 -2
@@ -759,7 +759,8 @@ static struct intel_pt_queue *intel_pt_alloc_queue(struct intel_pt *pt,if (pt->synth_opts.callchain) {size_t sz =sizeof (struct ip_callchain ); - sz += pt->synth_opts.callchain_sz *sizeof (u64); +/* Add 1 to callchain_sz for callchain context */ + sz += (pt->synth_opts.callchain_sz +1 ) *sizeof (u64); ptq->chain =zalloc (sz);if (!ptq->chain) goto out_free;
@@ -1160,7 +1161,8 @@ static void intel_pt_prep_sample(struct intel_pt *pt,if (pt->synth_opts.callchain) {thread_stack__sample (ptq->thread, ptq->chain, - pt->synth_opts.callchain_sz, sample->ip); + pt->synth_opts.callchain_sz +1 , + sample->ip, pt->kernel_start); sample->callchain = ptq->chain; }
tools/perf/util/thread-stack.c +35 -9
55 lines not on this page · raw message
tools/perf/util/thread-stack.h +1 -1
@@ -84,7 +84,7 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, u64 to_ip, u16 insn_len, u64 trace_nr);void thread_stack__set_trace_nr (struct thread *thread,u64 trace_nr);void thread_stack__sample (struct thread *thread,struct ip_callchain *chain, -size_t sz,u64 ip); +size_t sz,u64 ip,u64 kernel_start);int thread_stack__flush (struct thread *thread);void thread_stack__free (struct thread *thread);size_t thread_stack__depth (struct thread *thread);
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 4/5] perf intel-pt: Insert callchain context into synthesized callchains
In reply to Adrian HunterEm Wed, Oct 31, 2018 at 04:15:26PM +0200, Adrian Hunter escreveu:
Quoted 25 lines · Arnaldo Carvalho de Melo
Quoted 5 lines
Em Wed, Oct 31, 2018 at 11:10:42AM +0200, Adrian Hunter escreveu:
Quoted 2 lines
In the absence of a fallback, callchains must encode also the callchain context. Do that now there is no fallback.
So, this one is independent of the first 3 patches, right?
Yes. I was just going to test it separately when I noticed I had screwed up my earlier testing. When I re-tested I discovered this patch has an off-by-one error:
tools/perf/util/thread-stack.c +1
-1@@ -337,7 +337,7 @@ void thread_stack__sample(struct thread *thread, struct ip_callchain *chain, last_context = context; -for (i =2 , j =0 ; i < sz && j < thread->ts->cnt; i++, j++) { +for (i =2 , j =1 ; i < sz && j <= thread->ts->cnt; i++, j++) { ip = thread->ts->stack[thread->ts->cnt - j].ret_addr; context =callchain_context (ip, kernel_start);if (context != last_context) {
Quoted 1 line
Shall I send V2?
No need, I can fix this here
Quoted 113 lines
Quoted 113 lines
So, this one is independent of the first 3 patches, right? Ok, applying it first, I'll relook the first ones next.
- Arnaldo
Quoted 107 lines
Signed-off-by Adrian Hunter <[email protected]>Cc [email protected] # 4.19--- tools/perf/util/intel-pt.c | 6 +++-- tools/perf/util/thread-stack.c | 44 +++++++++++++++++++++++++++------- tools/perf/util/thread-stack.h | 2 +- 3 files changed, 40 insertions(+), 12 deletions(-)tools/perf/util/intel-pt.c +4
-2@@ -759,7 +759,8 @@ static struct intel_pt_queue *intel_pt_alloc_queue(struct intel_pt *pt,if (pt->synth_opts.callchain) {size_t sz =sizeof (struct ip_callchain ); - sz += pt->synth_opts.callchain_sz *sizeof (u64); +/* Add 1 to callchain_sz for callchain context */ + sz += (pt->synth_opts.callchain_sz +1 ) *sizeof (u64); ptq->chain =zalloc (sz);if (!ptq->chain) goto out_free;@@ -1160,7 +1161,8 @@ static void intel_pt_prep_sample(struct intel_pt *pt,if (pt->synth_opts.callchain) {thread_stack__sample (ptq->thread, ptq->chain, - pt->synth_opts.callchain_sz, sample->ip); + pt->synth_opts.callchain_sz +1 , + sample->ip, pt->kernel_start); sample->callchain = ptq->chain; }tools/perf/util/thread-stack.c +35
-9@@ -310,20 +310,46 @@ void thread_stack__free(struct thread *thread) } } +static inline u64 callchain_context (u64 ip,u64 kernel_start) +{ +return ip < kernel_start ?PERF_CONTEXT_USER :PERF_CONTEXT_KERNEL ; +} +void thread_stack__sample (struct thread *thread,struct ip_callchain *chain, -size_t sz,u64 ip) +size_t sz,u64 ip,u64 kernel_start) { -size_t i; +u64 context =callchain_context (ip, kernel_start); +u64 last_context; +size_t i, j; -if (!thread || !thread->ts) - chain->nr =1 ; -else - chain->nr =min (sz, thread->ts->cnt +1 ); +if (sz <2 ) { + chain->nr =0 ; +return ; + } - chain->ips[0 ] = ip; + chain->ips[0 ] = context; + chain->ips[1 ] = ip; + +if (!thread || !thread->ts) { + chain->nr =2 ; +return ; + } + + last_context = context; + +for (i =2 , j =0 ; i < sz && j < thread->ts->cnt; i++, j++) { + ip = thread->ts->stack[thread->ts->cnt - j].ret_addr; + context =callchain_context (ip, kernel_start); +if (context != last_context) { +if (i >= sz -1 ) +break ; + chain->ips[i++] = context; + last_context = context; + } + chain->ips[i] = ip; + } -for (i =1 ; i < chain->nr; i++) - chain->ips[i] = thread->ts->stack[thread->ts->cnt - i].ret_addr; + chain->nr = i; } struct call_return_processor *tools/perf/util/thread-stack.h +1
-1@@ -84,7 +84,7 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, u64 to_ip, u16 insn_len, u64 trace_nr);void thread_stack__set_trace_nr (struct thread *thread,u64 trace_nr);void thread_stack__sample (struct thread *thread,struct ip_callchain *chain, -size_t sz,u64 ip); +size_t sz,u64 ip,u64 kernel_start);int thread_stack__flush (struct thread *thread);void thread_stack__free (struct thread *thread);size_t thread_stack__depth (struct thread *thread);Signature · 1 line
2.17.1
Adrian Hunter
reviewerPATCH 5/5 perf intel-pt/bts: Calculate cpumode for synthesized samples
In reply to Adrian HunterIn the absence of a fallback, samples must provide a correct cpumode for the 'ip'. Do that now there is no fallback.
--- tools/perf/util/intel-bts.c | 17 ++++++++++++----- tools/perf/util/intel-pt.c | 22 +++++++++++++--------- 2 files changed, 25 insertions(+), 14 deletions(-)
tools/perf/util/intel-bts.c +12 -5
@@ -269,6 +269,13 @@ static int intel_bts_do_fix_overlap(struct auxtrace_queue *queue,return 0 ; } +static inline u8 intel_bts_cpumode (struct intel_bts *bts,uint64_t ip) +{ +return machine__kernel_ip (bts->machine, ip) ? +PERF_RECORD_MISC_KERNEL : +PERF_RECORD_MISC_USER ; +} +static int intel_bts_synth_branch_sample (struct intel_bts_queue *btsq,struct branch *branch) {
@@ -281,12 +288,8 @@ static int intel_bts_synth_branch_sample(struct intel_bts_queue *btsq, bts->num_events++ <= bts->synth_opts.initial_skip)return 0 ; - event.sample.header.type =PERF_RECORD_SAMPLE ; - event.sample.header.misc =PERF_RECORD_MISC_USER ; - event.sample.header.size =sizeof (struct perf_event_header ); - - sample.cpumode =PERF_RECORD_MISC_USER ; sample.ip =le64_to_cpu (branch->from); + sample.cpumode =intel_bts_cpumode (bts, sample.ip); sample.pid = btsq->pid; sample.tid = btsq->tid; sample.addr =le64_to_cpu (branch->to);
@@ -298,6 +301,10 @@ static int intel_bts_synth_branch_sample(struct intel_bts_queue *btsq, sample.insn_len = btsq->intel_pt_insn.length;memcpy (sample.insn, btsq->intel_pt_insn.buf,INTEL_PT_INSN_BUF_SZ ); + event.sample.header.type =PERF_RECORD_SAMPLE ; + event.sample.header.misc = sample.cpumode; + event.sample.header.size =sizeof (struct perf_event_header ); +if (bts->synth_opts.inject) { event.sample.header.size = bts->branches_event_size; ret = perf_event__synthesize_sample(&event,
tools/perf/util/intel-pt.c +13 -9
@@ -407,6 +407,13 @@ intel_pt_cache_lookup(struct dso *dso, struct machine *machine, u64 offset)return auxtrace_cache__lookup (dso->auxtrace_cache, offset); } +static inline u8 intel_pt_cpumode (struct intel_pt *pt,uint64_t ip) +{ +return ip >= pt->kernel_start ? +PERF_RECORD_MISC_KERNEL : +PERF_RECORD_MISC_USER ; +} + static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn, uint64_t *insn_cnt_ptr, uint64_t *ip, uint64_t to_ip, uint64_t max_insn_cnt,
@@ -429,10 +436,7 @@ static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn,if (to_ip && *ip == to_ip) goto out_no_cache; -if (*ip >= ptq->pt->kernel_start) - cpumode =PERF_RECORD_MISC_KERNEL ; -else - cpumode =PERF_RECORD_MISC_USER ; + cpumode =intel_pt_cpumode (ptq->pt, *ip); thread = ptq->thread;if (!thread) {
@@ -1059,15 +1063,11 @@ static void intel_pt_prep_b_sample(struct intel_pt *pt, union perf_event *event, struct perf_sample *sample) { - event->sample.header.type = PERF_RECORD_SAMPLE; - event->sample.header.misc =PERF_RECORD_MISC_USER ; - event->sample.header.size =sizeof (struct perf_event_header ); -if (!pt->timeless_decoding) sample->time =tsc_to_perf_time (ptq->timestamp, &pt->tc); - sample->cpumode =PERF_RECORD_MISC_USER ; sample->ip = ptq->state->from_ip; + sample->cpumode =intel_pt_cpumode (pt, sample->ip); sample->pid = ptq->pid; sample->tid = ptq->tid; sample->addr = ptq->state->to_ip;
@@ -1076,6 +1076,10 @@ static void intel_pt_prep_b_sample(struct intel_pt *pt, sample->flags = ptq->flags; sample->insn_len = ptq->insn_len;memcpy (sample->insn, ptq->insn,INTEL_PT_INSN_BUF_SZ ); + + event->sample.header.type =PERF_RECORD_SAMPLE ; + event->sample.header.misc = sample->cpumode; + event->sample.header.size =sizeof (struct perf_event_header ); } static int intel_pt_inject_event(union perf_event *event,
Signature · 1 line
2.17.1
tip-bot for Adrian Hunter
merge bot[tip:perf/urgent] perf intel-pt/bts: Calculate cpumode for synthesized samples
perf intel-pt/bts: Calculate cpumode for synthesized samples
Commit-ID: 5d4f0edaa3ac4f1844ed7c64cd2bae6f1912bac5 Gitweb: https://git.kernel.org/tip/5d4f0edaa3ac4f1844ed7c64cd2bae6f1912bac5 Author: Adrian Hunter <[email protected]> AuthorDate: Wed, 31 Oct 2018 11:10:43 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Wed, 31 Oct 2018 12:56:26 -0300
perf intel-pt/bts: Calculate cpumode for synthesized samples
In the absence of a fallback, samples must provide a correct cpumode for the 'ip'. Do that now there is no fallback.
--- tools/perf/util/intel-bts.c | 17 ++++++++++++----- tools/perf/util/intel-pt.c | 22 +++++++++++++--------- 2 files changed, 25 insertions(+), 14 deletions(-)
tools/perf/util/intel-bts.c +12 -5
@@ -269,6 +269,13 @@ static int intel_bts_do_fix_overlap(struct auxtrace_queue *queue,return 0 ; } +static inline u8 intel_bts_cpumode (struct intel_bts *bts,uint64_t ip) +{ +return machine__kernel_ip (bts->machine, ip) ? +PERF_RECORD_MISC_KERNEL : +PERF_RECORD_MISC_USER ; +} +static int intel_bts_synth_branch_sample (struct intel_bts_queue *btsq,struct branch *branch) {
@@ -281,12 +288,8 @@ static int intel_bts_synth_branch_sample(struct intel_bts_queue *btsq, bts->num_events++ <= bts->synth_opts.initial_skip)return 0 ; - event.sample.header.type =PERF_RECORD_SAMPLE ; - event.sample.header.misc =PERF_RECORD_MISC_USER ; - event.sample.header.size =sizeof (struct perf_event_header ); - - sample.cpumode =PERF_RECORD_MISC_USER ; sample.ip =le64_to_cpu (branch->from); + sample.cpumode =intel_bts_cpumode (bts, sample.ip); sample.pid = btsq->pid; sample.tid = btsq->tid; sample.addr =le64_to_cpu (branch->to);
@@ -298,6 +301,10 @@ static int intel_bts_synth_branch_sample(struct intel_bts_queue *btsq, sample.insn_len = btsq->intel_pt_insn.length;memcpy (sample.insn, btsq->intel_pt_insn.buf,INTEL_PT_INSN_BUF_SZ ); + event.sample.header.type =PERF_RECORD_SAMPLE ; + event.sample.header.misc = sample.cpumode; + event.sample.header.size =sizeof (struct perf_event_header ); +if (bts->synth_opts.inject) { event.sample.header.size = bts->branches_event_size; ret = perf_event__synthesize_sample(&event,
tools/perf/util/intel-pt.c +13 -9
@@ -407,6 +407,13 @@ intel_pt_cache_lookup(struct dso *dso, struct machine *machine, u64 offset)return auxtrace_cache__lookup (dso->auxtrace_cache, offset); } +static inline u8 intel_pt_cpumode (struct intel_pt *pt,uint64_t ip) +{ +return ip >= pt->kernel_start ? +PERF_RECORD_MISC_KERNEL : +PERF_RECORD_MISC_USER ; +} + static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn, uint64_t *insn_cnt_ptr, uint64_t *ip, uint64_t to_ip, uint64_t max_insn_cnt,
@@ -429,10 +436,7 @@ static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn,if (to_ip && *ip == to_ip) goto out_no_cache; -if (*ip >= ptq->pt->kernel_start) - cpumode =PERF_RECORD_MISC_KERNEL ; -else - cpumode =PERF_RECORD_MISC_USER ; + cpumode =intel_pt_cpumode (ptq->pt, *ip); thread = ptq->thread;if (!thread) {
@@ -1059,15 +1063,11 @@ static void intel_pt_prep_b_sample(struct intel_pt *pt, union perf_event *event, struct perf_sample *sample) { - event->sample.header.type = PERF_RECORD_SAMPLE; - event->sample.header.misc =PERF_RECORD_MISC_USER ; - event->sample.header.size =sizeof (struct perf_event_header ); -if (!pt->timeless_decoding) sample->time =tsc_to_perf_time (ptq->timestamp, &pt->tc); - sample->cpumode =PERF_RECORD_MISC_USER ; sample->ip = ptq->state->from_ip; + sample->cpumode =intel_pt_cpumode (pt, sample->ip); sample->pid = ptq->pid; sample->tid = ptq->tid; sample->addr = ptq->state->to_ip;
@@ -1076,6 +1076,10 @@ static void intel_pt_prep_b_sample(struct intel_pt *pt, sample->flags = ptq->flags; sample->insn_len = ptq->insn_len;memcpy (sample->insn, ptq->insn,INTEL_PT_INSN_BUF_SZ ); + + event->sample.header.type =PERF_RECORD_SAMPLE ; + event->sample.header.misc = sample->cpumode; + event->sample.header.size =sizeof (struct perf_event_header ); } static int intel_pt_inject_event(union perf_event *event,
Quoted 8 lines · Adrian Hunter
Hi
These patches probably deal with most cases except the one fixed by David Miller's "perf callchain: Honour the ordering of PERF_CONTEXT_{USER,KERNEL,etc}" patch, and also
cat_backtrace()which looks like it has the same problem, and the cs-etm fix.
Quoted 6 lines
Adrian Hunter (5):
perf tools: Add fallback functions for cases where cpumode is insufficient perf tools: Use fallback forsample_addr_correlates_sym()cases perf tools: Use fallbacks for branch stacks
perf intel-pt: Insert callchain context into synthesized callchains perf intel-pt/bts: Calculate cpumode for synthesized samples
thanks,
jirka
Quoted 16 lines
tools/perf/builtin-script.c | 12 +++--- tools/perf/util/event.c | 29 +++++++++++++- tools/perf/util/intel-bts.c | 17 ++++++--- tools/perf/util/intel-pt.c | 28 ++++++++------ tools/perf/util/machine.c | 40 ++++++++++++++++++++ tools/perf/util/machine.h | 2 + .../util/scripting-engines/trace-event-python.c | 16 ++++---- tools/perf/util/thread-stack.c | 44 +++++++++++++++++----- tools/perf/util/thread-stack.h | 2 +- tools/perf/util/thread.h | 4 ++ 10 files changed, 153 insertions(+), 41 deletions(-)Regards
Adrian