Re: [PATCH RFC 4/4] kvm: Implement atomic memory region resizes via region_resize()

[David Hildenbrand]

On 06.03.20 10:50, Paolo Bonzini wrote:
> On 03/03/20 15:19, David Hildenbrand wrote:
>> virtio-mem wants to resize (esp. grow) ram memory regions while the guest
>> is already aware of them and makes use of them. Resizing a KVM slot can
>> only currently be done by removing it and re-adding it. While the kvm slot
>> is temporarily removed, VCPUs that try to read from these slots will fault.
> 

s/try to read/try to access/

> Only fetches I think?  Data reads and write would be treated as MMIO
> accesses and they should just work (using either the old or new FlatView).

On x86-64, I saw KVM fault printks getting printed (it was about 1-2
years ago, though, when I realized this was a problem). Could be that
these were fetches. At least the guest eventually crashed :)

On other archs (esp. s390x) guests will directly receive a
PGM_ADDRESSING from KVM if they stumble over memory that is not covered
by a kvm slot.

> 
>> But also, other ioctls might depend on all slots being in place.
>>
>> Let's inhibit most KVM ioctls while performing the resize. Once we have an
>> ioctl that can perform atomic resizes (e.g., KVM_SET_USER_MEMORY_REGION
>> extensions), we can make inhibiting optional at runtime.
>>
>> Also, make sure to hold the kvm_slots_lock while performing both
>> actions (removing+re-adding).
>>
>> Note: Resizes of memory regions currently seems to happen during bootup
>> only, so I don't think any existing RT users should be affected.
> 
> rwlocks are not efficient, they cause cache line contention.  For
> MMIO-heavy workloads the impact will be very large (well, not that large
> because right now they all take the BQL, but one can always hope).

Yeah, rwlocks are not optimal and I am still looking for better
alternatives (suggestions welcome :) ). Using RCU might not work,
because the rcu_read region might be too big (esp. while in KVM_RUN).

I had a prototype which used a bunch of atomics + qemu_cond_wait. But it
was quite elaborate and buggy.

(I assume only going into KVM_RUN is really affected, and I do wonder if
it will be noticeable at all. Doing an ioctl is always already an
expensive operation.)

I can look into per-cpu locks instead of the rwlock.

> 
> I would very much prefer to add a KVM_SET_USER_MEMORY_REGION extension
> right away.
> 

I really want to avoid dependencies on kernel features to at least make
it work for now. Especially, resizing memory slots in KVM (especially
while dirty bitmaps, rmaps, etc. are active) is non-trivial.

Thanks Paolo!

-- 
Thanks,

David / dhildenb