Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH 1/2] perfcounter: Ignore the nmi call frames in the x86-64 backtraces

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


fweisbec at gmail

Jul 1, 2009, 12:02 PM

Post #1 of 3 (143 views)
Permalink
[PATCH 1/2] perfcounter: Ignore the nmi call frames in the x86-64 backtraces

About every callchains recorded with perf record are
filled up including the internal perfcounter nmi frame:

perf_callchain
perf_counter_overflow
intel_pmu_handle_irq
perf_counter_nmi_handler
notifier_call_chain
atomic_notifier_call_chain
notify_die
do_nmi
nmi

We want ignore this frame as it's not interesting for instrumentation.
To solve this, we simply ignore every frames from nmi context.

New example of "perf report -s sym -c" after this patch:

9.59% [k] search_by_key
4.88%
search_by_key
reiserfs_read_locked_inode
reiserfs_iget
reiserfs_lookup
do_lookup
__link_path_walk
path_walk
do_path_lookup
user_path_at
vfs_fstatat
vfs_lstat
sys_newlstat
system_call_fastpath
__lxstat
0x406fb1

3.19%
search_by_key
search_by_entry_key
reiserfs_find_entry
reiserfs_lookup
do_lookup
__link_path_walk
path_walk
do_path_lookup
user_path_at
vfs_fstatat
vfs_lstat
sys_newlstat
system_call_fastpath
__lxstat
0x406fb1
[...]

For now this patch only solves the problem in x86-64.

Signed-off-by: Frederic Weisbecker <fweisbec [at] gmail>
---
arch/x86/include/asm/stacktrace.h | 2 ++
arch/x86/kernel/cpu/perf_counter.c | 8 +++++++-
arch/x86/kernel/dumpstack_32.c | 6 ++++++
arch/x86/kernel/dumpstack_64.c | 22 +++++++++++++++-------
4 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h
index f517944..cf86a5e 100644
--- a/arch/x86/include/asm/stacktrace.h
+++ b/arch/x86/include/asm/stacktrace.h
@@ -3,6 +3,8 @@

extern int kstack_depth_to_print;

+int x86_is_stack_id(int id, char *name);
+
/* Generic stack tracer with callbacks */

