[Qemu-devel] [PATCH v2] migration: optimize the downtime

[Qemu-devel] [PATCH v2] migration: optimize the downtime

[Jay Zhou]

Qemu_savevm_state_cleanup takes about 300ms in my ram migration tests
with a 8U24G vm(20G is really occupied), the main cost comes from
KVM_SET_USER_MEMORY_REGION ioctl when mem.memory_size = 0 in
kvm_set_user_memory_region. In kmod, the main cost is
kvm_zap_obsolete_pages, which traverses the active_mmu_pages list to
zap the unsync sptes.

It can be optimized by delaying memory_global_dirty_log_stop to the next
vm_start.

Changes v1->v2:
 - create a VMChangeStateHandler in memory.c to reduce the coupling [Paolo]

Signed-off-by: Jay Zhou <jianjay.zhou@huawei.com>
---
 memory.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/memory.c b/memory.c
index a7bc70a..4c22b7e 100644
--- a/memory.c
+++ b/memory.c
@@ -2357,8 +2357,14 @@ void memory_global_dirty_log_sync(void)
     }
 }
 
+static VMChangeStateEntry *vmstate_change;
+
 void memory_global_dirty_log_start(void)
 {
+    if (vmstate_change) {
+        qemu_del_vm_change_state_handler(vmstate_change);
+    }
+
     global_dirty_log = true;
 
     MEMORY_LISTENER_CALL_GLOBAL(log_global_start, Forward);
@@ -2369,7 +2375,7 @@ void memory_global_dirty_log_start(void)
     memory_region_transaction_commit();
 }
 
-void memory_global_dirty_log_stop(void)
+static void memory_global_dirty_log_do_stop(void)
 {
     global_dirty_log = false;
 
@@ -2381,6 +2387,25 @@ void memory_global_dirty_log_stop(void)
     MEMORY_LISTENER_CALL_GLOBAL(log_global_stop, Reverse);
 }
 
+static void memory_vm_change_state_handler(void *opaque, int running,
+                                           RunState state)
+{
+    if (running) {
+        memory_global_dirty_log_do_stop();
+    }
+}
+
+void memory_global_dirty_log_stop(void)
+{
+    if (!runstate_is_running()) {
+        vmstate_change = qemu_add_vm_change_state_handler(
+                                memory_vm_change_state_handler, NULL);
+        return;
+    }
+
+    memory_global_dirty_log_do_stop();
+}
+
 static void listener_add_address_space(MemoryListener *listener,
                                        AddressSpace *as)
 {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2] migration: optimize the downtime

[Paolo Bonzini]

On 27/07/2017 04:36, Jay Zhou wrote:
> Qemu_savevm_state_cleanup takes about 300ms in my ram migration tests
> with a 8U24G vm(20G is really occupied), the main cost comes from
> KVM_SET_USER_MEMORY_REGION ioctl when mem.memory_size = 0 in
> kvm_set_user_memory_region. In kmod, the main cost is
> kvm_zap_obsolete_pages, which traverses the active_mmu_pages list to
> zap the unsync sptes.
> 
> It can be optimized by delaying memory_global_dirty_log_stop to the next
> vm_start.
> 
> Changes v1->v2:
>  - create a VMChangeStateHandler in memory.c to reduce the coupling [Paolo]
> 
> Signed-off-by: Jay Zhou <jianjay.zhou@huawei.com>

memory_vm_change_state_handler should delete the handler.

Apart from that, because there is no protection against nested
invocations of memory_global_dirty_log_start/stop, a little more work is
needed.

