All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.