Skip to contentSkip to conversation

perf tool: Haswell LBR call stack support (user)

PATCH Superseded [email protected] 4 participants 25 messages Nov 14, 2014 – Nov 21, 2014 perf events

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

[email protected]

author
#0 lore raw

v3 cover perf tool: Haswell LBR call stack support (user)

From: Kan Liang <[email protected]>

This is the user space patch for Haswell LBR call stack support. For many profiling tasks we need the callgraph. For example we often need to see the caller of a lock or the caller of a memcpy or other library function to actually tune the program. Frame pointer unwinding is efficient and works well. But frame pointers are off by default on 64bit code (and on modern 32bit gccs), so there are many binaries around that do not use frame pointers. Profiling unchanged production code is very useful in practice. On some CPUs frame pointer also has a high cost. Dwarf2 unwinding also does not always work and is extremely slow (upto 20% overhead).

Haswell has a new feature that utilizes the existing Last Branch Record facility to record call chains. When the feature is enabled, function call will be collected as normal, but as return instructions are executed the last captured branch record is popped from the on-chip LBR registers. The LBR call stack facility provides an alternative to get callgraph. It has some limitations too, but should work in most cases and is significantly faster than dwarf. Frame pointer unwinding is still the best default, but LBR call stack is a good alternative when nothing else works.

A new call chain recording option "lbr" is introduced into perf tool for LBR call stack. The user can use --call-graph lbr to get the call stack information from hardware.

When profiling bc(1) on Fedora 19:
echo 'scale=2000; 4*a(1)' > cmd; perf record --call-graph lbr bc -l < cmd If enabling LBR, perf report output looks like:

    50.36%       bc  bc                 [.] bc_divide
                 |
                 --- bc_divide
                     execute
                     run_code
                     yyparse
                     main
                     __libc_start_main
                     _start
    33.66%       bc  bc                 [.] _one_mult
                 |
                 --- _one_mult
                     bc_divide
                     execute
                     run_code
                     yyparse
                     main
                     __libc_start_main
                     _start
     7.62%       bc  bc                 [.] _bc_do_add
                 |
                 --- _bc_do_add
                    |
                    |--99.89%-- 0x2000186a8
                     --0.11%-- [...]
     6.83%       bc  bc                 [.] _bc_do_sub
                 |
                 --- _bc_do_sub
                    |
                    |--99.94%-- bc_add
                    |          execute
                    |          run_code
                    |          yyparse
                    |          main
                    |          __libc_start_main
                    |          _start
                     --0.06%-- [...]
     0.46%       bc  libc-2.17.so       [.] __memset_sse2
                 |
                 --- __memset_sse2
                    |
                    |--54.13%-- bc_new_num
                    |          |
                    |          |--51.00%-- bc_divide
                    |          |          execute
                    |          |          run_code
                    |          |          yyparse
                    |          |          main
                    |          |          __libc_start_main
                    |          |          _start
                    |          |
                    |          |--30.46%-- _bc_do_sub
                    |          |          bc_add
                    |          |          execute
                    |          |          run_code
                    |          |          yyparse
                    |          |          main
                    |          |          __libc_start_main
                    |          |          _start
                    |          |
                    |           --18.55%-- _bc_do_add
                    |                     bc_add
                    |                     execute
                    |                     run_code
                    |                     yyparse
                    |                     main
                    |                     __libc_start_main
                    |                     _start
                    |
                     --45.87%-- bc_divide
                               execute
                               run_code
                               yyparse
                               main
                               __libc_start_main
                               _start
If using FP, perf report output looks like:
echo 'scale=2000; 4*a(1)' > cmd; perf record --call-graph fp bc -l < cmd
    50.49%       bc  bc                 [.] bc_divide
                 |
                 --- bc_divide
    33.57%       bc  bc                 [.] _one_mult
                 |
                 --- _one_mult
     7.61%       bc  bc                 [.] _bc_do_add
                 |
                 --- _bc_do_add
                     0x2000186a8
     6.88%       bc  bc                 [.] _bc_do_sub
                 |
                 --- _bc_do_sub
     0.42%       bc  libc-2.17.so       [.] __memcpy_ssse3_back
                 |
                 --- __memcpy_ssse3_back

If using LBR, perf report -D output looks like:
11739295893248 0x4d0 [0xe0]: PERF_RECORD_SAMPLE(IP, 0x2): 10505/10505:
0x40054d period: 39255 addr: 0
... LBR call chain: nr:7

.....  0: fffffffffffffe00
.....  1: 0000000000400540
.....  2: 0000000000400587
.....  3: 00000000004005b3
.....  4: 00000000004005ef
.....  5: 0000003d1cc21b43
.....  6: 0000000000400474
... FP chain: nr:6
.....  0: fffffffffffffe00
.....  1: 000000000040054d
.....  2: 000000000040058c
.....  3: 00000000004005b8
.....  4: 00000000004005f4
.....  5: 0000003d1cc21b45

... thread: a.out:10505
...... dso: /home/lk/a.out

The LBR call stack has following known limitations
- Zero length calls are not filtered out by hardware
- Exception handing such as setjmp/longjmp will have calls/returns not match
- Pushing different return address onto the stack will have calls/returns not match
- If callstack is deeper than the LBR, only the last entries are captured

Changes since v1
- Update help document
- Force exclude_user to 0 with warning in LBR call stack
- Dump both lbr and fp info when report -D
- Reconstruct thread__resolve_callchain_sample and split it into two patches
- Use has_branch_callstack function to check LBR call stack available

Changes since v2
- Rebase to 025ce5d33373

Kan Liang (3):
perf tools: enable LBR call stack support
perf tool: Move cpumode resolve code to add_callchain_ip perf tools: Construct LBR call chain

 tools/perf/Documentation/perf-record.txt |   8 +-
 tools/perf/builtin-record.c              |   6 +-
 tools/perf/builtin-report.c              |   2 +
 tools/perf/util/callchain.c              |  10 ++-
 tools/perf/util/callchain.h              |   1 +
 tools/perf/util/evsel.c                  |  21 ++++-
 tools/perf/util/evsel.h                  |   4 +
 tools/perf/util/machine.c                | 134 +++++++++++++++++++++----------
 tools/perf/util/session.c                |  60 ++++++++++++--
 9 files changed, 192 insertions(+), 54 deletions(-)
Signature · 1 line
1.8.3.2
author
#1 lore raw

PATCH v3 1/3 perf tools: enable LBR call stack support

From: Kan Liang <[email protected]>

Currently, there are two call chain recording options, fp and dwarf. Haswell has a new feature that utilizes the existing LBR facility to record call chains. So it provides the third options to record call chain. This patch enables the lbr call stack support.

LBR call stack has some limitations. It reuses current LBR facility, so LBR call stack and branch record can not be enabled at the same time. It is only available for user callchain.
However, LBR call stack can work on the user app which doesn't have frame-pointer or dwarf debug info compiled. It is a good alternative when nothing else works.

Signed-off-by Kan Liang <[email protected]>
---
 tools/perf/Documentation/perf-record.txt |  8 +++++++-
 tools/perf/builtin-record.c              |  6 +++---
 tools/perf/builtin-report.c              |  2 ++
 tools/perf/util/callchain.c              | 10 +++++++++-
 tools/perf/util/callchain.h              |  1 +
 tools/perf/util/evsel.c                  | 21 +++++++++++++++++++--
 6 files changed, 41 insertions(+), 7 deletions(-)
tools/perf/Documentation/perf-record.txt +7 -1
index 398f8d5..03d9939 100644
@@ -100,13 +100,19 @@ OPTIONS
 	implies -g.
 
 	Allows specifying "fp" (frame pointer) or "dwarf"
