From 98d943b02f6f1b57787ff1aa6f34d019a407e3ee Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Wed, 29 Sep 2010 16:52:25 +0200 Subject: [PATCH 1/4] oprofile, ARM: Release resources on failure This patch fixes a resource leak on failure, where the oprofilefs and some counters may not released properly. Signed-off-by: Robert Richter Acked-by: Will Deacon Cc: linux-arm-kernel@lists.infradead.org Cc: # .35.x LKML-Reference: <20100929145225.GJ13563@erda.amd.com> Signed-off-by: Ingo Molnar --- arch/arm/oprofile/common.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c index 0691176899ff..72e09eb642dd 100644 --- a/arch/arm/oprofile/common.c +++ b/arch/arm/oprofile/common.c @@ -102,6 +102,7 @@ static int op_create_counter(int cpu, int event) if (IS_ERR(pevent)) { ret = PTR_ERR(pevent); } else if (pevent->state != PERF_EVENT_STATE_ACTIVE) { + perf_event_release_kernel(pevent); pr_warning("oprofile: failed to enable event %d " "on CPU %d\n", event, cpu); ret = -EBUSY; @@ -365,6 +366,7 @@ int __init oprofile_arch_init(struct oprofile_operations *ops) ret = init_driverfs(); if (ret) { kfree(counter_config); + counter_config = NULL; return ret; } @@ -402,7 +404,6 @@ void oprofile_arch_exit(void) struct perf_event *event; if (*perf_events) { - exit_driverfs(); for_each_possible_cpu(cpu) { for (id = 0; id < perf_num_counters; ++id) { event = perf_events[cpu][id]; @@ -413,8 +414,10 @@ void oprofile_arch_exit(void) } } - if (counter_config) + if (counter_config) { kfree(counter_config); + exit_driverfs(); + } } #else int __init oprofile_arch_init(struct oprofile_operations *ops) From 40c6b3cb64cd1d02322df5f729cca25084580f40 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Wed, 29 Sep 2010 10:46:46 -0400 Subject: [PATCH 2/4] oprofile, x86: Using struct stack_frame for 64bit processes dump Removing unnecessary struct frame_head and replacing it with struct stack_frame. The struct stack_frame is already defined and used in other places in kernel, so there's no reason to define new structure. Signed-off-by: Jiri Olsa Signed-off-by: Robert Richter --- arch/x86/oprofile/backtrace.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/arch/x86/oprofile/backtrace.c b/arch/x86/oprofile/backtrace.c index 3855096c59b8..d640a86198b1 100644 --- a/arch/x86/oprofile/backtrace.c +++ b/arch/x86/oprofile/backtrace.c @@ -48,35 +48,30 @@ static struct stacktrace_ops backtrace_ops = { .walk_stack = print_context_stack, }; -struct frame_head { - struct frame_head *bp; - unsigned long ret; -} __attribute__((packed)); - -static struct frame_head *dump_user_backtrace(struct frame_head *head) +static struct stack_frame *dump_user_backtrace(struct stack_frame *head) { - struct frame_head bufhead[2]; + struct stack_frame bufhead[2]; - /* Also check accessibility of one struct frame_head beyond */ + /* Also check accessibility of one struct stack_frame beyond */ if (!access_ok(VERIFY_READ, head, sizeof(bufhead))) return NULL; if (__copy_from_user_inatomic(bufhead, head, sizeof(bufhead))) return NULL; - oprofile_add_trace(bufhead[0].ret); + oprofile_add_trace(bufhead[0].return_address); /* frame pointers should strictly progress back up the stack * (towards higher addresses) */ - if (head >= bufhead[0].bp) + if (head >= bufhead[0].next_frame) return NULL; - return bufhead[0].bp; + return bufhead[0].next_frame; } void x86_backtrace(struct pt_regs * const regs, unsigned int depth) { - struct frame_head *head = (struct frame_head *)frame_pointer(regs); + struct stack_frame *head = (struct stack_frame *)frame_pointer(regs); if (!user_mode_vm(regs)) { unsigned long stack = kernel_stack_pointer(regs); From f6dedecc37164a58bb80ae2ed9d204669ffc4850 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Wed, 29 Sep 2010 10:46:47 -0400 Subject: [PATCH 3/4] oprofile, x86: Adding backtrace dump for 32bit process in compat mode This patch implements the oprofile backtrace generation for 32 bit applications running in the 64bit environment (compat mode). With this change it's possible to get backtrace for 32bits applications under the 64bits environment using oprofile's callgraph options. opcontrol --setup -c ... opreport -l -cg ... Signed-off-by: Jiri Olsa Signed-off-by: Robert Richter --- arch/x86/oprofile/backtrace.c | 53 +++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/arch/x86/oprofile/backtrace.c b/arch/x86/oprofile/backtrace.c index d640a86198b1..2d49d4e19a36 100644 --- a/arch/x86/oprofile/backtrace.c +++ b/arch/x86/oprofile/backtrace.c @@ -14,6 +14,7 @@ #include #include #include +#include static void backtrace_warning_symbol(void *data, char *msg, unsigned long symbol) @@ -48,6 +49,55 @@ static struct stacktrace_ops backtrace_ops = { .walk_stack = print_context_stack, }; +#ifdef CONFIG_COMPAT +static struct stack_frame_ia32 * +dump_user_backtrace_32(struct stack_frame_ia32 *head) +{ + struct stack_frame_ia32 bufhead[2]; + struct stack_frame_ia32 *fp; + + /* Also check accessibility of one struct frame_head beyond */ + if (!access_ok(VERIFY_READ, head, sizeof(bufhead))) + return NULL; + if (__copy_from_user_inatomic(bufhead, head, sizeof(bufhead))) + return NULL; + + fp = (struct stack_frame_ia32 *) compat_ptr(bufhead[0].next_frame); + + oprofile_add_trace(bufhead[0].return_address); + + /* frame pointers should strictly progress back up the stack + * (towards higher addresses) */ + if (head >= fp) + return NULL; + + return fp; +} + +static inline int +x86_backtrace_32(struct pt_regs * const regs, unsigned int depth) +{ + struct stack_frame_ia32 *head; + + /* User process is 32-bit */ + if (!current || !test_thread_flag(TIF_IA32)) + return 0; + + head = (struct stack_frame_ia32 *) regs->bp; + while (depth-- && head) + head = dump_user_backtrace_32(head); + + return 1; +} + +#else +static inline int +x86_backtrace_32(struct pt_regs * const regs, unsigned int depth) +{ + return 0; +} +#endif /* CONFIG_COMPAT */ + static struct stack_frame *dump_user_backtrace(struct stack_frame *head) { struct stack_frame bufhead[2]; @@ -81,6 +131,9 @@ x86_backtrace(struct pt_regs * const regs, unsigned int depth) return; } + if (x86_backtrace_32(regs, depth)) + return; + while (depth-- && head) head = dump_user_backtrace(head); } From 5140434d5f82f2e2119926272ada2e9731ec04f1 Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Thu, 30 Sep 2010 18:55:47 +0200 Subject: [PATCH 4/4] oprofile, x86: Simplify init/exit functions Now, that we only call the exit function if init succeeds with commit: 979048e oprofile: don't call arch exit code from init code on failure we can simplify the x86 init/exit functions too. Variable using_nmi becomes obsolete. Signed-off-by: Robert Richter --- arch/x86/oprofile/nmi_int.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c index f1575c9a2572..bd1489c3ce09 100644 --- a/arch/x86/oprofile/nmi_int.c +++ b/arch/x86/oprofile/nmi_int.c @@ -695,9 +695,6 @@ static int __init ppro_init(char **cpu_type) return 1; } -/* in order to get sysfs right */ -static int using_nmi; - int __init op_nmi_init(struct oprofile_operations *ops) { __u8 vendor = boot_cpu_data.x86_vendor; @@ -705,8 +702,6 @@ int __init op_nmi_init(struct oprofile_operations *ops) char *cpu_type = NULL; int ret = 0; - using_nmi = 0; - if (!cpu_has_apic) return -ENODEV; @@ -790,13 +785,11 @@ int __init op_nmi_init(struct oprofile_operations *ops) if (ret) return ret; - using_nmi = 1; printk(KERN_INFO "oprofile: using NMI interrupt.\n"); return 0; } void op_nmi_exit(void) { - if (using_nmi) - exit_sysfs(); + exit_sysfs(); }