kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: David Matlack <dmatlack@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>,
	Mingwei Zhang <mizhang@google.com>
Subject: Re: [PATCH v2 24/30] KVM: x86/mmu: Allow yielding when zapping GFNs for defunct TDP MMU root
Date: Wed, 5 Jan 2022 21:32:51 +0000	[thread overview]
Message-ID: <YdYOg6UnRPJsznjC@google.com> (raw)
In-Reply-To: <YdTzkjOi2w+MVA2V@google.com>

On Wed, Jan 05, 2022, David Matlack wrote:
> On Thu, Dec 23, 2021 at 10:23:12PM +0000, Sean Christopherson wrote:
> > +	/*
> > +	 * Invalidate the root to prevent it from being reused by a vCPU, and
> > +	 * mark it defunct so that kvm_tdp_mmu_zap_invalidated_roots() doesn't
> > +	 * try to put a reference it didn't acquire.
> > +	 */
> > +	root->role.invalid = true;
> > +	root->tdp_mmu_defunct_root = true;
> 
> Ah ok so tdp_mmu_defunct_root indicates the root became invalid due to
> losing all its references while it was valid. This is in contrast to
> kvm_tdp_mmu_invalidate_all_roots() which marks roots invalid while they
> still have valid references.
> 
> But I wonder if tdp_mmu_defunct_root is necessary? It's only used to
> skip a put in zap_invalidated_roots. Could we instead unconditionally
> grab a reference in invalidate_all_roots and then unconditionally drop
> it?

Hmm, it's probably not necessary.  I added tdp_mmu_defunct_root before realizing
that that kvm_tdp_mmu_invalidate_all_roots() was wrong about this:

 * Roots which have a zero refcount should be skipped as
 * they're already being torn down.

IIRC, I added the second flag because I was trying to honor that (incorrect) logic,
and never reconsidered the need for a second flag once I got everything working.
The only downside would be keeping the memory for defunct roots around a wee bit
longer, and that's not a big deal.

I'll yank it out for the next version, assuming I didn't forget some detail...

  reply	other threads:[~2022-01-05 21:34 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-23 22:22 [PATCH v2 00/30] KVM: x86/mmu: Overhaul TDP MMU zapping and flushing Sean Christopherson
2021-12-23 22:22 ` [PATCH v2 01/30] KVM: x86/mmu: Use common TDP MMU zap helper for MMU notifier unmap hook Sean Christopherson
2021-12-23 22:22 ` [PATCH v2 02/30] KVM: x86/mmu: Move "invalid" check out of kvm_tdp_mmu_get_root() Sean Christopherson
2021-12-23 22:22 ` [PATCH v2 03/30] KVM: x86/mmu: Zap _all_ roots when unmapping gfn range in TDP MMU Sean Christopherson
2021-12-23 22:22 ` [PATCH v2 04/30] KVM: x86/mmu: Use common iterator for walking invalid TDP MMU roots Sean Christopherson
2022-01-04 22:13   ` David Matlack
2021-12-23 22:22 ` [PATCH v2 05/30] KVM: x86/mmu: Check for present SPTE when clearing dirty bit in TDP MMU Sean Christopherson
2021-12-23 22:22 ` [PATCH v2 06/30] KVM: x86/mmu: Fix wrong/misleading comments in TDP MMU fast zap Sean Christopherson
2021-12-23 22:22 ` [PATCH v2 07/30] KVM: x86/mmu: Formalize TDP MMU's (unintended?) deferred TLB flush logic Sean Christopherson
2021-12-23 22:22 ` [PATCH v2 08/30] KVM: x86/mmu: Document that zapping invalidated roots doesn't need to flush Sean Christopherson
2021-12-23 22:22 ` [PATCH v2 09/30] KVM: x86/mmu: Drop unused @kvm param from kvm_tdp_mmu_get_root() Sean Christopherson
2021-12-23 22:22 ` [PATCH v2 10/30] KVM: x86/mmu: Require mmu_lock be held for write in unyielding root iter Sean Christopherson
2021-12-23 22:22 ` [PATCH v2 11/30] KVM: x86/mmu: Check for !leaf=>leaf, not PFN change, in TDP MMU SP removal Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 12/30] KVM: x86/mmu: Batch TLB flushes from TDP MMU for MMU notifier change_spte Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 13/30] KVM: x86/mmu: Drop RCU after processing each root in MMU notifier hooks Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 14/30] KVM: x86/mmu: Add helpers to read/write TDP MMU SPTEs and document RCU Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 15/30] KVM: x86/mmu: WARN if old _or_ new SPTE is REMOVED in non-atomic path Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 16/30] KVM: x86/mmu: Refactor low-level TDP MMU set SPTE helper to take raw vals Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 17/30] KVM: x86/mmu: Zap only the target TDP MMU shadow page in NX recovery Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 18/30] KVM: x86/mmu: Skip remote TLB flush when zapping all of TDP MMU Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 19/30] KVM: x86/mmu: Add dedicated helper to zap TDP MMU root shadow page Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 20/30] KVM: x86/mmu: Require mmu_lock be held for write to zap TDP MMU range Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 21/30] KVM: x86/mmu: Zap only TDP MMU leafs in kvm_zap_gfn_range() Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 22/30] KVM: x86/mmu: Do remote TLB flush before dropping RCU in TDP MMU resched Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 23/30] KVM: x86/mmu: Defer TLB flush to caller when freeing TDP MMU shadow pages Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 24/30] KVM: x86/mmu: Allow yielding when zapping GFNs for defunct TDP MMU root Sean Christopherson
2022-01-05  1:25   ` David Matlack
2022-01-05 21:32     ` Sean Christopherson [this message]
2021-12-23 22:23 ` [PATCH v2 25/30] KVM: x86/mmu: Zap roots in two passes to avoid inducing RCU stalls Sean Christopherson
2022-01-05  0:19   ` David Matlack
2022-01-05 16:33     ` Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 26/30] KVM: x86/mmu: Zap defunct roots via asynchronous worker Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 27/30] KVM: selftests: Move raw KVM_SET_USER_MEMORY_REGION helper to utils Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 28/30] KVM: selftests: Split out helper to allocate guest mem via memfd Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 29/30] KVM: selftests: Define cpu_relax() helpers for s390 and x86 Sean Christopherson
2021-12-23 22:23 ` [PATCH v2 30/30] KVM: selftests: Add test to populate a VM with the max possible guest mem Sean Christopherson

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=YdYOg6UnRPJsznjC@google.com \
    --to=seanjc@google.com \
    --cc=bgardon@google.com \
    --cc=dmatlack@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 \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).