Messages in this thread | | | Subject | Re: [PATCH v3 04/16] perf: Stop pretending that perf can handle multiple guest callbacks | From | Paolo Bonzini <> | Date | Wed, 22 Sep 2021 08:25:44 +0200 |
| |
On 22/09/21 02:05, Sean Christopherson wrote: > Drop the 'int' return value from the perf (un)register callbacks helpers > and stop pretending perf can support multiple callbacks. The 'int' > returns are not future proofing anything as none of the callers take > action on an error. It's also not obvious that there will ever be > co-tenant hypervisors, and if there are, that allowing multiple callbacks > to be registered is desirable or even correct. > > Opportunistically rename callbacks=>cbs in the affected declarations to > match their definitions. > > No functional change intended. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/arm64/include/asm/kvm_host.h | 4 ++-- > arch/arm64/kvm/perf.c | 8 ++++---- > include/linux/perf_event.h | 12 ++++++------ > kernel/events/core.c | 16 ++++------------ > 4 files changed, 16 insertions(+), 24 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 41911585ae0c..ed940aec89e0 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -670,8 +670,8 @@ unsigned long kvm_mmio_read_buf(const void *buf, unsigned int len); > int kvm_handle_mmio_return(struct kvm_vcpu *vcpu); > int io_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa); > > -int kvm_perf_init(void); > -int kvm_perf_teardown(void); > +void kvm_perf_init(void); > +void kvm_perf_teardown(void); > > long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu); > gpa_t kvm_init_stolen_time(struct kvm_vcpu *vcpu); > diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c > index 151c31fb9860..c37c0cf1bfe9 100644 > --- a/arch/arm64/kvm/perf.c > +++ b/arch/arm64/kvm/perf.c > @@ -48,15 +48,15 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = { > .get_guest_ip = kvm_get_guest_ip, > }; > > -int kvm_perf_init(void) > +void kvm_perf_init(void) > { > if (kvm_pmu_probe_pmuver() != 0xf && !is_protected_kvm_enabled()) > static_branch_enable(&kvm_arm_pmu_available); > > - return perf_register_guest_info_callbacks(&kvm_guest_cbs); > + perf_register_guest_info_callbacks(&kvm_guest_cbs); > } > > -int kvm_perf_teardown(void) > +void kvm_perf_teardown(void) > { > - return perf_unregister_guest_info_callbacks(&kvm_guest_cbs); > + perf_unregister_guest_info_callbacks(&kvm_guest_cbs); > } > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index 6b0405e578c1..317d4658afe9 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -1245,8 +1245,8 @@ static inline struct perf_guest_info_callbacks *perf_get_guest_cbs(void) > /* Prevent reloading between a !NULL check and dereferences. */ > return READ_ONCE(perf_guest_cbs); > } > -extern int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks); > -extern int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks); > +extern void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs); > +extern void perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs); > > extern void perf_event_exec(void); > extern void perf_event_comm(struct task_struct *tsk, bool exec); > @@ -1489,10 +1489,10 @@ perf_sw_event(u32 event_id, u64 nr, struct pt_regs *regs, u64 addr) { } > static inline void > perf_bp_event(struct perf_event *event, void *data) { } > > -static inline int perf_register_guest_info_callbacks > -(struct perf_guest_info_callbacks *callbacks) { return 0; } > -static inline int perf_unregister_guest_info_callbacks > -(struct perf_guest_info_callbacks *callbacks) { return 0; } > +static inline void perf_register_guest_info_callbacks > +(struct perf_guest_info_callbacks *cbs) { } > +static inline void perf_unregister_guest_info_callbacks > +(struct perf_guest_info_callbacks *cbs) { } > > static inline void perf_event_mmap(struct vm_area_struct *vma) { } > > diff --git a/kernel/events/core.c b/kernel/events/core.c > index 80ff050a7b55..d90a43572400 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -6482,31 +6482,23 @@ static void perf_pending_event(struct irq_work *entry) > perf_swevent_put_recursion_context(rctx); > } > > -/* > - * We assume there is only KVM supporting the callbacks. > - * Later on, we might change it to a list if there is > - * another virtualization implementation supporting the callbacks. > - */ > struct perf_guest_info_callbacks *perf_guest_cbs; > - > -int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs) > +void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs) > { > if (WARN_ON_ONCE(perf_guest_cbs)) > - return -EBUSY; > + return; > > WRITE_ONCE(perf_guest_cbs, cbs);
Maybe you want a smp_store_release or rcu_assign_pointer here?
> - return 0; > } > EXPORT_SYMBOL_GPL(perf_register_guest_info_callbacks); > > -int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs) > +void perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs) > { > if (WARN_ON_ONCE(perf_guest_cbs != cbs)) > - return -EINVAL; > + return; > > WRITE_ONCE(perf_guest_cbs, NULL); > synchronize_rcu(); > - return 0; > } > EXPORT_SYMBOL_GPL(perf_unregister_guest_info_callbacks); > >
Apart from the above,
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
| |