[PATCH 0/7] oprofile/x86: fix cpu hotplug

[PATCH 0/7] oprofile/x86: fix cpu hotplug

[Robert Richter]

This patch series fixes oprofile cpu hotplug. The code was broken and
causing crashes due to uninitialized counter usage. This can be only
fixed with a code rework. The first patch is to avoid kernel crashes
during cpu hotplug. This is a small fix for linux-stable.

Ingo, I will send a separate pull request.

-Robert

[PATCH 1/7] oprofile/x86: fix uninitialized counter usage during cpu hotplug

[Robert Richter]

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: [<ffffffff8132ad57>] 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:[<ffffffff8132ad57>]  [<ffffffff8132ad57>] 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:
 <IRQ>
 [<ffffffff8132a602>] nmi_cpu_stop+0x21/0x23
 [<ffffffff810521b3>] generic_smp_call_function_single_interrupt+0xdf/0x11b
 [<ffffffff8101804f>] smp_call_function_single_interrupt+0x22/0x31
 [<ffffffff810029f3>] call_function_single_interrupt+0x13/0x20
 <EOI>
 [<ffffffff8102c9a5>] ? wake_up_process+0x10/0x12
 [<ffffffff81008701>] ? default_idle+0x22/0x37
 [<ffffffff8100896d>] c1e_idle+0xdf/0xe6
 [<ffffffff813f1170>] ? atomic_notifier_call_chain+0x13/0x15
 [<ffffffff810012fb>] cpu_idle+0x4b/0x7e
 [<ffffffff813e8a4e>] 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  [<ffffffff8132ad57>] op_amd_stop+0x2d/0x8e
 RSP <ffff880001843f28>
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:
 <IRQ>  [<ffffffff813ebd6a>] panic+0x9e/0x10c
 [<ffffffff810474b0>] ? up+0x34/0x39
 [<ffffffff81031ccc>] ? kmsg_dump+0x112/0x12c
 [<ffffffff813eeff1>] oops_end+0x81/0x8e
 [<ffffffff8101efee>] no_context+0x1f3/0x202
 [<ffffffff8101f1b7>] __bad_area_nosemaphore+0x1ba/0x1e0
 [<ffffffff81028d24>] ? enqueue_task_fair+0x16d/0x17a
 [<ffffffff810264dc>] ? activate_task+0x42/0x53
 [<ffffffff8102c967>] ? try_to_wake_up+0x272/0x284
 [<ffffffff8101f1eb>] bad_area_nosemaphore+0xe/0x10
 [<ffffffff813f0f3f>] do_page_fault+0x1c8/0x37c
 [<ffffffff81028d24>] ? enqueue_task_fair+0x16d/0x17a
 [<ffffffff813ee55f>] page_fault+0x1f/0x30
 [<ffffffff8102c9a5>] ? wake_up_process+0x10/0x12
 [<ffffffff8132ad57>] ? op_amd_stop+0x2d/0x8e
 [<ffffffff8132ad46>] ? op_amd_stop+0x1c/0x8e
 [<ffffffff8132a602>] nmi_cpu_stop+0x21/0x23
 [<ffffffff810521b3>] generic_smp_call_function_single_interrupt+0xdf/0x11b
 [<ffffffff8101804f>] smp_call_function_single_interrupt+0x22/0x31
 [<ffffffff810029f3>] call_function_single_interrupt+0x13/0x20
 <EOI>  [<ffffffff8102c9a5>] ? wake_up_process+0x10/0x12
 [<ffffffff81008701>] ? default_idle+0x22/0x37
 [<ffffffff8100896d>] c1e_idle+0xdf/0xe6
 [<ffffffff813f1170>] ? atomic_notifier_call_chain+0x13/0x15
 [<ffffffff810012fb>] cpu_idle+0x4b/0x7e
 [<ffffffff813e8a4e>] 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:
 <IRQ>  [<ffffffff81017f32>] ? native_smp_send_reschedule+0x27/0x53
 [<ffffffff81030ee2>] warn_slowpath_common+0x77/0xa4
 [<ffffffff81030f1e>] warn_slowpath_null+0xf/0x11
 [<ffffffff81017f32>] native_smp_send_reschedule+0x27/0x53
 [<ffffffff8102634b>] resched_task+0x60/0x62
 [<ffffffff8102653a>] check_preempt_curr_idle+0x10/0x12
 [<ffffffff8102c8ea>] try_to_wake_up+0x1f5/0x284
 [<ffffffff8102c986>] default_wake_function+0xd/0xf
 [<ffffffff810a110d>] pollwake+0x57/0x5a
 [<ffffffff8102c979>] ? default_wake_function+0x0/0xf
 [<ffffffff81026be5>] __wake_up_common+0x46/0x75
 [<ffffffff81026ed0>] __wake_up+0x38/0x50
 [<ffffffff81031694>] printk_tick+0x39/0x3b
 [<ffffffff8103ac37>] update_process_times+0x3f/0x5c
 [<ffffffff8104dc63>] tick_periodic+0x5d/0x69
 [<ffffffff8104dc90>] tick_handle_periodic+0x21/0x71
 [<ffffffff81018fd0>] smp_apic_timer_interrupt+0x82/0x95
 [<ffffffff81002853>] apic_timer_interrupt+0x13/0x20
 [<ffffffff81030cb5>] ? panic_blink_one_second+0x0/0x7b
 [<ffffffff813ebdd6>] ? panic+0x10a/0x10c
 [<ffffffff810474b0>] ? up+0x34/0x39
 [<ffffffff81031ccc>] ? kmsg_dump+0x112/0x12c
 [<ffffffff813eeff1>] ? oops_end+0x81/0x8e
 [<ffffffff8101efee>] ? no_context+0x1f3/0x202
 [<ffffffff8101f1b7>] ? __bad_area_nosemaphore+0x1ba/0x1e0
 [<ffffffff81028d24>] ? enqueue_task_fair+0x16d/0x17a
 [<ffffffff810264dc>] ? activate_task+0x42/0x53
 [<ffffffff8102c967>] ? try_to_wake_up+0x272/0x284
 [<ffffffff8101f1eb>] ? bad_area_nosemaphore+0xe/0x10
 [<ffffffff813f0f3f>] ? do_page_fault+0x1c8/0x37c
 [<ffffffff81028d24>] ? enqueue_task_fair+0x16d/0x17a
 [<ffffffff813ee55f>] ? page_fault+0x1f/0x30
 [<ffffffff8102c9a5>] ? wake_up_process+0x10/0x12
 [<ffffffff8132ad57>] ? op_amd_stop+0x2d/0x8e
 [<ffffffff8132ad46>] ? op_amd_stop+0x1c/0x8e
 [<ffffffff8132a602>] ? nmi_cpu_stop+0x21/0x23
 [<ffffffff810521b3>] ? generic_smp_call_function_single_interrupt+0xdf/0x11b
 [<ffffffff8101804f>] ? smp_call_function_single_interrupt+0x22/0x31
 [<ffffffff810029f3>] ? call_function_single_interrupt+0x13/0x20
 <EOI>  [<ffffffff8102c9a5>] ? wake_up_process+0x10/0x12
 [<ffffffff81008701>] ? default_idle+0x22/0x37
 [<ffffffff8100896d>] ? c1e_idle+0xdf/0xe6
 [<ffffffff813f1170>] ? atomic_notifier_call_chain+0x13/0x15
 [<ffffffff810012fb>] ? cpu_idle+0x4b/0x7e
 [<ffffffff813e8a4e>] ? start_secondary+0x1ae/0x1b2
