Updated Haswell uncore PMU patchkit
Subsystems (2)
This version avoids registering the CPU notifier if nothing is detected.
-Andi
PATCH 1/3 x86, perf, uncore: Add support for ARB uncore PMU on Sandy/IvyBridge
From: Andi Kleen <[email protected]>
Add a new "ARB" uncore PMU that is used to monitor the uncore queue arbiter. This is useful to measure uncore queue occupancy and similar statistics. The registers all have the same format as the existing CBOX PMU.
Also move the event constraints from the CBOX to ARB. The 0x80+ events are ARB events and cannot be scheduled on a CBOX PMU.
--- arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c +21 -2
@@ -44,6 +44,11 @@#define SNB_UNC_CBO_0_PER_CTR0 0x706#define SNB_UNC_CBO_MSR_OFFSET 0x10 +/* SNB ARB register */ +#define SNB_UNC_ARB_PER_CTR0 0x3b0 +#define SNB_UNC_ARB_PERFEVTSEL0 0x3b2 +#define SNB_UNC_ARB_MSR_OFFSET 0x10 +/* NHM global control register */ #define NHM_UNC_PERF_GLOBAL_CTL 0x391#define NHM_UNC_FIXED_CTR 0x394
@@ -114,7 +119,7 @@ static struct intel_uncore_ops snb_uncore_msr_ops = { .read_counter = uncore_msr_read_counter, }; -static struct event_constraint snb_uncore_cbox_constraints[] = { +static struct event_constraint snb_uncore_arb_constraints[] = {UNCORE_EVENT_CONSTRAINT (0x80 ,0x1 ),UNCORE_EVENT_CONSTRAINT (0x83 ,0x1 ),EVENT_CONSTRAINT_END
@@ -133,14 +138,28 @@ static struct intel_uncore_type snb_uncore_cbox = { .single_fixed = 1, .event_mask = SNB_UNC_RAW_EVENT_MASK, .msr_offset = SNB_UNC_CBO_MSR_OFFSET, - .constraints = snb_uncore_cbox_constraints, .ops = &snb_uncore_msr_ops, .format_group = &snb_uncore_format_group, .event_descs = snb_uncore_events, }; +static struct intel_uncore_type snb_uncore_arb = { + .name ="arb" , + .num_counters =2 , + .num_boxes =1 , + .perf_ctr_bits =44 , + .perf_ctr =SNB_UNC_ARB_PER_CTR0 , + .event_ctl =SNB_UNC_ARB_PERFEVTSEL0 , + .event_mask =SNB_UNC_RAW_EVENT_MASK , + .msr_offset =SNB_UNC_ARB_MSR_OFFSET , + .constraints = snb_uncore_arb_constraints, + .ops = &snb_uncore_msr_ops, + .format_group = &snb_uncore_format_group, +}; +static struct intel_uncore_type *snb_msr_uncores[] = { &snb_uncore_cbox, + &snb_uncore_arb,NULL , };
Signature · 1 line
2.4.2
PATCH 2/3 x86, perf, uncore: Use Sandy Bridge client PMU on Haswell/Broadwell
In reply to Andi KleenFrom: Andi Kleen <[email protected]>
Haswell and Broadwell have the same uncore CBOX/ARB PMU as Sandy Bridge. Add the respective model numbers to enable the SNB uncore PMU.
--- arch/x86/kernel/cpu/perf_event_intel_uncore.c | 5 +++++ 1 file changed, 5 insertions(+)
arch/x86/kernel/cpu/perf_event_intel_uncore.c +5 -0
@@ -1202,6 +1202,11 @@ static int __init uncore_cpu_init(void)break ;case 42 :/* Sandy Bridge */ case 58 :/* Ivy Bridge */ +case 60 :/* Haswell */ +case 69 :/* Haswell */ +case 70 :/* Haswell */ +case 61 :/* Broadwell */ +case 71 :/* Broadwell */ snb_uncore_cpu_init ();break ;case 45 :/* Sandy Bridge-EP */
Signature · 1 line
2.4.2
PATCH 3/3 x86, perf, uncore: Don't make MSR uncore depend on PCI uncore
In reply to Andi KleenFrom: Andi Kleen <[email protected]>
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
--- arch/x86/kernel/cpu/perf_event_intel_uncore.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-)
arch/x86/kernel/cpu/perf_event_intel_uncore.c +8 -13
@@ -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,12 @@ 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; + ret1 =uncore_pci_init (); + ret2 =uncore_cpu_init (); +if (!ret1 || !ret2) { +uncore_cpumask_init (); +uncore_pmus_register (); } -uncore_cpumask_init (); - -uncore_pmus_register ();return 0 ; -fail: -return ret; }device_initcall (intel_uncore_init);
Signature · 1 line
2.4.2
Re: [PATCH 3/3] x86, perf, uncore: Don't make MSR uncore depend on PCI uncore
Quoted 18 lines · Andi Kleen
static int
__initintel_uncore_init(void)
{arch/x86/kernel/cpu/perf_event_intel_uncore.c +1
-8-int ret; +int ret1, ret2;if (boot_cpu_data.x86_vendor !=X86_VENDOR_INTEL )return -ENODEV ;@@ -1295,19 +1297,12 @@ 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;
arch/x86/kernel/cpu/perf_event_intel_uncore.c static int __init intel_uncore_init(void) · line 1306
Quoted 10 lines
arch/x86/kernel/cpu/perf_event_intel_uncore.c +5
-3+ ret1 =uncore_pci_init (); + ret2 =uncore_cpu_init (); +if (!ret1 || !ret2) { +uncore_cpumask_init (); +uncore_pmus_register (); } -uncore_cpumask_init (); - -uncore_pmus_register ();return 0 ;
So now we return success, if nothing is there or stuff failed?
One possible solution is to split the initcall and have one for uncore_pci and one for uncode_msr, but that does not work well if you want to make it a module.
But we should at least have some indication, what worked and what went wrong instead of unconditionally returning success.
Thanks,
tglx
Re: [PATCH 3/3] x86, perf, uncore: Don't make MSR uncore depend on PCI uncore
Quoted 1 line
So now we return success, if nothing is there or stuff failed?
Right.
Quoted 7 lines
One possible solution is to split the initcall and have one for
uncore_pciand one foruncode_msr, but that does not work well if you want to make it a module.But we should at least have some indication, what worked and what went wrong instead of unconditionally returning success.
Nobody uses the return value for builtin drivers (short of one debug printk). It would not load the module, but right now we don't have a module.
Generally it's a bad idea to print something when a probe doesn't work, as that just leads to lots of dmesg spam for large monolithic kernels.
-Andi
Signature · 1 line
[email protected] -- Speaking for myself only
Re: [PATCH 3/3] x86, perf, uncore: Don't make MSR uncore depend on PCI uncore
Quoted 9 lines · Andi Kleen
Quoted 6 lines
One possible solution is to split the initcall and have one for
uncore_pciand one foruncode_msr, but that does not work well if you want to make it a module.But we should at least have some indication, what worked and what went wrong instead of unconditionally returning success.
Nobody uses the return value for builtin drivers (short of one debug printk).
Which is a valuable tool to figure out WHY stuff does not work, which you broke for a particular case.
Quoted 1 line
It would not load the module, but right now we don't have a module.
Though you keep the exit function around for making this modular. So your argumentation does not make any sense at all.
Make it sloppy first, so it's more work to convert it to a module.
Quoted 2 lines
Generally it's a bad idea to print something when a probe doesn't work, as that just leads to lots of dmesg spam for large monolithic kernels.
Generally it's bad to just hack stuff into submission and leave the mess created for others to clean up.
You've been told that a gazillion times in the past, and I tell it another time, even if I realized long ago, that you are completely advisory resistant.
Thanks,
tglx