-	(DWARF's CFI - Call Frame Information) as the method to collect
+	(DWARF's CFI - Call Frame Information) or "lbr"
+	(Hardware Last Branch Record facility) as the method to collect
 	the information used to show the call graphs.
 
 	In some systems, where binaries are build with gcc
 	--fomit-frame-pointer, using the "fp" method will produce bogus
 	call graphs, using "dwarf", if available (perf tools linked to
 	the libunwind library) should be used instead.
+	Using the "lbr" method doesn't require any compiler options. It
+	will produce call graphs from the hardware LBR registers. The
+	main limition is that it is only available on new Intel
+	platforms, such as Haswell. It can only get user call chain. It
+	doesn't work with branch stack sampling at the same time.
 
 -q::
 --quiet::
tools/perf/builtin-record.c +3 -3
index 582c4da..e486627 100644
@@ -639,7 +639,7 @@ error:
 
 static void callchain_debug(void)
 {
-	static const char *str[CALLCHAIN_MAX] = { "NONE", "FP", "DWARF" };
+	static const char *str[CALLCHAIN_MAX] = { "NONE", "FP", "DWARF", "LBR" };
 
 	pr_debug("callchain: type %s\n", str[callchain_param.record_mode]);
 

@@ -725,9 +725,9 @@ static struct record record = {
 #define CALLCHAIN_HELP "setup and enables call-graph (stack chain/backtrace) recording: "
 
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
-const char record_callchain_help[] = CALLCHAIN_HELP "fp dwarf";
+const char record_callchain_help[] = CALLCHAIN_HELP "fp dwarf lbr";
 #else
-const char record_callchain_help[] = CALLCHAIN_HELP "fp";
+const char record_callchain_help[] = CALLCHAIN_HELP "fp lbr";
 #endif
 
 /*
tools/perf/builtin-report.c +2 -0
index 140a6cd..43babdb 100644
@@ -261,6 +261,8 @@ static int report__setup_sample_type(struct report *rep)
 		if ((sample_type & PERF_SAMPLE_REGS_USER) &&
 		    (sample_type & PERF_SAMPLE_STACK_USER))
 			callchain_param.record_mode = CALLCHAIN_DWARF;
+		else if (sample_type & PERF_SAMPLE_BRANCH_STACK)
+			callchain_param.record_mode = CALLCHAIN_LBR;
 		else
 			callchain_param.record_mode = CALLCHAIN_FP;
 	}
tools/perf/util/callchain.c +9 -1
index 38da69c..138c5d6 100644
@@ -77,7 +77,7 @@ int parse_callchain_record_opt(const char *arg)
 				ret = 0;
 			} else
 				pr_err("callchain: No more arguments "
-				       "needed for -g fp\n");
+				       "needed for --call-graph fp\n");
 			break;
 
 #ifdef HAVE_DWARF_UNWIND_SUPPORT

@@ -97,6 +97,14 @@ int parse_callchain_record_opt(const char *arg)
 				callchain_param.dump_size = size;
 			}
 #endif /* HAVE_DWARF_UNWIND_SUPPORT */
+		} else if (!strncmp(name, "lbr", sizeof("lbr"))) {
+			if (!strtok_r(NULL, ",", &saveptr)) {
+				callchain_param.record_mode = CALLCHAIN_LBR;
+				ret = 0;
+			} else
+				pr_err("callchain: No more arguments "
+					"needed for --call-graph lbr\n");
+			break;
 		} else {
 			pr_err("callchain: Unknown --call-graph option "
 			       "value: %s\n", arg);
tools/perf/util/callchain.h +1 -0
index 3e1ed15..a60a7b5 100644
@@ -11,6 +11,7 @@ enum perf_call_graph_mode {
 	CALLCHAIN_NONE,
 	CALLCHAIN_FP,
 	CALLCHAIN_DWARF,
+	CALLCHAIN_LBR,
 	CALLCHAIN_MAX
 };
tools/perf/util/evsel.c +19 -2
index 12b4396..7cbe2e9 100644
@@ -537,13 +537,30 @@ int perf_evsel__group_desc(struct perf_evsel *evsel, char *buf, size_t size)
 }
 
 static void
-perf_evsel__config_callgraph(struct perf_evsel *evsel)
+perf_evsel__config_callgraph(struct perf_evsel *evsel,
+			     struct record_opts *opts)
 {
 	bool function = perf_evsel__is_function_event(evsel);
 	struct perf_event_attr *attr = &evsel->attr;
 
 	perf_evsel__set_sample_bit(evsel, CALLCHAIN);
 
+	if (callchain_param.record_mode == CALLCHAIN_LBR) {
+		if (!opts->branch_stack) {
+			perf_evsel__set_sample_bit(evsel, BRANCH_STACK);
+			attr->branch_sample_type = PERF_SAMPLE_BRANCH_USER |
+						PERF_SAMPLE_BRANCH_CALL_STACK;
+			if (attr->exclude_user) {
+				attr->exclude_user = 0;
+
+				pr_warning("LBR callstack option is only available"
+					   " to get user callchain information."
+					   " Force exclude_user to 0.\n");
+			}
+		} else
+			 pr_info("Cannot use LBR callstack with branch stack\n");
+	}
+
 	if (callchain_param.record_mode == CALLCHAIN_DWARF) {
 		if (!function) {
 			perf_evsel__set_sample_bit(evsel, REGS_USER);

@@ -659,7 +676,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 	}
 
 	if (callchain_param.enabled && !evsel->no_aux_samples)
-		perf_evsel__config_callgraph(evsel);
+		perf_evsel__config_callgraph(evsel, opts);
 
 	if (target__has_cpu(&opts->target))
 		perf_evsel__set_sample_bit(evsel, CPU);
Signature · 1 line
1.8.3.2
maintainer
#2 lore raw

Re: [PATCH V3 1/3] perf tools: enable LBR call stack support

Quoted 7 lines · kan liang

From: Kan Liang <[email protected]>

Currently, there are two call chain recording options, fp and dwarf. Haswell has a new feature that utilizes the existing LBR facility to record call chains. So it provides the third options to record call chain. This patch enables the lbr call stack support.

Quoted 6 lines

LBR call stack has some limitations. It reuses current LBR facility, so LBR call stack and branch record can not be enabled at the same time. It is only available for user callchain.
However, LBR call stack can work on the user app which doesn't have frame-pointer or dwarf debug info compiled. It is a good alternative when nothing else works.

So this is only for Haswell or later, right? What if user gives it to the other CPUs - is it fail or ignore silently? I think we need to warn user about it more kindly..

Thanks,
Namhyung

author
#3 lore raw

RE: [PATCH V3 1/3] perf tools: enable LBR call stack support

Quoted 10 lines · kan liang
Quoted 7 lines

From: Kan Liang <[email protected]>

Currently, there are two call chain recording options, fp and dwarf. Haswell has a new feature that utilizes the existing LBR facility to record call chains. So it provides the third options to record call chain. This patch enables the lbr call stack support.

Quoted 8 lines
Quoted 6 lines

LBR call stack has some limitations. It reuses current LBR facility, so LBR call stack and branch record can not be enabled at the same time. It is only available for user callchain.
However, LBR call stack can work on the user app which doesn't have frame-pointer or dwarf debug info compiled. It is a good alternative when nothing else works.

So this is only for Haswell or later, right?

Right.

Quoted 3 lines

What if user gives it to the other
CPUs - is it fail or ignore silently? I think we need to warn user about it more kindly..

The kernel will handle it. EOPNOTSUPP will be return if there is no LBR Call stack support.

Thanks,
Kan

Quoted 3 lines

Thanks,
Namhyung

author
#4 lore raw

PATCH v3 2/3 perf tool: Move cpumode resolve code to add_callchain_ip

In reply to [email protected]

From: Kan Liang <[email protected]>

Move the cpumode resolve code to add_callchain_ip function. No change in behavior.

Signed-off-by Kan Liang <[email protected]>
---
 tools/perf/util/machine.c | 62 ++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 33 deletions(-)
tools/perf/util/machine.c +29 -33
index d97309c..dd8496a 100644
@@ -1384,11 +1384,35 @@ 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,
-			    int cpumode,
+			    u8 cpumode,
 			    u64 ip)
 {
 	struct addr_location al;
 
+	if (ip >= PERF_CONTEXT_MAX) {
+		switch (ip) {
+		case PERF_CONTEXT_HV:
+			cpumode = PERF_RECORD_MISC_HYPERVISOR;
+			break;
+		case PERF_CONTEXT_KERNEL:
+			cpumode = PERF_RECORD_MISC_KERNEL;
+			break;
+		case PERF_CONTEXT_USER:
+			cpumode = PERF_RECORD_MISC_USER;
+			break;
+		default:
+			pr_debug("invalid callchain context: "
+				 "%"PRId64"\n", (s64) ip);
+			/*
+			 * It seems the callchain is corrupted.
+			 * Discard all.
+			 */
+			callchain_cursor_reset(&callchain_cursor);
+			return 1;
+		}
+		return 0;	
+	}
+
 	al.filtered = 0;
 	al.sym = NULL;
 	thread__find_addr_location(thread, cpumode, MAP__FUNCTION,

@@ -1435,9 +1459,7 @@ static int thread__resolve_callchain_sample(struct thread *thread,
 {
 	u8 cpumode = PERF_RECORD_MISC_USER;
 	int chain_nr = min(max_stack, (int)chain->nr);
-	int i;
-	int j;
-	int err;
+	int i, j, err = 0;
 	int skip_idx __maybe_unused;
 
 	callchain_cursor_reset(&callchain_cursor);

@@ -1467,39 +1489,13 @@ static int thread__resolve_callchain_sample(struct thread *thread,
 #endif
 		ip = chain->ips[j];
 
-		if (ip >= PERF_CONTEXT_MAX) {
-			switch (ip) {
-			case PERF_CONTEXT_HV:
-				cpumode = PERF_RECORD_MISC_HYPERVISOR;
-				break;
-			case PERF_CONTEXT_KERNEL:
-				cpumode = PERF_RECORD_MISC_KERNEL;
-				break;
-			case PERF_CONTEXT_USER:
-				cpumode = PERF_RECORD_MISC_USER;
-				break;
-			default:
-				pr_debug("invalid callchain context: "
-					 "%"PRId64"\n", (s64) ip);
-				/*
-				 * It seems the callchain is corrupted.
-				 * Discard all.
-				 */
-				callchain_cursor_reset(&callchain_cursor);
-				return 0;
-			}
-			continue;
-		}
-
 		err = add_callchain_ip(thread, parent, root_al,
 				       cpumode, ip);
-		if (err == -EINVAL)
-			break;
 		if (err)
-			return err;
+			goto exit;
 	}
-
-	return 0;
+exit:
+	return (err < 0) ? err : 0;
 }
 
 static int unwind_entry(struct unwind_entry *entry, void *arg)
Signature · 1 line
1.8.3.2
#5 lore raw

Re: [PATCH V3 2/3] perf tool: Move cpumode resolve code to add_callchain_ip

Quoted 1 line · On Fri, Nov 14, 2014 at 08:44:11AM -0500, [email protected] wrote:

From: Kan Liang <[email protected]>

SNIP

Quoted 24 lines
tools/perf/util/machine.c +13 -1
@@ -1384,11 +1384,35 @@ 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,
-			    int cpumode,
+			    u8 cpumode,
 			    u64 ip)
 {
 	struct addr_location al;
 
+	if (ip >= PERF_CONTEXT_MAX) {
+		switch (ip) {
+		case PERF_CONTEXT_HV:
+			cpumode = PERF_RECORD_MISC_HYPERVISOR;
+			break;
+		case PERF_CONTEXT_KERNEL:
+			cpumode = PERF_RECORD_MISC_KERNEL;
+			break;
+		case PERF_CONTEXT_USER:
+			cpumode = PERF_RECORD_MISC_USER;
+			break;
+		default:

tools/perf/util/machine.c struct mem_info *sample__resolve_mem(struct perf_sample *sample, · line 1413

Quoted 10 lines
tools/perf/util/machine.c +10 -0
+			pr_debug("invalid callchain context: "
+				 "%"PRId64"\n", (s64) ip);
+			/*
+			 * It seems the callchain is corrupted.
+			 * Discard all.
+			 */
+			callchain_cursor_reset(&callchain_cursor);
+			return 1;
+		}
+		return 0;

extra whitespace ^^^^^^^

Quoted 5 lines
tools/perf/util/machine.c +2 -0
+	}
+
 	al.filtered = 0;
 	al.sym = NULL;
 	thread__find_addr_location(thread, cpumode, MAP__FUNCTION,

SNIP

tools/perf/util/machine.c static int thread__resolve_callchain_sample(struct thread *thread, · line 1493

Quoted 10 lines
tools/perf/util/machine.c +0 -8
-				 * Discard all.
-				 */
-				callchain_cursor_reset(&callchain_cursor);
-				return 0;
-			}
-			continue;
-		}
-
 		err = add_callchain_ip(thread, parent, root_al,
 				       cpumode, ip);

hm, why do we need to pass cpumode in here? it should be handled within add_callchain_ip in add_callchain_ip function, right?

jirka

#6 lore raw

Re: [PATCH V3 2/3] perf tool: Move cpumode resolve code to add_callchain_ip

In reply to [email protected]

On Fri, Nov 14, 2014 at 08:44:11AM -0500, [email protected] wrote:

SNIP

Quoted 29 lines
tools/perf/util/machine.c +1 -21
-	int i;
-	int j;
-	int err;
+	int i, j, err = 0;
 	int skip_idx __maybe_unused;
 
 	callchain_cursor_reset(&callchain_cursor);

@@ -1467,39 +1489,13 @@ static int thread__resolve_callchain_sample(struct thread *thread,
 #endif
 		ip = chain->ips[j];
 
-		if (ip >= PERF_CONTEXT_MAX) {
-			switch (ip) {
-			case PERF_CONTEXT_HV:
-				cpumode = PERF_RECORD_MISC_HYPERVISOR;
-				break;
-			case PERF_CONTEXT_KERNEL:
-				cpumode = PERF_RECORD_MISC_KERNEL;
-				break;
-			case PERF_CONTEXT_USER:
-				cpumode = PERF_RECORD_MISC_USER;
-				break;
-			default:
-				pr_debug("invalid callchain context: "
-					 "%"PRId64"\n", (s64) ip);
-				/*
-				 * It seems the callchain is corrupted.
-				 * Discard all.
-				 */

tools/perf/util/machine.c static int thread__resolve_callchain_sample(struct thread *thread, · line 1497

Quoted 10 lines
tools/perf/util/machine.c +0 -8
-				callchain_cursor_reset(&callchain_cursor);
-				return 0;
-			}
-			continue;
-		}
-
 		err = add_callchain_ip(thread, parent, root_al,
 				       cpumode, ip);
-		if (err == -EINVAL)
-			break;

yea, I was wondering, why we did not spot this nop before ;-)

tools/perf/util/machine.c static int thread__resolve_callchain_sample(struct thread *thread, · line 1495

Quoted 3 lines
tools/perf/util/machine.c +1 -1
 		if (err)
-			return err;
+			goto exit;

I dont understand this change seems like extra nop complication.. why not return in here and rather recheck at the end again?

jirka

Quoted 6 lines
tools/perf/util/machine.c +2 -2
 	}
-
-	return 0;
+exit:
+	return (err < 0) ? err : 0;
 }
#7 lore raw

Re: [PATCH V3 2/3] perf tool: Move cpumode resolve code to add_callchain_ip

In reply to [email protected]
Quoted 15 lines · On Fri, Nov 14, 2014 at 08:44:11AM -0500, [email protected] wrote:

From: Kan Liang <[email protected]>

Move the cpumode resolve code to add_callchain_ip function. No change in behavior.

Signed-off-by Kan Liang <[email protected]>
---
 tools/perf/util/machine.c | 62 ++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 33 deletions(-)
tools/perf/util/machine.c +0 -0
index d97309c..dd8496a 100644

tools/perf/util/machine.c struct mem_info *sample__resolve_mem(struct perf_sample *sample, · line 1387

Quoted 5 lines
tools/perf/util/machine.c +1 -1
@@ -1384,11 +1384,35 @@ 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,
-			    int cpumode,
+			    u8 cpumode,

Andi's patch got in:
b3340a5 perf callchain: Support handling complete branch stacks as histograms

and it uses the cpumode check for -1, so it needs to stay int, otherwise there's a compilation failure:

CC util/machine.o
util/machine.c: In function ‘add_callchain_ip’:
util/machine.c:1419:14: error: comparison is always false due to limited range of data type [-Werror=type-limits]
if (cpumode == -1)

              ^

jirka

author
#8 lore raw

RE: [PATCH V3 2/3] perf tool: Move cpumode resolve code to add_callchain_ip

Quoted 43 lines

-----Original Message-----
From: Jiri Olsa [mailto:[email protected]]
Sent: Tuesday, November 18, 2014 3:25 AM
To: Liang, Kan
Cc: [email protected]; [email protected]; [email protected]; linux- [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH V3 2/3] perf tool: Move cpumode resolve code to add_callchain_ip

tools/perf/util/machine.c struct mem_info

Quoted 16 lines · On Fri, Nov 14, 2014 at 08:44:11AM -0500, [email protected] wrote:

From: Kan Liang <[email protected]>

Move the cpumode resolve code to add_callchain_ip function. No change in behavior.

Signed-off-by Kan Liang <[email protected]>
---
 tools/perf/util/machine.c | 62
++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 33 deletions(-)
tools/perf/util/machine.c +0 -0
index d97309c..dd8496a 100644
@@ -1384,11 +1384,35 @@ struct mem_info

*sample__resolve_mem(struct

tools/perf/util/machine.c struct mem_info *sample__resolve_mem(struct perf_sample *sample, · line 1387

Quoted 5 lines
tools/perf/util/machine.c +1 -1
 perf_sample *sample,  static int add_callchain_ip(struct thread *thread,
 			    struct symbol **parent,
 			    struct addr_location *root_al,
-			    int cpumode,
+			    u8 cpumode,

Andi's patch got in:
b3340a5 perf callchain: Support handling complete branch stacks as histograms

and it uses the cpumode check for -1, so it needs to stay int, otherwise there's a compilation failure:

Quoted 7 lines

CC util/machine.o
util/machine.c: In function ‘add_callchain_ip’:
util/machine.c:1419:14: error: comparison is always false due to limited range of data type [-Werror=type-limits]
if (cpumode == -1)

              ^

Hi Arnaldo and Jirka,

According to recent feedback, the LBR call stack V4 patch only need to do small change. However, it looks the patch "b3340a5 perf callchain: Support handling complete branch stacks as histograms" has been removed from perf/core.

Should I wait for the patch to merge again and submit the V5 patch then, OR Should I rebase the V5 to the latest perf/core and submit it today?

Thanks,
Kan

Quoted 2 lines

jirka

���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�

maintainer
#9 lore raw

Re: [PATCH V3 2/3] perf tool: Move cpumode resolve code to add_callchain_ip

Em Fri, Nov 21, 2014 at 03:06:08PM +0000, Liang, Kan escreveu:

Quoted 51 lines
Quoted 43 lines

-----Original Message-----
From: Jiri Olsa [mailto:[email protected]]
Sent: Tuesday, November 18, 2014 3:25 AM
To: Liang, Kan
Cc: [email protected]; [email protected]; [email protected]; linux- [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH V3 2/3] perf tool: Move cpumode resolve code to add_callchain_ip

tools/perf/util/machine.c struct mem_info

Quoted 16 lines · On Fri, Nov 14, 2014 at 08:44:11AM -0500, [email protected] wrote:

From: Kan Liang <[email protected]>

Move the cpumode resolve code to add_callchain_ip function. No change in behavior.

Signed-off-by Kan Liang <[email protected]>
---
 tools/perf/util/machine.c | 62
++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 33 deletions(-)
tools/perf/util/machine.c +0 -0
index d97309c..dd8496a 100644
@@ -1384,11 +1384,35 @@ struct mem_info

*sample__resolve_mem(struct

tools/perf/util/machine.c struct mem_info *sample__resolve_mem(struct perf_sample *sample, · line 1387

Quoted 5 lines
tools/perf/util/machine.c +1 -1
 perf_sample *sample,  static int add_callchain_ip(struct thread *thread,
 			    struct symbol **parent,
 			    struct addr_location *root_al,
-			    int cpumode,
+			    u8 cpumode,

Andi's patch got in:
b3340a5 perf callchain: Support handling complete branch stacks as histograms

and it uses the cpumode check for -1, so it needs to stay int, otherwise there's a compilation failure:

Quoted 7 lines

CC util/machine.o
util/machine.c: In function ‘add_callchain_ip’:
util/machine.c:1419:14: error: comparison is always false due to limited range of data type [-Werror=type-limits]
if (cpumode == -1)

              ^
Quoted 5 lines

Hi Arnaldo and Jirka,

According to recent feedback, the LBR call stack V4 patch only need to do small change. However, it looks the patch "b3340a5 perf callchain: Support handling complete branch stacks as histograms" has been removed from perf/core.

Yeah, I should have left it on a separate branch while testing was underway, my bad.

Quoted 2 lines

Should I wait for the patch to merge again and submit the V5 patch then, OR Should I rebase the V5 to the latest perf/core and submit it today?

I'll try to send a screenshot of the differences among

perf report --branch-report --tui
perf report --branch-report --stdio (or --gtk, those two matches)

As requested by Andi, that isn't noticing any difference among these three output modes.

After this is clarified (and possibly fixed) I'll push to Ingo and then you can move on.

- Arnaldo

author
#10 lore raw

PATCH v3 3/3 perf tools: Construct LBR call chain

In reply to [email protected]

From: Kan Liang <[email protected]>

LBR call stack only has user callchain. It is output as PERF_SAMPLE_BRANCH_STACK data format. For the kernel callchain, it's still from PERF_SAMPLE_CALLCHAIN.
The perf tool has to handle both data sources to construct a complete callstack.
For perf report -D option, both lbr and fp information will be displayed.

Signed-off-by Kan Liang <[email protected]>
---
 tools/perf/util/evsel.h   |  4 +++
 tools/perf/util/machine.c | 72 +++++++++++++++++++++++++++++++++++++++++------
 tools/perf/util/session.c | 60 +++++++++++++++++++++++++++++++++++----
 3 files changed, 122 insertions(+), 14 deletions(-)
tools/perf/util/evsel.h +4 -0
index 9797909..1bbaa74 100644
@@ -368,4 +368,8 @@ for ((_evsel) = list_entry((_leader)->node.next, struct perf_evsel, node); 	\
      (_evsel) && (_evsel)->leader == (_leader);					\
      (_evsel) = list_entry((_evsel)->node.next, struct perf_evsel, node))
 
+static inline bool has_branch_callstack(struct perf_evsel *evsel)
+{
+	return evsel->attr.branch_sample_type & PERF_SAMPLE_BRANCH_CALL_STACK;
+}
 #endif /* __PERF_EVSEL_H */
tools/perf/util/machine.c +64 -8

101 lines not on this page · raw message

tools/perf/util/session.c +54 -6

78 lines not on this page · raw message

Signature · 1 line
1.8.3.2
#11 lore raw

Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

On Fri, Nov 14, 2014 at 08:44:12AM -0500, [email protected] wrote:

SNIP

Quoted 35 lines
tools/perf/util/session.c +20 -3
index f4478ce..335c3a9 100644
@@ -557,15 +557,63 @@ int perf_session_queue_event(struct perf_session *s, union perf_event *event,
 	return 0;
 }
 
-static void callchain__printf(struct perf_sample *sample)
+static void callchain__printf(struct perf_evsel *evsel,
+			      struct perf_sample *sample)
 {
 	unsigned int i;
+	struct ip_callchain *callchain = sample->callchain;
+	bool lbr = has_branch_callstack(evsel);
 
-	printf("... chain: nr:%" PRIu64 "\n", sample->callchain->nr);
+	if (lbr) {
+		struct branch_stack *lbr_stack = sample->branch_stack;
+		u64 kernel_callchain_nr = callchain->nr;
 
-	for (i = 0; i < sample->callchain->nr; i++)
+		for (i = 0; i < kernel_callchain_nr; i++) {
+			if (callchain->ips[i] == PERF_CONTEXT_USER)
+				break;
+		}
+
+		if ((i != kernel_callchain_nr) && lbr_stack->nr) {
+			u64 total_nr;
+			/*
+			 * LBR callstack can only get user call chain,
+			 * i is kernel call chain number,
+			 * 1 is PERF_CONTEXT_USER.
+			 *
+			 * The user call chain is stored in LBR registers.

tools/perf/util/session.c int perf_session_queue_event(struct perf_session *s, union perf_event *event, · line 593

Quoted 10 lines
tools/perf/util/session.c +10 -0
+			 * LBR are pair registers. The caller is stored
+			 * in "from" register, while the callee is stored
+			 * in "to" register.
+			 * For example, there is a call stack
+			 * "A"->"B"->"C"->"D".
+			 * The LBR registers will recorde like
+			 * "C"->"D", "B"->"C", "A"->"B".
+			 * So only the first "to" register and all "from"
+			 * registers are needed to construct the whole stack.
+			 */

Andi is using some sanity checks:
http://marc.info/?l=linux-kernel&m=141584447819894&w=2
I guess this could be applied in here, once his patch gets in.

jirka

author
#12 lore raw

RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

Quoted 50 lines

SNIP

tools/perf/util/session.c int perf_session_queue_event(struct

Quoted 5 lines
tools/perf/util/session.c +0 -0
index f4478ce..335c3a9 100644
@@ -557,15 +557,63 @@ int perf_session_queue_event(struct

perf_session *s, union perf_event *event,

Quoted 29 lines
tools/perf/util/session.c +19 -3
 	return 0;
 }
 
-static void callchain__printf(struct perf_sample *sample)
+static void callchain__printf(struct perf_evsel *evsel,
+			      struct perf_sample *sample)
 {
 	unsigned int i;
+	struct ip_callchain *callchain = sample->callchain;
+	bool lbr = has_branch_callstack(evsel);
 
-	printf("... chain: nr:%" PRIu64 "\n", sample->callchain->nr);
+	if (lbr) {
+		struct branch_stack *lbr_stack = sample->branch_stack;
+		u64 kernel_callchain_nr = callchain->nr;
 
-	for (i = 0; i < sample->callchain->nr; i++)
+		for (i = 0; i < kernel_callchain_nr; i++) {
+			if (callchain->ips[i] == PERF_CONTEXT_USER)
+				break;
+		}
+
+		if ((i != kernel_callchain_nr) && lbr_stack->nr) {
+			u64 total_nr;
+			/*
+			 * LBR callstack can only get user call chain,
+			 * i is kernel call chain number,
+			 * 1 is PERF_CONTEXT_USER.
+			 *

tools/perf/util/session.c int perf_session_queue_event(struct perf_session *s, union perf_event *event,

Quoted 10 lines
tools/perf/util/session.c +10 -0
+			 * The user call chain is stored in LBR registers.
+			 * LBR are pair registers. The caller is stored
+			 * in "from" register, while the callee is stored
+			 * in "to" register.
+			 * For example, there is a call stack
+			 * "A"->"B"->"C"->"D".
+			 * The LBR registers will recorde like
+			 * "C"->"D", "B"->"C", "A"->"B".
+			 * So only the first "to" register and all "from"
+			 * registers are needed to construct the whole

stack.

Quoted 6 lines

tools/perf/util/session.c int perf_session_queue_event(struct · line 593

Quoted 1 line
tools/perf/util/session.c +1 -0
+			 */

Andi is using some sanity checks:
http://marc.info/?l=linux-kernel&m=141584447819894&w=2
I guess this could be applied in here, once his patch gets in.

Are you suggesting me to remove the comments,
or rebase the whole patch to Andi's patch once it's merged?

The branch history in Andi's patch is different as the call stack, although they are both from LBR.
Andi's branch history recording branch records for taken branches, interrupts, and exceptions.
While the LBR call stack records for the call stack.

Thanks,
Kan

maintainer
#13 lore raw

Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

Quoted 55 lines · Kan Liang
Quoted 48 lines

SNIP

tools/perf/util/session.c int perf_session_queue_event(struct

Quoted 5 lines
tools/perf/util/session.c +0 -0
index f4478ce..335c3a9 100644
@@ -557,15 +557,63 @@ int perf_session_queue_event(struct

perf_session *s, union perf_event *event,

Quoted 29 lines
tools/perf/util/session.c +19 -3
 	return 0;
 }
 
-static void callchain__printf(struct perf_sample *sample)
+static void callchain__printf(struct perf_evsel *evsel,
+			      struct perf_sample *sample)
 {
 	unsigned int i;
+	struct ip_callchain *callchain = sample->callchain;
+	bool lbr = has_branch_callstack(evsel);
 
-	printf("... chain: nr:%" PRIu64 "\n", sample->callchain->nr);
+	if (lbr) {
+		struct branch_stack *lbr_stack = sample->branch_stack;
+		u64 kernel_callchain_nr = callchain->nr;
 
-	for (i = 0; i < sample->callchain->nr; i++)
+		for (i = 0; i < kernel_callchain_nr; i++) {
+			if (callchain->ips[i] == PERF_CONTEXT_USER)
+				break;
+		}
+
+		if ((i != kernel_callchain_nr) && lbr_stack->nr) {
+			u64 total_nr;
+			/*
+			 * LBR callstack can only get user call chain,
+			 * i is kernel call chain number,
+			 * 1 is PERF_CONTEXT_USER.
+			 *

tools/perf/util/session.c int perf_session_queue_event(struct perf_session *s, union perf_event *event,

Quoted 10 lines
tools/perf/util/session.c +10 -0
+			 * The user call chain is stored in LBR registers.
+			 * LBR are pair registers. The caller is stored
+			 * in "from" register, while the callee is stored
+			 * in "to" register.
+			 * For example, there is a call stack
+			 * "A"->"B"->"C"->"D".
+			 * The LBR registers will recorde like
+			 * "C"->"D", "B"->"C", "A"->"B".
+			 * So only the first "to" register and all "from"
+			 * registers are needed to construct the whole

stack.

Quoted 6 lines

tools/perf/util/session.c int perf_session_queue_event(struct · line 593

Quoted 1 line
tools/perf/util/session.c +1 -0
+			 */

Andi is using some sanity checks:
http://marc.info/?l=linux-kernel&m=141584447819894&w=2
I guess this could be applied in here, once his patch gets in.

Quoted 8 lines

Are you suggesting me to remove the comments,
or rebase the whole patch to Andi's patch once it's merged?

The branch history in Andi's patch is different as the call stack, although they are both from LBR.
Andi's branch history recording branch records for taken branches, interrupts, and exceptions.
While the LBR call stack records for the call stack.

Right. And branch history can overlap with normal callchains so additional check in there is to remove duplication. While LBR call stack is separated to user only so there should be no overlap.

Thanks,
Namhyung

#14 lore raw

Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

On Tue, Nov 18, 2014 at 03:13:50PM +0900, Namhyung Kim wrote:

SNIP

Quoted 25 lines
Quoted 16 lines
Quoted 9 lines

tools/perf/util/session.c int perf_session_queue_event(struct perf_session *s, union perf_event *event,

Quoted 8 lines
tools/perf/util/session.c +8 -0
+			 * in "from" register, while the callee is stored
+			 * in "to" register.
+			 * For example, there is a call stack
+			 * "A"->"B"->"C"->"D".
+			 * The LBR registers will recorde like
+			 * "C"->"D", "B"->"C", "A"->"B".
+			 * So only the first "to" register and all "from"
+			 * registers are needed to construct the whole

stack.

Quoted 6 lines

Review

Quoted 1 line
(quoted diff) +1 -0
+			 */

Andi is using some sanity checks:
http://marc.info/?l=linux-kernel&m=141584447819894&w=2
I guess this could be applied in here, once his patch gets in.

Quoted 8 lines

Are you suggesting me to remove the comments,
or rebase the whole patch to Andi's patch once it's merged?

The branch history in Andi's patch is different as the call stack, although they are both from LBR.
Andi's branch history recording branch records for taken branches, interrupts, and exceptions.
While the LBR call stack records for the call stack.

Quoted 3 lines

Right. And branch history can overlap with normal callchains so additional check in there is to remove duplication. While LBR call stack is separated to user only so there should be no overlap.

hum, it seemed to me like the remove_loops function could be used for this one as well.. but anyway I meant that this can be introduced later after Andi's change gets in

thanks,
jirka

author
#15 lore raw

RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

Quoted 34 lines · Namhyung Kim

SNIP

Quoted 1 line
Quoted 1 line
Quoted 1 line

Review

Quoted 1 line
(quoted diff) +1 -0
+			 * in "from" register, while the callee is

stored

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

tools/perf/util/session.c int perf_session_queue_event(struct perf_session *s, union perf_event *event,

Quoted 6 lines
tools/perf/util/session.c +6 -0
+			 * in "to" register.
+			 * For example, there is a call stack
+			 * "A"->"B"->"C"->"D".
+			 * The LBR registers will recorde like
+			 * "C"->"D", "B"->"C", "A"->"B".
+			 * So only the first "to" register and all

"from"

Quoted 1 line
Quoted 1 line
Quoted 1 line

Review

Quoted 1 line
(quoted diff) +1 -0
+			 * registers are needed to construct the

whole

Quoted 17 lines
Quoted 16 lines
Quoted 7 lines

stack.

Review

Quoted 1 line
(quoted diff) +1 -0
+			 */

Andi is using some sanity checks:
http://marc.info/?l=linux-kernel&m=141584447819894&w=2
I guess this could be applied in here, once his patch gets in.

Are you suggesting me to remove the comments, or rebase the whole patch to Andi's patch once it's merged?

The branch history in Andi's patch is different as the call stack, although they are both from LBR.
Andi's branch history recording branch records for taken branches, interrupts, and exceptions.
While the LBR call stack records for the call stack.

Quoted 7 lines
Quoted 3 lines

Right. And branch history can overlap with normal callchains so additional check in there is to remove duplication. While LBR call stack is separated to user only so there should be no overlap.

hum, it seemed to me like the remove_loops function could be used for this one as well.. but anyway I meant that this can be introduced later after Andi's change gets in

I see. I will apply Andi's remove_loops.

Thanks,
Kan

Quoted 3 lines

thanks,
jirka

author
#16 lore raw

RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

Quoted 25 lines
Quoted 12 lines

whole

Quoted 9 lines
Quoted 9 lines
Quoted 7 lines

stack.

Review

Quoted 1 line
(quoted diff) +1 -0
+			 */

Andi is using some sanity checks:
http://marc.info/?l=linux-kernel&m=141584447819894&w=2
I guess this could be applied in here, once his patch gets in.

Are you suggesting me to remove the comments, or rebase the

whole

Quoted 12 lines
Quoted 11 lines
Quoted 7 lines

patch to Andi's patch once it's merged?

The branch history in Andi's patch is different as the call stack, although they are both from LBR.
Andi's branch history recording branch records for taken branches, interrupts, and exceptions.
While the LBR call stack records for the call stack.

Right. And branch history can overlap with normal callchains so additional check in there is to remove duplication. While LBR call stack is separated to user only so there should be no overlap.

Quoted 6 lines
Quoted 3 lines

hum, it seemed to me like the remove_loops function could be used for this one as well.. but anyway I meant that this can be introduced later after Andi's change gets in

I see. I will apply Andi's remove_loops.

As Namhyung said, there is no overlap for LBR call stack. The user callchain is not a mix. It's from either LBR or FP. so remove_loops doesn't fit to the LBR call stack.
Sorry for the confusion from my last reply.

Thanks,
Kan

maintainer
#17 lore raw

Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

Hi Kan,

Quoted 27 lines · Kan Liang
Quoted 27 lines
Quoted 10 lines

whole

Quoted 9 lines
Quoted 9 lines
Quoted 7 lines

stack.

Review

Quoted 1 line
(quoted diff) +1 -0
+			 */

Andi is using some sanity checks:
http://marc.info/?l=linux-kernel&m=141584447819894&w=2
I guess this could be applied in here, once his patch gets in.

Are you suggesting me to remove the comments, or rebase the

whole

Quoted 15 lines
Quoted 11 lines
Quoted 7 lines

patch to Andi's patch once it's merged?

The branch history in Andi's patch is different as the call stack, although they are both from LBR.
Andi's branch history recording branch records for taken branches, interrupts, and exceptions.
While the LBR call stack records for the call stack.

Right. And branch history can overlap with normal callchains so additional check in there is to remove duplication. While LBR call stack is separated to user only so there should be no overlap.

hum, it seemed to me like the remove_loops function could be used for this one as well.. but anyway I meant that this can be introduced later after Andi's change gets in

Quoted 6 lines
Quoted 2 lines

I see. I will apply Andi's remove_loops.

As Namhyung said, there is no overlap for LBR call stack. The user callchain is not a mix. It's from either LBR or FP. so remove_loops doesn't fit to the LBR call stack.

IIUC the remove_loops() is for eliminating repeated branches in a loop in a single function (and it only checks LBRs not FP callchain). It's needed since branch history saves all branches, but not needed for LBR callstack as it saves CALLs only.

Thanks,
Namhyung

#18 lore raw

Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

In reply to [email protected]

On Fri, Nov 14, 2014 at 08:44:12AM -0500, [email protected] wrote:

SNIP

Quoted 42 lines
tools/perf/util/machine.c +42 -0
+	if (lbr) {
+		for (i = 0; i < chain_nr; i++) {
+			if (chain->ips[i] == PERF_CONTEXT_USER)
+				break;
+		}
+
+		/* LBR only affects the user callchain */
+		if (i != chain_nr) {
+			struct branch_stack *lbr_stack = sample->branch_stack;
+			int lbr_nr = lbr_stack->nr;
+			/*
+			 * LBR callstack can only get user call chain.
+			 * The mix_chain_nr is kernel call chain
+			 * number plus LBR user call chain number.
+			 * i is kernel call chain number,
+			 * 1 is PERF_CONTEXT_USER,
+			 * lbr_nr + 1 is the user call chain number.
+			 * For details, please refer to the comments
+			 * in callchain__printf
+			 */
+			int mix_chain_nr = i + 1 + lbr_nr + 1;
+
+			if (mix_chain_nr > PERF_MAX_STACK_DEPTH) {
+				pr_warning("corrupted callchain. skipping...\n");
+				return 0;
+			}
+
+			for (j = 0; j < mix_chain_nr; j++) {
+				if (callchain_param.order == ORDER_CALLEE) {
+					if (j < i + 1)
+						ip = chain->ips[j];
+					else if (j > i + 1)
+						ip = lbr_stack->entries[j - i - 2].from;
+					else
+						ip = lbr_stack->entries[0].to;
+				} else {
+					if (j < lbr_nr)
+						ip = lbr_stack->entries[lbr_nr - j - 1].from;
+					else if (j > lbr_nr)
+						ip = chain->ips[i + 1 - (j - lbr_nr)];
+					else
+						ip = lbr_stack->entries[0].to;

tools/perf/util/machine.c static int thread__resolve_callchain_sample(struct thread *thread, · line 1527

Quoted 10 lines
tools/perf/util/machine.c +10 -0
+				}
+
+				err = add_callchain_ip(thread, parent, root_al,
+						       cpumode, ip);
+				if (err)
+					goto exit;
+			}
+			return 0;
+		}
+	}

also could you please move whole block above into separated function? Andi has another change for this function and it's becoming really big.

other then that I think it's ok IMO and I could ack next version

thanks,
jirka

maintainer
#19 lore raw

Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

Quoted 51 lines · Jiri Olsa

On Fri, Nov 14, 2014 at 08:44:12AM -0500, [email protected] wrote:

SNIP

Quoted 47 lines
tools/perf/util/machine.c +47 -0
+	if (lbr) {
+		for (i = 0; i < chain_nr; i++) {
+			if (chain->ips[i] == PERF_CONTEXT_USER)
+				break;
+		}
+
+		/* LBR only affects the user callchain */
+		if (i != chain_nr) {
+			struct branch_stack *lbr_stack = sample->branch_stack;
+			int lbr_nr = lbr_stack->nr;
+			/*
+			 * LBR callstack can only get user call chain.
+			 * The mix_chain_nr is kernel call chain
+			 * number plus LBR user call chain number.
+			 * i is kernel call chain number,
+			 * 1 is PERF_CONTEXT_USER,
+			 * lbr_nr + 1 is the user call chain number.
+			 * For details, please refer to the comments
+			 * in callchain__printf
+			 */
+			int mix_chain_nr = i + 1 + lbr_nr + 1;
+
+			if (mix_chain_nr > PERF_MAX_STACK_DEPTH) {
+				pr_warning("corrupted callchain. skipping...\n");
+				return 0;
+			}
+
+			for (j = 0; j < mix_chain_nr; j++) {
+				if (callchain_param.order == ORDER_CALLEE) {
+					if (j < i + 1)
+						ip = chain->ips[j];
+					else if (j > i + 1)
+						ip = lbr_stack->entries[j - i - 2].from;
+					else
+						ip = lbr_stack->entries[0].to;
+				} else {
+					if (j < lbr_nr)
+						ip = lbr_stack->entries[lbr_nr - j - 1].from;
+					else if (j > lbr_nr)
+						ip = chain->ips[i + 1 - (j - lbr_nr)];
+					else
+						ip = lbr_stack->entries[0].to;
+				}
+
+				err = add_callchain_ip(thread, parent, root_al,
+						       cpumode, ip);
+				if (err)
Quoted 8 lines

tools/perf/util/machine.c static int thread__resolve_callchain_sample(struct thread *thread, · line 1527

Quoted 5 lines
tools/perf/util/machine.c +5 -0
+					goto exit;
+			}
+			return 0;
+		}
+	}

also could you please move whole block above into separated function? Andi has another change for this function and it's becoming really big.

Agreed.

Thanks,
Namhyung

maintainer
#20 lore raw

Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

In reply to [email protected]
Quoted 10 lines · kan liang
tools/perf/util/machine.c +10 -0
+		/* LBR only affects the user callchain */
+		if (i != chain_nr) {
+			struct branch_stack *lbr_stack = sample->branch_stack;
+			int lbr_nr = lbr_stack->nr;
+			/*
+			 * LBR callstack can only get user call chain.
+			 * The mix_chain_nr is kernel call chain
+			 * number plus LBR user call chain number.
+			 * i is kernel call chain number,
+			 * 1 is PERF_CONTEXT_USER,

tools/perf/util/machine.c static int thread__resolve_callchain_sample(struct thread *thread, · line 1501

Quoted 10 lines
tools/perf/util/machine.c +10 -0
+			 * lbr_nr + 1 is the user call chain number.
+			 * For details, please refer to the comments
+			 * in callchain__printf
+			 */
+			int mix_chain_nr = i + 1 + lbr_nr + 1;
+
+			if (mix_chain_nr > PERF_MAX_STACK_DEPTH) {
+				pr_warning("corrupted callchain. skipping...\n");
+				return 0;
+			}

I'm not sure whether this is really a corrupted callchain. If a single chain is greater than the max it should be corrupted, but we're now summing up with other values..

What about checking callchain_nr and lbr_nr separately or mix_nr with 2*max?

Thanks,
Namhyung

author
#21 lore raw

RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

Quoted 30 lines · kan liang
Quoted 9 lines
tools/perf/util/machine.c +3 -0
+		/* LBR only affects the user callchain */
+		if (i != chain_nr) {
+			struct branch_stack *lbr_stack = sample-

branch_stack;

tools/perf/util/machine.c +5 -0
+			int lbr_nr = lbr_stack->nr;
+			/*
+			 * LBR callstack can only get user call chain.
+			 * The mix_chain_nr is kernel call chain
+			 * number plus LBR user call chain number.

tools/perf/util/machine.c static int thread__resolve_callchain_sample(struct thread *thread,

Quoted 10 lines
tools/perf/util/machine.c +10 -0
+			 * i is kernel call chain number,
+			 * 1 is PERF_CONTEXT_USER,
+			 * lbr_nr + 1 is the user call chain number.
+			 * For details, please refer to the comments
+			 * in callchain__printf
+			 */
+			int mix_chain_nr = i + 1 + lbr_nr + 1;
+
+			if (mix_chain_nr > PERF_MAX_STACK_DEPTH) {
+				pr_warning("corrupted callchain.

skipping...\n");

Review

Quoted 2 lines
(quoted diff) +2 -0
+				return 0;
+			}

I'm not sure whether this is really a corrupted callchain. If a single chain is greater than the max it should be corrupted, but we're now summing up with other values..

Quoted 2 lines

What about checking callchain_nr and lbr_nr separately or mix_nr with 2*max?

The lbr_nr max is 16. I will change it to max+16.

Thanks,
Kan

Quoted 3 lines

Thanks,
Namhyung

maintainer
#22 lore raw

Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

Quoted 28 lines · Kan Liang
Quoted 28 lines · kan liang
Quoted 9 lines
tools/perf/util/machine.c +3 -0
+		/* LBR only affects the user callchain */
+		if (i != chain_nr) {
+			struct branch_stack *lbr_stack = sample-

branch_stack;

tools/perf/util/machine.c +5 -0
+			int lbr_nr = lbr_stack->nr;
+			/*
+			 * LBR callstack can only get user call chain.
+			 * The mix_chain_nr is kernel call chain
+			 * number plus LBR user call chain number.

tools/perf/util/machine.c static int thread__resolve_callchain_sample(struct thread *thread,

Quoted 10 lines
tools/perf/util/machine.c +10 -0
+			 * i is kernel call chain number,
+			 * 1 is PERF_CONTEXT_USER,
+			 * lbr_nr + 1 is the user call chain number.
+			 * For details, please refer to the comments
+			 * in callchain__printf
+			 */
+			int mix_chain_nr = i + 1 + lbr_nr + 1;
+
+			if (mix_chain_nr > PERF_MAX_STACK_DEPTH) {
+				pr_warning("corrupted callchain.

skipping...\n");

Review

Quoted 2 lines
(quoted diff) +2 -0
+				return 0;
+			}

I'm not sure whether this is really a corrupted callchain. If a single chain is greater than the max it should be corrupted, but we're now summing up with other values..

Quoted 4 lines
Quoted 2 lines

What about checking callchain_nr and lbr_nr separately or mix_nr with 2*max?

The lbr_nr max is 16. I will change it to max+16.

Shouldn't it be 18 (16 + 1 + 1) at least?

Thanks,
Namhyung

author
#23 lore raw

RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

Quoted 32 lines · Kan Liang
Quoted 28 lines
Quoted 28 lines · kan liang
Quoted 9 lines
tools/perf/util/machine.c +3 -0
+		/* LBR only affects the user callchain */
+		if (i != chain_nr) {
+			struct branch_stack *lbr_stack = sample-

branch_stack;

tools/perf/util/machine.c +5 -0
+			int lbr_nr = lbr_stack->nr;
+			/*
+			 * LBR callstack can only get user call chain.
+			 * The mix_chain_nr is kernel call chain
+			 * number plus LBR user call chain number.

tools/perf/util/machine.c static int thread__resolve_callchain_sample(struct thread *thread,

Quoted 10 lines
tools/perf/util/machine.c +10 -0
+			 * i is kernel call chain number,
+			 * 1 is PERF_CONTEXT_USER,
+			 * lbr_nr + 1 is the user call chain number.
+			 * For details, please refer to the comments
+			 * in callchain__printf
+			 */
+			int mix_chain_nr = i + 1 + lbr_nr + 1;
+
+			if (mix_chain_nr > PERF_MAX_STACK_DEPTH) {
+				pr_warning("corrupted callchain.

skipping...\n");

Review

Quoted 2 lines
(quoted diff) +2 -0
+				return 0;
+			}

I'm not sure whether this is really a corrupted callchain. If a single chain is greater than the max it should be corrupted, but we're now summing up with other values..

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

What about checking callchain_nr and lbr_nr separately or mix_nr with 2*max?

The lbr_nr max is 16. I will change it to max+16.

Shouldn't it be 18 (16 + 1 + 1) at least?

In the new patch, I use PERF_MAX_BRANCH_DEPTH from Andi's patch. It's
127. It should be enough.

+ if (mix_chain_nr > PERF_MAX_STACK_DEPTH +
PERF_MAX_BRANCH_DEPTH) {
+ pr_warning("corrupted callchain. skipping...\n");
+ return 0;
+ }

Thanks,
Kan

Quoted 3 lines

Thanks,
Namhyung

#24 lore raw
In reply to [email protected]
Quoted 13 lines · On Fri, Nov 14, 2014 at 08:44:09AM -0500, [email protected] wrote:

From: Kan Liang <[email protected]>

This is the user space patch for Haswell LBR call stack support. For many profiling tasks we need the callgraph. For example we often need to see the caller of a lock or the caller of a memcpy or other library function to actually tune the program. Frame pointer unwinding is efficient and works well. But frame pointers are off by default on 64bit code (and on modern 32bit gccs), so there are many binaries around that do not use frame pointers. Profiling unchanged production code is very useful in practice. On some CPUs frame pointer also has a high cost. Dwarf2 unwinding also does not always work and is extremely slow (upto 20% overhead).

Quoted 10 lines

Haswell has a new feature that utilizes the existing Last Branch Record facility to record call chains. When the feature is enabled, function call will be collected as normal, but as return instructions are executed the last captured branch record is popped from the on-chip LBR registers. The LBR call stack facility provides an alternative to get callgraph. It has some limitations too, but should work in most cases and is significantly faster than dwarf. Frame pointer unwinding is still the best default, but LBR call stack is a good alternative when nothing else works.

---

Quoted 124 lines

A new call chain recording option "lbr" is introduced into perf tool for LBR call stack. The user can use --call-graph lbr to get the call stack information from hardware.

When profiling bc(1) on Fedora 19:
echo 'scale=2000; 4*a(1)' > cmd; perf record --call-graph lbr bc -l < cmd If enabling LBR, perf report output looks like:

    50.36%       bc  bc                 [.] bc_divide
                 |
                 --- bc_divide
                     execute
                     run_code
                     yyparse
                     main
                     __libc_start_main
                     _start
    33.66%       bc  bc                 [.] _one_mult
                 |
                 --- _one_mult
                     bc_divide
                     execute
                     run_code
                     yyparse
                     main
                     __libc_start_main
                     _start
     7.62%       bc  bc                 [.] _bc_do_add
                 |
                 --- _bc_do_add
                    |
                    |--99.89%-- 0x2000186a8
                     --0.11%-- [...]
     6.83%       bc  bc                 [.] _bc_do_sub
                 |
                 --- _bc_do_sub
                    |
                    |--99.94%-- bc_add
                    |          execute
                    |          run_code
                    |          yyparse
                    |          main
                    |          __libc_start_main
                    |          _start
                     --0.06%-- [...]
     0.46%       bc  libc-2.17.so       [.] __memset_sse2
                 |
                 --- __memset_sse2
                    |
                    |--54.13%-- bc_new_num
                    |          |
                    |          |--51.00%-- bc_divide
                    |          |          execute
                    |          |          run_code
                    |          |          yyparse
                    |          |          main
                    |          |          __libc_start_main
                    |          |          _start
                    |          |
                    |          |--30.46%-- _bc_do_sub
                    |          |          bc_add
                    |          |          execute
                    |          |          run_code
                    |          |          yyparse
                    |          |          main
                    |          |          __libc_start_main
                    |          |          _start
                    |          |
                    |           --18.55%-- _bc_do_add
                    |                     bc_add
                    |                     execute
                    |                     run_code
                    |                     yyparse
                    |                     main
                    |                     __libc_start_main
                    |                     _start
                    |
                     --45.87%-- bc_divide
                               execute
                               run_code
                               yyparse
                               main
                               __libc_start_main
                               _start
If using FP, perf report output looks like:
echo 'scale=2000; 4*a(1)' > cmd; perf record --call-graph fp bc -l < cmd
    50.49%       bc  bc                 [.] bc_divide
                 |
                 --- bc_divide
    33.57%       bc  bc                 [.] _one_mult
                 |
                 --- _one_mult
     7.61%       bc  bc                 [.] _bc_do_add
                 |
                 --- _bc_do_add
                     0x2000186a8
     6.88%       bc  bc                 [.] _bc_do_sub
                 |
                 --- _bc_do_sub
     0.42%       bc  libc-2.17.so       [.] __memcpy_ssse3_back
                 |
                 --- __memcpy_ssse3_back

If using LBR, perf report -D output looks like:
11739295893248 0x4d0 [0xe0]: PERF_RECORD_SAMPLE(IP, 0x2): 10505/10505:
0x40054d period: 39255 addr: 0
... LBR call chain: nr:7

.....  0: fffffffffffffe00
.....  1: 0000000000400540
.....  2: 0000000000400587
.....  3: 00000000004005b3
.....  4: 00000000004005ef
.....  5: 0000003d1cc21b43
.....  6: 0000000000400474
... FP chain: nr:6
.....  0: fffffffffffffe00
.....  1: 000000000040054d
.....  2: 000000000040058c
.....  3: 00000000004005b8
.....  4: 00000000004005f4
.....  5: 0000003d1cc21b45

... thread: a.out:10505
...... dso: /home/lk/a.out

Quoted 7 lines

The LBR call stack has following known limitations
- Zero length calls are not filtered out by hardware
- Exception handing such as setjmp/longjmp will have calls/returns not match
- Pushing different return address onto the stack will have calls/returns not match
- If callstack is deeper than the LBR, only the last entries are captured

---

also could you please add all above ^^^ as an additional text for patch 3/3 changelog (perf tools: Construct LBR call chain)?

looks too nice to lose it ;-)

thanks,
jirka