perf: oprofile spring cleanup
Subsystems (7)
Marc Zyngier
maintainercover perf: oprofile spring cleanup
This small series builds on top of the work that was started with [1].
It recently became apparent that KVM/arm64 is the last bit of the kernel that still uses perf_num_counters().
As I went ahead to address this, it became obvious that all traces of oprofile had been eradicated from all architectures but arm64, s390 and sh (plus a bit of cruft in the core perf code). With KVM fixed, perf_num_counters() and perf_pmu_name() are finally gone.
Thanks,
M.
[1] https://lore.kernel.org/lkml/20210215050618.hgftdmfmslbdrg3j@vireshk-i7
Marc Zyngier (5):
KVM: arm64: Divorce the perf code from oprofile helpers
arm64: Get rid of oprofile leftovers
s390: Get rid of oprofile leftovers
sh: Get rid of oprofile leftovers
perf: Get rid of oprofile leftovers
arch/arm64/kvm/perf.c | 7 +------ arch/arm64/kvm/pmu-emul.c | 2 +- arch/s390/kernel/perf_event.c | 21 --------------------- arch/sh/kernel/perf_event.c | 18 ------------------ drivers/perf/arm_pmu.c | 30 ------------------------------ include/kvm/arm_pmu.h | 4 ++++ include/linux/perf_event.h | 2 -- kernel/events/core.c | 5 ----- 8 files changed, 6 insertions(+), 83 deletions(-)
Signature · 1 line
2.29.2
Marc Zyngier
maintainerPATCH 1/5 KVM: arm64: Divorce the perf code from oprofile helpers
KVM/arm64 is the sole user of perf_num_counters(), and really could do without it. Stop using the obsolete API by relying on the existing probing code.
--- arch/arm64/kvm/perf.c | 7 +------ arch/arm64/kvm/pmu-emul.c | 2 +- include/kvm/arm_pmu.h | 4 ++++ 3 files changed, 6 insertions(+), 7 deletions(-)
arch/arm64/kvm/perf.c +1 -6
@@ -50,12 +50,7 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {int kvm_perf_init (void ) { -/* -* Check if HW_PERF_EVENTS are supported by checking the number of -* hardware performance counters. This could ensure the presence of -* a physical PMU and CONFIG_PERF_EVENT is selected. -*/ -if (IS_ENABLED (CONFIG_ARM_PMU ) &&perf_num_counters () >0 ) +if (kvm_pmu_probe_pmuver () !=0xf )static_branch_enable (&kvm_arm_pmu_available);return perf_register_guest_info_callbacks (&kvm_guest_cbs);
arch/arm64/kvm/pmu-emul.c +1 -1
@@ -739,7 +739,7 @@ void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,kvm_pmu_create_perf_event (vcpu, select_idx); } -static int kvm_pmu_probe_pmuver (void ) +int kvm_pmu_probe_pmuver (void ) {struct perf_event_attr attr = { };struct perf_event *event;
include/kvm/arm_pmu.h +4 -0
@@ -61,6 +61,7 @@ int kvm_arm_pmu_v3_get_attr(struct kvm_vcpu *vcpu,int kvm_arm_pmu_v3_has_attr (struct kvm_vcpu *vcpu,struct kvm_device_attr *attr);int kvm_arm_pmu_v3_enable (struct kvm_vcpu *vcpu); +int kvm_pmu_probe_pmuver (void );#else struct kvm_pmu { };
@@ -116,6 +117,9 @@ static inline u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1) {return 0 ; } + +static inline int kvm_pmu_probe_pmuver (void ) {return 0xf ; } +#endif #endif
Signature · 1 line
2.29.2
Re: [PATCH 1/5] KVM: arm64: Divorce the perf code from oprofile helpers
Hi Marc,
Quoted 16 lines · Marc Zyngier
KVM/arm64 is the sole user of
perf_num_counters(), and really could do without it. Stop using the obsolete API by relying on the existing probing code.Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/perf.c | 7 +------ arch/arm64/kvm/pmu-emul.c | 2 +- include/kvm/arm_pmu.h | 4 ++++ 3 files changed, 6 insertions(+), 7 deletions(-)arch/arm64/kvm/perf.c +0
-0
arch/arm64/kvm/perf.c static struct perf_guest_info_callbacks kvm_guest_cbs = { · line 53
Quoted 10 lines
arch/arm64/kvm/perf.c +1
-6@@ -50,12 +50,7 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {int kvm_perf_init (void ) { -/* -* Check if HW_PERF_EVENTS are supported by checking the number of -* hardware performance counters. This could ensure the presence of -* a physical PMU and CONFIG_PERF_EVENT is selected. -*/ -if (IS_ENABLED (CONFIG_ARM_PMU ) &&perf_num_counters () >0 ) +if (kvm_pmu_probe_pmuver () !=0xf )
The probe() function may be called many times (kvm_arm_pmu_v3_set_attr also calls it). I don't know whether the first calling is enough. If so, can we use a static variable in it, so the following calling can return the result right away?
Thanks,
Keqian
Marc Zyngier
maintainerRe: [PATCH 1/5] KVM: arm64: Divorce the perf code from oprofile helpers
Quoted 21 lines · Keqian Zhu
Hi Marc,
Quoted 17 lines · Marc Zyngier
KVM/arm64 is the sole user of
perf_num_counters(), and really could do without it. Stop using the obsolete API by relying on the existing probing code.Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/perf.c | 7 +------ arch/arm64/kvm/pmu-emul.c | 2 +- include/kvm/arm_pmu.h | 4 ++++ 3 files changed, 6 insertions(+), 7 deletions(-)arch/arm64/kvm/perf.c +0
-0@@ -50,12 +50,7 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {
Quoted 13 lines
arch/arm64/kvm/perf.c static struct perf_guest_info_callbacks kvm_guest_cbs = { · line 53
Quoted 9 lines
arch/arm64/kvm/perf.c +1
-6int kvm_perf_init (void ) { -/* -* Check if HW_PERF_EVENTS are supported by checking the number of -* hardware performance counters. This could ensure the presence of -* a physical PMU and CONFIG_PERF_EVENT is selected. -*/ -if (IS_ENABLED (CONFIG_ARM_PMU ) &&perf_num_counters () >0 ) +if (kvm_pmu_probe_pmuver () !=0xf )The
probe()function may be called many times (kvm_arm_pmu_v3_set_attralso calls it). I don't know whether the first calling is enough. If so, can we use a static variable in it, so the following calling can return the result right away?
No, because that wouldn't help with crappy big-little implementations that could have PMUs with different versions. We want to find the version at the point where the virtual PMU is created, which is why we call the probe function once per vcpu.
This of course is broken in other ways (BL+KVM is a total disaster when it comes to PMU), but making this static would just make it worse.
M.
Signature · 5 lines
Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 1/5] KVM: arm64: Divorce the perf code from oprofile helpers
Hi Marc,
Quoted 37 lines · Marc Zyngier
Quoted 21 lines · Keqian Zhu
Hi Marc,
Quoted 17 lines · Marc Zyngier
KVM/arm64 is the sole user of
perf_num_counters(), and really could do without it. Stop using the obsolete API by relying on the existing probing code.Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/perf.c | 7 +------ arch/arm64/kvm/pmu-emul.c | 2 +- include/kvm/arm_pmu.h | 4 ++++ 3 files changed, 6 insertions(+), 7 deletions(-)arch/arm64/kvm/perf.c +0
-0@@ -50,12 +50,7 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {Quoted 13 lines
arch/arm64/kvm/perf.c static struct perf_guest_info_callbacks kvm_guest_cbs = { · line 53
Quoted 9 lines
arch/arm64/kvm/perf.c +1
-6int kvm_perf_init (void ) { -/* -* Check if HW_PERF_EVENTS are supported by checking the number of -* hardware performance counters. This could ensure the presence of -* a physical PMU and CONFIG_PERF_EVENT is selected. -*/ -if (IS_ENABLED (CONFIG_ARM_PMU ) &&perf_num_counters () >0 ) +if (kvm_pmu_probe_pmuver () !=0xf )The
probe()function may be called many times (kvm_arm_pmu_v3_set_attralso calls it). I don't know whether the first calling is enough. If so, can we use a static variable in it, so the following calling can return the result right away?
Quoted 4 lines
No, because that wouldn't help with crappy big-little implementations that could have PMUs with different versions. We want to find the version at the point where the virtual PMU is created, which is why we call the probe function once per vcpu.
I see.
But AFAICS the pmuver is placed in kvm->arch, and the probe function is called once per VM. Maybe I miss something.
Quoted 4 lines
This of course is broken in other ways (BL+KVM is a total disaster when it comes to PMU), but making this static would just make it worse.
OK.
Thanks,
Keqian
Marc Zyngier
maintainerRe: [PATCH 1/5] KVM: arm64: Divorce the perf code from oprofile helpers
Quoted 41 lines · Keqian Zhu
Hi Marc,
Quoted 37 lines · Marc Zyngier
Quoted 34 lines · Keqian Zhu
Hi Marc,
Quoted 16 lines · Marc Zyngier
KVM/arm64 is the sole user of
perf_num_counters(), and really could do without it. Stop using the obsolete API by relying on the existing probing code.Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/perf.c | 7 +------ arch/arm64/kvm/pmu-emul.c | 2 +- include/kvm/arm_pmu.h | 4 ++++ 3 files changed, 6 insertions(+), 7 deletions(-)arch/arm64/kvm/perf.c +0
-0arch/arm64/kvm/perf.c static struct perf_guest_info_callbacks kvm_guest_cbs = { · line 53
Quoted 10 lines
arch/arm64/kvm/perf.c +1
-6@@ -50,12 +50,7 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {int kvm_perf_init (void ) { -/* -* Check if HW_PERF_EVENTS are supported by checking the number of -* hardware performance counters. This could ensure the presence of -* a physical PMU and CONFIG_PERF_EVENT is selected. -*/ -if (IS_ENABLED (CONFIG_ARM_PMU ) &&perf_num_counters () >0 ) +if (kvm_pmu_probe_pmuver () !=0xf )The
probe()function may be called many times (kvm_arm_pmu_v3_set_attralso calls it). I don't know whether the first calling is enough. If so, can we use a static variable in it, so the following calling can return the result right away?
Quoted 8 lines
Quoted 4 lines
No, because that wouldn't help with crappy big-little implementations that could have PMUs with different versions. We want to find the version at the point where the virtual PMU is created, which is why we call the probe function once per vcpu.
I see.
But AFAICS the pmuver is placed in
kvm->arch, and the probe function is called once per VM. Maybe I miss something.
You're right, I mis-remembered. This doesn't change much though.
Thanks,
M.
Signature · 5 lines
Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 1/5] KVM: arm64: Divorce the perf code from oprofile helpers
In reply to Marc ZyngierQuoted 16 lines · Marc Zyngier
KVM/arm64 is the sole user of
perf_num_counters(), and really could do without it. Stop using the obsolete API by relying on the existing probing code.Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/perf.c | 7 +------ arch/arm64/kvm/pmu-emul.c | 2 +- include/kvm/arm_pmu.h | 4 ++++ 3 files changed, 6 insertions(+), 7 deletions(-)arch/arm64/kvm/perf.c +0
-0
arch/arm64/kvm/perf.c static struct perf_guest_info_callbacks kvm_guest_cbs = { · line 53
Quoted 10 lines
arch/arm64/kvm/perf.c +1
-6@@ -50,12 +50,7 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {int kvm_perf_init (void ) { -/* -* Check if HW_PERF_EVENTS are supported by checking the number of -* hardware performance counters. This could ensure the presence of -* a physical PMU and CONFIG_PERF_EVENT is selected. -*/ -if (IS_ENABLED (CONFIG_ARM_PMU ) &&perf_num_counters () >0 ) +if (kvm_pmu_probe_pmuver () !=0xf )
Took me a while to figure out that this returns 0xf if the hardware has a PMUVer of 0x0, so it's all good:
Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Marc Zyngier
maintainerPATCH 2/5 arm64: Get rid of oprofile leftovers
In reply to Marc Zyngierperf_pmu_name() and perf_num_counters() are now unused. Drop them.
--- drivers/perf/arm_pmu.c | 30 ------------------------------ 1 file changed, 30 deletions(-)
drivers/perf/arm_pmu.c +0 -30
@@ -581,33 +581,6 @@ static const struct attribute_group armpmu_common_attr_group = { .attrs = armpmu_common_attrs, }; -/* Set at runtime when we know what CPU type we are. */ -static struct arm_pmu *__oprofile_cpu_pmu; - -/* -* Despite the names, these two functions are CPU-specific and are used -* by the OProfile/perf code. -*/ -const char *perf_pmu_name (void ) -{ -if (!__oprofile_cpu_pmu) -return NULL ; - -return __oprofile_cpu_pmu->name; -} -EXPORT_SYMBOL_GPL (perf_pmu_name); - -int perf_num_counters (void ) -{ -int max_events =0 ; - -if (__oprofile_cpu_pmu !=NULL ) - max_events = __oprofile_cpu_pmu->num_events; - -return max_events; -} -EXPORT_SYMBOL_GPL (perf_num_counters); -static int armpmu_count_irq_users (const int irq) {int cpu, count =0 ;
@@ -979,9 +952,6 @@ int armpmu_register(struct arm_pmu *pmu)if (ret) goto out_destroy; -if (!__oprofile_cpu_pmu) - __oprofile_cpu_pmu = pmu; -pr_info ("enabled with %s PMU driver, %d counters available%s\n" , pmu->name, pmu->num_events, has_nmi ?", using NMIs" :"" );
Signature · 1 line
2.29.2
Re: [PATCH 2/5] arm64: Get rid of oprofile leftovers
Quoted 6 lines · Marc Zyngier
perf_pmu_name()andperf_num_counters()are now unused. Drop them.Signed-off-by Marc Zyngier <[email protected]>--- drivers/perf/arm_pmu.c | 30 ------------------------------ 1 file changed, 30 deletions(-)
Nice! This was some of the first code I ever wrote in the kernel but I can't say I miss it:
Will
Marc Zyngier
maintainerPATCH 3/5 s390: Get rid of oprofile leftovers
In reply to Marc Zyngierperf_pmu_name() and perf_num_counters() are unused. Drop them.
--- arch/s390/kernel/perf_event.c | 21 --------------------- 1 file changed, 21 deletions(-)
arch/s390/kernel/perf_event.c +0 -21
@@ -23,27 +23,6 @@#include <asm/sysinfo.h> #include <asm/unwind.h> -const char *perf_pmu_name (void ) -{ -if (cpum_cf_avail () ||cpum_sf_avail ()) -return "CPU-Measurement Facilities (CPU-MF)" ; -return "pmu" ; -} -EXPORT_SYMBOL (perf_pmu_name); - -int perf_num_counters (void ) -{ -int num =0 ; - -if (cpum_cf_avail ()) - num +=PERF_CPUM_CF_MAX_CTR ; -if (cpum_sf_avail ()) - num +=PERF_CPUM_SF_MAX_CTR ; - -return num; -} -EXPORT_SYMBOL (perf_num_counters); -static struct kvm_s390_sie_block *sie_block (struct pt_regs *regs) {struct stack_frame *stack = (struct stack_frame *) regs->gprs[15 ];
Signature · 1 line
2.29.2
Re: [PATCH 3/5] s390: Get rid of oprofile leftovers
Quoted 6 lines · Marc Zyngier
perf_pmu_name()andperf_num_counters()are unused. Drop them.Signed-off-by Marc Zyngier <[email protected]>--- arch/s390/kernel/perf_event.c | 21 --------------------- 1 file changed, 21 deletions(-)
...or do you want me to pick this up and route via the s390 tree(?).
Marc Zyngier
maintainerRe: [PATCH 3/5] s390: Get rid of oprofile leftovers
Quoted 4 lines · Heiko Carstens
Quoted 2 lines · Marc Zyngier
perf_pmu_name()andperf_num_counters()are unused. Drop them.
Quoted 8 lines
Quoted 4 lines
Signed-off-by Marc Zyngier <[email protected]>--- arch/s390/kernel/perf_event.c | 21 --------------------- 1 file changed, 21 deletions(-)Acked-by Heiko Carstens <[email protected]>...or do you want me to pick this up and route via the s390 tree(?).
Either way work for me, but I just want to make sure the last patch doesn't get applied before the previous ones.
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Heiko Carstens
maintainerRe: [PATCH 3/5] s390: Get rid of oprofile leftovers
Quoted 11 lines · Marc Zyngier
Quoted 9 lines · Heiko Carstens
Quoted 6 lines · Marc Zyngier
perf_pmu_name()andperf_num_counters()are unused. Drop them.Signed-off-by Marc Zyngier <[email protected]>--- arch/s390/kernel/perf_event.c | 21 --------------------- 1 file changed, 21 deletions(-)
Quoted 6 lines
Quoted 3 lines
Acked-by Heiko Carstens <[email protected]>...or do you want me to pick this up and route via the s390 tree(?).
Either way work for me, but I just want to make sure the last patch doesn't get applied before the previous ones.
Ok, I applied this one to the s390 tree. Thanks!
Marc Zyngier
maintainerPATCH 4/5 sh: Get rid of oprofile leftovers
In reply to Marc Zyngierperf_pmu_name() and perf_num_counters() are unused. Drop them.
--- arch/sh/kernel/perf_event.c | 18 ------------------ 1 file changed, 18 deletions(-)
arch/sh/kernel/perf_event.c +0 -18
@@ -57,24 +57,6 @@ static inline int sh_pmu_initialized(void)return !!sh_pmu; } -const char *perf_pmu_name (void ) -{ -if (!sh_pmu) -return NULL ; - -return sh_pmu->name; -} -EXPORT_SYMBOL_GPL (perf_pmu_name); - -int perf_num_counters (void ) -{ -if (!sh_pmu) -return 0 ; - -return sh_pmu->num_events; -} -EXPORT_SYMBOL_GPL (perf_num_counters); -/* * Release the PMU if this is the last perf_event. */
Signature · 1 line
2.29.2
Re: [PATCH 4/5] sh: Get rid of oprofile leftovers
Quoted 3 lines · Marc Zyngier
perf_pmu_name()andperf_num_counters()are unused. Drop them.Signed-off-by Marc Zyngier <[email protected]>
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Marc Zyngier
maintainerPATCH 5/5 perf: Get rid of oprofile leftovers
In reply to Marc Zyngierperf_pmu_name() and perf_num_counters() are unused. Drop them.
--- include/linux/perf_event.h | 2 -- kernel/events/core.c | 5 ----- 2 files changed, 7 deletions(-)
include/linux/perf_event.h +0 -2
@@ -951,8 +951,6 @@ extern void perf_event_itrace_started(struct perf_event *event);extern int perf_pmu_register (struct pmu *pmu,const char *name,int type);extern void perf_pmu_unregister (struct pmu *pmu); -extern int perf_num_counters (void ); -extern const char *perf_pmu_name (void );extern void __perf_event_task_sched_in (struct task_struct *prev,struct task_struct *task); extern void __perf_event_task_sched_out(struct task_struct *prev,
kernel/events/core.c +0 -5
@@ -580,11 +580,6 @@ static u64 perf_event_time(struct perf_event *event);void __weak perf_event_print_debug (void ) { } -extern __weak const char *perf_pmu_name (void ) -{ -return "pmu" ; -} -static inline u64 perf_clock (void ) {return local_clock ();
Signature · 1 line
2.29.2
Re: [PATCH 5/5] perf: Get rid of oprofile leftovers
Quoted 7 lines · Marc Zyngier
perf_pmu_name()andperf_num_counters()are unused. Drop them.Signed-off-by Marc Zyngier <[email protected]>--- include/linux/perf_event.h | 2 -- kernel/events/core.c | 5 ----- 2 files changed, 7 deletions(-)
Will
Quoted 11 lines · Marc Zyngier
This small series builds on top of the work that was started with [1].
It recently became apparent that KVM/arm64 is the last bit of the kernel that still uses
perf_num_counters().As I went ahead to address this, it became obvious that all traces of oprofile had been eradicated from all architectures but arm64, s390 and sh (plus a bit of cruft in the core perf code). With KVM fixed,
perf_num_counters()andperf_pmu_name()are finally gone.[...]
Applied to kvm-arm64/kill_oprofile_dependency, thanks!
[1/5] KVM: arm64: Divorce the perf code from oprofile helpers
commit: 5421db1be3b11c5e469cce3760d5c8a013a90f2c
[2/5] arm64: Get rid of oprofile leftovers
commit: e9c74a686a45e54b2e1c4586b14c84f3ee2f2014
[3/5] s390: Get rid of oprofile leftovers
commit: 8c3f7913a106aa8b94d331cb59709c84a9a1d55b
[4/5] sh: Get rid of oprofile leftovers
commit: ac21ecf5ad32b89909bee2b50161ce93d6462b7d
[5/5] perf: Get rid of oprofile leftovers
commit: 7f318847a0f37b96d8927e8d30ae7b8f149b11f1Cheers,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.