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

[Jay Zhou <jianjay.zhou@huawei.com>]

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