---[ end trace 679ac372d674b758 ]---

Cc: Andi Kleen <andi@firstfloor.org>
Cc: stable <stable@kernel.org>
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/nmi_int.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index 9f001d9..2458204 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)
-- 
1.7.0.3

[PATCH 2/7] oprofile/x86: remove CONFIG_SMP macros

[Robert Richter]

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 <andi@firstfloor.org>
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/nmi_int.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index 2458204..c5df8ee 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();
-- 
1.7.0.3

[PATCH 3/7] oprofile/x86: protect cpu hotplug sections

[Robert Richter]

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 <andi@firstfloor.org>
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/nmi_int.c |   52 ++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index c5df8ee..b56601e 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();
-- 
1.7.0.3

[PATCH 4/7] oprofile/x86: stop disabled counters in nmi handler

[Robert Richter]

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 <andi@firstfloor.org>
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/nmi_int.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index b56601e..94b5481 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);
-- 
1.7.0.3

[PATCH 5/7] oprofile/x86: reordering some functions

[Robert Richter]

Reordering some functions. Necessary for the next patch. No functional
changes.

Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/nmi_int.c |  134 +++++++++++++++++++++---------------------
 1 files changed, 67 insertions(+), 67 deletions(-)

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index 94b5481..7de0572 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)
-- 
1.7.0.3

