linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>,
	Igor Mammedov <imammedo@redhat.com>,
	Marc Zyngier <maz@kernel.org>, James Morse <james.morse@arm.com>,
	Julien Thierry <julien.thierry.kdev@gmail.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Huacai Chen <chenhuacai@kernel.org>,
	Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>,
	Paul Mackerras <paulus@ozlabs.org>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Janosch Frank <frankja@linux.ibm.com>,
	David Hildenbrand <david@redhat.com>,
	Cornelia Huck <cohuck@redhat.com>,
	Claudio Imbrenda <imbrenda@linux.ibm.com>,
	Joerg Roedel <joro@8bytes.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 11/13] KVM: Keep memslots in tree-based structures instead of array-based ones
Date: Thu, 28 Oct 2021 22:22:37 +0000	[thread overview]
Message-ID: <YXsirUMNdgmWXKxC@google.com> (raw)
In-Reply-To: <YXnmrqhvc4mzxZfn@google.com>

On Wed, Oct 27, 2021, Sean Christopherson wrote:
> And doling out the work to helpers also mostly eliminates the need to track the
> inactive vs. active slot.  There's still some slightly twisty logic, especially for
> the MOVE case (which nobody uses, *sigh*).  If we really want, even that mess can
> be cleaned up by pre-allocating an extra memslot, but I don't think it's worth the
> effort so long as things are well documented.

Aha!  I figured out how to do this in a way that reduces complexity and fixes the
wart of "new" being an on-stack object, which has always bugged me but wasn't
really fixable given the old/current memslots approach of duplicating the entire
array.  I belive it "needs" to be done as cleanup on top, i.e. what I've proposed
here would still exist as an intermediate step, but the final result would be clean.

Rather than reuse a single working slot, dynamically allocate "new" except in the
DELETE case.  That way a "working" slot isn't needed for CREATE or FLAGS_ONLY since
they simply replace "old" (NULL for CREATE) with "new".

It requires an extra allocation for MOVE, but (a) it's only a single memslot and
(b) AFAIK no real VMM actually uses MOVE, and the benefit is that it significantly
reduces the weirdness for the code as a whole.  And even MOVE is cleaned up because
it more clearly replace old and invalid_slot with new, as opposed to doing a weird
dance where it uses the old slot as the "working" slot.

