perf callchain: fix kernel symbol resolution by remembering the cpumode

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 <[email protected]>

---
 tools/perf/util/machine.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

tools/perf/util/machine.c index renamed from tools/perf/util/machine.c +0 -0

e335330..c843652 100644

tools/perf/util/machine.c +8 -10

@@ -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);
 	}

tools/perf/util/machine.c +1 -0

 	if (al.sym != NULL) {

@@ -1604,6 +1602,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;

tools/perf/util/machine.c +2 -2

 	int first_call = 0;

@@ -1669,10 +1668,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)

tools/perf/util/machine.c +1 -1

 				break;
 			if (err)

@@ -1701,7 +1700,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

Quoted 5 lines

Commit 2e77784bb7d8 ("perf callchain: Move cpumode resolve code to add_callchain_ip") promised "No change in behavior.".

Quoted 4 lines

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.

Indeed.
Besides thread__resolve_callchain_sample, lbr path also need to be patched.

tools/perf/util/machine.c +2 -1

@@ -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;

                }

Thanks,
Kan

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.

Quoted 4 lines

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?

Quoted 5 lines

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 <[email protected]>