Re: [PATCH v2] perf callchain: fix kernel symbol resolution by remembering the cpumode

2015-03-30 Thread David Hildenbrand
> On Mon, Mar 30, 2015 at 10:11:00AM +0200, David Hildenbrand wrote:
> > Commit 2e77784bb7d8 ("perf callchain: Move cpumode resolve code to
> > add_callchain_ip") promised "No change in behavior.".
> > 
> > As this commit breaks callchains on s390x (symbols not getting resolved,
> 
> I think it's a generic problem not just s390x
> 
> the x86 archs were safe due to the (al->map == NULL) fallback
> in thread__find_addr_map, where we rerun the lookup for kernel
> maps.. I need to rethink this check :-\
> 
> perhaps s390x did not match the machine__kernel_ip condition?

Most probably yes, in contrast to other archs, we can't really decide based on
the address if it belongs to user or kernel space. So we need the context.

> 
> 
> > observed when profiling the kernel), this statement is wrong. The cpumode
> > must be kept when iterating over all ips, otherwise the default
> > (PERF_RECORD_MISC_USER) will be used by error.
> 
> anyway
> 
> Acked-by: Jiri Olsa 
> 
> 

Thanks!

David

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] perf callchain: fix kernel symbol resolution by remembering the cpumode

2015-03-30 Thread Jiri Olsa
On Mon, Mar 30, 2015 at 10:11:00AM +0200, David Hildenbrand wrote:
> Commit 2e77784bb7d8 ("perf callchain: Move cpumode resolve code to
> add_callchain_ip") promised "No change in behavior.".
> 
> As this commit breaks callchains on s390x (symbols not getting resolved,

I think it's a generic problem not just s390x

the x86 archs were safe due to the (al->map == NULL) fallback
in thread__find_addr_map, where we rerun the lookup for kernel
maps.. I need to rethink this check :-\

perhaps s390x did not match the machine__kernel_ip condition?


> observed when profiling the kernel), this statement is wrong. The cpumode
> must be kept when iterating over all ips, otherwise the default
> (PERF_RECORD_MISC_USER) will be used by error.

anyway

Acked-by: Jiri Olsa 


thanks,
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2] perf callchain: fix kernel symbol resolution by remembering the cpumode

