Browse Source

Revert "perf/x86/intel/uncore: Move uncore_box_init() out of driver initialization"

This reverts commit c05199e5a57a579fea1e8fa65e2b511ceb524ffc.

Vince Weaver reported the following crash while perf fuzzing:

[   79.473121] kernel BUG at mm/vmalloc.c:1335!
[   79.694391] Call Trace:
[   79.696997]  <IRQ>
[   79.699090]  [<ffffffff811b2130>] get_vm_area_caller+0x40/0x50
[   79.705505]  [<ffffffff81039f4d>] ? snb_uncore_imc_init_box+0x6d/0x90
[   79.712414]  [<ffffffff810635e5>] __ioremap_caller+0x195/0x350
[   79.718610]  [<ffffffff81039f4d>] ? snb_uncore_imc_init_box+0x6d/0x90
[   79.725462]  [<ffffffff81427f6b>] ? debug_object_activate+0x14b/0x1e0
[   79.732346]  [<ffffffff810637b7>] ioremap_nocache+0x17/0x20
[   79.738283]  [<ffffffff81039f4d>] snb_uncore_imc_init_box+0x6d/0x90
[   79.744945]  [<ffffffff81039cf7>] snb_uncore_imc_event_start+0xb7/0x110
[   79.752020]  [<ffffffff81039d97>] snb_uncore_imc_event_add+0x47/0x60
[   79.758832]  [<ffffffff81162cbb>] event_sched_in.isra.85+0xfb/0x330
[   79.765519]  [<ffffffff81162f5f>] group_sched_in+0x6f/0x1e0
[   79.771481]  [<ffffffff8101df1a>] ? native_sched_clock+0x2a/0x90
[   79.777858]  [<ffffffff811637bc>] __perf_event_enable+0x25c/0x2a0
[   79.784418]  [<ffffffff810f3e69>] ? tick_nohz_irq_exit+0x29/0x30
[   79.790820]  [<ffffffff8115ef30>] ? cpu_clock_event_start+0x40/0x40
[   79.797546]  [<ffffffff8115ef80>] remote_function+0x50/0x60
[   79.803535]  [<ffffffff810f8cd1>] flush_smp_call_function_queue+0x81/0x180
[   79.810840]  [<ffffffff810f9763>] generic_smp_call_function_single_interrupt+0x13/0x60
[   79.819328]  [<ffffffff8104b5e8>] smp_trace_call_function_single_interrupt+0x38/0xc0
[   79.827614]  [<ffffffff816de9be>] trace_call_function_single_interrupt+0x6e/0x80
[   79.835465]  <EOI>
[   79.837543]  [<ffffffff8156e8b5>] ? cpuidle_enter_state+0x65/0x160
[   79.844377]  [<ffffffff8156e8a1>] ? cpuidle_enter_state+0x51/0x160
[   79.851015]  [<ffffffff8156e9e7>] cpuidle_enter+0x17/0x20
[   79.856791]  [<ffffffff810b6e39>] cpu_startup_entry+0x399/0x440
[   79.863165]  [<ffffffff816c9ddb>] rest_init+0xbb/0xd0

The offending commit is clearly confused as it moves heavy initialization
work into IPI context.

Revert it.

Reported-by: Vince Weaver <vincent.weaver@maine.edu>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Yan, Zheng <zheng.z.yan@intel.com>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Ingo Molnar 10 years ago
parent
commit
15c1247953

+ 7 - 2
arch/x86/kernel/cpu/perf_event_intel_uncore.c

@@ -839,6 +839,7 @@ static int uncore_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id
 	box->phys_id = phys_id;
 	box->phys_id = phys_id;
 	box->pci_dev = pdev;
 	box->pci_dev = pdev;
 	box->pmu = pmu;
 	box->pmu = pmu;
+	uncore_box_init(box);
 	pci_set_drvdata(pdev, box);
 	pci_set_drvdata(pdev, box);
 
 
 	raw_spin_lock(&uncore_box_lock);
 	raw_spin_lock(&uncore_box_lock);
@@ -1002,8 +1003,10 @@ static int uncore_cpu_starting(int cpu)
 			pmu = &type->pmus[j];
 			pmu = &type->pmus[j];
 			box = *per_cpu_ptr(pmu->box, cpu);
 			box = *per_cpu_ptr(pmu->box, cpu);
 			/* called by uncore_cpu_init? */
 			/* called by uncore_cpu_init? */
-			if (box && box->phys_id >= 0)
+			if (box && box->phys_id >= 0) {
+				uncore_box_init(box);
 				continue;
 				continue;
+			}
 
 
 			for_each_online_cpu(k) {
 			for_each_online_cpu(k) {
 				exist = *per_cpu_ptr(pmu->box, k);
 				exist = *per_cpu_ptr(pmu->box, k);
@@ -1019,8 +1022,10 @@ static int uncore_cpu_starting(int cpu)
 				}
 				}
 			}
 			}
 
 
-			if (box)
+			if (box) {
 				box->phys_id = phys_id;
 				box->phys_id = phys_id;
+				uncore_box_init(box);
+			}
 		}
 		}
 	}
 	}
 	return 0;
 	return 0;

+ 8 - 10
arch/x86/kernel/cpu/perf_event_intel_uncore.h

@@ -258,14 +258,6 @@ static inline int uncore_num_counters(struct intel_uncore_box *box)
 	return box->pmu->type->num_counters;
 	return box->pmu->type->num_counters;
 }
 }
 
 
-static inline void uncore_box_init(struct intel_uncore_box *box)
-{
-	if (!test_and_set_bit(UNCORE_BOX_FLAG_INITIATED, &box->flags)) {
-		if (box->pmu->type->ops->init_box)
-			box->pmu->type->ops->init_box(box);
-	}
-}
-
 static inline void uncore_disable_box(struct intel_uncore_box *box)
 static inline void uncore_disable_box(struct intel_uncore_box *box)
 {
 {
 	if (box->pmu->type->ops->disable_box)
 	if (box->pmu->type->ops->disable_box)
@@ -274,8 +266,6 @@ static inline void uncore_disable_box(struct intel_uncore_box *box)
 
 
 static inline void uncore_enable_box(struct intel_uncore_box *box)
 static inline void uncore_enable_box(struct intel_uncore_box *box)
 {
 {
-	uncore_box_init(box);
-
 	if (box->pmu->type->ops->enable_box)
 	if (box->pmu->type->ops->enable_box)
 		box->pmu->type->ops->enable_box(box);
 		box->pmu->type->ops->enable_box(box);
 }
 }
@@ -298,6 +288,14 @@ static inline u64 uncore_read_counter(struct intel_uncore_box *box,
 	return box->pmu->type->ops->read_counter(box, event);
 	return box->pmu->type->ops->read_counter(box, event);
 }
 }
 
 
+static inline void uncore_box_init(struct intel_uncore_box *box)
+{
+	if (!test_and_set_bit(UNCORE_BOX_FLAG_INITIATED, &box->flags)) {
+		if (box->pmu->type->ops->init_box)
+			box->pmu->type->ops->init_box(box);
+	}
+}
+
 static inline bool uncore_box_is_fake(struct intel_uncore_box *box)
 static inline bool uncore_box_is_fake(struct intel_uncore_box *box)
 {
 {
 	return (box->phys_id < 0);
 	return (box->phys_id < 0);