Skip to contentSkip to conversation

callchain fixes and cleanups

RFC PATCH In review Frederic Weisbecker 5 participants 20 messages Aug 16, 2010 – Aug 18, 2010

Subsystems (7)
Arm (32-bit) x86 PowerPC perf events +3 more subsystems
lore
Share & tools
lore

Frederic Weisbecker

author
#0 lore raw

v3 cover callchain fixes and cleanups

Hi,

This set factorizes a lot of callchain code and fixes concurrent callchain buffers accesses.

The previous set has revealed we can't decently use the per cpu allocation api for memory that can be accessed from NMI. Hence this temporary uses a manual per cpu allocation until that gets fixed up.

Built-tested on PowerPc, Arm and SuperH (can someone point me to a ready to use sparc cross compiler, like those that can be found there for other archs: http://www.codesourcery.com/sgpp/lite_edition.html ?)

And runtime tested on x86-64 withiut troubles.

You can test it by pulling that branch:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
perf/callchains

If you have no comments, I'll propose it to -tip in a few days.

Thanks,
Frederic
---

Frederic Weisbecker (6):
perf: Drop unappropriate tests on arch callchains
perf: Generalize callchain_store()
perf: Generalize some arch callchain code
perf: Factorize callchain context handling
perf: Fix race in callchains
perf: Humanize the number of contexts

 arch/arm/kernel/perf_event.c         |   62 +---------
 arch/powerpc/kernel/perf_callchain.c |   86 ++++---------
 arch/sh/kernel/perf_callchain.c      |   50 +-------
 arch/sparc/kernel/perf_event.c       |   69 ++++-------
 arch/x86/kernel/cpu/perf_event.c     |   82 +++----------
 include/linux/perf_event.h           |   30 ++++-
 kernel/perf_event.c                  |  226 +++++++++++++++++++++++++++++-----
 7 files changed, 295 insertions(+), 310 deletions(-)
author
#1 lore raw

RFC PATCH 1/6 perf: Drop unappropriate tests on arch callchains

Drop the TASK_RUNNING test on user tasks for callchains as this check doesn't seem to make any sense.

Also remove the tests for !current that is not supposed to happen and current->pid as this should be handled at the generic level, with exclude_idle attribute.

Signed-off-by Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc David Miller <[email protected]>
Cc Paul Mundt <[email protected]>
Cc Will Deacon <[email protected]>
Cc Borislav Petkov <[email protected]>
---
 arch/arm/kernel/perf_event.c     |    6 ------
 arch/sh/kernel/perf_callchain.c  |    3 ---
 arch/x86/kernel/cpu/perf_event.c |    3 ---
 3 files changed, 0 insertions(+), 12 deletions(-)
arch/arm/kernel/perf_event.c +0 -6
index 417c392..fdcb0be 100644
@@ -3107,12 +3107,6 @@ perf_do_callchain(struct pt_regs *regs,
 
 	is_user = user_mode(regs);
 
-	if (!current || !current->pid)
-		return;
-
-	if (is_user && current->state != TASK_RUNNING)
-		return;
-
 	if (!is_user)
 		perf_callchain_kernel(regs, entry);
arch/sh/kernel/perf_callchain.c +0 -3
index a9dd3ab..1d6dbce 100644
@@ -68,9 +68,6 @@ perf_do_callchain(struct pt_regs *regs, struct perf_callchain_entry *entry)
 
 	is_user = user_mode(regs);
 
-	if (is_user && current->state != TASK_RUNNING)
-		return;
-
 	/*
 	 * Only the kernel side is implemented for now.
 	 */
arch/x86/kernel/cpu/perf_event.c +0 -3
index f2da20f..4a4d191 100644
@@ -1703,9 +1703,6 @@ perf_do_callchain(struct pt_regs *regs, struct perf_callchain_entry *entry)
 
 	is_user = user_mode(regs);
 
-	if (is_user && current->state != TASK_RUNNING)
-		return;
-
 	if (!is_user)
 		perf_callchain_kernel(regs, entry);
Signature · 1 line
1.6.2.3
author
#2 lore raw

RFC PATCH 2/6 perf: Generalize callchain_store()

In reply to Frederic Weisbecker

callchain_store() is the same on every archs, inline it in perf_event.h and rename it to perf_callchain_store() to avoid any collision.

This removes repetitive code.

Signed-off-by Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc David Miller <[email protected]>
Cc Paul Mundt <[email protected]>
Cc Will Deacon <[email protected]>
Cc Borislav Petkov <[email protected]>
---
 arch/arm/kernel/perf_event.c         |   15 +++---------
 arch/powerpc/kernel/perf_callchain.c |   40 ++++++++++++----------------------
 arch/sh/kernel/perf_callchain.c      |   11 ++------
 arch/sparc/kernel/perf_event.c       |   26 ++++++++-------------
 arch/x86/kernel/cpu/perf_event.c     |   20 ++++++-----------
 include/linux/perf_event.h           |    7 ++++++
 6 files changed, 45 insertions(+), 74 deletions(-)
arch/arm/kernel/perf_event.c +4 -11
index fdcb0be..a07c3b1 100644
@@ -3001,13 +3001,6 @@ arch_initcall(init_hw_perf_events);
 /*
  * Callchain handling code.
  */
-static inline void
-callchain_store(struct perf_callchain_entry *entry,
-		u64 ip)
-{
-	if (entry->nr < PERF_MAX_STACK_DEPTH)
-		entry->ip[entry->nr++] = ip;
-}
 
 /*
  * The registers we're interested in are at the end of the variable

@@ -3039,7 +3032,7 @@ user_backtrace(struct frame_tail *tail,
 	if (__copy_from_user_inatomic(&buftail, tail, sizeof(buftail)))
 		return NULL;
 
-	callchain_store(entry, buftail.lr);
+	perf_callchain_store(entry, buftail.lr);
 
 	/*
 	 * Frame pointers should strictly progress back up the stack

@@ -3057,7 +3050,7 @@ perf_callchain_user(struct pt_regs *regs,
 {
 	struct frame_tail *tail;
 
-	callchain_store(entry, PERF_CONTEXT_USER);
+	perf_callchain_store(entry, PERF_CONTEXT_USER);
 
 	if (!user_mode(regs))
 		regs = task_pt_regs(current);

@@ -3078,7 +3071,7 @@ callchain_trace(struct stackframe *fr,
 		void *data)
 {
 	struct perf_callchain_entry *entry = data;
-	callchain_store(entry, fr->pc);
+	perf_callchain_store(entry, fr->pc);
 	return 0;
 }
 

@@ -3088,7 +3081,7 @@ perf_callchain_kernel(struct pt_regs *regs,
 {
 	struct stackframe fr;
 
-	callchain_store(entry, PERF_CONTEXT_KERNEL);
+	perf_callchain_store(entry, PERF_CONTEXT_KERNEL);
 	fr.fp = regs->ARM_fp;
 	fr.sp = regs->ARM_sp;
 	fr.lr = regs->ARM_lr;
arch/powerpc/kernel/perf_callchain.c +14 -26
index 95ad9da..a286c2e 100644
@@ -23,18 +23,6 @@
 #include "ppc32.h"
 #endif
 
-/*
- * Store another value in a callchain_entry.
- */
-static inline void callchain_store(struct perf_callchain_entry *entry, u64 ip)
-{
-	unsigned int nr = entry->nr;
-
-	if (nr < PERF_MAX_STACK_DEPTH) {
-		entry->ip[nr] = ip;
-		entry->nr = nr + 1;
-	}
-}
 
 /*
  * Is sp valid as the address of the next kernel stack frame after prev_sp?

@@ -69,8 +57,8 @@ static void perf_callchain_kernel(struct pt_regs *regs,
 
 	lr = regs->link;
 	sp = regs->gpr[1];
-	callchain_store(entry, PERF_CONTEXT_KERNEL);
-	callchain_store(entry, regs->nip);
+	perf_callchain_store(entry, PERF_CONTEXT_KERNEL);
+	perf_callchain_store(entry, regs->nip);
 
 	if (!validate_sp(sp, current, STACK_FRAME_OVERHEAD))
 		return;

@@ -89,7 +77,7 @@ static void perf_callchain_kernel(struct pt_regs *regs,
 			next_ip = regs->nip;
 			lr = regs->link;
 			level = 0;
-			callchain_store(entry, PERF_CONTEXT_KERNEL);
+			perf_callchain_store(entry, PERF_CONTEXT_KERNEL);
 
 		} else {
 			if (level == 0)

@@ -111,7 +99,7 @@ static void perf_callchain_kernel(struct pt_regs *regs,
 			++level;
 		}
 
-		callchain_store(entry, next_ip);
+		perf_callchain_store(entry, next_ip);
 		if (!valid_next_sp(next_sp, sp))
 			return;
 		sp = next_sp;

@@ -246,8 +234,8 @@ static void perf_callchain_user_64(struct pt_regs *regs,
 	next_ip = regs->nip;
 	lr = regs->link;
 	sp = regs->gpr[1];
-	callchain_store(entry, PERF_CONTEXT_USER);
-	callchain_store(entry, next_ip);
+	perf_callchain_store(entry, PERF_CONTEXT_USER);
+	perf_callchain_store(entry, next_ip);
 
 	for (;;) {
 		fp = (unsigned long __user *) sp;

@@ -276,14 +264,14 @@ static void perf_callchain_user_64(struct pt_regs *regs,
 			    read_user_stack_64(&uregs[PT_R1], &sp))
 				return;
 			level = 0;
-			callchain_store(entry, PERF_CONTEXT_USER);
-			callchain_store(entry, next_ip);
+			perf_callchain_store(entry, PERF_CONTEXT_USER);
+			perf_callchain_store(entry, next_ip);
 			continue;
 		}
 
 		if (level == 0)
 			next_ip = lr;
-		callchain_store(entry, next_ip);
+		perf_callchain_store(entry, next_ip);
 		++level;
 		sp = next_sp;
 	}

@@ -447,8 +435,8 @@ static void perf_callchain_user_32(struct pt_regs *regs,
 	next_ip = regs->nip;
 	lr = regs->link;
 	sp = regs->gpr[1];
-	callchain_store(entry, PERF_CONTEXT_USER);
-	callchain_store(entry, next_ip);
+	perf_callchain_store(entry, PERF_CONTEXT_USER);
+	perf_callchain_store(entry, next_ip);
 
 	while (entry->nr < PERF_MAX_STACK_DEPTH) {
 		fp = (unsigned int __user *) (unsigned long) sp;

@@ -470,14 +458,14 @@ static void perf_callchain_user_32(struct pt_regs *regs,
 			    read_user_stack_32(&uregs[PT_R1], &sp))
 				return;
 			level = 0;
-			callchain_store(entry, PERF_CONTEXT_USER);
-			callchain_store(entry, next_ip);
+			perf_callchain_store(entry, PERF_CONTEXT_USER);
+			perf_callchain_store(entry, next_ip);
 			continue;
 		}
 
 		if (level == 0)
 			next_ip = lr;
-		callchain_store(entry, next_ip);
+		perf_callchain_store(entry, next_ip);
 		++level;
 		sp = next_sp;
 	}
arch/sh/kernel/perf_callchain.c +3 -8
index 1d6dbce..00143f3 100644
@@ -14,11 +14,6 @@
 #include <asm/unwinder.h>
 #include <asm/ptrace.h>
 
-static inline void callchain_store(struct perf_callchain_entry *entry, u64 ip)
-{
-	if (entry->nr < PERF_MAX_STACK_DEPTH)
-		entry->ip[entry->nr++] = ip;
-}
 
 static void callchain_warning(void *data, char *msg)
 {

@@ -39,7 +34,7 @@ static void callchain_address(void *data, unsigned long addr, int reliable)
 	struct perf_callchain_entry *entry = data;
 
 	if (reliable)
-		callchain_store(entry, addr);
+		perf_callchain_store(entry, addr);
 }
 
 static const struct stacktrace_ops callchain_ops = {

@@ -52,8 +47,8 @@ static const struct stacktrace_ops callchain_ops = {
 static void
 perf_callchain_kernel(struct pt_regs *regs, struct perf_callchain_entry *entry)
 {
-	callchain_store(entry, PERF_CONTEXT_KERNEL);
-	callchain_store(entry, regs->pc);
+	perf_callchain_store(entry, PERF_CONTEXT_KERNEL);
+	perf_callchain_store(entry, regs->pc);
 
 	unwind_stack(NULL, regs, NULL, &callchain_ops, entry);
 }
arch/sparc/kernel/perf_event.c +10 -16
index 357ced3..2a95a90 100644
@@ -1283,12 +1283,6 @@ void __init init_hw_perf_events(void)
 	register_die_notifier(&perf_event_nmi_notifier);
 }
 
-static inline void callchain_store(struct perf_callchain_entry *entry, u64 ip)
-{
-	if (entry->nr < PERF_MAX_STACK_DEPTH)
-		entry->ip[entry->nr++] = ip;
-}
-
 static void perf_callchain_kernel(struct pt_regs *regs,
 				  struct perf_callchain_entry *entry)
 {

@@ -1297,8 +1291,8 @@ static void perf_callchain_kernel(struct pt_regs *regs,
 	int graph = 0;
 #endif
 
-	callchain_store(entry, PERF_CONTEXT_KERNEL);
-	callchain_store(entry, regs->tpc);
+	perf_callchain_store(entry, PERF_CONTEXT_KERNEL);
+	perf_callchain_store(entry, regs->tpc);
 
 	ksp = regs->u_regs[UREG_I6];
 	fp = ksp + STACK_BIAS;

@@ -1322,13 +1316,13 @@ static void perf_callchain_kernel(struct pt_regs *regs,
 			pc = sf->callers_pc;
 			fp = (unsigned long)sf->fp + STACK_BIAS;
 		}
-		callchain_store(entry, pc);
+		perf_callchain_store(entry, pc);
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 		if ((pc + 8UL) == (unsigned long) &return_to_handler) {
 			int index = current->curr_ret_stack;
 			if (current->ret_stack && index >= graph) {
 				pc = current->ret_stack[index - graph].ret;
-				callchain_store(entry, pc);
+				perf_callchain_store(entry, pc);
 				graph++;
 			}
 		}

@@ -1341,8 +1335,8 @@ static void perf_callchain_user_64(struct pt_regs *regs,
 {
 	unsigned long ufp;
 
-	callchain_store(entry, PERF_CONTEXT_USER);
-	callchain_store(entry, regs->tpc);
+	perf_callchain_store(entry, PERF_CONTEXT_USER);
+	perf_callchain_store(entry, regs->tpc);
 
 	ufp = regs->u_regs[UREG_I6] + STACK_BIAS;
 	do {

@@ -1355,7 +1349,7 @@ static void perf_callchain_user_64(struct pt_regs *regs,
 
 		pc = sf.callers_pc;
 		ufp = (unsigned long)sf.fp + STACK_BIAS;
-		callchain_store(entry, pc);
+		perf_callchain_store(entry, pc);
 	} while (entry->nr < PERF_MAX_STACK_DEPTH);
 }
 

@@ -1364,8 +1358,8 @@ static void perf_callchain_user_32(struct pt_regs *regs,
 {
 	unsigned long ufp;
 
-	callchain_store(entry, PERF_CONTEXT_USER);
-	callchain_store(entry, regs->tpc);
+	perf_callchain_store(entry, PERF_CONTEXT_USER);
+	perf_callchain_store(entry, regs->tpc);
 
 	ufp = regs->u_regs[UREG_I6] & 0xffffffffUL;
 	do {

@@ -1378,7 +1372,7 @@ static void perf_callchain_user_32(struct pt_regs *regs,
 
 		pc = sf.callers_pc;
 		ufp = (unsigned long)sf.fp;
-		callchain_store(entry, pc);
+		perf_callchain_store(entry, pc);
 	} while (entry->nr < PERF_MAX_STACK_DEPTH);
 }
arch/x86/kernel/cpu/perf_event.c +7 -13
index 4a4d191..8af28ca 100644
@@ -1571,12 +1571,6 @@ const struct pmu *hw_perf_event_init(struct perf_event *event)
  * callchain support
  */
 
-static inline
-void callchain_store(struct perf_callchain_entry *entry, u64 ip)
-{
-	if (entry->nr < PERF_MAX_STACK_DEPTH)
-		entry->ip[entry->nr++] = ip;
-}
 
 static DEFINE_PER_CPU(struct perf_callchain_entry, pmc_irq_entry);
 static DEFINE_PER_CPU(struct perf_callchain_entry, pmc_nmi_entry);

@@ -1602,7 +1596,7 @@ static void backtrace_address(void *data, unsigned long addr, int reliable)
 {
 	struct perf_callchain_entry *entry = data;
 
-	callchain_store(entry, addr);
+	perf_callchain_store(entry, addr);
 }
 
 static const struct stacktrace_ops backtrace_ops = {

@@ -1616,8 +1610,8 @@ static const struct stacktrace_ops backtrace_ops = {
 static void
 perf_callchain_kernel(struct pt_regs *regs, struct perf_callchain_entry *entry)
 {
-	callchain_store(entry, PERF_CONTEXT_KERNEL);
-	callchain_store(entry, regs->ip);
+	perf_callchain_store(entry, PERF_CONTEXT_KERNEL);
+	perf_callchain_store(entry, regs->ip);
 
 	dump_trace(NULL, regs, NULL, regs->bp, &backtrace_ops, entry);
 }

@@ -1646,7 +1640,7 @@ perf_callchain_user32(struct pt_regs *regs, struct perf_callchain_entry *entry)
 		if (fp < compat_ptr(regs->sp))
 			break;
 
-		callchain_store(entry, frame.return_address);
+		perf_callchain_store(entry, frame.return_address);
 		fp = compat_ptr(frame.next_frame);
 	}
 	return 1;

@@ -1670,8 +1664,8 @@ perf_callchain_user(struct pt_regs *regs, struct perf_callchain_entry *entry)
 
 	fp = (void __user *)regs->bp;
 
-	callchain_store(entry, PERF_CONTEXT_USER);
-	callchain_store(entry, regs->ip);
+	perf_callchain_store(entry, PERF_CONTEXT_USER);
+	perf_callchain_store(entry, regs->ip);
 
 	if (perf_callchain_user32(regs, entry))
 		return;

@@ -1688,7 +1682,7 @@ perf_callchain_user(struct pt_regs *regs, struct perf_callchain_entry *entry)
 		if ((unsigned long)fp < regs->sp)
 			break;
 
-		callchain_store(entry, frame.return_address);
+		perf_callchain_store(entry, frame.return_address);
 		fp = frame.next_frame;
 	}
 }
include/linux/perf_event.h +7 -0
index 937495c..3588804 100644
@@ -978,6 +978,13 @@ extern void perf_event_fork(struct task_struct *tsk);
 
 extern struct perf_callchain_entry *perf_callchain(struct pt_regs *regs);
 
+static inline void
+perf_callchain_store(struct perf_callchain_entry *entry, u64 ip)
+{
+	if (entry->nr < PERF_MAX_STACK_DEPTH)
+		entry->ip[entry->nr++] = ip;
+}
+
 extern int sysctl_perf_event_paranoid;
 extern int sysctl_perf_event_mlock;
 extern int sysctl_perf_event_sample_rate;
Signature · 1 line
1.6.2.3
Acked-by on v1 2/6
#3 lore raw

Re: [RFC PATCH 2/6] perf: Generalize callchain_store()

Quoted 5 lines · Frederic Weisbecker

callchain_store() is the same on every archs, inline it in perf_event.h and rename it to perf_callchain_store() to avoid any collision.

This removes repetitive code.

Acked-by Paul Mackerras <[email protected]>
author
#4 lore raw

RFC PATCH 3/6 perf: Generalize some arch callchain code

In reply to Frederic Weisbecker

- Most archs use one callchain buffer per cpu, except x86 that needs to deal with NMIs. Provide a default perf_callchain_buffer() implementation that x86 overrides.

- Centralize all the kernel/user regs handling and invoke new arch handlers from there: perf_callchain_user() / perf_callchain_kernel() That avoid all the user_mode(), current->mm checks and so...

- Invert some parameters in perf_callchain_*() helpers: entry to the left, regs to the right, following the traditional (src, dst).

Signed-off-by Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc David Miller <[email protected]>
Cc Paul Mundt <[email protected]>
Cc Will Deacon <[email protected]>
Cc Borislav Petkov <[email protected]>
---
 arch/arm/kernel/perf_event.c         |   43 +++---------------------------
 arch/powerpc/kernel/perf_callchain.c |   49 +++++++++------------------------
 arch/sh/kernel/perf_callchain.c      |   37 +------------------------
 arch/sparc/kernel/perf_event.c       |   46 ++++++++++---------------------
 arch/x86/kernel/cpu/perf_event.c     |   45 +++++-------------------------
 include/linux/perf_event.h           |   10 ++++++-
 kernel/perf_event.c                  |   40 ++++++++++++++++++++++++++-
 7 files changed, 90 insertions(+), 180 deletions(-)
arch/arm/kernel/perf_event.c +4 -39
index a07c3b1..0e3bbdb 100644
@@ -3044,17 +3044,13 @@ user_backtrace(struct frame_tail *tail,
 	return buftail.fp - 1;
 }
 
-static void
-perf_callchain_user(struct pt_regs *regs,
-		    struct perf_callchain_entry *entry)
+void
+perf_callchain_user(struct perf_callchain_entry *entry, struct pt_regs *regs)
 {
 	struct frame_tail *tail;
 
 	perf_callchain_store(entry, PERF_CONTEXT_USER);
 
-	if (!user_mode(regs))
-		regs = task_pt_regs(current);
-
 	tail = (struct frame_tail *)regs->ARM_fp - 1;
 
 	while (tail && !((unsigned long)tail & 0x3))

@@ -3075,9 +3071,8 @@ callchain_trace(struct stackframe *fr,
 	return 0;
 }
 
-static void
-perf_callchain_kernel(struct pt_regs *regs,
-		      struct perf_callchain_entry *entry)
+void
+perf_callchain_kernel(struct perf_callchain_entry *entry, struct pt_regs *regs)
 {
 	struct stackframe fr;
 

@@ -3088,33 +3083,3 @@ perf_callchain_kernel(struct pt_regs *regs,
 	fr.pc = regs->ARM_pc;
 	walk_stackframe(&fr, callchain_trace, entry);
 }
-
-static void
-perf_do_callchain(struct pt_regs *regs,
-		  struct perf_callchain_entry *entry)
-{
-	int is_user;
-
-	if (!regs)
-		return;
-
-	is_user = user_mode(regs);
-
-	if (!is_user)
-		perf_callchain_kernel(regs, entry);
-
-	if (current->mm)
-		perf_callchain_user(regs, entry);
-}
-
-static DEFINE_PER_CPU(struct perf_callchain_entry, pmc_irq_entry);
-
-struct perf_callchain_entry *
-perf_callchain(struct pt_regs *regs)
-{
-	struct perf_callchain_entry *entry = &__get_cpu_var(pmc_irq_entry);
-
-	entry->nr = 0;
-	perf_do_callchain(regs, entry);
-	return entry;
-}
arch/powerpc/kernel/perf_callchain.c +14 -35

78 lines not on this page · raw message

arch/sh/kernel/perf_callchain.c +2 -35
index 00143f3..ef076a9 100644
@@ -44,44 +44,11 @@ static const struct stacktrace_ops callchain_ops = {
 	.address	= callchain_address,
 };
 
-static void
-perf_callchain_kernel(struct pt_regs *regs, struct perf_callchain_entry *entry)
+void
+perf_callchain_kernel(struct perf_callchain_entry *entry, struct pt_regs *regs)
 {
 	perf_callchain_store(entry, PERF_CONTEXT_KERNEL);
 	perf_callchain_store(entry, regs->pc);
 
 	unwind_stack(NULL, regs, NULL, &callchain_ops, entry);
 }
-
-static void
-perf_do_callchain(struct pt_regs *regs, struct perf_callchain_entry *entry)
-{
-	int is_user;
-
-	if (!regs)
-		return;
-
-	is_user = user_mode(regs);
-
-	/*
-	 * Only the kernel side is implemented for now.
-	 */
-	if (!is_user)
-		perf_callchain_kernel(regs, entry);
-}
-
-/*
- * No need for separate IRQ and NMI entries.
- */
-static DEFINE_PER_CPU(struct perf_callchain_entry, callchain);
-
-struct perf_callchain_entry *perf_callchain(struct pt_regs *regs)
-{
-	struct perf_callchain_entry *entry = &__get_cpu_var(callchain);
-
-	entry->nr = 0;
-
-	perf_do_callchain(regs, entry);
-
-	return entry;
-}
arch/sparc/kernel/perf_event.c +15 -31

75 lines not on this page · raw message

arch/x86/kernel/cpu/perf_event.c +8 -37

81 lines not on this page · raw message

include/linux/perf_event.h +9 -1
index 3588804..4db61dd 100644
@@ -976,7 +976,15 @@ extern int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks
 extern void perf_event_comm(struct task_struct *tsk);
 extern void perf_event_fork(struct task_struct *tsk);
 
-extern struct perf_callchain_entry *perf_callchain(struct pt_regs *regs);
+/* Callchains */
+DECLARE_PER_CPU(struct perf_callchain_entry, perf_callchain_entry);
+
+extern void perf_callchain_user(struct perf_callchain_entry *entry,
+				struct pt_regs *regs);
+extern void perf_callchain_kernel(struct perf_callchain_entry *entry,
+				  struct pt_regs *regs);
+extern struct perf_callchain_entry *perf_callchain_buffer(void);
+
 
 static inline void
 perf_callchain_store(struct perf_callchain_entry *entry, u64 ip)
kernel/perf_event.c +38 -2
index c772a3d..02efde6 100644
@@ -2937,13 +2937,49 @@ void perf_event_do_pending(void)
 	__perf_pending_run();
 }
 
+DEFINE_PER_CPU(struct perf_callchain_entry, perf_callchain_entry);
+
 /*
  * Callchain support -- arch specific
  */
 
-__weak struct perf_callchain_entry *perf_callchain(struct pt_regs *regs)
+__weak struct perf_callchain_entry *perf_callchain_buffer(void)
 {
-	return NULL;
+	return &__get_cpu_var(perf_callchain_entry);
+}
+
+__weak void perf_callchain_kernel(struct perf_callchain_entry *entry,
+				  struct pt_regs *regs)
+{
+}
+
+__weak void perf_callchain_user(struct perf_callchain_entry *entry,
+				struct pt_regs *regs)
+{
+}
+
+static struct perf_callchain_entry *perf_callchain(struct pt_regs *regs)
+{
+	struct perf_callchain_entry *entry;
+
+	entry = perf_callchain_buffer();
+	if (!entry)
+		return NULL;
+
+	entry->nr = 0;
+
+	if (!user_mode(regs)) {
+		perf_callchain_kernel(entry, regs);
+		if (current->mm)
+			regs = task_pt_regs(current);
+		else
+			regs = NULL;
+	}
+
+	if (regs)
+		perf_callchain_user(entry, regs);
+
+	return entry;
 }
Signature · 1 line
1.6.2.3
Acked-by on v1 3/6
#5 lore raw

Re: [RFC PATCH 3/6] perf: Generalize some arch callchain code

Quoted 10 lines · Frederic Weisbecker

- Most archs use one callchain buffer per cpu, except x86 that needs to deal with NMIs. Provide a default perf_callchain_buffer() implementation that x86 overrides.

- Centralize all the kernel/user regs handling and invoke new arch handlers from there: perf_callchain_user() / perf_callchain_kernel() That avoid all the user_mode(), current->mm checks and so...

- Invert some parameters in perf_callchain_*() helpers: entry to the left, regs to the right, following the traditional (src, dst).

I think you mean (dst, src), don't you?

Apart from that, the patch looks OK, so

Acked-by Paul Mackerras <[email protected]>

Paul.

author
#6 lore raw

Re: [RFC PATCH 3/6] perf: Generalize some arch callchain code

Quoted 6 lines · Paul Mackerras
Quoted 4 lines · Frederic Weisbecker

- Most archs use one callchain buffer per cpu, except x86 that needs to deal with NMIs. Provide a default perf_callchain_buffer() implementation that x86 overrides.

Quoted 8 lines
Quoted 6 lines

- Centralize all the kernel/user regs handling and invoke new arch handlers from there: perf_callchain_user() / perf_callchain_kernel() That avoid all the user_mode(), current->mm checks and so...

- Invert some parameters in perf_callchain_*() helpers: entry to the left, regs to the right, following the traditional (src, dst).

I think you mean (dst, src), don't you?

Indeed yeah :)

Quoted 3 lines

Apart from that, the patch looks OK, so

Acked-by Paul Mackerras <[email protected]>

Thanks a lot for your reviews and acks!

author
#7 lore raw

RFC PATCH 4/6 perf: Factorize callchain context handling

In reply to Frederic Weisbecker

Store the kernel and user contexts from the generic layer instead of archs, this gathers some repetitive code.

Signed-off-by Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc David Miller <[email protected]>
Cc Paul Mundt <[email protected]>
Cc Will Deacon <[email protected]>
Cc Borislav Petkov <[email protected]>
---
 arch/arm/kernel/perf_event.c         |    2 --
 arch/powerpc/kernel/perf_callchain.c |    3 ---
 arch/sh/kernel/perf_callchain.c      |    1 -
 arch/sparc/kernel/perf_event.c       |    3 ---
 arch/x86/kernel/cpu/perf_event.c     |    2 --
 kernel/perf_event.c                  |    5 ++++-
 6 files changed, 4 insertions(+), 12 deletions(-)
arch/arm/kernel/perf_event.c +0 -2
index 0e3bbdb..64ca8c3 100644
@@ -3049,7 +3049,6 @@ perf_callchain_user(struct perf_callchain_entry *entry, struct pt_regs *regs)
 {
 	struct frame_tail *tail;
 
-	perf_callchain_store(entry, PERF_CONTEXT_USER);
 
 	tail = (struct frame_tail *)regs->ARM_fp - 1;
 

@@ -3076,7 +3075,6 @@ perf_callchain_kernel(struct perf_callchain_entry *entry, struct pt_regs *regs)
 {
 	struct stackframe fr;
 
-	perf_callchain_store(entry, PERF_CONTEXT_KERNEL);
 	fr.fp = regs->ARM_fp;
 	fr.sp = regs->ARM_sp;
 	fr.lr = regs->ARM_lr;
arch/powerpc/kernel/perf_callchain.c +0 -3
index f7a85ed..d05ae42 100644
@@ -57,7 +57,6 @@ perf_callchain_kernel(struct perf_callchain_entry *entry, struct pt_regs *regs)
 
 	lr = regs->link;
 	sp = regs->gpr[1];
-	perf_callchain_store(entry, PERF_CONTEXT_KERNEL);
 	perf_callchain_store(entry, regs->nip);
 
 	if (!validate_sp(sp, current, STACK_FRAME_OVERHEAD))

@@ -234,7 +233,6 @@ static void perf_callchain_user_64(struct perf_callchain_entry *entry,
 	next_ip = regs->nip;
 	lr = regs->link;
 	sp = regs->gpr[1];
-	perf_callchain_store(entry, PERF_CONTEXT_USER);
 	perf_callchain_store(entry, next_ip);
 
 	for (;;) {

@@ -435,7 +433,6 @@ static void perf_callchain_user_32(struct perf_callchain_entry *entry,
 	next_ip = regs->nip;
 	lr = regs->link;
 	sp = regs->gpr[1];
-	perf_callchain_store(entry, PERF_CONTEXT_USER);
 	perf_callchain_store(entry, next_ip);
 
 	while (entry->nr < PERF_MAX_STACK_DEPTH) {
arch/sh/kernel/perf_callchain.c +0 -1
index ef076a9..d5ca1ef 100644
@@ -47,7 +47,6 @@ static const struct stacktrace_ops callchain_ops = {
 void
 perf_callchain_kernel(struct perf_callchain_entry *entry, struct pt_regs *regs)
 {
-	perf_callchain_store(entry, PERF_CONTEXT_KERNEL);
 	perf_callchain_store(entry, regs->pc);
 
 	unwind_stack(NULL, regs, NULL, &callchain_ops, entry);
arch/sparc/kernel/perf_event.c +0 -3
index 460162d..4bc4029 100644
@@ -1293,7 +1293,6 @@ void perf_callchain_kernel(struct perf_callchain_entry *entry,
 
 	stack_trace_flush();
 
-	perf_callchain_store(entry, PERF_CONTEXT_KERNEL);
 	perf_callchain_store(entry, regs->tpc);
 
 	ksp = regs->u_regs[UREG_I6];

@@ -1337,7 +1336,6 @@ static void perf_callchain_user_64(struct perf_callchain_entry *entry,
 {
 	unsigned long ufp;
 
-	perf_callchain_store(entry, PERF_CONTEXT_USER);
 	perf_callchain_store(entry, regs->tpc);
 
 	ufp = regs->u_regs[UREG_I6] + STACK_BIAS;

@@ -1360,7 +1358,6 @@ static void perf_callchain_user_32(struct perf_callchain_entry *entry,
 {
 	unsigned long ufp;
 
-	perf_callchain_store(entry, PERF_CONTEXT_USER);
 	perf_callchain_store(entry, regs->tpc);
 
 	ufp = regs->u_regs[UREG_I6] & 0xffffffffUL;
arch/x86/kernel/cpu/perf_event.c +0 -2
index 39f8421..a3c9222 100644
@@ -1608,7 +1608,6 @@ static const struct stacktrace_ops backtrace_ops = {
 void
 perf_callchain_kernel(struct perf_callchain_entry *entry, struct pt_regs *regs)
 {
-	perf_callchain_store(entry, PERF_CONTEXT_KERNEL);
 	perf_callchain_store(entry, regs->ip);
 
 	dump_trace(NULL, regs, NULL, regs->bp, &backtrace_ops, entry);

@@ -1660,7 +1659,6 @@ perf_callchain_user(struct perf_callchain_entry *entry, struct pt_regs *regs)
 
 	fp = (void __user *)regs->bp;
 
-	perf_callchain_store(entry, PERF_CONTEXT_USER);
 	perf_callchain_store(entry, regs->ip);
 
 	if (perf_callchain_user32(regs, entry))
kernel/perf_event.c +4 -1
index 02efde6..615d024 100644
@@ -2969,6 +2969,7 @@ static struct perf_callchain_entry *perf_callchain(struct pt_regs *regs)
 	entry->nr = 0;
 
 	if (!user_mode(regs)) {
+		perf_callchain_store(entry, PERF_CONTEXT_KERNEL);
 		perf_callchain_kernel(entry, regs);
 		if (current->mm)
 			regs = task_pt_regs(current);

@@ -2976,8 +2977,10 @@ static struct perf_callchain_entry *perf_callchain(struct pt_regs *regs)
 			regs = NULL;
 	}
 
-	if (regs)
+	if (regs) {
+		perf_callchain_store(entry, PERF_CONTEXT_USER);
 		perf_callchain_user(entry, regs);
+	}
 
 	return entry;
 }
Signature · 1 line
1.6.2.3
Acked-by on v1 4/6
#8 lore raw

Re: [RFC PATCH 4/6] perf: Factorize callchain context handling

Quoted 2 lines · Frederic Weisbecker

Store the kernel and user contexts from the generic layer instead of archs, this gathers some repetitive code.

Acked-by Paul Mackerras <[email protected]>
author
#9 lore raw

RFC PATCH 5/6 perf: Fix race in callchains

In reply to Frederic Weisbecker

Now that software events don't have interrupt disabled anymore in the event path, callchains can nest on any context. So seperating nmi and others contexts in two buffers has become racy.

Fix this by providing one buffer per nesting level. Given the size of the callchain entries (2040 bytes * 4), we now need to allocate them dynamically.

v2: Fixed put_callchain_entry call after recursion.
Fix the type of the recursion, it must be an array.

v3: Use a manual per cpu allocation (temporary solution until NMIs
can safely access vmalloc'ed memory).
Do a better separation between callchain reference tracking and allocation. Make the "put" path lockless for non-release cases.

Signed-off-by Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc Will Deacon <[email protected]>
Cc Paul Mundt <[email protected]>
Cc David Miller <[email protected]>
Cc Borislav Petkov <[email protected]>
---
 arch/x86/kernel/cpu/perf_event.c |   22 ++--
 include/linux/perf_event.h       |    1 -
 kernel/perf_event.c              |  265 ++++++++++++++++++++++++++++----------
 3 files changed, 205 insertions(+), 83 deletions(-)
arch/x86/kernel/cpu/perf_event.c +9 -13
index a3c9222..8e91cf3 100644
@@ -1608,6 +1608,11 @@ static const struct stacktrace_ops backtrace_ops = {
 void
 perf_callchain_kernel(struct perf_callchain_entry *entry, struct pt_regs *regs)
 {
+	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
+		/* TODO: We don't support guest os callchain now */
+		return NULL;
+	}
+
 	perf_callchain_store(entry, regs->ip);
 
 	dump_trace(NULL, regs, NULL, regs->bp, &backtrace_ops, entry);

@@ -1656,6 +1661,10 @@ perf_callchain_user(struct perf_callchain_entry *entry, struct pt_regs *regs)
 	struct stack_frame frame;
 	const void __user *fp;
 
+	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
+		/* TODO: We don't support guest os callchain now */
+		return NULL;
+	}
 
 	fp = (void __user *)regs->bp;
 

@@ -1681,19 +1690,6 @@ perf_callchain_user(struct perf_callchain_entry *entry, struct pt_regs *regs)
 	}
 }
 
-struct perf_callchain_entry *perf_callchain_buffer(void)
-{
-	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
-		/* TODO: We don't support guest os callchain now */
-		return NULL;
-	}
-
-	if (in_nmi())
-		return &__get_cpu_var(perf_callchain_entry_nmi);
-
-	return &__get_cpu_var(perf_callchain_entry);
-}
-
 unsigned long perf_instruction_pointer(struct pt_regs *regs)
 {
 	unsigned long ip;
include/linux/perf_event.h +0 -1
index 4db61dd..d7e8ea6 100644
@@ -983,7 +983,6 @@ extern void perf_callchain_user(struct perf_callchain_entry *entry,
 				struct pt_regs *regs);
 extern void perf_callchain_kernel(struct perf_callchain_entry *entry,
 				  struct pt_regs *regs);
-extern struct perf_callchain_entry *perf_callchain_buffer(void);
 
 
 static inline void
kernel/perf_event.c +196 -69

315 lines not on this page · raw message

Signature · 1 line
1.6.2.3
author
#10 lore raw

RFC PATCH v4 5/6 perf: Fix race in callchains

Now that software events don't have interrupt disabled anymore in the event path, callchains can nest on any context. So seperating nmi and others contexts in two buffers has become racy.

Fix this by providing one buffer per nesting level. Given the size of the callchain entries (2040 bytes * 4), we now need to allocate them dynamically.

v2: Fixed put_callchain_entry call after recursion.
Fix the type of the recursion, it must be an array.

v3: Use a manual pr cpu allocation (temporary solution until NMIs
can safely access vmalloc'ed memory).
Do a better separation between callchain reference tracking and allocation. Make the "put" path lockless for non-release cases.

v4: Protect the callchain buffers with rcu.

Signed-off-by Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc Will Deacon <[email protected]>
Cc Paul Mundt <[email protected]>
Cc David Miller <[email protected]>
Cc Borislav Petkov <[email protected]>
---
 arch/x86/kernel/cpu/perf_event.c |   22 ++--
 include/linux/perf_event.h       |    1 -
 kernel/perf_event.c              |  297 +++++++++++++++++++++++++++++---------
 3 files changed, 237 insertions(+), 83 deletions(-)
arch/x86/kernel/cpu/perf_event.c +9 -13
index a3c9222..8e91cf3 100644
@@ -1608,6 +1608,11 @@ static const struct stacktrace_ops backtrace_ops = {
 void
 perf_callchain_kernel(struct perf_callchain_entry *entry, struct pt_regs *regs)
 {
+	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
+		/* TODO: We don't support guest os callchain now */
+		return NULL;
+	}
+
 	perf_callchain_store(entry, regs->ip);
 
 	dump_trace(NULL, regs, NULL, regs->bp, &backtrace_ops, entry);

@@ -1656,6 +1661,10 @@ perf_callchain_user(struct perf_callchain_entry *entry, struct pt_regs *regs)
 	struct stack_frame frame;
 	const void __user *fp;
 
+	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
+		/* TODO: We don't support guest os callchain now */
+		return NULL;
+	}
 
 	fp = (void __user *)regs->bp;
 

@@ -1681,19 +1690,6 @@ perf_callchain_user(struct perf_callchain_entry *entry, struct pt_regs *regs)
 	}
 }
 
-struct perf_callchain_entry *perf_callchain_buffer(void)
-{
-	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
-		/* TODO: We don't support guest os callchain now */
-		return NULL;
-	}
-
-	if (in_nmi())
-		return &__get_cpu_var(perf_callchain_entry_nmi);
-
-	return &__get_cpu_var(perf_callchain_entry);
-}
-
 unsigned long perf_instruction_pointer(struct pt_regs *regs)
 {
 	unsigned long ip;
include/linux/perf_event.h +0 -1
index 4db61dd..d7e8ea6 100644
@@ -983,7 +983,6 @@ extern void perf_callchain_user(struct perf_callchain_entry *entry,
 				struct pt_regs *regs);
 extern void perf_callchain_kernel(struct perf_callchain_entry *entry,
 				  struct pt_regs *regs);
-extern struct perf_callchain_entry *perf_callchain_buffer(void);
 
 
 static inline void
kernel/perf_event.c +228 -69

347 lines not on this page · raw message

Signature · 1 line
1.6.2.3
#11 lore raw

Re: [RFC PATCH 5/6 v4] perf: Fix race in callchains

Quoted 8 lines · Frederic Weisbecker

Now that software events don't have interrupt disabled anymore in the event path, callchains can nest on any context. So seperating nmi and others contexts in two buffers has become racy.

Fix this by providing one buffer per nesting level. Given the size of the callchain entries (2040 bytes * 4), we now need to allocate them dynamically.

Quoted 5 lines

v2: Fixed put_callchain_entry call after recursion.
Fix the type of the recursion, it must be an array.

v3: Use a manual pr cpu allocation (temporary solution until NMIs
can safely access vmalloc'ed memory).

It would be nice to make these allocations node-local.

Also, I see that we're allocating 4 buffers per cpu on powerpc when we strictly only need 3, but I don't suppose that really matters.

Paul.

author
#12 lore raw

Re: [RFC PATCH 5/6 v4] perf: Fix race in callchains

Quoted 10 lines · Paul Mackerras
Quoted 8 lines · Frederic Weisbecker

Now that software events don't have interrupt disabled anymore in the event path, callchains can nest on any context. So seperating nmi and others contexts in two buffers has become racy.

Fix this by providing one buffer per nesting level. Given the size of the callchain entries (2040 bytes * 4), we now need to allocate them dynamically.

Quoted 7 lines
Quoted 5 lines

v2: Fixed put_callchain_entry call after recursion.
Fix the type of the recursion, it must be an array.

v3: Use a manual pr cpu allocation (temporary solution until NMIs
can safely access vmalloc'ed memory).

It would be nice to make these allocations node-local.

You're right, I'll use kmalloc_node then.

Quoted 2 lines

Also, I see that we're allocating 4 buffers per cpu on powerpc when we strictly only need 3, but I don't suppose that really matters.

Yeah, we are allocating a similar per-context bunch of buffers for trace events and also for software events.

If we had a way to know if an arch has nmis, we could manage that. Whenever this is about simulated or really unmaskable, we don't care, we just need to know if events can nest on traditional irq disabled sections.

How do you deal with that in PPC? Is the event delayed to when irqs are restored?

author
#13 lore raw

RFC PATCH 6/6 perf: Humanize the number of contexts

In reply to Frederic Weisbecker

Instead of hardcoding the number of contexts for the recursions barriers, define a cpp constant to make the code more self-explanatory.

Signed-off-by Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 include/linux/perf_event.h |   14 ++++++++------
 kernel/perf_event.c        |    6 +++---
 2 files changed, 11 insertions(+), 9 deletions(-)
include/linux/perf_event.h +8 -6
index d7e8ea6..ae6fa60 100644
@@ -808,6 +808,12 @@ struct perf_event_context {
 	struct rcu_head			rcu_head;
 };
 
+/*
+ * Number of contexts where an event can trigger:
+ * 	task, softirq, hardirq, nmi.
+ */
+#define PERF_NR_CONTEXTS	4
+
 /**
  * struct perf_event_cpu_context - per cpu event context structure
  */

@@ -821,12 +827,8 @@ struct perf_cpu_context {
 	struct mutex			hlist_mutex;
 	int				hlist_refcount;
 
-	/*
-	 * Recursion avoidance:
-	 *
-	 * task, softirq, irq, nmi context
-	 */
-	int				recursion[4];
+	/* Recursion avoidance in each contexts */
+	int				recursion[PERF_NR_CONTEXTS];
 };
 
 struct perf_output_handle {
kernel/perf_event.c +3 -3
index 6ad61fb..e41e29b 100644
@@ -1767,7 +1767,7 @@ static u64 perf_event_read(struct perf_event *event)
  * Callchain support
  */
 
-static DEFINE_PER_CPU(int, callchain_recursion[4]);
+static DEFINE_PER_CPU(int, callchain_recursion[PERF_NR_CONTEXTS]);
 static atomic_t nr_callchain_events;
 static DEFINE_MUTEX(callchain_mutex);
 static struct perf_callchain_entry **callchain_cpu_entries;

@@ -1812,8 +1812,8 @@ static int alloc_callchain_buffers(void)
 	callchain_cpu_entries = entries;
 
 	for_each_possible_cpu(cpu) {
-		entries[cpu] = kmalloc(sizeof(struct perf_callchain_entry) * 4,
-					GFP_KERNEL);
+		entries[cpu] = kmalloc(sizeof(struct perf_callchain_entry) *
+					PERF_NR_CONTEXTS, GFP_KERNEL);
 		if (!entries[cpu])
 			return -ENOMEM;
 	}
Signature · 1 line
1.6.2.3
#14 lore raw
In reply to Frederic Weisbecker

From: Frederic Weisbecker <[email protected]>
Date: Mon, Aug 16, 2010 at 04:48:29PM -0400

Hey Frederic,

Quoted 3 lines

Built-tested on PowerPc, Arm and SuperH (can someone point me to a ready to use sparc cross compiler, like those that can be found there for other archs: http://www.codesourcery.com/sgpp/lite_edition.html ?)

Have you by chance tried http://www.kernel.org/pub/tools/crosstool/
already?

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
author
#15 lore raw
Quoted 11 lines · Borislav Petkov

From: Frederic Weisbecker <[email protected]>
Date: Mon, Aug 16, 2010 at 04:48:29PM -0400

Hey Frederic,

Quoted 3 lines

Built-tested on PowerPc, Arm and SuperH (can someone point me to a ready to use sparc cross compiler, like those that can be found there for other archs: http://www.codesourcery.com/sgpp/lite_edition.html ?)

Have you by chance tried http://www.kernel.org/pub/tools/crosstool/
already?

Nice, I'll give it a try.

Thanks :)

Tested-by on the v3 series
#16 lore raw
In reply to Frederic Weisbecker
Quoted 2 lines · Frederic Weisbecker

Hi,

Hi Frederic,

Quoted 9 lines

This set factorizes a lot of callchain code and fixes concurrent callchain buffers accesses.

You can test it by pulling that branch:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
perf/callchains

I've tested this on an ARM Cortex-A9 board and it all seems fine [plus the code is a lot cleaner!].

Tested-by Will Deacon <[email protected]>

Cheers,

Will

author
#17 lore raw
Quoted 10 lines · Will Deacon
Quoted 2 lines · Frederic Weisbecker

Hi,

Hi Frederic,

Quoted 6 lines

This set factorizes a lot of callchain code and fixes concurrent callchain buffers accesses.

You can test it by pulling that branch:

Quoted 8 lines
Quoted 3 lines

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
perf/callchains

I've tested this on an ARM Cortex-A9 board and it all seems fine [plus the code is a lot cleaner!].

Tested-by Will Deacon <[email protected]>

Thanks a lot!

BTW, out of curiosity, do you have NMIs on ARM and do the hardware events make use of them? Or may be you use FIQ to simulate NMIs?

#18 lore raw
Quoted 11 lines · Frederic Weisbecker
Quoted 4 lines · Will Deacon

I've tested this on an ARM Cortex-A9 board and it all seems fine [plus the code is a lot cleaner!].

Tested-by Will Deacon <[email protected]>

Thanks a lot!

BTW, out of curiosity, do you have NMIs on ARM and do the hardware events make use of them? Or may be you use FIQ to simulate NMIs?

We don't have NMIs on ARM [so obviously we can't use them!] but you're right to point out the FIQ. I've actually been thinking about this during the past week, but there are the following problems:

(1) The FIQ isn't always wired up in the hardware, so you can't assume that it is available.

(2) The FIQ can only have a single handler at a given time. This is because it is a separate exception mode, with its own banked registers. Consequently, we might not be able to use it if it's being used for something else.

(3) The Trustzone security extensions may reserve the FIQ for secure use only or make it available only via the secure monitor [which will increase latency].

Of course, the advantage is that we could then use sample-based profiling techniques in sections of code where the interrupts are disabled.

The only way I can think of adding this is as a Kconfig option, which, when selected, tries to use the FIQ and then falls back to normal IRQs if it fails.

Cheers,

Will

#19 lore raw
Quoted 13 lines · Will Deacon
Quoted 11 lines · Frederic Weisbecker
Quoted 4 lines · Will Deacon

I've tested this on an ARM Cortex-A9 board and it all seems fine [plus the code is a lot cleaner!].

Tested-by Will Deacon <[email protected]>

Thanks a lot!

BTW, out of curiosity, do you have NMIs on ARM and do the hardware events make use of them? Or may be you use FIQ to simulate NMIs?

Quoted 6 lines

We don't have NMIs on ARM [so obviously we can't use them!] but you're right to point out the FIQ. I've actually been thinking about this during the past week, but there are the following problems:

(1) The FIQ isn't always wired up in the hardware, so you can't assume that it is available.

We dont always have NMIs on x86 either - we fall back to hrtimers in that case.

Quoted 4 lines

(2) The FIQ can only have a single handler at a given time. This is because it is a separate exception mode, with its own banked registers. Consequently, we might not be able to use it if it's being used for something else.

Technically the NMI is only a single exception source on x86 as well. We multiplex from there - if there are multiple users we call them using a notifier chain.

Quoted 3 lines

(3) The Trustzone security extensions may reserve the FIQ for secure use only or make it available only via the secure monitor [which will increase latency].

As long as it can still be detected during PMU init and set up safely, it should be OK.

Quoted 2 lines

Of course, the advantage is that we could then use sample-based profiling techniques in sections of code where the interrupts are disabled.

Once you've tried NMI profiling you wont be going back - the difference is day and night ;-)

Here's the profile of a scheduling-intense workload using a timer based fallback path:

# Events: 586 cycles
#
# Overhead Command Shared Object Symbol

# ........  ............  .................  ...........................
#
    21.33%  pipe-test-1m  [kernel.kallsyms]  [k] finish_task_switch
    14.33%  pipe-test-1m  [kernel.kallsyms]  [k] _raw_spin_unlock_irqrestore
     4.61%  pipe-test-1m  [kernel.kallsyms]  [k] avc_has_perm_noaudit
     3.75%  pipe-test-1m  [kernel.kallsyms]  [k] pipe_read
     3.58%  pipe-test-1m  libc-2.12.so       [.] __write_nocancel
     3.58%  pipe-test-1m  [kernel.kallsyms]  [k] copy_user_generic_string
     3.41%  pipe-test-1m  libc-2.12.so       [.] __GI___libc_read
     3.07%  pipe-test-1m  [kernel.kallsyms]  [k] system_call_after_swapgs
     3.07%  pipe-test-1m  [kernel.kallsyms]  [k] pipe_write
     3.07%  pipe-test-1m  [kernel.kallsyms]  [k] file_has_perm
     2.22%  pipe-test-1m  pipe-test-1m       [.] main
     2.22%  pipe-test-1m  [kernel.kallsyms]  [k] selinux_file_permission
     1.88%  pipe-test-1m  [kernel.kallsyms]  [k] rw_verify_area
     1.88%  pipe-test-1m  [kernel.kallsyms]  [k] fsnotify

# Events: 23K cycles
#
# Overhead Command Shared Object Symbol

# ........  ............  .................  ...................................
#
     7.14%  pipe-test-1m  [kernel.kallsyms]  [k] __default_send_IPI_dest_field
     4.34%  pipe-test-1m  [kernel.kallsyms]  [k] schedule
     4.27%  pipe-test-1m  [kernel.kallsyms]  [k] __switch_to
     3.88%  pipe-test-1m  [kernel.kallsyms]  [k] pipe_read
     3.57%  pipe-test-1m  [kernel.kallsyms]  [k] switch_mm
     3.45%  pipe-test-1m  [kernel.kallsyms]  [k] file_has_perm
     3.37%  pipe-test-1m  [kernel.kallsyms]  [k] copy_user_generic_string
     3.37%  pipe-test-1m  pipe-test-1m       [.] main
     3.20%  pipe-test-1m  [kernel.kallsyms]  [k] avc_has_perm_noaudit
     2.62%  pipe-test-1m  libc-2.12.so       [.] __GI___libc_read
     2.09%  pipe-test-1m  [kernel.kallsyms]  [k] fsnotify
     1.96%  pipe-test-1m  [kernel.kallsyms]  [k] system_call
     1.94%  pipe-test-1m  [kernel.kallsyms]  [k] pipe_write
     1.90%  pipe-test-1m  libc-2.12.so       [.] __write_nocancel
     1.88%  pipe-test-1m  [kernel.kallsyms]  [k] mutex_lock
     1.88%  pipe-test-1m  [kernel.kallsyms]  [k] selinux_file_permission
     1.78%  pipe-test-1m  [kernel.kallsyms]  [k] mutex_unlock
     1.66%  pipe-test-1m  [kernel.kallsyms]  [k] _raw_spin_lock_irqsave
     1.50%  pipe-test-1m  [kernel.kallsyms]  [k] _raw_spin_unlock_irqrestore
     1.23%  pipe-test-1m  [kernel.kallsyms]  [k] vfs_read
     1.19%  pipe-test-1m  [kernel.kallsyms]  [k] do_sync_read
     1.18%  pipe-test-1m  [kernel.kallsyms]  [k] update_curr

The NMI output is an order of magnitude richer in information.

Quoted 3 lines

The only way I can think of adding this is as a Kconfig option, which, when selected, tries to use the FIQ and then falls back to normal IRQs if it fails.

Dynamic detection and a fallback path, should be perfectly OK. Kconfig options have the disadvantage of doubling the test space and halving the tester base (or worse).

Thanks,

Ingo