2015-03-30 Thread David Hildenbrand
Commit 2e77784bb7d8 ("perf callchain: Move cpumode resolve code to
add_callchain_ip") promised "No change in behavior.".

As this commit breaks callchains on s390x (symbols not getting resolved,
observed when profiling the kernel), this statement is wrong. The cpumode
must be kept when iterating over all ips, otherwise the default
(PERF_RECORD_MISC_USER) will be used by error.

Signed-off-by: David Hildenbrand 
---
 tools/perf/util/machine.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index e335330..e45c8f3 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1408,29 +1408,27 @@ struct mem_info *sample__resolve_mem(struct perf_sample 
*sample,
 static int add_callchain_ip(struct thread *thread,
struct symbol **parent,
struct addr_location *root_al,
-   bool branch_history,
+   u8 *cpumode,
u64 ip)
 {
struct addr_location al;
 
al.filtered = 0;
al.sym = NULL;
-   if (branch_history)
+   if (!cpumode) {
thread__find_cpumode_addr_location(thread, MAP__FUNCTION,
   ip, );
-   else {
-   u8 cpumode = PERF_RECORD_MISC_USER;
-
+   } else {
if (ip >= PERF_CONTEXT_MAX) {
switch (ip) {
case PERF_CONTEXT_HV:
-   cpumode = PERF_RECORD_MISC_HYPERVISOR;
+   *cpumode = PERF_RECORD_MISC_HYPERVISOR;
break;
case PERF_CONTEXT_KERNEL:
-   cpumode = PERF_RECORD_MISC_KERNEL;
+   *cpumode = PERF_RECORD_MISC_KERNEL;
break;
case PERF_CONTEXT_USER:
-   cpumode = PERF_RECORD_MISC_USER;
+   *cpumode = PERF_RECORD_MISC_USER;
break;
default:
pr_debug("invalid callchain context: "
@@ -1444,8 +1442,8 @@ static int add_callchain_ip(struct thread *thread,
}
return 0;
}
-   thread__find_addr_location(thread, cpumode, MAP__FUNCTION,
-  ip, );
+   thread__find_addr_location(thread, *cpumode, MAP__FUNCTION,
+  ip, );
}
 
if (al.sym != NULL) {
@@ -1538,6 +1536,7 @@ static int resolve_lbr_callchain_sample(struct thread 
*thread,
 {
struct ip_callchain *chain = sample->callchain;
int chain_nr = min(max_stack, (int)chain->nr);
+   u8 cpumode = PERF_RECORD_MISC_USER;
int i, j, err;
u64 ip;
 
@@ -1584,7 +1583,7 @@ static int resolve_lbr_callchain_sample(struct thread 
*thread,
ip = lbr_stack->entries[0].to;
}
 
-   err = add_callchain_ip(thread, parent, root_al, false, 
ip);
+   err = add_callchain_ip(thread, parent, root_al, 
, ip);
if (err)
return (err < 0) ? err : 0;
}
@@ -1604,6 +1603,7 @@ static int thread__resolve_callchain_sample(struct thread 
*thread,
struct branch_stack *branch = sample->branch_stack;
struct ip_callchain *chain = sample->callchain;
int chain_nr = min(max_stack, (int)chain->nr);
+   u8 cpumode = PERF_RECORD_MISC_USER;
int i, j, err;
int skip_idx = -1;
int first_call = 0;
@@ -1669,10 +1669,10 @@ static int thread__resolve_callchain_sample(struct 
thread *thread,
 
for (i = 0; i < nr; i++) {
err = add_callchain_ip(thread, parent, root_al,
-  true, be[i].to);
+  NULL, be[i].to);
if (!err)
err = add_callchain_ip(thread, parent, root_al,
-  true, be[i].from);
+  NULL, be[i].from);
if (err == -EINVAL)
break;
if (err)
@@ -1701,7 +1701,7 @@ check_calls:
 #endif
ip = chain->ips[j];
 
-   err = add_callchain_ip(thread, parent, root_al, false, ip);
+   err = add_callchain_ip(thread, parent, root_al, , ip);
 
if (err)
return (err < 0) ? err : 0;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to 

[PATCH v2] perf callchain: fix kernel symbol resolution by remembering the cpumode

2015-03-30 Thread David Hildenbrand
Commit 2e77784bb7d8 (perf callchain: Move cpumode resolve code to
add_callchain_ip) promised No change in behavior..

As this commit breaks callchains on s390x (symbols not getting resolved,
observed when profiling the kernel), this statement is wrong. The cpumode
must be kept when iterating over all ips, otherwise the default
(PERF_RECORD_MISC_USER) will be used by error.

Signed-off-by: David Hildenbrand d...@linux.vnet.ibm.com
---
 tools/perf/util/machine.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index e335330..e45c8f3 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1408,29 +1408,27 @@ struct mem_info *sample__resolve_mem(struct perf_sample 
*sample,
 static int add_callchain_ip(struct thread *thread,
struct symbol **parent,
struct addr_location *root_al,
-   bool branch_history,
+   u8 *cpumode,
u64 ip)
 {
struct addr_location al;
 
al.filtered = 0;
al.sym = NULL;
-   if (branch_history)
+   if (!cpumode) {
thread__find_cpumode_addr_location(thread, MAP__FUNCTION,
   ip, al);
-   else {
-   u8 cpumode = PERF_RECORD_MISC_USER;
-
+   } else {
if (ip = PERF_CONTEXT_MAX) {
switch (ip) {
case PERF_CONTEXT_HV:
-   cpumode = PERF_RECORD_MISC_HYPERVISOR;
+   *cpumode = PERF_RECORD_MISC_HYPERVISOR;
break;
case PERF_CONTEXT_KERNEL:
-   cpumode = PERF_RECORD_MISC_KERNEL;
+   *cpumode = PERF_RECORD_MISC_KERNEL;
break;
case PERF_CONTEXT_USER:
-   cpumode = PERF_RECORD_MISC_USER;
+   *cpumode = PERF_RECORD_MISC_USER;
break;
default:
pr_debug(invalid callchain context: 
@@ -1444,8 +1442,8 @@ static int add_callchain_ip(struct thread *thread,
}
return 0;
}
-   thread__find_addr_location(thread, cpumode, MAP__FUNCTION,
-  ip, al);
+   thread__find_addr_location(thread, *cpumode, MAP__FUNCTION,
+  ip, al);
}
 
if (al.sym != NULL) {
@@ -1538,6 +1536,7 @@ static int resolve_lbr_callchain_sample(struct thread 
*thread,
 {
struct ip_callchain *chain = sample-callchain;
int chain_nr = min(max_stack, (int)chain-nr);
+   u8 cpumode = PERF_RECORD_MISC_USER;
int i, j, err;
u64 ip;
 
@@ -1584,7 +1583,7 @@ static int resolve_lbr_callchain_sample(struct thread 
*thread,
ip = lbr_stack-entries[0].to;
}
 
-   err = add_callchain_ip(thread, parent, root_al, false, 
ip);
+   err = add_callchain_ip(thread, parent, root_al, 
cpumode, ip);
if (err)
return (err  0) ? err : 0;
}
@@ -1604,6 +1603,7 @@ static int thread__resolve_callchain_sample(struct thread 
*thread,
struct branch_stack *branch = sample-branch_stack;
struct ip_callchain *chain = sample-callchain;
int chain_nr = min(max_stack, (int)chain-nr);
+   u8 cpumode = PERF_RECORD_MISC_USER;
int i, j, err;
int skip_idx = -1;
int first_call = 0;
@@ -1669,10 +1669,10 @@ static int thread__resolve_callchain_sample(struct 
thread *thread,
 
for (i = 0; i  nr; i++) {
err = add_callchain_ip(thread, parent, root_al,
-  true, be[i].to);
+  NULL, be[i].to);
if (!err)
err = add_callchain_ip(thread, parent, root_al,
-  true, be[i].from);
+  NULL, be[i].from);
if (err == -EINVAL)
break;
if (err)
@@ -1701,7 +1701,7 @@ check_calls:
 #endif
ip = chain-ips[j];
 
-   err = add_callchain_ip(thread, parent, root_al, false, ip);
+   err = add_callchain_ip(thread, parent, root_al, cpumode, ip);
 
if (err)
return (err  0) ? err : 0;
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of 

Re: [PATCH v2] perf callchain: fix kernel symbol resolution by remembering the cpumode

2015-03-30 Thread Jiri Olsa
On Mon, Mar 30, 2015 at 10:11:00AM +0200, David Hildenbrand wrote:
 Commit 2e77784bb7d8 (perf callchain: Move cpumode resolve code to
 add_callchain_ip) promised No change in behavior..
 
 As this commit breaks callchains on s390x (symbols not getting resolved,

I think it's a generic problem not just s390x

the x86 archs were safe due to the (al-map == NULL) fallback
in thread__find_addr_map, where we rerun the lookup for kernel
maps.. I need to rethink this check :-\

perhaps s390x did not match the machine__kernel_ip condition?


 observed when profiling the kernel), this statement is wrong. The cpumode
 must be kept when iterating over all ips, otherwise the default
 (PERF_RECORD_MISC_USER) will be used by error.

anyway

Acked-by: Jiri Olsa jo...@kernel.org


thanks,
jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] perf callchain: fix kernel symbol resolution by remembering the cpumode

2015-03-30 Thread David Hildenbrand
 On Mon, Mar 30, 2015 at 10:11:00AM +0200, David Hildenbrand wrote:
  Commit 2e77784bb7d8 (perf callchain: Move cpumode resolve code to
  add_callchain_ip) promised No change in behavior..
  
  As this commit breaks callchains on s390x (symbols not getting resolved,
 
 I think it's a generic problem not just s390x
 
 the x86 archs were safe due to the (al-map == NULL) fallback
 in thread__find_addr_map, where we rerun the lookup for kernel
 maps.. I need to rethink this check :-\
 
 perhaps s390x did not match the machine__kernel_ip condition?

Most probably yes, in contrast to other archs, we can't really decide based on
the address if it belongs to user or kernel space. So we need the context.

 
 
  observed when profiling the kernel), this statement is wrong. The cpumode
  must be kept when iterating over all ips, otherwise the default
  (PERF_RECORD_MISC_USER) will be used by error.
 
 anyway
 
 Acked-by: Jiri Olsa jo...@kernel.org
 
 

Thanks!

David

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/