LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Mingwei Zhang <mizhang@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Ben Gardon <bgardon@google.com>,
	Jing Zhang <jingzhangos@google.com>,
	David Matlack <dmatlack@google.com>
Subject: Re: [PATCH v3 3/3] KVM: x86/mmu: Add detailed page size stats
Date: Mon, 2 Aug 2021 23:02:05 +0000	[thread overview]
Message-ID: <YQh5baC5cYAuj6y0@google.com> (raw)
In-Reply-To: <20210730225939.3852712-4-mizhang@google.com>

On Fri, Jul 30, 2021, Mingwei Zhang wrote:
> Existing KVM code tracks the number of large pages regardless of their
> sizes. Therefore, when large page of 1GB (or larger) is adopted, the
> information becomes less useful because lpages counts a mix of 1G and 2M
> pages.
> 
> So remove the lpages since it is easy for user space to aggregate the info.
> Instead, provide a comprehensive page stats of all sizes from 4K to 512G.
> 
> Suggested-by: Ben Gardon <bgardon@google.com>
> Reviewed-by: Ben Gardon <bgardon@google.com>
> Signed-off-by: Mingwei Zhang <mizhang@google.com>
> Cc: Jing Zhang <jingzhangos@google.com>
> Cc: David Matlack <dmatlack@google.com>
> Cc: Sean Christopherson <seanjc@google.com>

Looks good, but you'll probably need/want to rebase to kvm/queue once that settles
down (I suspect a forced push is coming this week).  This has quite a few conflicts
with other stuff sitting in kvm/queue.

> ---
>  arch/x86/include/asm/kvm_host.h | 10 +++++++++-
>  arch/x86/kvm/mmu.h              |  4 ++++
>  arch/x86/kvm/mmu/mmu.c          | 26 +++++++++++++-------------
>  arch/x86/kvm/mmu/tdp_mmu.c      | 15 ++-------------
>  arch/x86/kvm/x86.c              |  7 +++++--
>  5 files changed, 33 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 974cbfb1eefe..eb6edc36b3ed 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1206,9 +1206,17 @@ struct kvm_vm_stat {
>  	u64 mmu_recycled;
>  	u64 mmu_cache_miss;
>  	u64 mmu_unsync;
> -	u64 lpages;
>  	u64 nx_lpage_splits;
>  	u64 max_mmu_page_hash_collisions;
> +	union {
> +		struct {
> +			atomic64_t pages_4k;
> +			atomic64_t pages_2m;
> +			atomic64_t pages_1g;
> +			atomic64_t pages_512g;
> +		};
> +		atomic64_t pages[4];
> +	};

...

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 8166ad113fb2..e4dfcd5d83ad 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -235,9 +235,12 @@ const struct _kvm_stats_desc kvm_vm_stats_desc[] = {
>  	STATS_DESC_COUNTER(VM, mmu_recycled),
>  	STATS_DESC_COUNTER(VM, mmu_cache_miss),
>  	STATS_DESC_ICOUNTER(VM, mmu_unsync),
> -	STATS_DESC_ICOUNTER(VM, lpages),
>  	STATS_DESC_ICOUNTER(VM, nx_lpage_splits),
> -	STATS_DESC_PCOUNTER(VM, max_mmu_page_hash_collisions)
> +	STATS_DESC_PCOUNTER(VM, max_mmu_page_hash_collisions),
> +	STATS_DESC_ICOUNTER(VM, pages_4k),
> +	STATS_DESC_ICOUNTER(VM, pages_2m),
> +	STATS_DESC_ICOUNTER(VM, pages_1g),
> +	STATS_DESC_ICOUNTER(VM, pages_512g)

Uber nit that I wouldn't even have noticed if this didn't conflict, but there's
no need to land the union and the stats definitions at the end of the structs,
i.e. the new fields can directly replace lpages.  I don't think it will actually
avoid a conflict, but it would avoid modifying the max_mmu_page_hash_collisions
line.

>  };
>  static_assert(ARRAY_SIZE(kvm_vm_stats_desc) ==
>  		sizeof(struct kvm_vm_stat) / sizeof(u64));
> -- 
> 2.32.0.554.ge1b32706d8-goog
> 

  parent reply	other threads:[~2021-08-02 23:02 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-30 22:59 [PATCH v3 0/3] Add detailed page size stats in KVM stats Mingwei Zhang
2021-07-30 22:59 ` [PATCH v3 1/3] KVM: x86/mmu: Remove redundant spte present check in mmu_set_spte Mingwei Zhang
2021-08-02 22:29   ` David Matlack
2021-07-30 22:59 ` [PATCH v3 2/3] KVM: x86/mmu: Avoid collision with !PRESENT SPTEs in TDP MMU lpage stats Mingwei Zhang
2021-08-02 22:40   ` David Matlack
2021-07-30 22:59 ` [PATCH v3 3/3] KVM: x86/mmu: Add detailed page size stats Mingwei Zhang
2021-08-02 22:41   ` David Matlack
2021-08-02 23:02   ` Sean Christopherson [this message]
2021-08-02 23:45     ` Mingwei Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YQh5baC5cYAuj6y0@google.com \
    --to=seanjc@google.com \
    --cc=bgardon@google.com \
    --cc=dmatlack@google.com \
    --cc=jingzhangos@google.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mizhang@google.com \
    --cc=pbonzini@redhat.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --subject='Re: [PATCH v3 3/3] KVM: x86/mmu: Add detailed page size stats' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).