[PATCH v5.5 23/30] KVM: Resolve memslot ID via a hash table instead of via a static array

Sean Christopherson seanjc at google.com
Thu Nov 11 17:03:51 PST 2021


On Fri, Nov 12, 2021, Maciej S. Szmigiero wrote:
> On 04.11.2021 01:25, Sean Christopherson wrote:
> > From: Maciej S. Szmigiero <maciej.szmigiero at oracle.com>
> > 
> > Memslot ID to the corresponding memslot mappings are currently kept as
> > indices in static id_to_index array.
> > The size of this array depends on the maximum allowed memslot count
> > (regardless of the number of memslots actually in use).
> > 
> > This has become especially problematic recently, when memslot count cap was
> > removed, so the maximum count is now full 32k memslots - the maximum
> > allowed by the current KVM API.
> > 
> > Keeping these IDs in a hash table (instead of an array) avoids this
> > problem.
> > 
> > Resolving a memslot ID to the actual memslot (instead of its index) will
> > also enable transitioning away from an array-based implementation of the
> > whole memslots structure in a later commit.
> > 
> > Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero at oracle.com>
> > Co-developed-by: Sean Christopherson <seanjc at google.com>
> > Signed-off-by: Sean Christopherson <seanjc at google.com>
> > ---
> >   include/linux/kvm_host.h | 16 +++----
> >   virt/kvm/kvm_main.c      | 96 +++++++++++++++++++++++++++++++---------
> >   2 files changed, 84 insertions(+), 28 deletions(-)
> > 
> (..)
> > @@ -1259,17 +1257,49 @@ static int kvm_alloc_dirty_bitmap(struct kvm_memory_slot *memslot)
> >   	return 0;
> >   }
> > +static void kvm_replace_memslot(struct kvm_memslots *slots,
> > +				struct kvm_memory_slot *old,
> > +				struct kvm_memory_slot *new)
> > +{
> > +	/*
> > +	 * Remove the old memslot from the hash list, copying the node data
> > +	 * would corrupt the list.
> > +	 */
> > +	if (old) {
> > +		hash_del(&old->id_node);
> > +
> > +		if (!new)
> > +			return;
> > +	}
> > +
> > +	/* Copy the source *data*, not the pointer, to the destination. */
> > +	if (old)
> > +		*new = *old;
> 
> This way of writing it (that, is re-checking whether "old" is not-NULL)
> suggests that it could have been set to NULL inside the previous block
> (since the last check), which isn't true.

Yeah, I think I was trying to minimize the logic delta in future patches, but
looking back at the diffs, that didn't pan out.  I've no objection to folding
the two together.



More information about the linux-arm-kernel mailing list