From 8f5a2dd83a1f8e89fdc17eb0f2f07c2e713e635a Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Tue, 23 Mar 2010 19:09:51 +0100 Subject: oprofile/x86: rework error handler in nmi_setup() This patch improves the error handler in nmi_setup(). Most parts of the code are moved to allocate_msrs(). In case of an error allocate_msrs() also frees already allocated memory. nmi_setup() becomes easier and better extendable. Signed-off-by: Robert Richter --- arch/x86/oprofile/nmi_int.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) (limited to 'arch/x86/oprofile/nmi_int.c') diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c index 2c505ee71014..c0c21f200faf 100644 --- a/arch/x86/oprofile/nmi_int.c +++ b/arch/x86/oprofile/nmi_int.c @@ -295,6 +295,7 @@ static void free_msrs(void) kfree(per_cpu(cpu_msrs, i).controls); per_cpu(cpu_msrs, i).controls = NULL; } + nmi_shutdown_mux(); } static int allocate_msrs(void) @@ -307,14 +308,21 @@ static int allocate_msrs(void) per_cpu(cpu_msrs, i).counters = kzalloc(counters_size, GFP_KERNEL); if (!per_cpu(cpu_msrs, i).counters) - return 0; + goto fail; per_cpu(cpu_msrs, i).controls = kzalloc(controls_size, GFP_KERNEL); if (!per_cpu(cpu_msrs, i).controls) - return 0; + goto fail; } + if (!nmi_setup_mux()) + goto fail; + return 1; + +fail: + free_msrs(); + return 0; } static void nmi_cpu_setup(void *dummy) @@ -342,17 +350,7 @@ static int nmi_setup(void) int cpu; if (!allocate_msrs()) - err = -ENOMEM; - else if (!nmi_setup_mux()) - err = -ENOMEM; - else - err = register_die_notifier(&profile_exceptions_nb); - - if (err) { - free_msrs(); - nmi_shutdown_mux(); - return err; - } + return -ENOMEM; /* We need to serialize save and setup for HT because the subset * of msrs are distinct for save and setup operations @@ -374,9 +372,17 @@ static int nmi_setup(void) mux_clone(cpu); } + + err = register_die_notifier(&profile_exceptions_nb); + if (err) + goto fail; + on_each_cpu(nmi_cpu_setup, NULL, 1); nmi_enabled = 1; return 0; +fail: + free_msrs(); + return err; } static void nmi_cpu_restore_registers(struct op_msrs *msrs) @@ -421,7 +427,6 @@ static void nmi_shutdown(void) nmi_enabled = 0; on_each_cpu(nmi_cpu_shutdown, NULL, 1); unregister_die_notifier(&profile_exceptions_nb); - nmi_shutdown_mux(); msrs = &get_cpu_var(cpu_msrs); model->shutdown(msrs); free_msrs(); -- cgit v1.2.3 From 8617f98c001d00b176422d707e6a67b88bcd7e0d Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Fri, 26 Feb 2010 17:20:55 +0100 Subject: oprofile/x86: return -EBUSY if counters are already reserved In case a counter is already reserved by the watchdog or perf_event subsystem, oprofile ignored this counters silently. This case is handled now and oprofile_setup() now reports an error. Signed-off-by: Robert Richter --- arch/x86/oprofile/nmi_int.c | 5 ++++- arch/x86/oprofile/op_model_amd.c | 24 +++++++++++++----------- arch/x86/oprofile/op_model_p4.c | 14 +++++++++++++- arch/x86/oprofile/op_model_ppro.c | 24 +++++++++++++----------- arch/x86/oprofile/op_x86_model.h | 2 +- 5 files changed, 44 insertions(+), 25 deletions(-) (limited to 'arch/x86/oprofile/nmi_int.c') diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c index c0c21f200faf..9f001d904599 100644 --- a/arch/x86/oprofile/nmi_int.c +++ b/arch/x86/oprofile/nmi_int.c @@ -357,7 +357,10 @@ static int nmi_setup(void) */ /* Assume saved/restored counters are the same on all CPUs */ - model->fill_in_addresses(&per_cpu(cpu_msrs, 0)); + err = model->fill_in_addresses(&per_cpu(cpu_msrs, 0)); + if (err) + goto fail; + for_each_possible_cpu(cpu) { if (!cpu) continue; diff --git a/arch/x86/oprofile/op_model_amd.c b/arch/x86/oprofile/op_model_amd.c index 7e5886d54bd5..536d0b0b39a5 100644 --- a/arch/x86/oprofile/op_model_amd.c +++ b/arch/x86/oprofile/op_model_amd.c @@ -138,21 +138,30 @@ static void op_amd_shutdown(struct op_msrs const * const msrs) } } -static void op_amd_fill_in_addresses(struct op_msrs * const msrs) +static int op_amd_fill_in_addresses(struct op_msrs * const msrs) { int i; for (i = 0; i < NUM_COUNTERS; i++) { if (!reserve_perfctr_nmi(MSR_K7_PERFCTR0 + i)) - continue; + goto fail; if (!reserve_evntsel_nmi(MSR_K7_EVNTSEL0 + i)) { release_perfctr_nmi(MSR_K7_PERFCTR0 + i); - continue; + goto fail; } /* both registers must be reserved */ msrs->counters[i].addr = MSR_K7_PERFCTR0 + i; msrs->controls[i].addr = MSR_K7_EVNTSEL0 + i; + continue; + fail: + if (!counter_config[i].enabled) + continue; + op_x86_warn_reserved(i); + op_amd_shutdown(msrs); + return -EBUSY; } + + return 0; } static void op_amd_setup_ctrs(struct op_x86_model_spec const *model, @@ -172,15 +181,8 @@ static void op_amd_setup_ctrs(struct op_x86_model_spec const *model, /* clear all counters */ for (i = 0; i < NUM_COUNTERS; ++i) { - if (unlikely(!msrs->controls[i].addr)) { - if (counter_config[i].enabled && !smp_processor_id()) - /* - * counter is reserved, this is on all - * cpus, so report only for cpu #0 - */ - op_x86_warn_reserved(i); + if (!msrs->controls[i].addr) continue; - } rdmsrl(msrs->controls[i].addr, val); if (val & ARCH_PERFMON_EVENTSEL_ENABLE) op_x86_warn_in_use(i); diff --git a/arch/x86/oprofile/op_model_p4.c b/arch/x86/oprofile/op_model_p4.c index 7cc80df330d5..182558dd5515 100644 --- a/arch/x86/oprofile/op_model_p4.c +++ b/arch/x86/oprofile/op_model_p4.c @@ -404,7 +404,7 @@ static void p4_shutdown(struct op_msrs const * const msrs) } } -static void p4_fill_in_addresses(struct op_msrs * const msrs) +static int p4_fill_in_addresses(struct op_msrs * const msrs) { unsigned int i; unsigned int addr, cccraddr, stag; @@ -486,6 +486,18 @@ static void p4_fill_in_addresses(struct op_msrs * const msrs) msrs->controls[i++].addr = MSR_P4_CRU_ESCR5; } } + + for (i = 0; i < num_counters; ++i) { + if (!counter_config[i].enabled) + continue; + if (msrs->controls[i].addr) + continue; + op_x86_warn_reserved(i); + p4_shutdown(msrs); + return -EBUSY; + } + + return 0; } diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c index b07d25a52f02..1fd17cfb956b 100644 --- a/arch/x86/oprofile/op_model_ppro.c +++ b/arch/x86/oprofile/op_model_ppro.c @@ -46,21 +46,30 @@ static void ppro_shutdown(struct op_msrs const * const msrs) } } -static void ppro_fill_in_addresses(struct op_msrs * const msrs) +static int ppro_fill_in_addresses(struct op_msrs * const msrs) { int i; for (i = 0; i < num_counters; i++) { if (!reserve_perfctr_nmi(MSR_P6_PERFCTR0 + i)) - continue; + goto fail; if (!reserve_evntsel_nmi(MSR_P6_EVNTSEL0 + i)) { release_perfctr_nmi(MSR_P6_PERFCTR0 + i); - continue; + goto fail; } /* both registers must be reserved */ msrs->counters[i].addr = MSR_P6_PERFCTR0 + i; msrs->controls[i].addr = MSR_P6_EVNTSEL0 + i; + continue; + fail: + if (!counter_config[i].enabled) + continue; + op_x86_warn_reserved(i); + ppro_shutdown(msrs); + return -EBUSY; } + + return 0; } @@ -96,15 +105,8 @@ static void ppro_setup_ctrs(struct op_x86_model_spec const *model, /* clear all counters */ for (i = 0; i < num_counters; ++i) { - if (unlikely(!msrs->controls[i].addr)) { - if (counter_config[i].enabled && !smp_processor_id()) - /* - * counter is reserved, this is on all - * cpus, so report only for cpu #0 - */ - op_x86_warn_reserved(i); + if (!msrs->controls[i].addr) continue; - } rdmsrl(msrs->controls[i].addr, val); if (val & ARCH_PERFMON_EVENTSEL_ENABLE) op_x86_warn_in_use(i); diff --git a/arch/x86/oprofile/op_x86_model.h b/arch/x86/oprofile/op_x86_model.h index ff82a755edd4..551401398fba 100644 --- a/arch/x86/oprofile/op_x86_model.h +++ b/arch/x86/oprofile/op_x86_model.h @@ -41,7 +41,7 @@ struct op_x86_model_spec { u16 event_mask; int (*init)(struct oprofile_operations *ops); void (*exit)(void); - void (*fill_in_addresses)(struct op_msrs * const msrs); + int (*fill_in_addresses)(struct op_msrs * const msrs); void (*setup_ctrs)(struct op_x86_model_spec const *model, struct op_msrs const * const msrs); int (*check_ctrs)(struct pt_regs * const regs, -- cgit v1.2.3 From 2623a1d55a6260c855e1f6d1895900b50b40a896 Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Mon, 3 May 2010 19:44:32 +0200 Subject: oprofile/x86: fix uninitialized counter usage during cpu hotplug This fixes a NULL pointer dereference that is triggered when taking a cpu offline after oprofile was initialized, e.g.: $ opcontrol --init $ opcontrol --start-daemon $ opcontrol --shutdown $ opcontrol --deinit $ echo 0 > /sys/devices/system/cpu/cpu1/online See the crash dump below. Though the counter has been disabled the cpu notifier is still active and trying to use already freed counter data. This fix is for linux-stable. To proper fix this, the hotplug code must be rewritten. Thus I will leave a WARN_ON_ONCE() message with this patch. BUG: unable to handle kernel NULL pointer dereference at (null) IP: [] op_amd_stop+0x2d/0x8e PGD 0 Oops: 0000 [#1] SMP last sysfs file: /sys/devices/system/cpu/cpu1/online CPU 1 Modules linked in: Pid: 0, comm: swapper Not tainted 2.6.34-rc5-oprofile-x86_64-standard-00210-g8c00f06 #16 Anaheim/Anaheim RIP: 0010:[] [] op_amd_stop+0x2d/0x8e RSP: 0018:ffff880001843f28 EFLAGS: 00010006 RAX: 0000000000000000 RBX: 0000000000000000 RCX: dead000000200200 RDX: ffff880001843f68 RSI: dead000000100100 RDI: 0000000000000000 RBP: ffff880001843f48 R08: 0000000000000000 R09: ffff880001843f08 R10: ffffffff8102c9a5 R11: ffff88000184ea80 R12: 0000000000000000 R13: ffff88000184f6c0 R14: 0000000000000000 R15: 0000000000000000 FS: 00007fec6a92e6f0(0000) GS:ffff880001840000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b CR2: 0000000000000000 CR3: 000000000163b000 CR4: 00000000000006e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 Process swapper (pid: 0, threadinfo ffff88042fcd8000, task ffff88042fcd51d0) Stack: ffff880001843f48 0000000000000001 ffff88042e9f7d38 ffff880001843f68 <0> ffff880001843f58 ffffffff8132a602 ffff880001843f98 ffffffff810521b3 <0> ffff880001843f68 ffff880001843f68 ffff880001843f88 ffff88042fcd9fd8 Call Trace: [] nmi_cpu_stop+0x21/0x23 [] generic_smp_call_function_single_interrupt+0xdf/0x11b [] smp_call_function_single_interrupt+0x22/0x31 [] call_function_single_interrupt+0x13/0x20 [] ? wake_up_process+0x10/0x12 [] ? default_idle+0x22/0x37 [] c1e_idle+0xdf/0xe6 [] ? atomic_notifier_call_chain+0x13/0x15 [] cpu_idle+0x4b/0x7e [] start_secondary+0x1ae/0x1b2 Code: 89 e5 41 55 49 89 fd 41 54 45 31 e4 53 31 db 48 83 ec 08 89 df e8 be f8 ff ff 48 98 48 83 3c c5 10 67 7a 81 00 74 1f 49 8b 45 08 <42> 8b 0c 20 0f 32 48 c1 e2 20 25 ff ff bf ff 48 09 d0 48 89 c2 RIP [] op_amd_stop+0x2d/0x8e RSP CR2: 0000000000000000 ---[ end trace 679ac372d674b757 ]--- Kernel panic - not syncing: Fatal exception in interrupt Pid: 0, comm: swapper Tainted: G D 2.6.34-rc5-oprofile-x86_64-standard-00210-g8c00f06 #16 Call Trace: [] panic+0x9e/0x10c [] ? up+0x34/0x39 [] ? kmsg_dump+0x112/0x12c [] oops_end+0x81/0x8e [] no_context+0x1f3/0x202 [] __bad_area_nosemaphore+0x1ba/0x1e0 [] ? enqueue_task_fair+0x16d/0x17a [] ? activate_task+0x42/0x53 [] ? try_to_wake_up+0x272/0x284 [] bad_area_nosemaphore+0xe/0x10 [] do_page_fault+0x1c8/0x37c [] ? enqueue_task_fair+0x16d/0x17a [] page_fault+0x1f/0x30 [] ? wake_up_process+0x10/0x12 [] ? op_amd_stop+0x2d/0x8e [] ? op_amd_stop+0x1c/0x8e [] nmi_cpu_stop+0x21/0x23 [] generic_smp_call_function_single_interrupt+0xdf/0x11b [] smp_call_function_single_interrupt+0x22/0x31 [] call_function_single_interrupt+0x13/0x20 [] ? wake_up_process+0x10/0x12 [] ? default_idle+0x22/0x37 [] c1e_idle+0xdf/0xe6 [] ? atomic_notifier_call_chain+0x13/0x15 [] cpu_idle+0x4b/0x7e [] start_secondary+0x1ae/0x1b2 ------------[ cut here ]------------ WARNING: at /local/rrichter/.source/linux/arch/x86/kernel/smp.c:118 native_smp_send_reschedule+0x27/0x53() Hardware name: Anaheim Modules linked in: Pid: 0, comm: swapper Tainted: G D 2.6.34-rc5-oprofile-x86_64-standard-00210-g8c00f06 #16 Call Trace: [] ? native_smp_send_reschedule+0x27/0x53 [] warn_slowpath_common+0x77/0xa4 [] warn_slowpath_null+0xf/0x11 [] native_smp_send_reschedule+0x27/0x53 [] resched_task+0x60/0x62 [] check_preempt_curr_idle+0x10/0x12 [] try_to_wake_up+0x1f5/0x284 [] default_wake_function+0xd/0xf [] pollwake+0x57/0x5a [] ? default_wake_function+0x0/0xf [] __wake_up_common+0x46/0x75 [] __wake_up+0x38/0x50 [] printk_tick+0x39/0x3b [] update_process_times+0x3f/0x5c [] tick_periodic+0x5d/0x69 [] tick_handle_periodic+0x21/0x71 [] smp_apic_timer_interrupt+0x82/0x95 [] apic_timer_interrupt+0x13/0x20 [] ? panic_blink_one_second+0x0/0x7b [] ? panic+0x10a/0x10c [] ? up+0x34/0x39 [] ? kmsg_dump+0x112/0x12c [] ? oops_end+0x81/0x8e [] ? no_context+0x1f3/0x202 [] ? __bad_area_nosemaphore+0x1ba/0x1e0 [] ? enqueue_task_fair+0x16d/0x17a [] ? activate_task+0x42/0x53 [] ? try_to_wake_up+0x272/0x284 [] ? bad_area_nosemaphore+0xe/0x10 [] ? do_page_fault+0x1c8/0x37c [] ? enqueue_task_fair+0x16d/0x17a [] ? page_fault+0x1f/0x30 [] ? wake_up_process+0x10/0x12 [] ? op_amd_stop+0x2d/0x8e [] ? op_amd_stop+0x1c/0x8e [] ? nmi_cpu_stop+0x21/0x23 [] ? generic_smp_call_function_single_interrupt+0xdf/0x11b [] ? smp_call_function_single_interrupt+0x22/0x31 [] ? call_function_single_interrupt+0x13/0x20 [] ? wake_up_process+0x10/0x12 [] ? default_idle+0x22/0x37 [] ? c1e_idle+0xdf/0xe6 [] ? atomic_notifier_call_chain+0x13/0x15 [] ? cpu_idle+0x4b/0x7e [] ? start_secondary+0x1ae/0x1b2 ---[ end trace 679ac372d674b758 ]--- Cc: Andi Kleen Cc: stable Signed-off-by: Robert Richter --- arch/x86/oprofile/nmi_int.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'arch/x86/oprofile/nmi_int.c') diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c index 9f001d904599..24582040b718 100644 --- a/arch/x86/oprofile/nmi_int.c +++ b/arch/x86/oprofile/nmi_int.c @@ -95,7 +95,10 @@ static void nmi_cpu_save_registers(struct op_msrs *msrs) static void nmi_cpu_start(void *dummy) { struct op_msrs const *msrs = &__get_cpu_var(cpu_msrs); - model->start(msrs); + if (!msrs->controls) + WARN_ON_ONCE(1); + else + model->start(msrs); } static int nmi_start(void) @@ -107,7 +110,10 @@ static int nmi_start(void) static void nmi_cpu_stop(void *dummy) { struct op_msrs const *msrs = &__get_cpu_var(cpu_msrs); - model->stop(msrs); + if (!msrs->controls) + WARN_ON_ONCE(1); + else + model->stop(msrs); } static void nmi_stop(void) -- cgit v1.2.3 From 216f3d9b4e5121feea4b13fae9d4c83e8d7e1c8a Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Mon, 3 May 2010 11:58:46 +0200 Subject: oprofile/x86: remove CONFIG_SMP macros CPU notifier register functions also exist if CONFIG_SMP is disabled. This change is part of hotplug code rework and also necessary for later patches. Cc: Andi Kleen Signed-off-by: Robert Richter --- arch/x86/oprofile/nmi_int.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'arch/x86/oprofile/nmi_int.c') diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c index 24582040b718..c5df8ee76ee4 100644 --- a/arch/x86/oprofile/nmi_int.c +++ b/arch/x86/oprofile/nmi_int.c @@ -471,7 +471,6 @@ static int nmi_create_files(struct super_block *sb, struct dentry *root) return 0; } -#ifdef CONFIG_SMP static int oprofile_cpu_notifier(struct notifier_block *b, unsigned long action, void *data) { @@ -491,7 +490,6 @@ static int oprofile_cpu_notifier(struct notifier_block *b, unsigned long action, static struct notifier_block oprofile_cpu_nb = { .notifier_call = oprofile_cpu_notifier }; -#endif #ifdef CONFIG_PM @@ -701,9 +699,8 @@ int __init op_nmi_init(struct oprofile_operations *ops) return -ENODEV; } -#ifdef CONFIG_SMP register_cpu_notifier(&oprofile_cpu_nb); -#endif + /* default values, can be overwritten by model */ ops->create_files = nmi_create_files; ops->setup = nmi_setup; @@ -732,9 +729,7 @@ void op_nmi_exit(void) { if (using_nmi) { exit_sysfs(); -#ifdef CONFIG_SMP unregister_cpu_notifier(&oprofile_cpu_nb); -#endif } if (model->exit) model->exit(); -- cgit v1.2.3 From 6ae56b55bc364bc2f2342f599b46581627ba22da Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Thu, 29 Apr 2010 14:55:55 +0200 Subject: oprofile/x86: protect cpu hotplug sections This patch reworks oprofile cpu hotplug code as follows: Introduce ctr_running variable to check, if counters are running or not. The state must be known for taking a cpu on or offline and when switching counters during counter multiplexing. Protect on_each_cpu() sections with get_online_cpus()/put_online_cpu() functions. This is necessary if notifiers or states are modified. Within these sections the cpu mask may not change. Switch only between counters in nmi_cpu_switch(), if counters are running. Otherwise the switch may restart a counter though they are disabled. Add nmi_cpu_setup() and nmi_cpu_shutdown() to cpu hotplug code. The function must also be called to avoid uninitialzed counter usage. Cc: Andi Kleen Signed-off-by: Robert Richter --- arch/x86/oprofile/nmi_int.c | 52 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 6 deletions(-) (limited to 'arch/x86/oprofile/nmi_int.c') diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c index c5df8ee76ee4..b56601eaf29d 100644 --- a/arch/x86/oprofile/nmi_int.c +++ b/arch/x86/oprofile/nmi_int.c @@ -31,8 +31,9 @@ static struct op_x86_model_spec *model; static DEFINE_PER_CPU(struct op_msrs, cpu_msrs); static DEFINE_PER_CPU(unsigned long, saved_lvtpc); -/* 0 == registered but off, 1 == registered and on */ -static int nmi_enabled = 0; +/* must be protected with get_online_cpus()/put_online_cpus(): */ +static int nmi_enabled; +static int ctr_running; struct op_counter_config counter_config[OP_MAX_COUNTER]; @@ -103,7 +104,10 @@ static void nmi_cpu_start(void *dummy) static int nmi_start(void) { + get_online_cpus(); on_each_cpu(nmi_cpu_start, NULL, 1); + ctr_running = 1; + put_online_cpus(); return 0; } @@ -118,7 +122,10 @@ static void nmi_cpu_stop(void *dummy) static void nmi_stop(void) { + get_online_cpus(); on_each_cpu(nmi_cpu_stop, NULL, 1); + ctr_running = 0; + put_online_cpus(); } #ifdef CONFIG_OPROFILE_EVENT_MULTIPLEX @@ -258,7 +265,10 @@ static int nmi_switch_event(void) if (nmi_multiplex_on() < 0) return -EINVAL; /* not necessary */ - on_each_cpu(nmi_cpu_switch, NULL, 1); + get_online_cpus(); + if (ctr_running) + on_each_cpu(nmi_cpu_switch, NULL, 1); + put_online_cpus(); return 0; } @@ -386,8 +396,11 @@ static int nmi_setup(void) if (err) goto fail; + get_online_cpus(); on_each_cpu(nmi_cpu_setup, NULL, 1); nmi_enabled = 1; + put_online_cpus(); + return 0; fail: free_msrs(); @@ -433,8 +446,11 @@ static void nmi_shutdown(void) { struct op_msrs *msrs; - nmi_enabled = 0; + get_online_cpus(); on_each_cpu(nmi_cpu_shutdown, NULL, 1); + nmi_enabled = 0; + ctr_running = 0; + put_online_cpus(); unregister_die_notifier(&profile_exceptions_nb); msrs = &get_cpu_var(cpu_msrs); model->shutdown(msrs); @@ -442,6 +458,22 @@ static void nmi_shutdown(void) put_cpu_var(cpu_msrs); } +static void nmi_cpu_up(void *dummy) +{ + if (nmi_enabled) + nmi_cpu_setup(dummy); + if (ctr_running) + nmi_cpu_start(dummy); +} + +static void nmi_cpu_down(void *dummy) +{ + if (ctr_running) + nmi_cpu_stop(dummy); + if (nmi_enabled) + nmi_cpu_shutdown(dummy); +} + static int nmi_create_files(struct super_block *sb, struct dentry *root) { unsigned int i; @@ -478,10 +510,10 @@ static int oprofile_cpu_notifier(struct notifier_block *b, unsigned long action, switch (action) { case CPU_DOWN_FAILED: case CPU_ONLINE: - smp_call_function_single(cpu, nmi_cpu_start, NULL, 0); + smp_call_function_single(cpu, nmi_cpu_up, NULL, 0); break; case CPU_DOWN_PREPARE: - smp_call_function_single(cpu, nmi_cpu_stop, NULL, 1); + smp_call_function_single(cpu, nmi_cpu_down, NULL, 1); break; } return NOTIFY_DONE; @@ -699,7 +731,11 @@ int __init op_nmi_init(struct oprofile_operations *ops) return -ENODEV; } + get_online_cpus(); register_cpu_notifier(&oprofile_cpu_nb); + nmi_enabled = 0; + ctr_running = 0; + put_online_cpus(); /* default values, can be overwritten by model */ ops->create_files = nmi_create_files; @@ -729,7 +765,11 @@ void op_nmi_exit(void) { if (using_nmi) { exit_sysfs(); + get_online_cpus(); unregister_cpu_notifier(&oprofile_cpu_nb); + nmi_enabled = 0; + ctr_running = 0; + put_online_cpus(); } if (model->exit) model->exit(); -- cgit v1.2.3 From de654649737696ecf32873c341b305e30f3dc777 Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Mon, 3 May 2010 14:41:22 +0200 Subject: oprofile/x86: stop disabled counters in nmi handler This patch adds checks to the nmi handler. Now samples are only generated and counters reenabled, if the counters are running. Otherwise the counters are stopped, if oprofile is using the nmi. In other cases it will ignore the nmi notification. Cc: Andi Kleen Signed-off-by: Robert Richter --- arch/x86/oprofile/nmi_int.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'arch/x86/oprofile/nmi_int.c') diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c index b56601eaf29d..94b5481bb6c6 100644 --- a/arch/x86/oprofile/nmi_int.c +++ b/arch/x86/oprofile/nmi_int.c @@ -62,12 +62,16 @@ static int profile_exceptions_notify(struct notifier_block *self, { struct die_args *args = (struct die_args *)data; int ret = NOTIFY_DONE; - int cpu = smp_processor_id(); switch (val) { case DIE_NMI: case DIE_NMI_IPI: - model->check_ctrs(args->regs, &per_cpu(cpu_msrs, cpu)); + if (ctr_running) + model->check_ctrs(args->regs, &__get_cpu_var(cpu_msrs)); + else if (!nmi_enabled) + break; + else + model->stop(&__get_cpu_var(cpu_msrs)); ret = NOTIFY_STOP; break; default: @@ -392,6 +396,9 @@ static int nmi_setup(void) mux_clone(cpu); } + nmi_enabled = 0; + ctr_running = 0; + barrier(); err = register_die_notifier(&profile_exceptions_nb); if (err) goto fail; @@ -451,6 +458,7 @@ static void nmi_shutdown(void) nmi_enabled = 0; ctr_running = 0; put_online_cpus(); + barrier(); unregister_die_notifier(&profile_exceptions_nb); msrs = &get_cpu_var(cpu_msrs); model->shutdown(msrs); -- cgit v1.2.3 From d30d64c6da3ec7a0708bfffa7e05752d5b9a1093 Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Mon, 3 May 2010 15:52:26 +0200 Subject: oprofile/x86: reordering some functions Reordering some functions. Necessary for the next patch. No functional changes. Cc: Andi Kleen Signed-off-by: Robert Richter --- arch/x86/oprofile/nmi_int.c | 134 ++++++++++++++++++++++---------------------- 1 file changed, 67 insertions(+), 67 deletions(-) (limited to 'arch/x86/oprofile/nmi_int.c') diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c index 94b5481bb6c6..7de0572b0a5e 100644 --- a/arch/x86/oprofile/nmi_int.c +++ b/arch/x86/oprofile/nmi_int.c @@ -364,56 +364,6 @@ static struct notifier_block profile_exceptions_nb = { .priority = 2 }; -static int nmi_setup(void) -{ - int err = 0; - int cpu; - - if (!allocate_msrs()) - return -ENOMEM; - - /* We need to serialize save and setup for HT because the subset - * of msrs are distinct for save and setup operations - */ - - /* Assume saved/restored counters are the same on all CPUs */ - err = model->fill_in_addresses(&per_cpu(cpu_msrs, 0)); - if (err) - goto fail; - - for_each_possible_cpu(cpu) { - if (!cpu) - continue; - - memcpy(per_cpu(cpu_msrs, cpu).counters, - per_cpu(cpu_msrs, 0).counters, - sizeof(struct op_msr) * model->num_counters); - - memcpy(per_cpu(cpu_msrs, cpu).controls, - per_cpu(cpu_msrs, 0).controls, - sizeof(struct op_msr) * model->num_controls); - - mux_clone(cpu); - } - - nmi_enabled = 0; - ctr_running = 0; - barrier(); - err = register_die_notifier(&profile_exceptions_nb); - if (err) - goto fail; - - get_online_cpus(); - on_each_cpu(nmi_cpu_setup, NULL, 1); - nmi_enabled = 1; - put_online_cpus(); - - return 0; -fail: - free_msrs(); - return err; -} - static void nmi_cpu_restore_registers(struct op_msrs *msrs) { struct op_msr *counters = msrs->counters; @@ -449,23 +399,6 @@ static void nmi_cpu_shutdown(void *dummy) nmi_cpu_restore_registers(msrs); } -static void nmi_shutdown(void) -{ - struct op_msrs *msrs; - - get_online_cpus(); - on_each_cpu(nmi_cpu_shutdown, NULL, 1); - nmi_enabled = 0; - ctr_running = 0; - put_online_cpus(); - barrier(); - unregister_die_notifier(&profile_exceptions_nb); - msrs = &get_cpu_var(cpu_msrs); - model->shutdown(msrs); - free_msrs(); - put_cpu_var(cpu_msrs); -} - static void nmi_cpu_up(void *dummy) { if (nmi_enabled) @@ -531,6 +464,73 @@ static struct notifier_block oprofile_cpu_nb = { .notifier_call = oprofile_cpu_notifier }; +static int nmi_setup(void) +{ + int err = 0; + int cpu; + + if (!allocate_msrs()) + return -ENOMEM; + + /* We need to serialize save and setup for HT because the subset + * of msrs are distinct for save and setup operations + */ + + /* Assume saved/restored counters are the same on all CPUs */ + err = model->fill_in_addresses(&per_cpu(cpu_msrs, 0)); + if (err) + goto fail; + + for_each_possible_cpu(cpu) { + if (!cpu) + continue; + + memcpy(per_cpu(cpu_msrs, cpu).counters, + per_cpu(cpu_msrs, 0).counters, + sizeof(struct op_msr) * model->num_counters); + + memcpy(per_cpu(cpu_msrs, cpu).controls, + per_cpu(cpu_msrs, 0).controls, + sizeof(struct op_msr) * model->num_controls); + + mux_clone(cpu); + } + + nmi_enabled = 0; + ctr_running = 0; + barrier(); + err = register_die_notifier(&profile_exceptions_nb); + if (err) + goto fail; + + get_online_cpus(); + on_each_cpu(nmi_cpu_setup, NULL, 1); + nmi_enabled = 1; + put_online_cpus(); + + return 0; +fail: + free_msrs(); + return err; +} + +static void nmi_shutdown(void) +{ + struct op_msrs *msrs; + + get_online_cpus(); + on_each_cpu(nmi_cpu_shutdown, NULL, 1); + nmi_enabled = 0; + ctr_running = 0; + put_online_cpus(); + barrier(); + unregister_die_notifier(&profile_exceptions_nb); + msrs = &get_cpu_var(cpu_msrs); + model->shutdown(msrs); + free_msrs(); + put_cpu_var(cpu_msrs); +} + #ifdef CONFIG_PM static int nmi_suspend(struct sys_device *dev, pm_message_t state) -- cgit v1.2.3 From 3de668ee8d5b1e08da3200f926ff5a28aeb99bc2 Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Mon, 3 May 2010 15:00:25 +0200 Subject: oprofile/x86: notify cpus only when daemon is running This patch moves the cpu notifier registration from nmi_init() to nmi_setup(). The corresponding unregistration function is now in nmi_shutdown(). Thus, the hotplug code is only active, if the oprofile daemon is running. Cc: Andi Kleen Signed-off-by: Robert Richter --- arch/x86/oprofile/nmi_int.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) (limited to 'arch/x86/oprofile/nmi_int.c') diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c index 7de0572b0a5e..2a086726cad1 100644 --- a/arch/x86/oprofile/nmi_int.c +++ b/arch/x86/oprofile/nmi_int.c @@ -504,6 +504,7 @@ static int nmi_setup(void) goto fail; get_online_cpus(); + register_cpu_notifier(&oprofile_cpu_nb); on_each_cpu(nmi_cpu_setup, NULL, 1); nmi_enabled = 1; put_online_cpus(); @@ -519,6 +520,7 @@ static void nmi_shutdown(void) struct op_msrs *msrs; get_online_cpus(); + unregister_cpu_notifier(&oprofile_cpu_nb); on_each_cpu(nmi_cpu_shutdown, NULL, 1); nmi_enabled = 0; ctr_running = 0; @@ -739,12 +741,6 @@ int __init op_nmi_init(struct oprofile_operations *ops) return -ENODEV; } - get_online_cpus(); - register_cpu_notifier(&oprofile_cpu_nb); - nmi_enabled = 0; - ctr_running = 0; - put_online_cpus(); - /* default values, can be overwritten by model */ ops->create_files = nmi_create_files; ops->setup = nmi_setup; @@ -771,14 +767,8 @@ int __init op_nmi_init(struct oprofile_operations *ops) void op_nmi_exit(void) { - if (using_nmi) { + if (using_nmi) exit_sysfs(); - get_online_cpus(); - unregister_cpu_notifier(&oprofile_cpu_nb); - nmi_enabled = 0; - ctr_running = 0; - put_online_cpus(); - } if (model->exit) model->exit(); } -- cgit v1.2.3 From bae663bc635e2726c7c5228dbf0f2051e16d1c81 Mon Sep 17 00:00:00 2001 From: Robert Richter Date: Wed, 5 May 2010 17:47:17 +0200 Subject: oprofile/x86: make AMD IBS hotplug capable Current IBS code is not hotplug capable. An offline cpu might not be initialized or deinitialized properly. This patch fixes this by removing on_each_cpu() functions. The IBS init/deinit code is executed in the per-cpu functions model->setup_ctrs() and model->cpu_down() which are also called by hotplug notifiers. model->cpu_down() replaces model->exit() that became obsolete. Cc: Andi Kleen Signed-off-by: Robert Richter --- arch/x86/oprofile/nmi_int.c | 4 +-- arch/x86/oprofile/op_model_amd.c | 54 ++++++++++++---------------------------- arch/x86/oprofile/op_x86_model.h | 2 +- 3 files changed, 19 insertions(+), 41 deletions(-) (limited to 'arch/x86/oprofile/nmi_int.c') diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c index 2a086726cad1..b28d2f1253bb 100644 --- a/arch/x86/oprofile/nmi_int.c +++ b/arch/x86/oprofile/nmi_int.c @@ -397,6 +397,8 @@ static void nmi_cpu_shutdown(void *dummy) apic_write(APIC_LVTPC, per_cpu(saved_lvtpc, cpu)); apic_write(APIC_LVTERR, v); nmi_cpu_restore_registers(msrs); + if (model->cpu_down) + model->cpu_down(); } static void nmi_cpu_up(void *dummy) @@ -769,6 +771,4 @@ void op_nmi_exit(void) { if (using_nmi) exit_sysfs(); - if (model->exit) - model->exit(); } diff --git a/arch/x86/oprofile/op_model_amd.c b/arch/x86/oprofile/op_model_amd.c index 384c52410480..b67a6b5aa8d4 100644 --- a/arch/x86/oprofile/op_model_amd.c +++ b/arch/x86/oprofile/op_model_amd.c @@ -374,6 +374,15 @@ static void op_amd_setup_ctrs(struct op_x86_model_spec const *model, val |= op_x86_get_ctrl(model, &counter_config[virt]); wrmsrl(msrs->controls[i].addr, val); } + + if (ibs_caps) + setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_NMI, 0); +} + +static void op_amd_cpu_shutdown(void) +{ + if (ibs_caps) + setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_FIX, 1); } static int op_amd_check_ctrs(struct pt_regs * const regs, @@ -436,28 +445,16 @@ static void op_amd_stop(struct op_msrs const * const msrs) op_amd_stop_ibs(); } -static u8 ibs_eilvt_off; - -static inline void apic_init_ibs_nmi_per_cpu(void *arg) -{ - ibs_eilvt_off = setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_NMI, 0); -} - -static inline void apic_clear_ibs_nmi_per_cpu(void *arg) -{ - setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_FIX, 1); -} - -static int init_ibs_nmi(void) +static int __init_ibs_nmi(void) { #define IBSCTL_LVTOFFSETVAL (1 << 8) #define IBSCTL 0x1cc struct pci_dev *cpu_cfg; int nodes; u32 value = 0; + u8 ibs_eilvt_off; - /* per CPU setup */ - on_each_cpu(apic_init_ibs_nmi_per_cpu, NULL, 1); + ibs_eilvt_off = setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_FIX, 1); nodes = 0; cpu_cfg = NULL; @@ -487,21 +484,15 @@ static int init_ibs_nmi(void) return 0; } -/* uninitialize the APIC for the IBS interrupts if needed */ -static void clear_ibs_nmi(void) -{ - on_each_cpu(apic_clear_ibs_nmi_per_cpu, NULL, 1); -} - /* initialize the APIC for the IBS interrupts if available */ -static void ibs_init(void) +static void init_ibs(void) { ibs_caps = get_ibs_caps(); if (!ibs_caps) return; - if (init_ibs_nmi()) { + if (__init_ibs_nmi()) { ibs_caps = 0; return; } @@ -510,14 +501,6 @@ static void ibs_init(void) (unsigned)ibs_caps); } -static void ibs_exit(void) -{ - if (!ibs_caps) - return; - - clear_ibs_nmi(); -} - static int (*create_arch_files)(struct super_block *sb, struct dentry *root); static int setup_ibs_files(struct super_block *sb, struct dentry *root) @@ -566,17 +549,12 @@ static int setup_ibs_files(struct super_block *sb, struct dentry *root) static int op_amd_init(struct oprofile_operations *ops) { - ibs_init(); + init_ibs(); create_arch_files = ops->create_files; ops->create_files = setup_ibs_files; return 0; } -static void op_amd_exit(void) -{ - ibs_exit(); -} - struct op_x86_model_spec op_amd_spec = { .num_counters = NUM_COUNTERS, .num_controls = NUM_COUNTERS, @@ -584,9 +562,9 @@ struct op_x86_model_spec op_amd_spec = { .reserved = MSR_AMD_EVENTSEL_RESERVED, .event_mask = OP_EVENT_MASK, .init = op_amd_init, - .exit = op_amd_exit, .fill_in_addresses = &op_amd_fill_in_addresses, .setup_ctrs = &op_amd_setup_ctrs, + .cpu_down = &op_amd_cpu_shutdown, .check_ctrs = &op_amd_check_ctrs, .start = &op_amd_start, .stop = &op_amd_stop, diff --git a/arch/x86/oprofile/op_x86_model.h b/arch/x86/oprofile/op_x86_model.h index 551401398fba..89017fa1fd63 100644 --- a/arch/x86/oprofile/op_x86_model.h +++ b/arch/x86/oprofile/op_x86_model.h @@ -40,10 +40,10 @@ struct op_x86_model_spec { u64 reserved; u16 event_mask; int (*init)(struct oprofile_operations *ops); - void (*exit)(void); int (*fill_in_addresses)(struct op_msrs * const msrs); void (*setup_ctrs)(struct op_x86_model_spec const *model, struct op_msrs const * const msrs); + void (*cpu_down)(void); int (*check_ctrs)(struct pt_regs * const regs, struct op_msrs const * const msrs); void (*start)(struct op_msrs const * const msrs); -- cgit v1.2.3