struct stacktrace_ops {
diff --git a/arch/x86/kernel/cpu/perf_counter.c b/arch/x86/kernel/cpu/perf_counter.c
index d4cf4ce..36c3dc7 100644
--- a/arch/x86/kernel/cpu/perf_counter.c
+++ b/arch/x86/kernel/cpu/perf_counter.c
@@ -1561,6 +1561,7 @@ void callchain_store(struct perf_callchain_entry *entry, u64 ip)

static DEFINE_PER_CPU(struct perf_callchain_entry, irq_entry);
static DEFINE_PER_CPU(struct perf_callchain_entry, nmi_entry);
+static DEFINE_PER_CPU(int, in_nmi_frame);


static void
@@ -1576,7 +1577,9 @@ static void backtrace_warning(void *data, char *msg)

static int backtrace_stack(void *data, char *name)
{
- /* Process all stacks: */
+ per_cpu(in_nmi_frame, smp_processor_id()) =
+ x86_is_stack_id(NMI_STACK, name);
+
return 0;
}

@@ -1584,6 +1587,9 @@ static void backtrace_address(void *data, unsigned long addr, int reliable)
{
struct perf_callchain_entry *entry = data;

+ if (per_cpu(in_nmi_frame, smp_processor_id()))
+ return;
+
if (reliable)
callchain_store(entry, addr);
}
diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index d593cd1..bca5fba 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -19,6 +19,12 @@

#include "dumpstack.h"

+/* Just a stub for now */
+int x86_is_stack_id(int id, char *name)
+{
+ return 0;
+}
+
void dump_trace(struct task_struct *task, struct pt_regs *regs,
unsigned long *stack, unsigned long bp,
const struct stacktrace_ops *ops, void *data)
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index d35db59..54b0a32 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -19,10 +19,8 @@

#include "dumpstack.h"

-static unsigned long *in_exception_stack(unsigned cpu, unsigned long stack,
- unsigned *usedp, char **idp)
-{
- static char ids[][8] = {
+
+static char x86_stack_ids[][8] = {
[DEBUG_STACK - 1] = "#DB",
[NMI_STACK - 1] = "NMI",
[DOUBLEFAULT_STACK - 1] = "#DF",
@@ -33,6 +31,15 @@ static unsigned long *in_exception_stack(unsigned cpu, unsigned long stack,
N_EXCEPTION_STACKS + DEBUG_STKSZ / EXCEPTION_STKSZ - 2] = "#DB[?]"
#endif
};
+
+int x86_is_stack_id(int id, char *name)
+{
+ return x86_stack_ids[id - 1] == name;
+}
+
+static unsigned long *in_exception_stack(unsigned cpu, unsigned long stack,
+ unsigned *usedp, char **idp)
+{
unsigned k;

/*
@@ -61,7 +68,7 @@ static unsigned long *in_exception_stack(unsigned cpu, unsigned long stack,
if (*usedp & (1U << k))
break;
*usedp |= 1U << k;
- *idp = ids[k];
+ *idp = x86_stack_ids[k];
return (unsigned long *)end;
}
/*
@@ -81,12 +88,13 @@ static unsigned long *in_exception_stack(unsigned cpu, unsigned long stack,
do {
++j;
end -= EXCEPTION_STKSZ;
- ids[j][4] = '1' + (j - N_EXCEPTION_STACKS);
+ x86_stack_ids[j][4] = '1' +
+ (j - N_EXCEPTION_STACKS);
} while (stack < end - EXCEPTION_STKSZ);
if (*usedp & (1U << j))
break;
*usedp |= 1U << j;
- *idp = ids[j];
+ *idp = x86_stack_ids[j];
return (unsigned long *)end;
}
#endif
--
1.6.2.3

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


a.p.zijlstra at chello

Jul 6, 2009, 5:17 AM

Post #2 of 3 (119 views)
Permalink
Re: [PATCH 1/2] perfcounter: Ignore the nmi call frames in the x86-64 backtraces [In reply to]

On Wed, 2009-07-01 at 21:02 +0200, Frederic Weisbecker wrote:

> +int x86_is_stack_id(int id, char *name)
> +{
> + return x86_stack_ids[id - 1] == name;
> +}

OK, this bit would need a bit of a comment explaining why this works, as
I don't think the C language guarantees de-duplication of constant
strings.

Therefore the above only works correctly if its passed a pointer from
the x86_stack_ids[] array -- as it indeed is, since the string passed is
obtained through the below method:

> +static unsigned long *in_exception_stack(unsigned cpu, unsigned long stack,
> + unsigned *usedp, char **idp)
> +{
> unsigned k;
>
> /*
> @@ -61,7 +68,7 @@ static unsigned long *in_exception_stack(unsigned cpu, unsigned long stack,
> if (*usedp & (1U << k))
> break;
> *usedp |= 1U << k;
> - *idp = ids[k];
> + *idp = x86_stack_ids[k];
> return (unsigned long *)end;
> }
> /*


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


fweisbec at gmail

Jul 7, 2009, 12:40 AM

Post #3 of 3 (104 views)
Permalink
Re: [PATCH 1/2] perfcounter: Ignore the nmi call frames in the x86-64 backtraces [In reply to]

On Mon, Jul 06, 2009 at 02:17:48PM +0200, Peter Zijlstra wrote:
> On Wed, 2009-07-01 at 21:02 +0200, Frederic Weisbecker wrote:
>
> > +int x86_is_stack_id(int id, char *name)
> > +{
> > + return x86_stack_ids[id - 1] == name;
> > +}
>
> OK, this bit would need a bit of a comment explaining why this works, as
> I don't think the C language guarantees de-duplication of constant
> strings.
>
> Therefore the above only works correctly if its passed a pointer from
> the x86_stack_ids[] array -- as it indeed is, since the string passed is
> obtained through the below method:


Right, that need some comments, I will do that with x86-32 support at the same
time.

Thanks.



> > +static unsigned long *in_exception_stack(unsigned cpu, unsigned long stack,
> > + unsigned *usedp, char **idp)
> > +{
> > unsigned k;
> >
> > /*
> > @@ -61,7 +68,7 @@ static unsigned long *in_exception_stack(unsigned cpu, unsigned long stack,
> > if (*usedp & (1U << k))
> > break;
> > *usedp |= 1U << k;
> > - *idp = ids[k];
> > + *idp = x86_stack_ids[k];
> > return (unsigned long *)end;
> > }
> > /*
>
>

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

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.