Assuming my approach actually works, I really like the end result.  I got waylaid
yet again on other stuff, so I may not get a series posted today.  *sigh*

  reply	other threads:[~2021-10-28 22:22 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-20 21:38 [PATCH v5 00/13] KVM: Scalable memslots implementation Maciej S. Szmigiero
2021-09-20 21:38 ` [PATCH v5 01/13] KVM: x86: Cache total page count to avoid traversing the memslot array Maciej S. Szmigiero
2021-10-19 22:24   ` Sean Christopherson
2021-10-19 22:31     ` Sean Christopherson
2021-10-20 18:40       ` Maciej S. Szmigiero
2021-10-20 18:41     ` Maciej S. Szmigiero
2021-10-20 19:01       ` Sean Christopherson
2021-11-01 22:29         ` Sean Christopherson
2021-11-03 11:59           ` Maciej S. Szmigiero
2021-11-03 14:47             ` Sean Christopherson
2021-11-03 15:38               ` Maciej S. Szmigiero
2021-09-20 21:38 ` [PATCH v5 02/13] KVM: x86: Don't call kvm_mmu_change_mmu_pages() if the count hasn't changed Maciej S. Szmigiero
2021-09-20 21:38 ` [PATCH v5 03/13] KVM: Add "old" memslot parameter to kvm_arch_prepare_memory_region() Maciej S. Szmigiero
2021-09-20 21:38 ` [PATCH v5 04/13] KVM: x86: Move n_memslots_pages recalc " Maciej S. Szmigiero
2021-10-19 22:38   ` Sean Christopherson
2021-10-20 18:41     ` Maciej S. Szmigiero
2021-09-20 21:38 ` [PATCH v5 05/13] KVM: Integrate gfn_to_memslot_approx() into search_memslots() Maciej S. Szmigiero
2021-10-19 23:38   ` Sean Christopherson
2021-10-20 18:41     ` Maciej S. Szmigiero
2021-09-20 21:38 ` [PATCH v5 06/13] KVM: Move WARN on invalid memslot index to update_memslots() Maciej S. Szmigiero
2021-10-19 23:42   ` Sean Christopherson
2021-09-20 21:38 ` [PATCH v5 07/13] KVM: Just resync arch fields when slots_arch_lock gets reacquired Maciej S. Szmigiero
2021-10-19 23:55   ` Sean Christopherson
2021-10-20 18:41     ` Maciej S. Szmigiero
2021-10-20 18:57       ` Sean Christopherson
2021-10-20 18:58         ` Maciej S. Szmigiero
2021-09-20 21:38 ` [PATCH v5 08/13] KVM: Resolve memslot ID via a hash table instead of via a static array Maciej S. Szmigiero
2021-10-20  0:43   ` Sean Christopherson
2021-10-20 18:42     ` Maciej S. Szmigiero
2021-10-20 22:39   ` Sean Christopherson
2021-10-21 14:15     ` Maciej S. Szmigiero
2021-09-20 21:38 ` [PATCH v5 09/13] KVM: Use interval tree to do fast hva lookup in memslots Maciej S. Szmigiero
2021-10-26 18:19   ` Sean Christopherson
2021-10-26 18:46     ` Maciej S. Szmigiero
2021-09-20 21:38 ` [PATCH v5 10/13] KVM: s390: Introduce kvm_s390_get_gfn_end() Maciej S. Szmigiero
2021-09-20 21:38 ` [PATCH v5 11/13] KVM: Keep memslots in tree-based structures instead of array-based ones Maciej S. Szmigiero
2021-10-27  0:36   ` Sean Christopherson
2021-10-27 23:54     ` Sean Christopherson
2021-10-28 22:22       ` Sean Christopherson [this message]
2021-09-20 21:39 ` [PATCH v5 12/13] KVM: Optimize gfn lookup in kvm_zap_gfn_range() Maciej S. Szmigiero
2021-10-20 23:47   ` Sean Christopherson
2021-10-21 14:16     ` Maciej S. Szmigiero
2021-10-21 16:30       ` Sean Christopherson
2021-10-21 21:44         ` Maciej S. Szmigiero
2021-09-20 21:39 ` [PATCH v5 13/13] KVM: Optimize overlapping memslots check Maciej S. Szmigiero
2021-10-26 18:59   ` Sean Christopherson
2021-10-27 13:48     ` Maciej S. Szmigiero
2021-10-28 17:53       ` Sean Christopherson
2021-10-29 16:23         ` Maciej S. Szmigiero
2021-10-30  0:32           ` Sean Christopherson
2021-10-19 22:07 ` [PATCH v5 00/13] KVM: Scalable memslots implementation Sean Christopherson
2021-10-20 18:40   ` Maciej S. Szmigiero
2021-10-20 19:58     ` 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=YXsirUMNdgmWXKxC@google.com \
    --to=seanjc@google.com \
    --cc=aleksandar.qemu.devel@gmail.com \
    --cc=borntraeger@de.ibm.com \
    --cc=chenhuacai@kernel.org \
    --cc=cohuck@redhat.com \
    --cc=david@redhat.com \
    --cc=frankja@linux.ibm.com \
    --cc=imammedo@redhat.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=james.morse@arm.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=julien.thierry.kdev@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mail@maciej.szmigiero.name \
    --cc=maz@kernel.org \
    --cc=paulus@ozlabs.org \
    --cc=pbonzini@redhat.com \
    --cc=suzuki.poulose@arm.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).