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 <address@hidden>

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)
>  {
>