From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com ([205.139.110.120]:57764 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S2441175AbgDOIaP (ORCPT ); Wed, 15 Apr 2020 04:30:15 -0400 Received: by mail-wm1-f71.google.com with SMTP id n127so4669067wme.4 for ; Wed, 15 Apr 2020 01:30:10 -0700 (PDT) Subject: Re: [PATCH v2] kvm_host: unify VM_STAT and VCPU_STAT definitions in a single place References: <20200414155625.20559-1-eesposit@redhat.com> From: Emanuele Giuseppe Esposito Message-ID: <992ede27-3eae-f2da-ad38-1d3498853ad2@redhat.com> Date: Wed, 15 Apr 2020 10:30:06 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-s390-owner@vger.kernel.org List-ID: To: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , kvm@vger.kernel.org Cc: Marc Zyngier , James Morse , Julien Thierry , Suzuki K Poulose , Paul Mackerras , Christian Borntraeger , Janosch Frank , David Hildenbrand , Cornelia Huck , Paolo Bonzini , Sean Christopherson , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org, kvm-ppc@vger.kernel.org, linux-s390@vger.kernel.org On 4/15/20 8:36 AM, Philippe Mathieu-Daudé wrote: > On 4/14/20 5:56 PM, Emanuele Giuseppe Esposito wrote: >> The macros VM_STAT and VCPU_STAT are redundantly implemented in multiple >> files, each used by a different architecure to initialize the debugfs >> entries for statistics. Since they all have the same purpose, they can be >> unified in a single common definition in include/linux/kvm_host.h >> >> Signed-off-by: Emanuele Giuseppe Esposito >> --- >> arch/arm64/kvm/guest.c | 23 ++--- >> arch/mips/kvm/mips.c | 61 ++++++------ >> arch/powerpc/kvm/book3s.c | 61 ++++++------ >> arch/powerpc/kvm/booke.c | 41 ++++---- >> arch/s390/kvm/kvm-s390.c | 203 +++++++++++++++++++------------------- >> arch/x86/kvm/x86.c | 80 +++++++-------- >> include/linux/kvm_host.h | 5 + >> 7 files changed, 231 insertions(+), 243 deletions(-) >> >> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c >> index 23ebe51410f0..8417b200bec9 100644 >> --- a/arch/arm64/kvm/guest.c >> +++ b/arch/arm64/kvm/guest.c >> @@ -29,20 +29,17 @@ >> >> #include "trace.h" >> >> -#define VM_STAT(x) { #x, offsetof(struct kvm, stat.x), KVM_STAT_VM } >> -#define VCPU_STAT(x) { #x, offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU } >> - >> struct kvm_stats_debugfs_item debugfs_entries[] = { >> - 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_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), >> { NULL } >> }; > > Patch easily reviewed with --word-diff. > > [...] >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> index 6d58beb65454..2e6ead872957 100644 >> --- a/include/linux/kvm_host.h >> +++ b/include/linux/kvm_host.h >> @@ -1130,6 +1130,11 @@ 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, ...) \ > > Not sure while you use so many whitespaces here... (maybe Paolo can > strip some when applying?). I honestly am not sure why it added few more spaces than I wanted, but the idea was to follow the KVM_DBGFS_GET_MODE macro above and put the backslash at the end of the line (80th char). > > Otherwise it looks nicer that v1, thanks. > > Reviewed-by: Philippe Mathieu-Daudé > >> + { n, offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU, ## __VA_ARGS__ } >> + >> extern struct kvm_stats_debugfs_item debugfs_entries[]; >> extern struct dentry *kvm_debugfs_dir; >> >> >