Re: [PATCH RFC] memory: pause all vCPUs for the duration of memory transactions

[Vitaly Kuznetsov]

David Hildenbrand <david@redhat.com> writes:

>>> Same applies to all other kinds of operations (splitting, punching out,
>>> ...) as you also mentioned.
>> 
>> One question from a QEMU newbie though: why do you put
>> kvm_ioctl_inhibit_begin()/kvm_ioctl_inhibit_end() to kvm_region_resize()
>> only and not taking it all the way up to
>> memory_region_transaction_begin()/memory_region_transaction_end() to
>> support atomicity for all kinds of updates right away?
>
> The clean way to implement it for 
> memory_region_transaction_begin()/memory_region_transaction_end() is by 
> implementing
> ->begin()
> ->commit()
> callbacks for the KVM MemoryListener, and doing it in there, in KVM code.
>
>
> Now, I wasn't sure how this might affect real-time workloads, where you 
> really don't want to kick CPUs out of KVM. You can make a lot of 
> operations without requiring this handling like
>
> 1. Adding regions (memory hotplug)
> 2. Removing regions (memory hotunplug)
> 3. Enabling/disabling dirty logging
>
> Resize/split(/move/...) are the problematic operations where we would 
> need that handling. Modifying the size/location of existing slots.
>
> One way to tackle it would be to "sense" upfront if such "modifying" 
> operations will be required, communicating that via "->begin()", and 
> letting the KVM notifier decide based on that information whether to get 
> everything out of KVM. Sounds feasible.
>

I don't actually know if we have such use-cases but thinking about
e.g. punching a hole in a middle of an existing slot requires:
1) Resizing the existing slot to its first half
2) Creating the hole
3) Creating a new slot for the second half of the slot.
In case we'd like to make this atomic, we need to cover the whole
transaction. But again, I don't know if we have a use-case for it or
not.

>> 
>> The second question is whether you plan to sumbit this any time soon)
>
> Once we have an agreement on how to proceed, either I can try to 
> dedicate some time, or if you have some time, you can pickup my work and 
> make it also handle the other problematic cases (as discussed e.g., ^).

Ok, I plan to test your patch in the nearby future and if it works we
can start with just the 'resize' case as it seems to be causing real
issues. The infrastructure you create with kvm_ioctl_inhibit_begin()/
kvm_ioctl_inhibit_end() can be re-used later for other ops.

-- 
Vitaly