Thread (24 messages) 24 messages, 6 authors, 2021-11-16

Re: [PATCH 5/5] KVM: Convert the kvm->vcpus array to a xarray

From: Marc Zyngier <maz@kernel.org>
Date: 2021-11-06 11:49:16
Also in: kvm, kvmarm, linuxppc-dev

On Fri, 05 Nov 2021 20:21:36 +0000,
Sean Christopherson [off-list ref] wrote:
On Fri, Nov 05, 2021, Marc Zyngier wrote:
quoted
At least on arm64 and x86, the vcpus array is pretty huge (512 entries),
and is mostly empty in most cases (running 512 vcpu VMs is not that
common). This mean that we end-up with a 4kB block of unused memory
in the middle of the kvm structure.
Heh, x86 is now up to 1024 entries.
Humph. I don't want to know whether people are actually using that in
practice. The only time I create VMs with 512 vcpus is to check
whether it still works...
 
quoted
Instead of wasting away this memory, let's use an xarray instead,
which gives us almost the same flexibility as a normal array, but
with a reduced memory usage with smaller VMs.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
@@ -693,7 +694,7 @@ static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm *kvm, int i)
 
 	/* Pairs with smp_wmb() in kvm_vm_ioctl_create_vcpu.  */
 	smp_rmb();
-	return kvm->vcpus[i];
+	return xa_load(&kvm->vcpu_array, i);
 }
It'd be nice for this series to convert kvm_for_each_vcpu() to use
xa_for_each() as well.  Maybe as a patch on top so that potential
explosions from that are isolated from the initiali conversion?

Or maybe even use xa_for_each_range() to cap at online_vcpus?
That's technically a functional change, but IMO it's easier to
reason about iterating over a snapshot of vCPUs as opposed to being
able to iterate over vCPUs as their being added.  In practice I
doubt it matters.

#define kvm_for_each_vcpu(idx, vcpup, kvm) \
	xa_for_each_range(&kvm->vcpu_array, idx, vcpup, 0, atomic_read(&kvm->online_vcpus))
I think that's already the behaviour of this iterator (we stop at the
first empty slot capped to online_vcpus. The only change in behaviour
is that vcpup currently holds a pointer to the last vcpu in no empty
slot has been encountered. xa_for_each{,_range}() would set the
pointer to NULL at all times.

I doubt anyone relies on that, but it is probably worth eyeballing
some of the use cases...

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help