From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.0 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 242B5C433DB for ; Wed, 10 Mar 2021 14:19:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E6C8464FF4 for ; Wed, 10 Mar 2021 14:19:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231244AbhCJOTR (ORCPT ); Wed, 10 Mar 2021 09:19:17 -0500 Received: from mail.kernel.org ([198.145.29.99]:36280 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230373AbhCJOTK (ORCPT ); Wed, 10 Mar 2021 09:19:10 -0500 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id EF53364FEF; Wed, 10 Mar 2021 14:19:09 +0000 (UTC) Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94) (envelope-from ) id 1lJzg3-000mQq-PG; Wed, 10 Mar 2021 14:19:07 +0000 Date: Wed, 10 Mar 2021 14:19:06 +0000 Message-ID: <878s6vxfad.wl-maz@kernel.org> From: Marc Zyngier To: Jing Zhang Cc: KVM , KVM ARM , Linux MIPS , KVM PPC , Linux S390 , Linux kselftest , Paolo Bonzini , James Morse , Julien Thierry , Suzuki K Poulose , Will Deacon , Huacai Chen , Aleksandar Markovic , Thomas Bogendoerfer , Paul Mackerras , Christian Borntraeger , Janosch Frank , David Hildenbrand , Cornelia Huck , Claudio Imbrenda , Sean Christopherson , Vitaly Kuznetsov , Jim Mattson , Peter Shier , Oliver Upton , David Rientjes , Emanuele Giuseppe Esposito Subject: Re: [RFC PATCH 1/4] KVM: stats: Separate statistics name strings from debugfs code In-Reply-To: <20210310003024.2026253-2-jingzhangos@google.com> References: <20210310003024.2026253-1-jingzhangos@google.com> <20210310003024.2026253-2-jingzhangos@google.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: jingzhangos@google.com, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, linux-mips@vger.kernel.org, kvm-ppc@vger.kernel.org, linux-s390@vger.kernel.org, linux-kselftest@vger.kernel.org, pbonzini@redhat.com, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com, will@kernel.org, chenhuacai@kernel.org, aleksandar.qemu.devel@gmail.com, tsbogend@alpha.franken.de, paulus@ozlabs.org, borntraeger@de.ibm.com, frankja@linux.ibm.com, david@redhat.com, cohuck@redhat.com, imbrenda@linux.ibm.com, seanjc@google.com, vkuznets@redhat.com, jmattson@google.com, pshier@google.com, oupton@google.com, rientjes@google.com, eesposit@redhat.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Hi Jing, On Wed, 10 Mar 2021 00:30:21 +0000, Jing Zhang wrote: > > Prepare the statistics name strings for supporting binary format > aggregated statistics data retrieval. > > No functional change intended. > > Signed-off-by: Jing Zhang > --- > arch/arm64/kvm/guest.c | 47 ++++-- > arch/mips/kvm/mips.c | 114 ++++++++++---- > arch/powerpc/kvm/book3s.c | 107 +++++++++---- > arch/powerpc/kvm/booke.c | 84 +++++++--- > arch/s390/kvm/kvm-s390.c | 320 ++++++++++++++++++++++++++------------ > arch/x86/kvm/x86.c | 127 ++++++++++----- > include/linux/kvm_host.h | 31 +++- > 7 files changed, 589 insertions(+), 241 deletions(-) > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c > index 9bbd30e62799..fb3aafe76b52 100644 > --- a/arch/arm64/kvm/guest.c > +++ b/arch/arm64/kvm/guest.c > @@ -28,19 +28,42 @@ > > #include "trace.h" > > +const char kvm_vm_stat_strings[][KVM_STATS_NAME_LEN] = { > + "remote_tlb_flush", > +}; > +static_assert(sizeof(kvm_vm_stat_strings) == > + VM_STAT_COUNT * KVM_STATS_NAME_LEN); > + > +const char kvm_vcpu_stat_strings[][KVM_STATS_NAME_LEN] = { > + "halt_successful_poll", > + "halt_attempted_poll", > + "halt_poll_success_ns", > + "halt_poll_fail_ns", > + "halt_poll_invalid", > + "halt_wakeup", > + "hvc_exit_stat", > + "wfe_exit_stat", > + "wfi_exit_stat", > + "mmio_exit_user", > + "mmio_exit_kernel", > + "exits", > +}; > +static_assert(sizeof(kvm_vcpu_stat_strings) == > + VCPU_STAT_COUNT * KVM_STATS_NAME_LEN); > + > struct kvm_stats_debugfs_item debugfs_entries[] = { > - VCPU_STAT("halt_successful_poll", halt_successful_poll), > - VCPU_STAT("halt_attempted_poll", halt_attempted_poll), > - VCPU_STAT("halt_poll_invalid", halt_poll_invalid), > - VCPU_STAT("halt_wakeup", halt_wakeup), > - VCPU_STAT("hvc_exit_stat", hvc_exit_stat), > - VCPU_STAT("wfe_exit_stat", wfe_exit_stat), > - VCPU_STAT("wfi_exit_stat", wfi_exit_stat), > - VCPU_STAT("mmio_exit_user", mmio_exit_user), > - VCPU_STAT("mmio_exit_kernel", mmio_exit_kernel), > - VCPU_STAT("exits", exits), > - VCPU_STAT("halt_poll_success_ns", halt_poll_success_ns), > - VCPU_STAT("halt_poll_fail_ns", halt_poll_fail_ns), > + VCPU_STAT(halt_successful_poll), > + VCPU_STAT(halt_attempted_poll), > + VCPU_STAT(halt_poll_invalid), > + VCPU_STAT(halt_wakeup), > + VCPU_STAT(hvc_exit_stat), > + VCPU_STAT(wfe_exit_stat), > + VCPU_STAT(wfi_exit_stat), > + VCPU_STAT(mmio_exit_user), > + VCPU_STAT(mmio_exit_kernel), > + VCPU_STAT(exits), > + VCPU_STAT(halt_poll_success_ns), > + VCPU_STAT(halt_poll_fail_ns), So we now have two arrays that can easily deviate in their order, whilst we didn't have that risk before. What is the advantage of doing this? The commit message doesn't really say... [...] > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 1b65e7204344..1ea297458306 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -1162,6 +1162,18 @@ static inline bool kvm_is_error_gpa(struct kvm *kvm, gpa_t gpa) > return kvm_is_error_hva(hva); > } > > +#define VM_STAT_COUNT (sizeof(struct kvm_vm_stat)/sizeof(ulong)) > +#define VCPU_STAT_COUNT (sizeof(struct kvm_vcpu_stat)/sizeof(u64)) > +#define KVM_STATS_NAME_LEN 32 > + > +/* Make sure it is synced with fields in struct kvm_vm_stat. */ > +extern const char kvm_vm_stat_strings[][KVM_STATS_NAME_LEN]; > +/* Make sure it is synced with fields in struct kvm_vcpu_stat. */ > +extern const char kvm_vcpu_stat_strings[][KVM_STATS_NAME_LEN]; > + > +#define VM_STAT_NAME(id) (kvm_vm_stat_strings[id]) > +#define VCPU_STAT_NAME(id) (kvm_vcpu_stat_strings[id]) > + > enum kvm_stat_kind { > KVM_STAT_VM, > KVM_STAT_VCPU, > @@ -1182,10 +1194,21 @@ struct kvm_stats_debugfs_item { > #define KVM_DBGFS_GET_MODE(dbgfs_item) \ > ((dbgfs_item)->mode ? (dbgfs_item)->mode : 0644) > > -#define VM_STAT(n, x, ...) \ > - { n, offsetof(struct kvm, stat.x), KVM_STAT_VM, ## __VA_ARGS__ } > -#define VCPU_STAT(n, x, ...) \ > - { n, offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU, ## __VA_ARGS__ } > +#define VM_STAT(x, ...) \ > + { \ > + VM_STAT_NAME(offsetof(struct kvm_vm_stat, x)/sizeof(ulong)), \ > + offsetof(struct kvm, stat.x), \ > + KVM_STAT_VM, \ > + ## __VA_ARGS__ \ > + } > + > +#define VCPU_STAT(x, ...) \ > + { \ > + VCPU_STAT_NAME(offsetof(struct kvm_vcpu_stat, x)/sizeof(u64)), \ > + offsetof(struct kvm_vcpu, stat.x), \ > + KVM_STAT_VCPU, \ > + ## __VA_ARGS__ \ > + } Is there any reason why we want to keep kvm_vm_stat populated with ulong, while kvm_vcpu_stat is populated with u64? I have the feeling that this is a fairly pointless difference, and that some of the macros could be unified. Also, using names initialisers would help with the readability of the macros. Thanks, M. -- Without deviation from the norm, progress is not possible.