> ---
>  memory.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/memory.c b/memory.c
> index a7bc70a..4c22b7e 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -2357,8 +2357,14 @@ void memory_global_dirty_log_sync(void)
>      }
>  }
>  
> +static VMChangeStateEntry *vmstate_change;
> +
>  void memory_global_dirty_log_start(void)
>  {
> +    if (vmstate_change) {
> +        qemu_del_vm_change_state_handler(vmstate_change);

This should also NULL vmstate_change, so that you can detect the case
where the handler is already installed.

> +    }
> +
>      global_dirty_log = true;
>  
>      MEMORY_LISTENER_CALL_GLOBAL(log_global_start, Forward);
> @@ -2369,7 +2375,7 @@ void memory_global_dirty_log_start(void)
>      memory_region_transaction_commit();
>  }
>  
> -void memory_global_dirty_log_stop(void)
> +static void memory_global_dirty_log_do_stop(void)
>  {
>      global_dirty_log = false;
>  
> @@ -2381,6 +2387,25 @@ void memory_global_dirty_log_stop(void)
>      MEMORY_LISTENER_CALL_GLOBAL(log_global_stop, Reverse);
>  }
>  
> +static void memory_vm_change_state_handler(void *opaque, int running,
> +                                           RunState state)
> +{
> +    if (running) {
> +        memory_global_dirty_log_do_stop();

Because this will delete the handler, it should also NULL vmstate_change.

> +    }
> +}
> +
> +void memory_global_dirty_log_stop(void)
> +{
> +    if (!runstate_is_running()) {
> +        vmstate_change = qemu_add_vm_change_state_handler(
> +                                memory_vm_change_state_handler, NULL);

And this needs to exit immediately if you have an installed handler.

Thanks,

Paolo

> +        return;
> +    }
> +
> +    memory_global_dirty_log_do_stop();
> +}
> +
>  static void listener_add_address_space(MemoryListener *listener,
>                                         AddressSpace *as)
>  {
> 

Re: [Qemu-devel] [PATCH v2] migration: optimize the downtime

[Jay Zhou]

Hi Paolo,

On 2017/7/27 22:15, Paolo Bonzini wrote:
> On 27/07/2017 04:36, Jay Zhou wrote:
>> Qemu_savevm_state_cleanup takes about 300ms in my ram migration tests
>> with a 8U24G vm(20G is really occupied), the main cost comes from
>> KVM_SET_USER_MEMORY_REGION ioctl when mem.memory_size = 0 in
>> kvm_set_user_memory_region. In kmod, the main cost is
>> kvm_zap_obsolete_pages, which traverses the active_mmu_pages list to
>> zap the unsync sptes.
>>
>> It can be optimized by delaying memory_global_dirty_log_stop to the next
>> vm_start.
>>
>> Changes v1->v2:
>>   - create a VMChangeStateHandler in memory.c to reduce the coupling [Paolo]
>>
>> Signed-off-by: Jay Zhou <jianjay.zhou@huawei.com>
>
> memory_vm_change_state_handler should delete the handler.
>
> Apart from that, because there is no protection against nested
> invocations of memory_global_dirty_log_start/stop, a little more work is
> needed.

Thank for your patience and pointing everything out, I will fix all of them.

Jay

>> ---
>>   memory.c | 27 ++++++++++++++++++++++++++-
>>   1 file changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/memory.c b/memory.c
>> index a7bc70a..4c22b7e 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -2357,8 +2357,14 @@ void memory_global_dirty_log_sync(void)
>>       }
>>   }
>>
>> +static VMChangeStateEntry *vmstate_change;
>> +
>>   void memory_global_dirty_log_start(void)
>>   {
>> +    if (vmstate_change) {
>> +        qemu_del_vm_change_state_handler(vmstate_change);
>
> This should also NULL vmstate_change, so that you can detect the case
> where the handler is already installed.
>
>> +    }
>> +
>>       global_dirty_log = true;
>>
>>       MEMORY_LISTENER_CALL_GLOBAL(log_global_start, Forward);
>> @@ -2369,7 +2375,7 @@ void memory_global_dirty_log_start(void)
>>       memory_region_transaction_commit();
>>   }
>>
>> -void memory_global_dirty_log_stop(void)
>> +static void memory_global_dirty_log_do_stop(void)
>>   {
>>       global_dirty_log = false;
>>
>> @@ -2381,6 +2387,25 @@ void memory_global_dirty_log_stop(void)
>>       MEMORY_LISTENER_CALL_GLOBAL(log_global_stop, Reverse);
>>   }
>>
>> +static void memory_vm_change_state_handler(void *opaque, int running,
>> +                                           RunState state)
>> +{
>> +    if (running) {
>> +        memory_global_dirty_log_do_stop();
>
> Because this will delete the handler, it should also NULL vmstate_change.
>
>> +    }
>> +}
>> +
>> +void memory_global_dirty_log_stop(void)
>> +{
>> +    if (!runstate_is_running()) {
>> +        vmstate_change = qemu_add_vm_change_state_handler(
>> +                                memory_vm_change_state_handler, NULL);
>
> And this needs to exit immediately if you have an installed handler.
>
> Thanks,
>
> Paolo
>
>> +        return;
>> +    }
>> +
>> +    memory_global_dirty_log_do_stop();
>> +}
>> +
>>   static void listener_add_address_space(MemoryListener *listener,
>>                                          AddressSpace *as)
>>   {
>>
>
>
> .
>