[PATCH] x86, perf, uncore: Don't make MSR uncore depend on PCI uncore

[PATCH] x86, perf, uncore: Don't make MSR uncore depend on PCI uncore

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

Several sytems, such as my laptop, don't expose the PCI devices
for the PCI uncore measurements. But the MSRs for the CBOX and ARB
uncores are always available. Currently the init code doesn't
initialize the MSR uncores when the PCI uncore registration fails.
Stop it from doing that and always try to register the MSR uncores.

This makes the pci uncore exit function unused. We'll need it later
when the uncore becomes modular, so instead of removing it I just
marked it with module_exit right now (which discards it)

v2: Avoid registering notifier when both initialization calls fail
v3: Return error when both registrations fail.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/kernel/cpu/perf_event_intel_uncore.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
index b57a09d..31ac78b 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -952,6 +952,8 @@ static void __init uncore_pci_exit(void)
 		uncore_types_exit(uncore_pci_uncores);
 	}
 }
+module_exit(uncore_pci_exit);
+/* XXX: need exit code for rest of intel_uncore_init too */
 
 /* CPU hot plug/unplug are serialized by cpu_add_remove_lock mutex */
 static LIST_HEAD(boxes_to_free);
@@ -1287,7 +1289,7 @@ static void __init uncore_cpumask_init(void)
 
 static int __init intel_uncore_init(void)
 {
-	int ret;
+	int ret1, ret2;
 
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
 		return -ENODEV;
@@ -1295,19 +1297,13 @@ static int __init intel_uncore_init(void)
 	if (cpu_has_hypervisor)
 		return -ENODEV;
 
-	ret = uncore_pci_init();
-	if (ret)
-		goto fail;
-	ret = uncore_cpu_init();
-	if (ret) {
-		uncore_pci_exit();
-		goto fail;
-	}
-	uncore_cpumask_init();
-
-	uncore_pmus_register();
+	ret1 = uncore_pci_init();
+	ret2 = uncore_cpu_init();
+	if (!ret1 || !ret2) {
+		uncore_cpumask_init();
+		uncore_pmus_register();
+	} else
+		return -ENODEV;
 	return 0;
-fail:
-	return ret;
 }
 device_initcall(intel_uncore_init);
-- 
2.4.2

Re: [PATCH] x86, perf, uncore: Don't make MSR uncore depend on PCI uncore

[Thomas Gleixner]

On Wed, 9 Sep 2015, Andi Kleen wrote:
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -958,6 +958,8 @@ static void __init uncore_pci_exit(void)
>  		uncore_types_exit(uncore_pci_uncores);
>  	}
>  }
> +module_exit(uncore_pci_exit);
> +/* XXX: need exit code for rest of intel_uncore_init too */

That comment makes no sense at all.
  
Other than that:

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

[PATCH] x86, perf, uncore: Don't make MSR uncore depend on PCI uncore

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

[Note: I still need this patch to make memory bandwidth monitoring
work on my laptop. I believe all earlier review comments regarding
how to return later ignored error codes are addressed in this version.]

Several sytems, such as my laptop, don't expose the PCI devices
for the PCI uncore measurements. But the MSRs for the CBOX and ARB
uncores are always available. Currently the init code doesn't
initialize the MSR uncores when the PCI uncore registration fails.
Stop it from doing that and always try to register the MSR uncores.

This makes the pci uncore exit function unused. We'll need it later
when the uncore becomes modular, so instead of removing it I just
marked it with module_exit right now (which discards it)

v2: Avoid registering notifier when both initialization calls fail
v3: Return error when both registrations fail.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/kernel/cpu/perf_event_intel_uncore.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
index 560e525..01599cd6 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -958,6 +958,8 @@ static void __init uncore_pci_exit(void)
 		uncore_types_exit(uncore_pci_uncores);
 	}
 }
+module_exit(uncore_pci_exit);
+/* XXX: need exit code for rest of intel_uncore_init too */
 
 /* CPU hot plug/unplug are serialized by cpu_add_remove_lock mutex */
 static LIST_HEAD(boxes_to_free);
@@ -1300,7 +1302,7 @@ static void __init uncore_cpumask_init(void)
 
 static int __init intel_uncore_init(void)
 {
-	int ret;
+	int ret1, ret2;
 
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
 		return -ENODEV;
@@ -1308,19 +1310,13 @@ static int __init intel_uncore_init(void)
 	if (cpu_has_hypervisor)
 		return -ENODEV;
 
-	ret = uncore_pci_init();
-	if (ret)
-		goto fail;
-	ret = uncore_cpu_init();
-	if (ret) {
-		uncore_pci_exit();
-		goto fail;
-	}
-	uncore_cpumask_init();
-
-	uncore_pmus_register();
+	ret1 = uncore_pci_init();
+	ret2 = uncore_cpu_init();
+	if (!ret1 || !ret2) {
+		uncore_cpumask_init();
+		uncore_pmus_register();
+	} else
+		return -ENODEV;
 	return 0;
-fail:
-	return ret;
 }
 device_initcall(intel_uncore_init);
-- 
2.4.3