From 979048e1f26190d16b5aea87166177f37e614439 Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Sun, 29 Aug 2010 14:51:59 -0400 Subject: oprofile: don't call arch exit code from init code on failure oprofile_init calls oprofile_arch_init to initialise the architecture-specific backend code. If this backend code returns failure, oprofile_arch_exit is called immediately, making it difficult to allocate and free resources correctly. This patch removes the oprofile_arch_exit call from oprofile_init, meaning that all architectures must ensure that oprofile_arch_init cleans up any mess it's made before returning an error. As far as I can tell, this only affects the code for ARM. Cc: Robert Richter Cc: Matt Fleming Cc: Peter Zijlstra Cc: Ingo Molnar Signed-off-by: Will Deacon Signed-off-by: Robert Richter --- drivers/oprofile/oprof.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/oprof.c b/drivers/oprofile/oprof.c index b336cd9ee7a1..b4a685719dba 100644 --- a/drivers/oprofile/oprof.c +++ b/drivers/oprofile/oprof.c @@ -257,16 +257,9 @@ static int __init oprofile_init(void) printk(KERN_INFO "oprofile: using timer interrupt.\n"); err = oprofile_timer_init(&oprofile_ops); if (err) - goto out_arch; + return err; } - err = oprofilefs_register(); - if (err) - goto out_arch; - return 0; - -out_arch: - oprofile_arch_exit(); - return err; + return oprofilefs_register(); } -- cgit v1.2.3 From 4fdaa7b682b413dfb7ca9fa74ff45b1e0cb3dade Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Thu, 19 Aug 2010 10:50:26 +0200 Subject: oprofile: Remove duplicate code around __oprofilefs_create_file() Removing duplicate code by assigning the inodes private data pointer in __oprofilefs_create_file(). Extending the function interface to pass the pointer. Signed-off-by: Robert Richter --- drivers/oprofile/oprofilefs.c | 46 +++++++++++++------------------------------ 1 file changed, 14 insertions(+), 32 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/oprofilefs.c b/drivers/oprofile/oprofilefs.c index 2766a6d3c2e9..789a1a857ddf 100644 --- a/drivers/oprofile/oprofilefs.c +++ b/drivers/oprofile/oprofilefs.c @@ -126,50 +126,41 @@ static const struct file_operations ulong_ro_fops = { }; -static struct dentry *__oprofilefs_create_file(struct super_block *sb, +static int __oprofilefs_create_file(struct super_block *sb, struct dentry *root, char const *name, const struct file_operations *fops, - int perm) + int perm, void *priv) { struct dentry *dentry; struct inode *inode; dentry = d_alloc_name(root, name); if (!dentry) - return NULL; + return -ENOMEM; inode = oprofilefs_get_inode(sb, S_IFREG | perm); if (!inode) { dput(dentry); - return NULL; + return -ENOMEM; } inode->i_fop = fops; d_add(dentry, inode); - return dentry; + dentry->d_inode->i_private = priv; + return 0; } int oprofilefs_create_ulong(struct super_block *sb, struct dentry *root, char const *name, unsigned long *val) { - struct dentry *d = __oprofilefs_create_file(sb, root, name, - &ulong_fops, 0644); - if (!d) - return -EFAULT; - - d->d_inode->i_private = val; - return 0; + return __oprofilefs_create_file(sb, root, name, + &ulong_fops, 0644, val); } int oprofilefs_create_ro_ulong(struct super_block *sb, struct dentry *root, char const *name, unsigned long *val) { - struct dentry *d = __oprofilefs_create_file(sb, root, name, - &ulong_ro_fops, 0444); - if (!d) - return -EFAULT; - - d->d_inode->i_private = val; - return 0; + return __oprofilefs_create_file(sb, root, name, + &ulong_ro_fops, 0444, val); } @@ -189,31 +180,22 @@ static const struct file_operations atomic_ro_fops = { int oprofilefs_create_ro_atomic(struct super_block *sb, struct dentry *root, char const *name, atomic_t *val) { - struct dentry *d = __oprofilefs_create_file(sb, root, name, - &atomic_ro_fops, 0444); - if (!d) - return -EFAULT; - - d->d_inode->i_private = val; - return 0; + return __oprofilefs_create_file(sb, root, name, + &atomic_ro_fops, 0444, val); } int oprofilefs_create_file(struct super_block *sb, struct dentry *root, char const *name, const struct file_operations *fops) { - if (!__oprofilefs_create_file(sb, root, name, fops, 0644)) - return -EFAULT; - return 0; + return __oprofilefs_create_file(sb, root, name, fops, 0644, NULL); } int oprofilefs_create_file_perm(struct super_block *sb, struct dentry *root, char const *name, const struct file_operations *fops, int perm) { - if (!__oprofilefs_create_file(sb, root, name, fops, perm)) - return -EFAULT; - return 0; + return __oprofilefs_create_file(sb, root, name, fops, perm, NULL); } -- cgit v1.2.3 From 3d90a00763b51e1db344a7430c966be723b67a29 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Mon, 27 Sep 2010 20:45:08 +0100 Subject: oprofile: Abstract the perf-events backend Move the perf-events backend from arch/arm/oprofile into drivers/oprofile so that the code can be shared between architectures. This allows each architecture to maintain only a single copy of the PMU accessor functions instead of one for both perf and OProfile. It also becomes possible for other architectures to delete much of their OProfile code in favour of the common code now available in drivers/oprofile/oprofile_perf.c. Signed-off-by: Matt Fleming Tested-by: Will Deacon Signed-off-by: Robert Richter --- arch/arm/oprofile/Makefile | 4 + arch/arm/oprofile/common.c | 319 -------------------------------------- drivers/oprofile/oprofile_perf.c | 326 +++++++++++++++++++++++++++++++++++++++ include/linux/oprofile.h | 3 + 4 files changed, 333 insertions(+), 319 deletions(-) create mode 100644 drivers/oprofile/oprofile_perf.c (limited to 'drivers') diff --git a/arch/arm/oprofile/Makefile b/arch/arm/oprofile/Makefile index e666eafed152..b2215c61cdf0 100644 --- a/arch/arm/oprofile/Makefile +++ b/arch/arm/oprofile/Makefile @@ -6,4 +6,8 @@ DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \ oprofilefs.o oprofile_stats.o \ timer_int.o ) +ifeq ($(CONFIG_HW_PERF_EVENTS),y) +DRIVER_OBJS += $(addprefix ../../../drivers/oprofile/, oprofile_perf.o) +endif + oprofile-y := $(DRIVER_OBJS) common.o diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c index 8718311cb530..8aa974491dfc 100644 --- a/arch/arm/oprofile/common.c +++ b/arch/arm/oprofile/common.c @@ -25,136 +25,6 @@ #include #ifdef CONFIG_HW_PERF_EVENTS -/* - * Per performance monitor configuration as set via oprofilefs. - */ -struct op_counter_config { - unsigned long count; - unsigned long enabled; - unsigned long event; - unsigned long unit_mask; - unsigned long kernel; - unsigned long user; - struct perf_event_attr attr; -}; - -static int oprofile_perf_enabled; -static DEFINE_MUTEX(oprofile_perf_mutex); - -static struct op_counter_config *counter_config; -static struct perf_event **perf_events[nr_cpumask_bits]; -static int num_counters; - -/* - * Overflow callback for oprofile. - */ -static void op_overflow_handler(struct perf_event *event, int unused, - struct perf_sample_data *data, struct pt_regs *regs) -{ - int id; - u32 cpu = smp_processor_id(); - - for (id = 0; id < num_counters; ++id) - if (perf_events[cpu][id] == event) - break; - - if (id != num_counters) - oprofile_add_sample(regs, id); - else - pr_warning("oprofile: ignoring spurious overflow " - "on cpu %u\n", cpu); -} - -/* - * Called by oprofile_perf_setup to create perf attributes to mirror the oprofile - * settings in counter_config. Attributes are created as `pinned' events and - * so are permanently scheduled on the PMU. - */ -static void op_perf_setup(void) -{ - int i; - u32 size = sizeof(struct perf_event_attr); - struct perf_event_attr *attr; - - for (i = 0; i < num_counters; ++i) { - attr = &counter_config[i].attr; - memset(attr, 0, size); - attr->type = PERF_TYPE_RAW; - attr->size = size; - attr->config = counter_config[i].event; - attr->sample_period = counter_config[i].count; - attr->pinned = 1; - } -} - -static int op_create_counter(int cpu, int event) -{ - int ret = 0; - struct perf_event *pevent; - - if (!counter_config[event].enabled || (perf_events[cpu][event] != NULL)) - return ret; - - pevent = perf_event_create_kernel_counter(&counter_config[event].attr, - cpu, -1, - op_overflow_handler); - - if (IS_ERR(pevent)) { - ret = PTR_ERR(pevent); - } else if (pevent->state != PERF_EVENT_STATE_ACTIVE) { - pr_warning("oprofile: failed to enable event %d " - "on CPU %d\n", event, cpu); - ret = -EBUSY; - } else { - perf_events[cpu][event] = pevent; - } - - return ret; -} - -static void op_destroy_counter(int cpu, int event) -{ - struct perf_event *pevent = perf_events[cpu][event]; - - if (pevent) { - perf_event_release_kernel(pevent); - perf_events[cpu][event] = NULL; - } -} - -/* - * Called by oprofile_perf_start to create active perf events based on the - * perviously configured attributes. - */ -static int op_perf_start(void) -{ - int cpu, event, ret = 0; - - for_each_online_cpu(cpu) { - for (event = 0; event < num_counters; ++event) { - ret = op_create_counter(cpu, event); - if (ret) - goto out; - } - } - -out: - return ret; -} - -/* - * Called by oprofile_perf_stop at the end of a profiling run. - */ -static void op_perf_stop(void) -{ - int cpu, event; - - for_each_online_cpu(cpu) - for (event = 0; event < num_counters; ++event) - op_destroy_counter(cpu, event); -} - - char *op_name_from_perf_id(void) { enum arm_perf_pmu_ids id = armpmu_get_pmu_id(); @@ -177,116 +47,6 @@ char *op_name_from_perf_id(void) } } -static int oprofile_perf_create_files(struct super_block *sb, struct dentry *root) -{ - unsigned int i; - - for (i = 0; i < num_counters; i++) { - struct dentry *dir; - char buf[4]; - - snprintf(buf, sizeof buf, "%d", i); - dir = oprofilefs_mkdir(sb, root, buf); - oprofilefs_create_ulong(sb, dir, "enabled", &counter_config[i].enabled); - oprofilefs_create_ulong(sb, dir, "event", &counter_config[i].event); - oprofilefs_create_ulong(sb, dir, "count", &counter_config[i].count); - oprofilefs_create_ulong(sb, dir, "unit_mask", &counter_config[i].unit_mask); - oprofilefs_create_ulong(sb, dir, "kernel", &counter_config[i].kernel); - oprofilefs_create_ulong(sb, dir, "user", &counter_config[i].user); - } - - return 0; -} - -static int oprofile_perf_setup(void) -{ - spin_lock(&oprofilefs_lock); - op_perf_setup(); - spin_unlock(&oprofilefs_lock); - return 0; -} - -static int oprofile_perf_start(void) -{ - int ret = -EBUSY; - - mutex_lock(&oprofile_perf_mutex); - if (!oprofile_perf_enabled) { - ret = 0; - op_perf_start(); - oprofile_perf_enabled = 1; - } - mutex_unlock(&oprofile_perf_mutex); - return ret; -} - -static void oprofile_perf_stop(void) -{ - mutex_lock(&oprofile_perf_mutex); - if (oprofile_perf_enabled) - op_perf_stop(); - oprofile_perf_enabled = 0; - mutex_unlock(&oprofile_perf_mutex); -} - -#ifdef CONFIG_PM -static int oprofile_perf_suspend(struct platform_device *dev, pm_message_t state) -{ - mutex_lock(&oprofile_perf_mutex); - if (oprofile_perf_enabled) - op_perf_stop(); - mutex_unlock(&oprofile_perf_mutex); - return 0; -} - -static int oprofile_perf_resume(struct platform_device *dev) -{ - mutex_lock(&oprofile_perf_mutex); - if (oprofile_perf_enabled && op_perf_start()) - oprofile_perf_enabled = 0; - mutex_unlock(&oprofile_perf_mutex); - return 0; -} - -static struct platform_driver oprofile_driver = { - .driver = { - .name = "oprofile-perf", - }, - .resume = oprofile_perf_resume, - .suspend = oprofile_perf_suspend, -}; - -static struct platform_device *oprofile_pdev; - -static int __init init_driverfs(void) -{ - int ret; - - ret = platform_driver_register(&oprofile_driver); - if (ret) - goto out; - - oprofile_pdev = platform_device_register_simple( - oprofile_driver.driver.name, 0, NULL, 0); - if (IS_ERR(oprofile_pdev)) { - ret = PTR_ERR(oprofile_pdev); - platform_driver_unregister(&oprofile_driver); - } - -out: - return ret; -} - -static void __exit exit_driverfs(void) -{ - platform_device_unregister(oprofile_pdev); - platform_driver_unregister(&oprofile_driver); -} -#else -static int __init init_driverfs(void) { return 0; } -#define exit_driverfs() do { } while (0) -#endif /* CONFIG_PM */ - static int report_trace(struct stackframe *frame, void *d) { unsigned int *depth = d; @@ -349,66 +109,6 @@ static void arm_backtrace(struct pt_regs * const regs, unsigned int depth) tail = user_backtrace(tail); } -int __init oprofile_perf_init(struct oprofile_operations *ops) -{ - int cpu, ret = 0; - - memset(&perf_events, 0, sizeof(perf_events)); - - num_counters = perf_num_counters(); - if (num_counters <= 0) { - pr_info("oprofile: no performance counters\n"); - ret = -ENODEV; - goto out; - } - - counter_config = kcalloc(num_counters, - sizeof(struct op_counter_config), GFP_KERNEL); - - if (!counter_config) { - pr_info("oprofile: failed to allocate %d " - "counters\n", num_counters); - ret = -ENOMEM; - goto out; - } - - ret = init_driverfs(); - if (ret) - goto out; - - for_each_possible_cpu(cpu) { - perf_events[cpu] = kcalloc(num_counters, - sizeof(struct perf_event *), GFP_KERNEL); - if (!perf_events[cpu]) { - pr_info("oprofile: failed to allocate %d perf events " - "for cpu %d\n", num_counters, cpu); - ret = -ENOMEM; - goto out; - } - } - - ops->create_files = oprofile_perf_create_files; - ops->setup = oprofile_perf_setup; - ops->start = oprofile_perf_start; - ops->stop = oprofile_perf_stop; - ops->shutdown = oprofile_perf_stop; - ops->cpu_type = op_name_from_perf_id(); - - if (!ops->cpu_type) - ret = -ENODEV; - else - pr_info("oprofile: using %s\n", ops->cpu_type); - -out: - if (ret) { - for_each_possible_cpu(cpu) - kfree(perf_events[cpu]); - kfree(counter_config); - } - - return ret; -} - int __init oprofile_arch_init(struct oprofile_operations *ops) { ops->backtrace = arm_backtrace; @@ -416,25 +116,6 @@ int __init oprofile_arch_init(struct oprofile_operations *ops) return oprofile_perf_init(ops); } -void __exit oprofile_perf_exit(void) -{ - int cpu, id; - struct perf_event *event; - - for_each_possible_cpu(cpu) { - for (id = 0; id < num_counters; ++id) { - event = perf_events[cpu][id]; - if (event) - perf_event_release_kernel(event); - } - - kfree(perf_events[cpu]); - } - - kfree(counter_config); - exit_driverfs(); -} - void __exit oprofile_arch_exit(void) { oprofile_perf_exit(); diff --git a/drivers/oprofile/oprofile_perf.c b/drivers/oprofile/oprofile_perf.c new file mode 100644 index 000000000000..ebb40cb87474 --- /dev/null +++ b/drivers/oprofile/oprofile_perf.c @@ -0,0 +1,326 @@ +/* + * Copyright 2010 ARM Ltd. + * + * Perf-events backend for OProfile. + */ +#include +#include +#include + +/* + * Per performance monitor configuration as set via oprofilefs. + */ +struct op_counter_config { + unsigned long count; + unsigned long enabled; + unsigned long event; + unsigned long unit_mask; + unsigned long kernel; + unsigned long user; + struct perf_event_attr attr; +}; + +static int oprofile_perf_enabled; +static DEFINE_MUTEX(oprofile_perf_mutex); + +static struct op_counter_config *counter_config; +static struct perf_event **perf_events[nr_cpumask_bits]; +static int num_counters; + +/* + * Overflow callback for oprofile. + */ +static void op_overflow_handler(struct perf_event *event, int unused, + struct perf_sample_data *data, struct pt_regs *regs) +{ + int id; + u32 cpu = smp_processor_id(); + + for (id = 0; id < num_counters; ++id) + if (perf_events[cpu][id] == event) + break; + + if (id != num_counters) + oprofile_add_sample(regs, id); + else + pr_warning("oprofile: ignoring spurious overflow " + "on cpu %u\n", cpu); +} + +/* + * Called by oprofile_perf_setup to create perf attributes to mirror the oprofile + * settings in counter_config. Attributes are created as `pinned' events and + * so are permanently scheduled on the PMU. + */ +static void op_perf_setup(void) +{ + int i; + u32 size = sizeof(struct perf_event_attr); + struct perf_event_attr *attr; + + for (i = 0; i < num_counters; ++i) { + attr = &counter_config[i].attr; + memset(attr, 0, size); + attr->type = PERF_TYPE_RAW; + attr->size = size; + attr->config = counter_config[i].event; + attr->sample_period = counter_config[i].count; + attr->pinned = 1; + } +} + +static int op_create_counter(int cpu, int event) +{ + int ret = 0; + struct perf_event *pevent; + + if (!counter_config[event].enabled || (perf_events[cpu][event] != NULL)) + return ret; + + pevent = perf_event_create_kernel_counter(&counter_config[event].attr, + cpu, -1, + op_overflow_handler); + + if (IS_ERR(pevent)) { + ret = PTR_ERR(pevent); + } else if (pevent->state != PERF_EVENT_STATE_ACTIVE) { + pr_warning("oprofile: failed to enable event %d " + "on CPU %d\n", event, cpu); + ret = -EBUSY; + } else { + perf_events[cpu][event] = pevent; + } + + return ret; +} + +static void op_destroy_counter(int cpu, int event) +{ + struct perf_event *pevent = perf_events[cpu][event]; + + if (pevent) { + perf_event_release_kernel(pevent); + perf_events[cpu][event] = NULL; + } +} + +/* + * Called by oprofile_perf_start to create active perf events based on the + * perviously configured attributes. + */ +static int op_perf_start(void) +{ + int cpu, event, ret = 0; + + for_each_online_cpu(cpu) { + for (event = 0; event < num_counters; ++event) { + ret = op_create_counter(cpu, event); + if (ret) + goto out; + } + } + +out: + return ret; +} + +/* + * Called by oprofile_perf_stop at the end of a profiling run. + */ +static void op_perf_stop(void) +{ + int cpu, event; + + for_each_online_cpu(cpu) + for (event = 0; event < num_counters; ++event) + op_destroy_counter(cpu, event); +} + +static int oprofile_perf_create_files(struct super_block *sb, struct dentry *root) +{ + unsigned int i; + + for (i = 0; i < num_counters; i++) { + struct dentry *dir; + char buf[4]; + + snprintf(buf, sizeof buf, "%d", i); + dir = oprofilefs_mkdir(sb, root, buf); + oprofilefs_create_ulong(sb, dir, "enabled", &counter_config[i].enabled); + oprofilefs_create_ulong(sb, dir, "event", &counter_config[i].event); + oprofilefs_create_ulong(sb, dir, "count", &counter_config[i].count); + oprofilefs_create_ulong(sb, dir, "unit_mask", &counter_config[i].unit_mask); + oprofilefs_create_ulong(sb, dir, "kernel", &counter_config[i].kernel); + oprofilefs_create_ulong(sb, dir, "user", &counter_config[i].user); + } + + return 0; +} + +static int oprofile_perf_setup(void) +{ + spin_lock(&oprofilefs_lock); + op_perf_setup(); + spin_unlock(&oprofilefs_lock); + return 0; +} + +static int oprofile_perf_start(void) +{ + int ret = -EBUSY; + + mutex_lock(&oprofile_perf_mutex); + if (!oprofile_perf_enabled) { + ret = 0; + op_perf_start(); + oprofile_perf_enabled = 1; + } + mutex_unlock(&oprofile_perf_mutex); + return ret; +} + +static void oprofile_perf_stop(void) +{ + mutex_lock(&oprofile_perf_mutex); + if (oprofile_perf_enabled) + op_perf_stop(); + oprofile_perf_enabled = 0; + mutex_unlock(&oprofile_perf_mutex); +} + +#ifdef CONFIG_PM +static int oprofile_perf_suspend(struct platform_device *dev, pm_message_t state) +{ + mutex_lock(&oprofile_perf_mutex); + if (oprofile_perf_enabled) + op_perf_stop(); + mutex_unlock(&oprofile_perf_mutex); + return 0; +} + +static int oprofile_perf_resume(struct platform_device *dev) +{ + mutex_lock(&oprofile_perf_mutex); + if (oprofile_perf_enabled && op_perf_start()) + oprofile_perf_enabled = 0; + mutex_unlock(&oprofile_perf_mutex); + return 0; +} + +static struct platform_driver oprofile_driver = { + .driver = { + .name = "oprofile-perf", + }, + .resume = oprofile_perf_resume, + .suspend = oprofile_perf_suspend, +}; + +static struct platform_device *oprofile_pdev; + +static int __init init_driverfs(void) +{ + int ret; + + ret = platform_driver_register(&oprofile_driver); + if (ret) + goto out; + + oprofile_pdev = platform_device_register_simple( + oprofile_driver.driver.name, 0, NULL, 0); + if (IS_ERR(oprofile_pdev)) { + ret = PTR_ERR(oprofile_pdev); + platform_driver_unregister(&oprofile_driver); + } + +out: + return ret; +} + +static void __exit exit_driverfs(void) +{ + platform_device_unregister(oprofile_pdev); + platform_driver_unregister(&oprofile_driver); +} +#else +static int __init init_driverfs(void) { return 0; } +#define exit_driverfs() do { } while (0) +#endif /* CONFIG_PM */ + +int __init oprofile_perf_init(struct oprofile_operations *ops) +{ + int cpu, ret = 0; + + memset(&perf_events, 0, sizeof(perf_events)); + + num_counters = perf_num_counters(); + if (num_counters <= 0) { + pr_info("oprofile: no performance counters\n"); + ret = -ENODEV; + goto out; + } + + counter_config = kcalloc(num_counters, + sizeof(struct op_counter_config), GFP_KERNEL); + + if (!counter_config) { + pr_info("oprofile: failed to allocate %d " + "counters\n", num_counters); + ret = -ENOMEM; + goto out; + } + + ret = init_driverfs(); + if (ret) + goto out; + + for_each_possible_cpu(cpu) { + perf_events[cpu] = kcalloc(num_counters, + sizeof(struct perf_event *), GFP_KERNEL); + if (!perf_events[cpu]) { + pr_info("oprofile: failed to allocate %d perf events " + "for cpu %d\n", num_counters, cpu); + ret = -ENOMEM; + goto out; + } + } + + ops->create_files = oprofile_perf_create_files; + ops->setup = oprofile_perf_setup; + ops->start = oprofile_perf_start; + ops->stop = oprofile_perf_stop; + ops->shutdown = oprofile_perf_stop; + ops->cpu_type = op_name_from_perf_id(); + + if (!ops->cpu_type) + ret = -ENODEV; + else + pr_info("oprofile: using %s\n", ops->cpu_type); + +out: + if (ret) { + for_each_possible_cpu(cpu) + kfree(perf_events[cpu]); + kfree(counter_config); + } + + return ret; +} + +void __exit oprofile_perf_exit(void) +{ + int cpu, id; + struct perf_event *event; + + for_each_possible_cpu(cpu) { + for (id = 0; id < num_counters; ++id) { + event = perf_events[cpu][id]; + if (event) + perf_event_release_kernel(event); + } + + kfree(perf_events[cpu]); + } + + kfree(counter_config); + exit_driverfs(); +} diff --git a/include/linux/oprofile.h b/include/linux/oprofile.h index 1574d4aca721..d67a8330b41e 100644 --- a/include/linux/oprofile.h +++ b/include/linux/oprofile.h @@ -15,6 +15,7 @@ #include #include +#include #include /* Each escaped entry is prefixed by ESCAPE_CODE @@ -186,6 +187,8 @@ int oprofile_add_data64(struct op_entry *entry, u64 val); int oprofile_write_commit(struct op_entry *entry); #ifdef CONFIG_PERF_EVENTS +int __init oprofile_perf_init(struct oprofile_operations *ops); +void __exit oprofile_perf_exit(void); char *op_name_from_perf_id(void); #endif /* CONFIG_PERF_EVENTS */ -- cgit v1.2.3 From 81771974ae49bf79aab60c42eac7a6d730a9ef2b Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Wed, 29 Sep 2010 16:52:25 +0200 Subject: 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 --- drivers/oprofile/oprofile_perf.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/oprofile/oprofile_perf.c b/drivers/oprofile/oprofile_perf.c index ebb40cb87474..f3d3df229a43 100644 --- a/drivers/oprofile/oprofile_perf.c +++ b/drivers/oprofile/oprofile_perf.c @@ -84,6 +84,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; -- cgit v1.2.3 From 9c91283a19c2d998a83f50f113f8585709c15caf Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Fri, 27 Aug 2010 14:32:41 +0200 Subject: oprofile, ARM: Remove some goto statements This patch removes some unnecessary goto statements. Acked-by: Will Deacon Signed-off-by: Robert Richter --- drivers/oprofile/oprofile_perf.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/oprofile_perf.c b/drivers/oprofile/oprofile_perf.c index f3d3df229a43..6853634c4681 100644 --- a/drivers/oprofile/oprofile_perf.c +++ b/drivers/oprofile/oprofile_perf.c @@ -117,11 +117,10 @@ static int op_perf_start(void) for (event = 0; event < num_counters; ++event) { ret = op_create_counter(cpu, event); if (ret) - goto out; + return ret; } } -out: return ret; } @@ -224,7 +223,7 @@ static int __init init_driverfs(void) ret = platform_driver_register(&oprofile_driver); if (ret) - goto out; + return ret; oprofile_pdev = platform_device_register_simple( oprofile_driver.driver.name, 0, NULL, 0); @@ -233,7 +232,6 @@ static int __init init_driverfs(void) platform_driver_unregister(&oprofile_driver); } -out: return ret; } -- cgit v1.2.3 From 2bcb2b641a8f1524f7a9801eb9e52a00b8f18c6e Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Wed, 29 Sep 2010 14:43:29 +0200 Subject: oprofile, ARM: Rework op_create_counter() This patch simplifies op_create_counter(). Removing if/else if paths and return code variable by direct returning from function. Acked-by: Will Deacon Signed-off-by: Robert Richter --- drivers/oprofile/oprofile_perf.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/oprofile_perf.c b/drivers/oprofile/oprofile_perf.c index 6853634c4681..a34137f2e26c 100644 --- a/drivers/oprofile/oprofile_perf.c +++ b/drivers/oprofile/oprofile_perf.c @@ -71,28 +71,28 @@ static void op_perf_setup(void) static int op_create_counter(int cpu, int event) { - int ret = 0; struct perf_event *pevent; - if (!counter_config[event].enabled || (perf_events[cpu][event] != NULL)) - return ret; + if (!counter_config[event].enabled || perf_events[cpu][event]) + return 0; pevent = perf_event_create_kernel_counter(&counter_config[event].attr, cpu, -1, op_overflow_handler); - if (IS_ERR(pevent)) { - ret = PTR_ERR(pevent); - } else if (pevent->state != PERF_EVENT_STATE_ACTIVE) { + if (IS_ERR(pevent)) + return PTR_ERR(pevent); + + 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; - } else { - perf_events[cpu][event] = pevent; + return -EBUSY; } - return ret; + perf_events[cpu][event] = pevent; + + return 0; } static void op_destroy_counter(int cpu, int event) -- cgit v1.2.3 From e9677b3ce207a07fad5746b6f7ddc70cae79de0a Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Wed, 29 Sep 2010 15:42:30 +0200 Subject: oprofile, ARM: Use oprofile_arch_exit() to cleanup on failure There is duplicate cleanup code in the init and exit functions. Now, oprofile_arch_exit() is also used if oprofile_arch_init() fails. Acked-by: Will Deacon Signed-off-by: Robert Richter --- drivers/oprofile/oprofile_perf.c | 54 +++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 28 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/oprofile_perf.c b/drivers/oprofile/oprofile_perf.c index a34137f2e26c..b17235a24a4d 100644 --- a/drivers/oprofile/oprofile_perf.c +++ b/drivers/oprofile/oprofile_perf.c @@ -245,10 +245,33 @@ static int __init init_driverfs(void) { return 0; } #define exit_driverfs() do { } while (0) #endif /* CONFIG_PM */ +void oprofile_perf_exit(void) +{ + int cpu, id; + struct perf_event *event; + + for_each_possible_cpu(cpu) { + for (id = 0; id < num_counters; ++id) { + event = perf_events[cpu][id]; + if (event) + perf_event_release_kernel(event); + } + + kfree(perf_events[cpu]); + } + + kfree(counter_config); + exit_driverfs(); +} + int __init oprofile_perf_init(struct oprofile_operations *ops) { int cpu, ret = 0; + ret = init_driverfs(); + if (ret) + return ret; + memset(&perf_events, 0, sizeof(perf_events)); num_counters = perf_num_counters(); @@ -265,13 +288,10 @@ int __init oprofile_perf_init(struct oprofile_operations *ops) pr_info("oprofile: failed to allocate %d " "counters\n", num_counters); ret = -ENOMEM; + num_counters = 0; goto out; } - ret = init_driverfs(); - if (ret) - goto out; - for_each_possible_cpu(cpu) { perf_events[cpu] = kcalloc(num_counters, sizeof(struct perf_event *), GFP_KERNEL); @@ -296,30 +316,8 @@ int __init oprofile_perf_init(struct oprofile_operations *ops) pr_info("oprofile: using %s\n", ops->cpu_type); out: - if (ret) { - for_each_possible_cpu(cpu) - kfree(perf_events[cpu]); - kfree(counter_config); - } + if (ret) + oprofile_perf_exit(); return ret; } - -void __exit oprofile_perf_exit(void) -{ - int cpu, id; - struct perf_event *event; - - for_each_possible_cpu(cpu) { - for (id = 0; id < num_counters; ++id) { - event = perf_events[cpu][id]; - if (event) - perf_event_release_kernel(event); - } - - kfree(perf_events[cpu]); - } - - kfree(counter_config); - exit_driverfs(); -} -- cgit v1.2.3 From 7df01d96b295e400167e78061b81d4c91630b12d Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Mon, 4 Oct 2010 21:09:36 +0200 Subject: oprofile: disable write access to oprofilefs while profiler is running Oprofile counters are setup when profiling is disabled. Thus, writing to oprofilefs has no immediate effect. Changes are updated only after oprofile is reenabled. To keep userland and kernel states synchronized, we now allow configuration of oprofile only if profiling is disabled. In this case it checks if the profiler is running and then disables write access to oprofilefs by returning -EBUSY. The change should be backward compatible with current oprofile userland daemon. Acked-by: Maynard Johnson Cc: William Cohen Cc: Suravee Suthikulpanit Signed-off-by: Robert Richter --- drivers/oprofile/oprof.c | 21 ++++++--------------- drivers/oprofile/oprof.h | 2 +- drivers/oprofile/oprofile_files.c | 7 +++++-- drivers/oprofile/oprofilefs.c | 8 ++++++-- 4 files changed, 18 insertions(+), 20 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/oprof.c b/drivers/oprofile/oprof.c index b4a685719dba..f9bda64fcd1b 100644 --- a/drivers/oprofile/oprof.c +++ b/drivers/oprofile/oprof.c @@ -225,26 +225,17 @@ post_sync: mutex_unlock(&start_mutex); } -int oprofile_set_backtrace(unsigned long val) +int oprofile_set_ulong(unsigned long *addr, unsigned long val) { - int err = 0; + int err = -EBUSY; mutex_lock(&start_mutex); - - if (oprofile_started) { - err = -EBUSY; - goto out; + if (!oprofile_started) { + *addr = val; + err = 0; } - - if (!oprofile_ops.backtrace) { - err = -EINVAL; - goto out; - } - - oprofile_backtrace_depth = val; - -out: mutex_unlock(&start_mutex); + return err; } diff --git a/drivers/oprofile/oprof.h b/drivers/oprofile/oprof.h index 47e12cb4ee8b..177b73de5e5f 100644 --- a/drivers/oprofile/oprof.h +++ b/drivers/oprofile/oprof.h @@ -37,7 +37,7 @@ void oprofile_create_files(struct super_block *sb, struct dentry *root); int oprofile_timer_init(struct oprofile_operations *ops); void oprofile_timer_exit(void); -int oprofile_set_backtrace(unsigned long depth); +int oprofile_set_ulong(unsigned long *addr, unsigned long val); int oprofile_set_timeout(unsigned long time); #endif /* OPROF_H */ diff --git a/drivers/oprofile/oprofile_files.c b/drivers/oprofile/oprofile_files.c index bbd7516e0869..ccf099e684a4 100644 --- a/drivers/oprofile/oprofile_files.c +++ b/drivers/oprofile/oprofile_files.c @@ -79,14 +79,17 @@ static ssize_t depth_write(struct file *file, char const __user *buf, size_t cou if (*offset) return -EINVAL; + if (!oprofile_ops.backtrace) + return -EINVAL; + retval = oprofilefs_ulong_from_user(&val, buf, count); if (retval) return retval; - retval = oprofile_set_backtrace(val); - + retval = oprofile_set_ulong(&oprofile_backtrace_depth, val); if (retval) return retval; + return count; } diff --git a/drivers/oprofile/oprofilefs.c b/drivers/oprofile/oprofilefs.c index 789a1a857ddf..1944621930d9 100644 --- a/drivers/oprofile/oprofilefs.c +++ b/drivers/oprofile/oprofilefs.c @@ -91,16 +91,20 @@ static ssize_t ulong_read_file(struct file *file, char __user *buf, size_t count static ssize_t ulong_write_file(struct file *file, char const __user *buf, size_t count, loff_t *offset) { - unsigned long *value = file->private_data; + unsigned long value; int retval; if (*offset) return -EINVAL; - retval = oprofilefs_ulong_from_user(value, buf, count); + retval = oprofilefs_ulong_from_user(&value, buf, count); + if (retval) + return retval; + retval = oprofile_set_ulong(file->private_data, value); if (retval) return retval; + return count; } -- cgit v1.2.3 From 277dd984172b063497d2ff6cfa7f2355f13a292d Mon Sep 17 00:00:00 2001 From: Anand Gadiyar Date: Thu, 14 Oct 2010 11:31:43 -0400 Subject: oprofile: include platform_device.h to fix build break oprofile_perf.c needs to include platform_device.h Otherwise we get the following build break. CC arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.o arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:192: warning: 'struct platform_device' declared inside parameter list arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:192: warning: its scope is only this definition or declaration, which is probably not what you want arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:201: warning: 'struct platform_device' declared inside parameter list arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:210: error: variable 'oprofile_driver' has initializer but incomplete type arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:211: error: unknown field 'driver' specified in initializer arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:211: error: extra brace group at end of initializer arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:211: error: (near initialization for 'oprofile_driver') arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:213: warning: excess elements in struct initializer arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:213: warning: (near initialization for 'oprofile_driver') arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:214: error: unknown field 'resume' specified in initializer arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:214: warning: excess elements in struct initializer arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:214: warning: (near initialization for 'oprofile_driver') arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:215: error: unknown field 'suspend' specified in initializer arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:215: warning: excess elements in struct initializer arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:215: warning: (near initialization for 'oprofile_driver') arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c: In function 'init_driverfs': Signed-off-by: Anand Gadiyar Cc: Matt Fleming Cc: Will Deacon Signed-off-by: Robert Richter --- drivers/oprofile/oprofile_perf.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/oprofile/oprofile_perf.c b/drivers/oprofile/oprofile_perf.c index 79c0005134a1..e707a72a3799 100644 --- a/drivers/oprofile/oprofile_perf.c +++ b/drivers/oprofile/oprofile_perf.c @@ -4,6 +4,7 @@ * Perf-events backend for OProfile. */ #include +#include #include #include -- cgit v1.2.3 From b3b3a9b63f2deacfd59137e3781211d21a568ca9 Mon Sep 17 00:00:00 2001 From: Anand Gadiyar Date: Thu, 14 Oct 2010 11:31:42 -0400 Subject: oprofile: fix linker errors Commit e9677b3ce (oprofile, ARM: Use oprofile_arch_exit() to cleanup on failure) caused oprofile_perf_exit to be called in the cleanup path of oprofile_perf_init. The __exit tag for oprofile_perf_exit should therefore be dropped. The same has to be done for exit_driverfs as well, as this function is called from oprofile_perf_exit. Else, we get the following two linker errors. LD .tmp_vmlinux1 `oprofile_perf_exit' referenced in section `.init.text' of arch/arm/oprofile/built-in.o: defined in discarded section `.exit.text' of arch/arm/oprofile/built-in.o make: *** [.tmp_vmlinux1] Error 1 LD .tmp_vmlinux1 `exit_driverfs' referenced in section `.text' of arch/arm/oprofile/built-in.o: defined in discarded section `.exit.text' of arch/arm/oprofile/built-in.o make: *** [.tmp_vmlinux1] Error 1 Signed-off-by: Anand Gadiyar Cc: Will Deacon Signed-off-by: Robert Richter --- drivers/oprofile/oprofile_perf.c | 2 +- include/linux/oprofile.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/oprofile_perf.c b/drivers/oprofile/oprofile_perf.c index e707a72a3799..36ec67e3d91d 100644 --- a/drivers/oprofile/oprofile_perf.c +++ b/drivers/oprofile/oprofile_perf.c @@ -236,7 +236,7 @@ static int __init init_driverfs(void) return ret; } -static void __exit exit_driverfs(void) +static void exit_driverfs(void) { platform_device_unregister(oprofile_pdev); platform_driver_unregister(&oprofile_driver); diff --git a/include/linux/oprofile.h b/include/linux/oprofile.h index d67a8330b41e..32fb81212fd1 100644 --- a/include/linux/oprofile.h +++ b/include/linux/oprofile.h @@ -188,7 +188,7 @@ int oprofile_write_commit(struct op_entry *entry); #ifdef CONFIG_PERF_EVENTS int __init oprofile_perf_init(struct oprofile_operations *ops); -void __exit oprofile_perf_exit(void); +void oprofile_perf_exit(void); char *op_name_from_perf_id(void); #endif /* CONFIG_PERF_EVENTS */ -- cgit v1.2.3 From cd254f295248c98b62ea824f361e10d80a081fe7 Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Fri, 15 Oct 2010 11:28:07 +0200 Subject: oprofile: make !CONFIG_PM function stubs static inline Make !CONFIG_PM function stubs static inline and remove section attribute. Signed-off-by: Robert Richter --- drivers/oprofile/oprofile_perf.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/oprofile/oprofile_perf.c b/drivers/oprofile/oprofile_perf.c index 36ec67e3d91d..9046f7b2ed79 100644 --- a/drivers/oprofile/oprofile_perf.c +++ b/drivers/oprofile/oprofile_perf.c @@ -190,6 +190,7 @@ static void oprofile_perf_stop(void) } #ifdef CONFIG_PM + static int oprofile_perf_suspend(struct platform_device *dev, pm_message_t state) { mutex_lock(&oprofile_perf_mutex); @@ -241,9 +242,12 @@ static void exit_driverfs(void) platform_device_unregister(oprofile_pdev); platform_driver_unregister(&oprofile_driver); } + #else -static int __init init_driverfs(void) { return 0; } -#define exit_driverfs() do { } while (0) + +static inline int init_driverfs(void) { return 0; } +static inline void exit_driverfs(void) { } + #endif /* CONFIG_PM */ void oprofile_perf_exit(void) -- cgit v1.2.3