[PATCH 6/7] oprofile/x86: notify cpus only when daemon is running

[Robert Richter]

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 <andi@firstfloor.org>
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/nmi_int.c |   16 +++-------------
 1 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index 7de0572..2a08672 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();
 }
-- 
1.7.0.3

[PATCH 7/7] oprofile/x86: make AMD IBS hotplug capable

[Robert Richter]

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 <andi@firstfloor.org>
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 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(-)

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index 2a08672..b28d2f1 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 384c524..b67a6b5 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 5514013..89017fa 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);
-- 
1.7.0.3

[GIT PULL] oprofile hotplug fixes for x86

[Robert Richter]

On 06.05.10 15:26:07, Robert Richter wrote:
> Ingo, I will send a separate pull request.

Ingo,

please pull oprofile hotplug fixes for x86 from:

 git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core

Thanks,

-Robert

--

The following changes since commit 5bdb7934ca4115a12c7d585c5a45312b1c36909b:
  Robert Richter (1):
        oprofile/x86: remove duplicate IBS capability check

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core

Robert Richter (7):
      oprofile/x86: fix uninitialized counter usage during cpu hotplug
      oprofile/x86: remove CONFIG_SMP macros
      oprofile/x86: protect cpu hotplug sections
      oprofile/x86: stop disabled counters in nmi handler
      oprofile/x86: reordering some functions
      oprofile/x86: notify cpus only when daemon is running
      oprofile/x86: make AMD IBS hotplug capable

 arch/x86/oprofile/nmi_int.c      |  187 +++++++++++++++++++++++---------------
 arch/x86/oprofile/op_model_amd.c |   54 +++--------
 arch/x86/oprofile/op_x86_model.h |    2 +-
 3 files changed, 130 insertions(+), 113 deletions(-)

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: [GIT PULL] oprofile hotplug fixes for x86

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> On 06.05.10 15:26:07, Robert Richter wrote:
> > Ingo, I will send a separate pull request.
> 
> Ingo,
> 
> please pull oprofile hotplug fixes for x86 from:
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core
> 
> Thanks,
> 
> -Robert
> 
> --
> 
> The following changes since commit 5bdb7934ca4115a12c7d585c5a45312b1c36909b:
>   Robert Richter (1):
>         oprofile/x86: remove duplicate IBS capability check
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core
> 
> Robert Richter (7):
>       oprofile/x86: fix uninitialized counter usage during cpu hotplug
>       oprofile/x86: remove CONFIG_SMP macros
>       oprofile/x86: protect cpu hotplug sections
>       oprofile/x86: stop disabled counters in nmi handler
>       oprofile/x86: reordering some functions
>       oprofile/x86: notify cpus only when daemon is running
>       oprofile/x86: make AMD IBS hotplug capable
> 
>  arch/x86/oprofile/nmi_int.c      |  187 +++++++++++++++++++++++---------------
>  arch/x86/oprofile/op_model_amd.c |   54 +++--------
>  arch/x86/oprofile/op_x86_model.h |    2 +-
>  3 files changed, 130 insertions(+), 113 deletions(-)

Pulled, thanks Robert!

	Ingo